On Tue, 2026-03-17 at 03:06 +0900, Ryusuke Konishi wrote:
> 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.
>
Yep, it is:
static inline u64 div_u64(u64 dividend, u32 divisor)
> > >
> > > > + 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 */
>
Even better, lets use a compile-time assertion:
BUILD_BUG_ON_NOT_POWER_OF_TWO(NILFS_BMAP_GROUP_DIV);
I'll send a respin.
Thanks!
--
Jeff Layton <[email protected]>
