All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Wilcox <willy@infradead.org>
To: Dave Chinner <david@fromorbit.com>
Cc: Josef Bacik <josef@toxicpanda.com>,
	Johannes Thumshirn <johannes.thumshirn@wdc.com>,
	David Sterba <dsterba@suse.com>,
	"linux-btrfs @ vger . kernel . org" <linux-btrfs@vger.kernel.org>,
	Filipe Manana <fdmanana@gmail.com>,
	Christoph Hellwig <hch@lst.de>,
	Linux FS Devel <linux-fsdevel@vger.kernel.org>
Subject: Re: [RFC PATCH] btrfs: don't call btrfs_sync_file from iomap context
Date: Wed, 2 Sep 2020 12:44:14 +0100	[thread overview]
Message-ID: <20200902114414.GX14765@casper.infradead.org> (raw)
In-Reply-To: <20200901235830.GI12096@dread.disaster.area>

On Wed, Sep 02, 2020 at 09:58:30AM +1000, Dave Chinner wrote:
> Put simply: converting a filesystem to use iomap is not a "change
> the filesystem interfacing code and it will work" modification.  We
> ask that filesystems are modified to conform to the iomap IO
> exclusion model; adding special cases for every potential
> locking and mapping quirk every different filesystem has is part of
> what turned the old direct IO code into an unmaintainable nightmare.
> 
> > That's fine, but this is kind of a bad way to find
> > out.  We really shouldn't have generic helper's that have different generic
> > locking rules based on which file system uses them.
> 
> We certainly can change the rules for new infrastructure. Indeed, we
> had to change the rules to support DAX.  The whole point of the
> iomap infrastructure was that it enabled us to use code that already
> worked for DAX (the XFS code) in multiple filesystems. And as people
> have realised that the DIO via iomap is much faster than the old DIO
> code and is a much more efficient way of doing large buffered IO,
> other filesystems have started to use it.
> 
> However....
> 
> > Because then we end up
> > with situations like this, where suddenly we're having to come up with some
> > weird solution because the generic thing only works for a subset of file
> > systems.  Thanks,
> 
> .... we've always said "you need to change the filesystem code to
> use iomap". This is simply a reflection on the fact that iomap has
> different rules and constraints to the old code and so it's not a
> direct plug in replacement. There are no short cuts here...

Can you point me (and I suspect Josef!) towards the documentation of the
locking model?  I was hoping to find Documentation/filesystems/iomap.rst
but all the 'iomap' strings in Documentation/ refer to pci_iomap and
similar, except for this in the DAX documentation:

: - implementing ->read_iter and ->write_iter operations which use dax_iomap_rw()
:   when inode has S_DAX flag set
: - implementing an mmap file operation for DAX files which sets the
:   VM_MIXEDMAP and VM_HUGEPAGE flags on the VMA, and setting the vm_ops to
:   include handlers for fault, pmd_fault, page_mkwrite, pfn_mkwrite. These
:   handlers should probably call dax_iomap_fault() passing the appropriate
:   fault size and iomap operations.
: - calling iomap_zero_range() passing appropriate iomap operations instead of
:   block_truncate_page() for DAX files
: - ensuring that there is sufficient locking between reads, writes,
:   truncates and page faults
: 
: The iomap handlers for allocating blocks must make sure that allocated blocks
: are zeroed out and converted to written extents before being returned to avoid
: exposure of uninitialized data through mmap.

which doesn't bear on this situation.

  parent reply	other threads:[~2020-09-02 11:44 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-01 13:06 [RFC PATCH] btrfs: don't call btrfs_sync_file from iomap context Johannes Thumshirn
2020-09-01 13:11 ` Johannes Thumshirn
2020-09-01 14:17 ` Goldwyn Rodrigues
2020-09-01 14:20   ` Johannes Thumshirn
2020-09-01 14:37 ` Filipe Manana
2020-09-01 14:44   ` Johannes Thumshirn
2020-09-01 18:40     ` Goldwyn Rodrigues
2020-09-01 15:11 ` Josef Bacik
2020-09-01 17:45   ` Darrick J. Wong
2020-09-01 17:55     ` Josef Bacik
2020-09-01 21:46   ` Dave Chinner
2020-09-01 22:19     ` Josef Bacik
2020-09-01 23:58       ` Dave Chinner
2020-09-02  0:22         ` Josef Bacik
2020-09-02  7:12           ` Johannes Thumshirn
2020-09-02 11:10             ` Josef Bacik
2020-09-02 16:29               ` Darrick J. Wong
2020-09-02 16:47                 ` Josef Bacik
2020-09-02 11:44         ` Matthew Wilcox [this message]
2020-09-02 12:20           ` Dave Chinner
2020-09-02 12:42             ` Josef Bacik
2020-09-03  2:28               ` Dave Chinner
2020-09-03  9:49                 ` Filipe Manana
2020-09-03 16:32   ` Christoph Hellwig
2020-09-03 16:46     ` Josef Bacik
2020-09-07  0:04     ` Dave Chinner
2020-09-15 21:48       ` Goldwyn Rodrigues
2020-09-17  3:09         ` Dave Chinner
2020-09-17  5:52           ` Christoph Hellwig
2020-09-17  6:29             ` Dave Chinner
2020-09-17  6:42               ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200902114414.GX14765@casper.infradead.org \
    --to=willy@infradead.org \
    --cc=david@fromorbit.com \
    --cc=dsterba@suse.com \
    --cc=fdmanana@gmail.com \
    --cc=hch@lst.de \
    --cc=johannes.thumshirn@wdc.com \
    --cc=josef@toxicpanda.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.