[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZrD0TKDHWhwiEoz_@casper.infradead.org>
Date: Mon, 5 Aug 2024 16:48:28 +0100
From: Matthew Wilcox <willy@...radead.org>
To: Jan Kara <jack@...e.cz>
Cc: Zhang Yi <yi.zhang@...weicloud.com>, Dave Chinner <david@...morbit.com>,
linux-xfs@...r.kernel.org, linux-fsdevel@...r.kernel.org,
linux-kernel@...r.kernel.org, djwong@...nel.org, hch@...radead.org,
brauner@...nel.org, yi.zhang@...wei.com, chengzhihao1@...wei.com,
yukuai3@...wei.com
Subject: Re: [PATCH 5/6] iomap: drop unnecessary state_lock when setting ifs
uptodate bits
On Mon, Aug 05, 2024 at 04:00:23PM +0200, Jan Kara wrote:
> Actually add Matthew to CC ;)
It's OK, I was reading.
FWIW, I agree with Dave; the locking complexity in this patch was
horrendous. I was going to get to the same critique he had, but I first
wanted to understand what the thought process was.
> > > Ha, right, I missed the comments of this function, it means that there are
> > > some special callers that hold table lock instead of folio lock, is it
> > > pte_alloc_map_lock?
> > >
> > > I checked all the filesystem related callers and didn't find any real
> > > caller that mark folio dirty without holding folio lock and that could
> > > affect current filesystems which are using iomap framework, it's just
> > > a potential possibility in the future, am I right?
Filesystems are normally quite capable of taking the folio lock to
prevent truncation. It's the MM code that needs the "or holding the
page table lock" get-out clause. I forget exactly which callers it
is; I worked through them a few times. It's not hard to put a
WARN_ON_RATELIMIT() into folio_mark_dirty() and get a good sampling.
There's also a "or holding a buffer_head locked" get-out clause that
I'm not sure is documented anywhere, but obviously that doesn't apply
to the iomap code.
> > There used to be quite a few places doing that. Now that I've checked all
> > places I was aware of got actually converted to call folio_mark_dirty() under
> > a folio lock (in particular all the cases happening on IO completion, folio
> > unmap etc.). Matthew, are you aware of any place where folio_mark_dirty()
> > would be called for regular file page cache (block device page cache is in a
> > different situation obviously) without folio lock held?
Yes, the MM code definitely applies to regular files as well as block
devices.
Powered by blists - more mailing lists