On Wed, Sep 3, 2025 at 5:16 PM Mateusz Guzik <mjguzik@xxxxxxxxx> wrote: > > On Wed, Sep 3, 2025 at 4:03 PM Mark Tinguely <mark.tinguely@xxxxxxxxxx> wrote: > > > > On 9/2/25 10:44 PM, Matthew Wilcox wrote: > > > On Tue, Sep 02, 2025 at 04:54:28PM +0200, Mateusz Guzik wrote: > > >> Following up on my response to the refcount patchset, here is a churn > > >> patch to retire I_WILL_FREE. > > >> > > >> The only consumer is the drop inode routine in ocfs2. > > > > > > If the only consumer is ocfs2 ... then you should cc the ocfs2 people, > > > right? > > > > > Fair point, I just copy pasted the list from the patchset, too sloppy. > > > >> For the life of me I could not figure out if write_inode_now() is legal > > >> to call in ->evict_inode later and have no means to test, so I devised a > > >> hack: let the fs set I_FREEING ahead of time. Also note iput_final() > > >> issues write_inode_now() anyway but only for the !drop case, which is the > > >> opposite of what is being returned. > > >> > > >> One could further hack around it by having ocfs2 return *DON'T* drop but > > >> also set I_DONTCACHE, which would result in both issuing the write in > > >> iput_final() and dropping. I think the hack I did implement is cleaner. > > >> Preferred option is ->evict_inode from ocfs handling the i/o, but per > > >> the above I don't know how to do it. > > > > I am a lurker in this series and ocfs2. My history has been mostly in > > XFS/CXFS/DMAPI. I removed the other CC entries because I did not want > > to blast my opinion unnecessaially. > > > > Hello Mark, > > This needs the opinion of the vfs folk though, so I'm adding some of > the cc list back. ;) > > > The flushing in ocfs2_drop_inode() predates the I_DONTCACHE addition. > > IMO, it would be safest and best to maintain to let ocfs2_drop_inode() > > return 0 and set I_DONTCACHE and let iput_final() do the correct thing. > > > wow, I'm sorry for really bad case of engrish in the mail. some of it gets slightly corrected below. > For now that would indeed work in the sense of providing the expected > behavior, but there is the obvious mismatch of the filesystem claiming > the inode should not be dropped (by returning 0) and but using a side > indicator to drop it anyway. This looks like a split-brain scenario > and sooner or later someone is going to complain about it when they do > other work in iput_final(). If I was maintaining the layer I would > reject the idea, but if the actual gatekeepers are fine with it... > > The absolute best thing to do long run is to move the i/o in > ->evict_inode, but someone familiar with the APIs here would do the > needful(tm) and that's not me. I mean the best thing to do in the long run is to move the the write to ->evict_inode, but I don't know how to do it and don't have any means to test ocfs2 anyway. Hopefully the ocfs2 folk will be willing to sort this out? -- Mateusz Guzik <mjguzik gmail.com>
