On Wed, Feb 21, 2018 at 04:43:53PM -0800, Matthew Wilcox wrote:
> On Wed, Feb 21, 2018 at 11:39:05PM +0100, Luc Van Oostenryck wrote:
> > When using this series on the kernel (v4.15, defconfig) we got
> > a few new sparse warning:
> > 1) a problem with (fs/aio.c)
> > struct kioctx {
> > ...
> > struct __percpu kioctx_cpu *cpu;
> > ...
> > };
> >
> > should be:
> > __percpu struct kioctx_cpu *cpu;
>
> There's another example of this:
>
> drivers/net/wireless/intel/iwlwifi/pcie/internal.h: struct __percpu iwl_tso_hdr_page *tso_hdr_page;
>
> I assume that "struct kioctx_cpu __percpu *cpu" is still acceptable?
> That seems to be the majority of uses of __percpu in the kernel today.
I think we should accept what GCC accept for non-sparse specific
attributes. I'll need to check a bit.
Since I write:
const unsigned int *ptr;
and it seems really weird to me to write:
unsigned int const *ptr;
I tend to see
__percpu struct tag *ptr;
as much 'more natural' than
struct tag __percpy *ptr;
> > I can restore current behaviour of 1) but I think it should simply
> > use the correct syntax. In fact I think that declaring:
> > struct name { ... };
> > and then using 'struct __some_attribute__ name' should be an error.
>
> I think it depends on the attribute. __bitwise is different from __percpu
> here; __bitwise tells you something about the type whereas __percpu tells
> you something about this particular pointer. It's quite legitimate for
> me to have a non-percpu struct kioctx_cpu, and percpu struct kioctx_cpu.
> I'd say __rcu is in the same category as __percpu here.
Here I disagree strongly. It should *not* depends on the attribute.
It's just a question of syntax for attaching the attribute to:
- the type itself. Correct example would be:
struct __attr__ name { };
struct name { ... } __attr__;
- or to the variable.
If the type 'struct kioctx_cpu' exist in itself, ok. And if you need
to use an attribute for a variable of this type, fine, and this
variable will have as type the original type + the attribute.
What I tried to say here above is that if you declare a type as:
struct name ...;
then the name 'struct name' will be attached to the type 'struct name'
so if later you use somewhere 'struct __attr__ name' (which is the
syntax for attaching an attribute to a type) you necessarily create
a problem because you're trying to redefine the type 'struct name'.
This situation can only arise for struct, union and enums.
But yes, __bitwise only make sense as an attribute for a type so
it must be used as such. So, it makes no sense to write:
__bitwise unsigned int var;
because we won't be able to ue any value for this var.
On the contrary, I suppose that for __rcu or __percpu it (almost)
always make sense to be able to access the non-attributed
version of the type, so it make no or little sense to write;
struct __percpu name { ... };
because then all variables declared as:
struct name var;
will be 'percpu'.
-- Luc
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
