Re: [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers
> New helpers take pointers to uuid_{be|le} as parameters. > > When using them on a raw data we don't need to do an ugly dereference and, > in some cases, a type casting. > > Cc: Andrew Morton> Cc: Arnd Bergmann > Cc: Liam Girdwood > Cc: Mark Brown > Cc: Vinod Koul > Cc: Srinivas Pandruvada > Cc: Benjamin Tissoires > Cc: Kirti Wankhede > Cc: Alex Williamson > Cc: "K. Y. Srinivasan" > Cc: Haiyang Zhang > Cc: Stephen Hemminger > Cc: Tomas Winkler > Cc: Matt Fleming > Cc: Ard Biesheuvel > Cc: "Rafael J. Wysocki" > > Signed-off-by: Andy Shevchenko > --- > include/linux/uuid.h | 20 > 1 file changed, 20 insertions(+) > > diff --git a/include/linux/uuid.h b/include/linux/uuid.h index > 4dff73a89758..45312cb5ac65 100644 > --- a/include/linux/uuid.h > +++ b/include/linux/uuid.h > @@ -58,6 +58,26 @@ static inline int uuid_be_cmp(const uuid_be u1, const > uuid_be u2) > return memcmp(, , sizeof(uuid_be)); } > > +static inline int uuid_le_cmp_p(const uuid_le *pu1, const uuid_le u2) { > + return memcmp(pu1, , sizeof(uuid_le)); } > + > +static inline int uuid_be_cmp_p(const uuid_be *pu1, const uuid_be u2) { > + return memcmp(pu1, , sizeof(uuid_be)); } > + > +static inline int uuid_le_cmp_pp(const uuid_le *pu1, const uuid_le > +*pu2) { > + return memcmp(pu1, pu2, sizeof(uuid_le)); } > + > +static inline int uuid_be_cmp_pp(const uuid_be *pu1, const uuid_be > +*pu2) { > + return memcmp(pu1, pu2, sizeof(uuid_be)); } > + > void generate_random_uuid(unsigned char uuid[16]); > > extern void uuid_le_gen(uuid_le *u); There's a bug in gcc wherein constant compound literals are generated on the stack at runtime, rather than stored in rodata. The bug occurs if the compound literal is passed by reference. It does not manifest itself when passing by value: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68725 Hence this patch is unfortunately counterproductive if the UUIDs are declared const. FWIW I've posted a series back in January to constify UUIDs as much as possible, but I got some objections and lacked the time so far to address them. In fact I'm thinking that gcc needs to be fixed first, then we can focus on improving the kernel side of things: https://www.spinics.net/lists/linux-efi/msg10093.html Best regards, Lukas
Re: [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers
> New helpers take pointers to uuid_{be|le} as parameters. > > When using them on a raw data we don't need to do an ugly dereference and, > in some cases, a type casting. > > Cc: Andrew Morton > Cc: Arnd Bergmann > Cc: Liam Girdwood > Cc: Mark Brown > Cc: Vinod Koul > Cc: Srinivas Pandruvada > Cc: Benjamin Tissoires > Cc: Kirti Wankhede > Cc: Alex Williamson > Cc: "K. Y. Srinivasan" > Cc: Haiyang Zhang > Cc: Stephen Hemminger > Cc: Tomas Winkler > Cc: Matt Fleming > Cc: Ard Biesheuvel > Cc: "Rafael J. Wysocki" > > Signed-off-by: Andy Shevchenko > --- > include/linux/uuid.h | 20 > 1 file changed, 20 insertions(+) > > diff --git a/include/linux/uuid.h b/include/linux/uuid.h index > 4dff73a89758..45312cb5ac65 100644 > --- a/include/linux/uuid.h > +++ b/include/linux/uuid.h > @@ -58,6 +58,26 @@ static inline int uuid_be_cmp(const uuid_be u1, const > uuid_be u2) > return memcmp(, , sizeof(uuid_be)); } > > +static inline int uuid_le_cmp_p(const uuid_le *pu1, const uuid_le u2) { > + return memcmp(pu1, , sizeof(uuid_le)); } > + > +static inline int uuid_be_cmp_p(const uuid_be *pu1, const uuid_be u2) { > + return memcmp(pu1, , sizeof(uuid_be)); } > + > +static inline int uuid_le_cmp_pp(const uuid_le *pu1, const uuid_le > +*pu2) { > + return memcmp(pu1, pu2, sizeof(uuid_le)); } > + > +static inline int uuid_be_cmp_pp(const uuid_be *pu1, const uuid_be > +*pu2) { > + return memcmp(pu1, pu2, sizeof(uuid_be)); } > + > void generate_random_uuid(unsigned char uuid[16]); > > extern void uuid_le_gen(uuid_le *u); There's a bug in gcc wherein constant compound literals are generated on the stack at runtime, rather than stored in rodata. The bug occurs if the compound literal is passed by reference. It does not manifest itself when passing by value: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68725 Hence this patch is unfortunately counterproductive if the UUIDs are declared const. FWIW I've posted a series back in January to constify UUIDs as much as possible, but I got some objections and lacked the time so far to address them. In fact I'm thinking that gcc needs to be fixed first, then we can focus on improving the kernel side of things: https://www.spinics.net/lists/linux-efi/msg10093.html Best regards, Lukas
Re: [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers
On Sun, 2017-04-23 at 20:20 +, Winkler, Tomas wrote: > On Sun, 2017-04-23 at 15:42 +0300, Andy Shevchenko wrote: > > On Sun, Apr 23, 2017 at 1:29 PM, Winkler, Tomas> . > > com> wrote: > > > > New helpers take pointers to uuid_{be|le} as parameters. > > > > > > > > When using them on a raw data we don't need to do an ugly > > > > dereference and, > > > > in some cases, a type casting. > > > > > > I think this going overboard, the _pp types are just enough. > > > > I looked at existing users and there are cases like > > #define XXX_UUID UUID_...(a, b, c, ...) > > > > uuid_.*_cmp(value, XXX_UUID) > > > > For _pp variant if would be _cmp_pp(value, _UUID) which is > > slightly worse than for _p variant. > > > Maybe it's worth to actually replace the defines with variables than > to > create an interface with all the permutations. Maybe. I didn't estimate the number of users that would be in a scope. -- Andy Shevchenko Intel Finland Oy
Re: [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers
On Sun, 2017-04-23 at 20:20 +, Winkler, Tomas wrote: > On Sun, 2017-04-23 at 15:42 +0300, Andy Shevchenko wrote: > > On Sun, Apr 23, 2017 at 1:29 PM, Winkler, Tomas > . > > com> wrote: > > > > New helpers take pointers to uuid_{be|le} as parameters. > > > > > > > > When using them on a raw data we don't need to do an ugly > > > > dereference and, > > > > in some cases, a type casting. > > > > > > I think this going overboard, the _pp types are just enough. > > > > I looked at existing users and there are cases like > > #define XXX_UUID UUID_...(a, b, c, ...) > > > > uuid_.*_cmp(value, XXX_UUID) > > > > For _pp variant if would be _cmp_pp(value, _UUID) which is > > slightly worse than for _p variant. > > > Maybe it's worth to actually replace the defines with variables than > to > create an interface with all the permutations. Maybe. I didn't estimate the number of users that would be in a scope. -- Andy Shevchenko Intel Finland Oy
Re: [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers
On Sun, 2017-04-23 at 15:42 +0300, Andy Shevchenko wrote: > On Sun, Apr 23, 2017 at 1:29 PM, Winkler, Tomascom> wrote: > > > New helpers take pointers to uuid_{be|le} as parameters. > > > > > > When using them on a raw data we don't need to do an ugly > > > dereference and, > > > in some cases, a type casting. > > I think this going overboard, the _pp types are just enough. > > I looked at existing users and there are cases like > #define XXX_UUID UUID_...(a, b, c, ...) > > uuid_.*_cmp(value, XXX_UUID) > > For _pp variant if would be _cmp_pp(value, _UUID) which is > slightly worse than for _p variant. Maybe it's worth to actually replace the defines with variables than to create an interface with all the permutations. Tomas
Re: [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers
On Sun, 2017-04-23 at 15:42 +0300, Andy Shevchenko wrote: > On Sun, Apr 23, 2017 at 1:29 PM, Winkler, Tomas com> wrote: > > > New helpers take pointers to uuid_{be|le} as parameters. > > > > > > When using them on a raw data we don't need to do an ugly > > > dereference and, > > > in some cases, a type casting. > > I think this going overboard, the _pp types are just enough. > > I looked at existing users and there are cases like > #define XXX_UUID UUID_...(a, b, c, ...) > > uuid_.*_cmp(value, XXX_UUID) > > For _pp variant if would be _cmp_pp(value, _UUID) which is > slightly worse than for _p variant. Maybe it's worth to actually replace the defines with variables than to create an interface with all the permutations. Tomas
Re: [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers
On Sun, Apr 23, 2017 at 1:29 PM, Winkler, Tomaswrote: >> New helpers take pointers to uuid_{be|le} as parameters. >> >> When using them on a raw data we don't need to do an ugly dereference and, >> in some cases, a type casting. > I think this going overboard, the _pp types are just enough. I looked at existing users and there are cases like #define XXX_UUID UUID_...(a, b, c, ...) uuid_.*_cmp(value, XXX_UUID) For _pp variant if would be _cmp_pp(value, _UUID) which is slightly worse than for _p variant. -- With Best Regards, Andy Shevchenko
Re: [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers
On Sun, Apr 23, 2017 at 1:29 PM, Winkler, Tomas wrote: >> New helpers take pointers to uuid_{be|le} as parameters. >> >> When using them on a raw data we don't need to do an ugly dereference and, >> in some cases, a type casting. > I think this going overboard, the _pp types are just enough. I looked at existing users and there are cases like #define XXX_UUID UUID_...(a, b, c, ...) uuid_.*_cmp(value, XXX_UUID) For _pp variant if would be _cmp_pp(value, _UUID) which is slightly worse than for _p variant. -- With Best Regards, Andy Shevchenko
RE: [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers
> New helpers take pointers to uuid_{be|le} as parameters. > > When using them on a raw data we don't need to do an ugly dereference and, > in some cases, a type casting. > > Cc: Andrew Morton> Cc: Arnd Bergmann > Cc: Liam Girdwood > Cc: Mark Brown > Cc: Vinod Koul > Cc: Srinivas Pandruvada > Cc: Benjamin Tissoires > Cc: Kirti Wankhede > Cc: Alex Williamson > Cc: "K. Y. Srinivasan" > Cc: Haiyang Zhang > Cc: Stephen Hemminger > Cc: Tomas Winkler > Cc: Matt Fleming > Cc: Ard Biesheuvel > Cc: "Rafael J. Wysocki" > > Signed-off-by: Andy Shevchenko > --- > include/linux/uuid.h | 20 > 1 file changed, 20 insertions(+) > > diff --git a/include/linux/uuid.h b/include/linux/uuid.h index > 4dff73a89758..45312cb5ac65 100644 > --- a/include/linux/uuid.h > +++ b/include/linux/uuid.h > @@ -58,6 +58,26 @@ static inline int uuid_be_cmp(const uuid_be u1, const > uuid_be u2) > return memcmp(, , sizeof(uuid_be)); } > > +static inline int uuid_le_cmp_p(const uuid_le *pu1, const uuid_le u2) { > + return memcmp(pu1, , sizeof(uuid_le)); } > + > +static inline int uuid_be_cmp_p(const uuid_be *pu1, const uuid_be u2) { > + return memcmp(pu1, , sizeof(uuid_be)); } > + > +static inline int uuid_le_cmp_pp(const uuid_le *pu1, const uuid_le > +*pu2) { > + return memcmp(pu1, pu2, sizeof(uuid_le)); } > + > +static inline int uuid_be_cmp_pp(const uuid_be *pu1, const uuid_be > +*pu2) { > + return memcmp(pu1, pu2, sizeof(uuid_be)); } > + > void generate_random_uuid(unsigned char uuid[16]); > > extern void uuid_le_gen(uuid_le *u); I think this going overboard, the _pp types are just enough. Tomas
RE: [PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers
> New helpers take pointers to uuid_{be|le} as parameters. > > When using them on a raw data we don't need to do an ugly dereference and, > in some cases, a type casting. > > Cc: Andrew Morton > Cc: Arnd Bergmann > Cc: Liam Girdwood > Cc: Mark Brown > Cc: Vinod Koul > Cc: Srinivas Pandruvada > Cc: Benjamin Tissoires > Cc: Kirti Wankhede > Cc: Alex Williamson > Cc: "K. Y. Srinivasan" > Cc: Haiyang Zhang > Cc: Stephen Hemminger > Cc: Tomas Winkler > Cc: Matt Fleming > Cc: Ard Biesheuvel > Cc: "Rafael J. Wysocki" > > Signed-off-by: Andy Shevchenko > --- > include/linux/uuid.h | 20 > 1 file changed, 20 insertions(+) > > diff --git a/include/linux/uuid.h b/include/linux/uuid.h index > 4dff73a89758..45312cb5ac65 100644 > --- a/include/linux/uuid.h > +++ b/include/linux/uuid.h > @@ -58,6 +58,26 @@ static inline int uuid_be_cmp(const uuid_be u1, const > uuid_be u2) > return memcmp(, , sizeof(uuid_be)); } > > +static inline int uuid_le_cmp_p(const uuid_le *pu1, const uuid_le u2) { > + return memcmp(pu1, , sizeof(uuid_le)); } > + > +static inline int uuid_be_cmp_p(const uuid_be *pu1, const uuid_be u2) { > + return memcmp(pu1, , sizeof(uuid_be)); } > + > +static inline int uuid_le_cmp_pp(const uuid_le *pu1, const uuid_le > +*pu2) { > + return memcmp(pu1, pu2, sizeof(uuid_le)); } > + > +static inline int uuid_be_cmp_pp(const uuid_be *pu1, const uuid_be > +*pu2) { > + return memcmp(pu1, pu2, sizeof(uuid_be)); } > + > void generate_random_uuid(unsigned char uuid[16]); > > extern void uuid_le_gen(uuid_le *u); I think this going overboard, the _pp types are just enough. Tomas
[PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers
New helpers take pointers to uuid_{be|le} as parameters. When using them on a raw data we don't need to do an ugly dereference and, in some cases, a type casting. Cc: Andrew MortonCc: Arnd Bergmann Cc: Liam Girdwood Cc: Mark Brown Cc: Vinod Koul Cc: Srinivas Pandruvada Cc: Benjamin Tissoires Cc: Kirti Wankhede Cc: Alex Williamson Cc: "K. Y. Srinivasan" Cc: Haiyang Zhang Cc: Stephen Hemminger Cc: Tomas Winkler Cc: Matt Fleming Cc: Ard Biesheuvel Cc: "Rafael J. Wysocki" Signed-off-by: Andy Shevchenko --- include/linux/uuid.h | 20 1 file changed, 20 insertions(+) diff --git a/include/linux/uuid.h b/include/linux/uuid.h index 4dff73a89758..45312cb5ac65 100644 --- a/include/linux/uuid.h +++ b/include/linux/uuid.h @@ -58,6 +58,26 @@ static inline int uuid_be_cmp(const uuid_be u1, const uuid_be u2) return memcmp(, , sizeof(uuid_be)); } +static inline int uuid_le_cmp_p(const uuid_le *pu1, const uuid_le u2) +{ + return memcmp(pu1, , sizeof(uuid_le)); +} + +static inline int uuid_be_cmp_p(const uuid_be *pu1, const uuid_be u2) +{ + return memcmp(pu1, , sizeof(uuid_be)); +} + +static inline int uuid_le_cmp_pp(const uuid_le *pu1, const uuid_le *pu2) +{ + return memcmp(pu1, pu2, sizeof(uuid_le)); +} + +static inline int uuid_be_cmp_pp(const uuid_be *pu1, const uuid_be *pu2) +{ + return memcmp(pu1, pu2, sizeof(uuid_be)); +} + void generate_random_uuid(unsigned char uuid[16]); extern void uuid_le_gen(uuid_le *u); -- 2.11.0
[PATCH v1 1/8] lib/uuid: Introduce uuid_{be|le}_cmp_p{p}() helpers
New helpers take pointers to uuid_{be|le} as parameters. When using them on a raw data we don't need to do an ugly dereference and, in some cases, a type casting. Cc: Andrew Morton Cc: Arnd Bergmann Cc: Liam Girdwood Cc: Mark Brown Cc: Vinod Koul Cc: Srinivas Pandruvada Cc: Benjamin Tissoires Cc: Kirti Wankhede Cc: Alex Williamson Cc: "K. Y. Srinivasan" Cc: Haiyang Zhang Cc: Stephen Hemminger Cc: Tomas Winkler Cc: Matt Fleming Cc: Ard Biesheuvel Cc: "Rafael J. Wysocki" Signed-off-by: Andy Shevchenko --- include/linux/uuid.h | 20 1 file changed, 20 insertions(+) diff --git a/include/linux/uuid.h b/include/linux/uuid.h index 4dff73a89758..45312cb5ac65 100644 --- a/include/linux/uuid.h +++ b/include/linux/uuid.h @@ -58,6 +58,26 @@ static inline int uuid_be_cmp(const uuid_be u1, const uuid_be u2) return memcmp(, , sizeof(uuid_be)); } +static inline int uuid_le_cmp_p(const uuid_le *pu1, const uuid_le u2) +{ + return memcmp(pu1, , sizeof(uuid_le)); +} + +static inline int uuid_be_cmp_p(const uuid_be *pu1, const uuid_be u2) +{ + return memcmp(pu1, , sizeof(uuid_be)); +} + +static inline int uuid_le_cmp_pp(const uuid_le *pu1, const uuid_le *pu2) +{ + return memcmp(pu1, pu2, sizeof(uuid_le)); +} + +static inline int uuid_be_cmp_pp(const uuid_be *pu1, const uuid_be *pu2) +{ + return memcmp(pu1, pu2, sizeof(uuid_be)); +} + void generate_random_uuid(unsigned char uuid[16]); extern void uuid_le_gen(uuid_le *u); -- 2.11.0