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. > 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. -- Mateusz Guzik <mjguzik gmail.com>
