Re: [PATCH v2 1/2] nilfs2: fix 64-bit division operations in nilfs_bmap_find_target_in_group()

[Jeff Layton <jlayton@kernel.org>]

On Sat, 2026-03-14 at 12:47 +0000, David Laight wrote:
> On Fri, 13 Mar 2026 14:45:20 -0400
> Jeff Layton <jlayton@kernel.org> wrote:
> 
> > With the change to make inode->i_ino a u64, the build started failing on
> > 32-bit ARM with:
> > 
> >     ERROR: modpost: "__aeabi_uldivmod" [fs/nilfs2/nilfs2.ko] undefined!
> > 
> > Fix this by using the 64-bit division interfaces in
> > nilfs_bmap_find_target_in_group().
> > 
> > Fixes: 998a59d371c2 ("treewide: fix missed i_ino format specifier conversions")
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202603100602.KPxiClIO-lkp@intel.com/
> > Reviewed-by: Viacheslav Dubeyko <slava@dubeyko.com>
> > Acked-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/nilfs2/bmap.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/fs/nilfs2/bmap.c b/fs/nilfs2/bmap.c
> > index 824f2bd91c167965ec3a660202b6e6c5f1fe007e..abcf5252578ad24f694bfccf525893674bfcb4bc 100644
> > --- a/fs/nilfs2/bmap.c
> > +++ b/fs/nilfs2/bmap.c
> > @@ -455,11 +455,14 @@ __u64 nilfs_bmap_find_target_in_group(const struct nilfs_bmap *bmap)
> >  {
> >  	struct inode *dat = nilfs_bmap_get_dat(bmap);
> >  	unsigned long entries_per_group = nilfs_palloc_entries_per_group(dat);
> > -	unsigned long group = bmap->b_inode->i_ino / entries_per_group;
> 
> Are you sure entries_per_group can be more than 32 bits?
> It looks like something that will be the same size on 32 and 64bit.
> 

I'm not sure of anything here. I'm just want to get this to compile on
all arches. FWIW, I'm not looking to optimize anything in this patch.

> > +	unsigned long group;
> > +	u32 index;
> > +
> > +	group = div_u64(bmap->b_inode->i_ino, entries_per_group);
> 
> You don't need the full 64 by 64 divide.
> IIRC there are both div_u64_u32() and div_u64_ulong().
>
> > +	div_u64_rem(bmap->b_inode->i_ino, NILFS_BMAP_GROUP_DIV, &index);
> 
> NILFD_BMAP_GROUP_DIV is 8 (and probably has to be a power of 2).
> So:
> 	index = bmap->b_inode->i_ino & (NILFS_BMAP_GROUP_DIV - 1);
> is the same and likely much faster to calculate.
> (The compiler will have done that optimisation before.)
> 
> 

That all sounds reasonable to me. At this point though, it would be
better if the NILFS2 folks stepped in with how they'd prefer this be
done.

> 
> >  
> >  	return group * entries_per_group +
> > -		(bmap->b_inode->i_ino % NILFS_BMAP_GROUP_DIV) *
> > -		(entries_per_group / NILFS_BMAP_GROUP_DIV);
> > +	       index * (entries_per_group / NILFS_BMAP_GROUP_DIV);
> >  }
> >  
> >  static struct lock_class_key nilfs_bmap_dat_lock_key;
> > 

-- 
Jeff Layton <jlayton@kernel.org>