On Sat, Mar 14, 2026 at 9:59 PM Jeff Layton wrote: > > On Sat, 2026-03-14 at 12:47 +0000, David Laight wrote: > > On Fri, 13 Mar 2026 14:45:20 -0400 > > Jeff Layton <[email protected]> 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 <[email protected]> > > > Closes: > > > https://lore.kernel.org/oe-kbuild-all/[email protected]/ > > > Reviewed-by: Viacheslav Dubeyko <[email protected]> > > > Acked-by: Ryusuke Konishi <[email protected]> > > > Signed-off-by: Jeff Layton <[email protected]> > > > --- > > > 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().
Isn't the type of divisor in div_u64() u32? Since entries_per_group cannot exceed 32 bits according to the current specification, I think using div_u64() is fine. > > > > > + 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. Yes, indeed. It seems that the application of optimizations will change, so this proposed correction is better. Since NILFS_BMAP_GROUP_DIV is a fixed constant and cannot be anything other than a power of 2, could you please adopt this proposed correction with the following comment? #define NILFS_BMAP_GROUP_DIV 8 /* must be a power of 2 */ Thanks, Ryusuke Konishi > > > > > > > > > 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 <[email protected]>
