On Tue 11-06-24 13:26:45, Mateusz Guzik wrote: > 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? Well, I'd do it as 4 patches actually: 1) xfs i_state workaround in xfs_reinit_inode() 2) add i_state zeroing to inode_init_always(), drop pointless zeroing from VFS. 3) drop now pointless zeroing from xfs 4) drop now pointless zeroing from bcachefs This way also respective maintainers can easily ack the bits they care about. I can live with two as you suggest since the changes are tiny but four is IMHO a "proper" way to do things ;). Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
