On Tue, Jun 11, 2024 at 08:56:40PM +1000, Dave Chinner wrote: > On Tue, Jun 11, 2024 at 12:23:59PM +0200, Mateusz Guzik wrote: > > I did not patch inode_init_always because it is exported and xfs uses it > > in 2 spots, only one of which zeroing the thing immediately after. > > Another one is a little more involved, it probably would not be a > > problem as the value is set altered later anyway, but I don't want to > > mess with semantics of the func if it can be easily avoided. > > Better to move the zeroing to inode_init_always(), do the basic > save/restore mod to xfs_reinit_inode(), and let us XFS people worry > about whether inode_init_always() is the right thing to be calling > in their code... > > All you'd need to do in xfs_reinit_inode() is this > > + unsigned long state = inode->i_state; > > ..... > error = inode_init_always(mp->m_super, inode); > ..... > + inode->i_state = state; > ..... > > And it should behave as expected. > Ok, so what would be the logistics of submitting this? Can I submit one patch which includes the above + i_state moved to inode_init_always? Do I instead ship a two-part patchset, starting with the xfs change and stating it was your idea? Something else? Fwiw inode_init_always consumer rundown is: - fs/inode.c which is automagically covered - bcachefs pre-zeroing state before even calling inode_init_always - xfs with one spot which zeroes immediately after the call - xfs with one spot which possibly avoids zeroing
