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

From: Arnd Bergmann
Date: Tue Apr 05 2022 - 03:08:42 EST


?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.

> > 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.

It may be helpful to add a __print_flags_u32(), but it's unrelated
to the problem at hand.

Arnd