Re: Build regressions/improvements in v5.18-rc1

[Dave Chinner]

On Tue, Apr 05, 2022 at 09:08:13AM +0200, Arnd Bergmann wrote:
> ?On Tue, Apr 5, 2022 at 8:47 AM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> > On Tue, Apr 5, 2022 at 12:16 AM Dave Chinner <david@xxxxxxxxxxxxx> wrote:
> > > So XFS only uses these flags in unsigned int fields that are
> > > typed via:
> > >
> > > typedef unsigned int xfs_buf_flags_t;
> > >
> > > So on the surface, declaring the flag values as ULONG and then writing
> > > them into a UINT field is not a nice thing to be doing.
> > >
> > > I really don't want to change the xfs_buf_flags_t type to an
> > > unsigned long, because that changes the packing of the first
> > > cacheline of the struct xfs_buf and the contents of that cacheline
> > > are performance critical for the lookup fastpath....
> >
> > Hence just use "1u << n" instead of "1ul << n"?
> 
> Right, that avoids the error as well. I picked '1ul' to match the type of
> the variable it's assigned to, but as Dave said the intended type is
> 'u32', so '1u' is better here.

Ok, I'll queue up a patch to make this modification. I'll also need
to check all the other trace flags fields we print as well, as they
likely have the same issue but there's no warnings from them because
they don't use the high bit in the 32 bit field yet...

> > > Looking at __print_flags, the internal array type declaration is:
> > >
> > > struct trace_print_flags {
> > >         unsigned long           mask;
> > >         const char              *name;
> > > };
> > >
> > > and that's the source of the problem.  I notice __print_flags_u64()
> > > exists, but __print_flags_u32() does not. Should it?
> 
> It's not the source of the error, as there is no signed integer
> overflow when assigning an unsigned int to an unsigned long.

Thanks for the clarification!

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx