From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-fsdevel-owner@kernel.org>
X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on
	aws-us-west-2-korg-lkml-1.web.codeaurora.org
Received: from vger.kernel.org (vger.kernel.org [23.128.96.18])
	by smtp.lore.kernel.org (Postfix) with ESMTP id BCA64C433EF
	for <linux-fsdevel@archiver.kernel.org>; Sun, 22 May 2022 07:43:36 +0000 (UTC)
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S233865AbiEVHnf (ORCPT
        <rfc822;linux-fsdevel@archiver.kernel.org>);
        Sun, 22 May 2022 03:43:35 -0400
Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44808 "EHLO
        lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S229771AbiEVHnf (ORCPT
        <rfc822;linux-fsdevel@vger.kernel.org>);
        Sun, 22 May 2022 03:43:35 -0400
Received: from verein.lst.de (verein.lst.de [213.95.11.211])
        by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E69613F8AE
        for <linux-fsdevel@vger.kernel.org>; Sun, 22 May 2022 00:43:32 -0700 (PDT)
Received: by verein.lst.de (Postfix, from userid 2407)
        id D3DCA68B05; Sun, 22 May 2022 09:43:27 +0200 (CEST)
Date:   Sun, 22 May 2022 09:43:27 +0200
From:   Christoph Hellwig <hch@lst.de>
To:     Al Viro <viro@zeniv.linux.org.uk>
Cc:     Christoph Hellwig <hch@lst.de>,
        Matthew Wilcox <willy@infradead.org>,
        linux-fsdevel@vger.kernel.org, Jens Axboe <axboe@kernel.dk>
Subject: Re: [RFC] what to do with IOCB_DSYNC?
Message-ID: <20220522074327.GA15562@lst.de>
References: <YM/hZgxPM+2cP+I7@zeniv-ca.linux.org.uk> <20210621135958.GA1013@lst.de> <YNCcG97WwRlSZpoL@casper.infradead.org> <20210621140956.GA1887@lst.de> <YNCfUoaTNyi4xiF+@casper.infradead.org> <20210621142235.GA2391@lst.de> <YNCjDmqeomXagKIe@zeniv-ca.linux.org.uk> <20210621143501.GA3789@lst.de> <Yokl+uHTVWFxoQGn@zeniv-ca.linux.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Yokl+uHTVWFxoQGn@zeniv-ca.linux.org.uk>
User-Agent: Mutt/1.5.17 (2007-11-01)
Precedence: bulk
List-ID: <linux-fsdevel.vger.kernel.org>
X-Mailing-List: linux-fsdevel@vger.kernel.org

On Sat, May 21, 2022 at 05:48:42PM +0000, Al Viro wrote:
> There's a problem with that variant.  Take a look at btrfs_direct_write():
>         const bool is_sync_write = (iocb->ki_flags & IOCB_DSYNC);
> 	...
>         /*
> 	 * We remove IOCB_DSYNC so that we don't deadlock when iomap_dio_rw()
> 	 * calls generic_write_sync() (through iomap_dio_complete()), because
> 	 * that results in calling fsync (btrfs_sync_file()) which will try to
> 	 * lock the inode in exclusive/write mode.
> 	 */
> 	if (is_sync_write)
> 		iocb->ki_flags &= ~IOCB_DSYNC;
> 	...
> 
> 	/*
> 	 * Add back IOCB_DSYNC. Our caller, btrfs_file_write_iter(), will do  
> 	 * the fsync (call generic_write_sync()).
> 	 */
> 	if (is_sync_write)
> 		iocb->ki_flags |= IOCB_DSYNC;
> 
> will run into trouble.  How about this (completely untested):

Which is pretty gross.  We can just add a IOMAP_DIO_NOSYNC flag
to do what btrfs wants is a much less gross way.

> Precalculate iocb_flags()
> 
> Store the value in file->f_i_flags; calculate at open time (do_dentry_open()
> for opens, alloc_file() for pipe(2)/socket(2)/etc.), update at FCNTL_SETFL
> time.

Eww.  I don't think we should grow struct file for tht.