On 2024/8/5 23:48, Matthew Wilcox wrote: > 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. Yes, I'd like to change to use the solution as Dave suggested. > >>>> 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. Thanks for your answer, I've found some callers. Thanks, Yi. > >>> 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. >
