* [PATCH 0/3] container_of: refactors
@ 2026-07-14 18:18 Vincent Mailhol
2026-07-14 18:18 ` [PATCH 1/3] container_of: apply typeof_member() to container_of() Vincent Mailhol
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Vincent Mailhol @ 2026-07-14 18:18 UTC (permalink / raw)
To: Greg Kroah-Hartman, Andrew Morton
Cc: Rasmus Villemoes, Sakari Ailus, Nick Desaulniers,
Alexander Lobakin, Arnd Bergmann, David Sterba, Ivo van Doorn,
Alexey Dobriyan, linux-kernel, Vincent Mailhol
This series refactors the container_of() function-like macro to improve
readability and remove a sparse/W=2 shadow warning. Further details in
each patch.
While I was expecting this series to be boring and purely cosmetic, the
bloat-o-meter stats gave some unexpected results:
$ ./scripts/bloat-o-meter vmlinux7.2-rc3_before.o vmlinux7.2-rc3_after.o
add/remove: 0/0 grow/shrink: 133/93 up/down: 5914901/-14344137 (-8429236)
< ... 227 lines redacted >
Total: Before=2641674349, After=2633245113, chg -0.32%
(done on v7.2-rc3 with GCC 15.3.0 on an x86_64 defconfig)
Upon analysis, this change in size can be tracked down to places where
container_of() is used in combination with __builtin_constant_p().
Here is a minimal reproducer:
struct foo {
int a;
};
#define to_foo(a_ptr) container_of(a_ptr, struct foo, a)
int f(int *a)
{
return __builtin_constant_p(to_foo(a)->a) || a;
}
The assembly code before this series...:
xor eax, eax
test rdi, rdi
setne al
ret
...and after:
mov eax, 1
ret
Link: https://godbolt.org/z/fenbGexjY
__builtin_constant_p(to_foo(a)->a) evaluates to false but gives the
optimiser the hint that pointer a is not NULL because of the
assumption that no undefined behaviour occurs. With this, the
expression:
__builtin_constant_p(to_foo(a)->a) || a
could be evaluated as true by the optimiser.
But the small variation in container_of() makes it that the optimiser
currently misses this optimisation but manages to do it after the
simplification of patch #3 of this series.
When __builtin_constant_p()'s argument is not trivially a compile time
constant, the result of __builtin_constant_p() comes late in the
evaluation process. And if it comes too late, after some other
optimisations were already done, the compiler will not retry and simply
miss these optimisations.
Note that the above example is very fragile and the results shown in
the godbolt link might not be reproducible under very small
variations.
Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
Vincent Mailhol (3):
container_of: apply typeof_member() to container_of()
container_of: remove useless pair of parentheses
container_of: remove local __mptr variable
include/linux/container_of.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
---
base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
change-id: 20260110-containerof_refactor-63acf8118a18
Best regards,
--
Vincent Mailhol <mailhol@kernel.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] container_of: apply typeof_member() to container_of()
2026-07-14 18:18 [PATCH 0/3] container_of: refactors Vincent Mailhol
@ 2026-07-14 18:18 ` Vincent Mailhol
2026-07-14 18:18 ` [PATCH 2/3] container_of: remove useless pair of parentheses Vincent Mailhol
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Vincent Mailhol @ 2026-07-14 18:18 UTC (permalink / raw)
To: Greg Kroah-Hartman, Andrew Morton
Cc: Rasmus Villemoes, Sakari Ailus, Nick Desaulniers,
Alexander Lobakin, Arnd Bergmann, David Sterba, Ivo van Doorn,
Alexey Dobriyan, linux-kernel, Vincent Mailhol
container_of() uses the construct below:
((type *)0)->member
to retrieve the type of the structure's member and then ensure that it
matches the type of the given pointer.
This construct being rather difficult to understand, the typeof_member()
macro was created to encapsulate it and give it a descriptive name.
Apply typeof_member() to container_of() to make it easier to read and
understand what this macro does.
Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
include/linux/container_of.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/container_of.h b/include/linux/container_of.h
index 1f6ebf27d962..28500a62ab7e 100644
--- a/include/linux/container_of.h
+++ b/include/linux/container_of.h
@@ -18,7 +18,7 @@
*/
#define container_of(ptr, type, member) ({ \
void *__mptr = (void *)(ptr); \
- static_assert(__same_type(*(ptr), ((type *)0)->member) || \
+ static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
__same_type(*(ptr), void), \
"pointer type mismatch in container_of()"); \
((type *)(__mptr - offsetof(type, member))); })
--
2.54.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] container_of: remove useless pair of parentheses
2026-07-14 18:18 [PATCH 0/3] container_of: refactors Vincent Mailhol
2026-07-14 18:18 ` [PATCH 1/3] container_of: apply typeof_member() to container_of() Vincent Mailhol
@ 2026-07-14 18:18 ` Vincent Mailhol
2026-07-15 14:30 ` Alexey Dobriyan
2026-07-14 18:18 ` [PATCH 3/3] container_of: remove local __mptr variable Vincent Mailhol
2026-07-15 4:54 ` [PATCH 0/3] container_of: refactors Greg Kroah-Hartman
3 siblings, 1 reply; 11+ messages in thread
From: Vincent Mailhol @ 2026-07-14 18:18 UTC (permalink / raw)
To: Greg Kroah-Hartman, Andrew Morton
Cc: Rasmus Villemoes, Sakari Ailus, Nick Desaulniers,
Alexander Lobakin, Arnd Bergmann, David Sterba, Ivo van Doorn,
Alexey Dobriyan, linux-kernel, Vincent Mailhol
The last expression in container_of() doesn't need an extra pair of
parenthesis. Remove it.
Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
include/linux/container_of.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/container_of.h b/include/linux/container_of.h
index 28500a62ab7e..68153170db32 100644
--- a/include/linux/container_of.h
+++ b/include/linux/container_of.h
@@ -21,7 +21,7 @@
static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
__same_type(*(ptr), void), \
"pointer type mismatch in container_of()"); \
- ((type *)(__mptr - offsetof(type, member))); })
+ (type *)(__mptr - offsetof(type, member)); })
/**
* container_of_const - cast a member of a structure out to the containing
--
2.54.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] container_of: remove local __mptr variable
2026-07-14 18:18 [PATCH 0/3] container_of: refactors Vincent Mailhol
2026-07-14 18:18 ` [PATCH 1/3] container_of: apply typeof_member() to container_of() Vincent Mailhol
2026-07-14 18:18 ` [PATCH 2/3] container_of: remove useless pair of parentheses Vincent Mailhol
@ 2026-07-14 18:18 ` Vincent Mailhol
2026-07-15 4:54 ` [PATCH 0/3] container_of: refactors Greg Kroah-Hartman
3 siblings, 0 replies; 11+ messages in thread
From: Vincent Mailhol @ 2026-07-14 18:18 UTC (permalink / raw)
To: Greg Kroah-Hartman, Andrew Morton
Cc: Rasmus Villemoes, Sakari Ailus, Nick Desaulniers,
Alexander Lobakin, Arnd Bergmann, David Sterba, Ivo van Doorn,
Alexey Dobriyan, linux-kernel, Vincent Mailhol
container_of() can be called in a nested manner to retrieve the grand
parent structure as illustrated below:
struct foo {
int a;
};
struct bar {
struct foo foo;
};
#define to_foo(a_ptr) container_of(a_ptr, struct foo, a)
#define to_bar(a_ptr) container_of(to_foo(a_ptr), struct bar, foo)
The issue is that the above construct will cause __mptr, the local
variable of container_of(), to shadow itself because of the nested
call. This then triggers a warning in sparse and W=2 builds.
While this warning is benign, it still causes some overhead as proven by
below list of commits in which people made local workarounds:
- commit 7eab14de73a8 ("mdio, phy: fix -Wshadow warnings triggered by
nested container_of()")
- commit 8d8c3131248d ("clk: define to_clk_regmap() as inline
function")
- commit bfb972c5e1cb ("IB/verbs: avoid nested container_of()")
- commit 093adbcedf12 ("btrfs: switch helper macros to static inlines
in sysfs.h")
- commit c1d35dfa0f7d ("rt2x00: Fix sparse warning on nested
container_of()")
(the list is probably not exhaustive).
As a matter of fact, the local variable __mptr is only used once in
container_of(). As such, it is not strictly needed. Inline that local
__mptr variable to remove once and for all the risk of variable
shadowing when nesting container_of() and prevent people from writing
further local fixes.
Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
include/linux/container_of.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/linux/container_of.h b/include/linux/container_of.h
index 68153170db32..28db38e9ee3e 100644
--- a/include/linux/container_of.h
+++ b/include/linux/container_of.h
@@ -17,11 +17,10 @@
* Do not use container_of() in new code.
*/
#define container_of(ptr, type, member) ({ \
- void *__mptr = (void *)(ptr); \
static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
__same_type(*(ptr), void), \
"pointer type mismatch in container_of()"); \
- (type *)(__mptr - offsetof(type, member)); })
+ (type *)((void *)(ptr) - offsetof(type, member)); })
/**
* container_of_const - cast a member of a structure out to the containing
--
2.54.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] container_of: refactors
2026-07-14 18:18 [PATCH 0/3] container_of: refactors Vincent Mailhol
` (2 preceding siblings ...)
2026-07-14 18:18 ` [PATCH 3/3] container_of: remove local __mptr variable Vincent Mailhol
@ 2026-07-15 4:54 ` Greg Kroah-Hartman
2026-07-15 5:33 ` Vincent Mailhol
3 siblings, 1 reply; 11+ messages in thread
From: Greg Kroah-Hartman @ 2026-07-15 4:54 UTC (permalink / raw)
To: Vincent Mailhol
Cc: Andrew Morton, Rasmus Villemoes, Sakari Ailus, Nick Desaulniers,
Alexander Lobakin, Arnd Bergmann, David Sterba, Ivo van Doorn,
Alexey Dobriyan, linux-kernel
On Tue, Jul 14, 2026 at 08:18:00PM +0200, Vincent Mailhol wrote:
> This series refactors the container_of() function-like macro to improve
> readability and remove a sparse/W=2 shadow warning. Further details in
> each patch.
>
> While I was expecting this series to be boring and purely cosmetic, the
> bloat-o-meter stats gave some unexpected results:
>
> $ ./scripts/bloat-o-meter vmlinux7.2-rc3_before.o vmlinux7.2-rc3_after.o
> add/remove: 0/0 grow/shrink: 133/93 up/down: 5914901/-14344137 (-8429236)
> < ... 227 lines redacted >
> Total: Before=2641674349, After=2633245113, chg -0.32%
>
> (done on v7.2-rc3 with GCC 15.3.0 on an x86_64 defconfig)
>
> Upon analysis, this change in size can be tracked down to places where
> container_of() is used in combination with __builtin_constant_p().
>
> Here is a minimal reproducer:
>
> struct foo {
> int a;
> };
>
> #define to_foo(a_ptr) container_of(a_ptr, struct foo, a)
>
> int f(int *a)
> {
> return __builtin_constant_p(to_foo(a)->a) || a;
> }
>
> The assembly code before this series...:
>
> xor eax, eax
> test rdi, rdi
> setne al
> ret
>
> ...and after:
>
> mov eax, 1
> ret
>
> Link: https://godbolt.org/z/fenbGexjY
>
> __builtin_constant_p(to_foo(a)->a) evaluates to false but gives the
> optimiser the hint that pointer a is not NULL because of the
> assumption that no undefined behaviour occurs. With this, the
> expression:
>
> __builtin_constant_p(to_foo(a)->a) || a
>
> could be evaluated as true by the optimiser.
>
> But the small variation in container_of() makes it that the optimiser
> currently misses this optimisation but manages to do it after the
> simplification of patch #3 of this series.
>
> When __builtin_constant_p()'s argument is not trivially a compile time
> constant, the result of __builtin_constant_p() comes late in the
> evaluation process. And if it comes too late, after some other
> optimisations were already done, the compiler will not retry and simply
> miss these optimisations.
>
> Note that the above example is very fragile and the results shown in
> the godbolt link might not be reproducible under very small
> variations.
>
> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
"fun" thing is, clang gets this right without your change, so this only
seems to help the gcc users.
Anyway, very nice optimizations, thanks for this! I'll queue these up
later today.
greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] container_of: refactors
2026-07-15 4:54 ` [PATCH 0/3] container_of: refactors Greg Kroah-Hartman
@ 2026-07-15 5:33 ` Vincent Mailhol
0 siblings, 0 replies; 11+ messages in thread
From: Vincent Mailhol @ 2026-07-15 5:33 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Andrew Morton, Rasmus Villemoes, Sakari Ailus, Nick Desaulniers,
Alexander Lobakin, Arnd Bergmann, David Sterba, Ivo van Doorn,
Alexey Dobriyan, linux-kernel
On 15/07/2026 at 06:54, Greg Kroah-Hartman wrote:
> On Tue, Jul 14, 2026 at 08:18:00PM +0200, Vincent Mailhol wrote:
>> This series refactors the container_of() function-like macro to improve
>> readability and remove a sparse/W=2 shadow warning. Further details in
>> each patch.
>>
>> While I was expecting this series to be boring and purely cosmetic, the
>> bloat-o-meter stats gave some unexpected results:
>>
>> $ ./scripts/bloat-o-meter vmlinux7.2-rc3_before.o vmlinux7.2-rc3_after.o
>> add/remove: 0/0 grow/shrink: 133/93 up/down: 5914901/-14344137 (-8429236)
>> < ... 227 lines redacted >
>> Total: Before=2641674349, After=2633245113, chg -0.32%
>>
>> (done on v7.2-rc3 with GCC 15.3.0 on an x86_64 defconfig)
>>
>> Upon analysis, this change in size can be tracked down to places where
>> container_of() is used in combination with __builtin_constant_p().
>>
>> Here is a minimal reproducer:
>>
>> struct foo {
>> int a;
>> };
>>
>> #define to_foo(a_ptr) container_of(a_ptr, struct foo, a)
>>
>> int f(int *a)
>> {
>> return __builtin_constant_p(to_foo(a)->a) || a;
>> }
>>
>> The assembly code before this series...:
>>
>> xor eax, eax
>> test rdi, rdi
>> setne al
>> ret
>>
>> ...and after:
>>
>> mov eax, 1
>> ret
>>
>> Link: https://godbolt.org/z/fenbGexjY
>>
>> __builtin_constant_p(to_foo(a)->a) evaluates to false but gives the
>> optimiser the hint that pointer a is not NULL because of the
>> assumption that no undefined behaviour occurs. With this, the
>> expression:
>>
>> __builtin_constant_p(to_foo(a)->a) || a
>>
>> could be evaluated as true by the optimiser.
>>
>> But the small variation in container_of() makes it that the optimiser
>> currently misses this optimisation but manages to do it after the
>> simplification of patch #3 of this series.
>>
>> When __builtin_constant_p()'s argument is not trivially a compile time
>> constant, the result of __builtin_constant_p() comes late in the
>> evaluation process. And if it comes too late, after some other
>> optimisations were already done, the compiler will not retry and simply
>> miss these optimisations.
>>
>> Note that the above example is very fragile and the results shown in
>> the godbolt link might not be reproducible under very small
>> variations.
>>
>> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
>
> "fun" thing is, clang gets this right without your change, so this only
> seems to help the gcc users.
Indeed. I forgot to mention it, but I also observed that clang is not
impacted by these weird __builtin_constant_p() intricacies.
One lesson learned it that in GCC __builtin_constant_p() can become an
optimisation killer.
> Anyway, very nice optimizations, thanks for this! I'll queue these up
> later today.
Thanks!
Yours sincerely,
Vincent Mailhol
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] container_of: remove useless pair of parentheses
2026-07-14 18:18 ` [PATCH 2/3] container_of: remove useless pair of parentheses Vincent Mailhol
@ 2026-07-15 14:30 ` Alexey Dobriyan
2026-07-15 14:51 ` Vincent Mailhol
0 siblings, 1 reply; 11+ messages in thread
From: Alexey Dobriyan @ 2026-07-15 14:30 UTC (permalink / raw)
To: Vincent Mailhol
Cc: Greg Kroah-Hartman, Andrew Morton, Rasmus Villemoes, Sakari Ailus,
Nick Desaulniers, Alexander Lobakin, Arnd Bergmann, David Sterba,
Ivo van Doorn, linux-kernel
On Tue, Jul 14, 2026 at 08:18:02PM +0200, Vincent Mailhol wrote:
> The last expression in container_of() doesn't need an extra pair of
> parenthesis. Remove it.
>
> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
> ---
> include/linux/container_of.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/container_of.h b/include/linux/container_of.h
> index 28500a62ab7e..68153170db32 100644
> --- a/include/linux/container_of.h
> +++ b/include/linux/container_of.h
> @@ -21,7 +21,7 @@
> static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
> __same_type(*(ptr), void), \
> "pointer type mismatch in container_of()"); \
> - ((type *)(__mptr - offsetof(type, member))); })
> + (type *)(__mptr - offsetof(type, member)); })
container_of_const() has the same problem. And it should parenthesize
first argument to _Generic.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] container_of: remove useless pair of parentheses
2026-07-15 14:30 ` Alexey Dobriyan
@ 2026-07-15 14:51 ` Vincent Mailhol
2026-07-15 14:57 ` Greg Kroah-Hartman
0 siblings, 1 reply; 11+ messages in thread
From: Vincent Mailhol @ 2026-07-15 14:51 UTC (permalink / raw)
To: Alexey Dobriyan
Cc: Greg Kroah-Hartman, Andrew Morton, Rasmus Villemoes, Sakari Ailus,
Nick Desaulniers, Alexander Lobakin, Arnd Bergmann, David Sterba,
Ivo van Doorn, linux-kernel
On 15/07/2026 at 16:30, Alexey Dobriyan wrote:
> On Tue, Jul 14, 2026 at 08:18:02PM +0200, Vincent Mailhol wrote:
>> The last expression in container_of() doesn't need an extra pair of
>> parenthesis. Remove it.
>>
>> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
>> ---
>> include/linux/container_of.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/container_of.h b/include/linux/container_of.h
>> index 28500a62ab7e..68153170db32 100644
>> --- a/include/linux/container_of.h
>> +++ b/include/linux/container_of.h
>> @@ -21,7 +21,7 @@
>> static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
>> __same_type(*(ptr), void), \
>> "pointer type mismatch in container_of()"); \
>> - ((type *)(__mptr - offsetof(type, member))); })
>> + (type *)(__mptr - offsetof(type, member)); })
>
> container_of_const() has the same problem.
Yes. But scoped I my series to the container_of() macro only.
I don't plan to send a follow-up patch to remove the other useless
parenthesis. The things which really bothered me was the __mptr
variable which I removed in patch #3. And with that, I am done with
what I wanted to contribute in this file.
But you are welcome to do this clean-up if you feel the need for it.
> And it should parenthesize first argument to _Generic.
Do you mean like this:
#define container_of_const(ptr, type, member) \
_Generic((ptr), \
const typeof(*(ptr)) *: ((const type *)container_of(ptr, type, member)),\
default: ((type *)container_of(ptr, type, member)) \
)
?
Why would parenthesis be needed here? I don't see what kind of
expression would make the current implementation unsafe.
Yours sincerely,
Vincent Mailhol
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] container_of: remove useless pair of parentheses
2026-07-15 14:51 ` Vincent Mailhol
@ 2026-07-15 14:57 ` Greg Kroah-Hartman
2026-07-15 16:27 ` Vincent Mailhol
0 siblings, 1 reply; 11+ messages in thread
From: Greg Kroah-Hartman @ 2026-07-15 14:57 UTC (permalink / raw)
To: Vincent Mailhol
Cc: Alexey Dobriyan, Andrew Morton, Rasmus Villemoes, Sakari Ailus,
Nick Desaulniers, Alexander Lobakin, Arnd Bergmann, David Sterba,
Ivo van Doorn, linux-kernel
On Wed, Jul 15, 2026 at 04:51:28PM +0200, Vincent Mailhol wrote:
> On 15/07/2026 at 16:30, Alexey Dobriyan wrote:
> > On Tue, Jul 14, 2026 at 08:18:02PM +0200, Vincent Mailhol wrote:
> >> The last expression in container_of() doesn't need an extra pair of
> >> parenthesis. Remove it.
> >>
> >> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
> >> ---
> >> include/linux/container_of.h | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/include/linux/container_of.h b/include/linux/container_of.h
> >> index 28500a62ab7e..68153170db32 100644
> >> --- a/include/linux/container_of.h
> >> +++ b/include/linux/container_of.h
> >> @@ -21,7 +21,7 @@
> >> static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
> >> __same_type(*(ptr), void), \
> >> "pointer type mismatch in container_of()"); \
> >> - ((type *)(__mptr - offsetof(type, member))); })
> >> + (type *)(__mptr - offsetof(type, member)); })
> >
> > container_of_const() has the same problem.
>
> Yes. But scoped I my series to the container_of() macro only.
>
> I don't plan to send a follow-up patch to remove the other useless
> parenthesis. The things which really bothered me was the __mptr
> variable which I removed in patch #3. And with that, I am done with
> what I wanted to contribute in this file.
>
> But you are welcome to do this clean-up if you feel the need for it.
>
> > And it should parenthesize first argument to _Generic.
>
> Do you mean like this:
>
> #define container_of_const(ptr, type, member) \
> _Generic((ptr), \
> const typeof(*(ptr)) *: ((const type *)container_of(ptr, type, member)),\
> default: ((type *)container_of(ptr, type, member)) \
> )
>
> ?
>
> Why would parenthesis be needed here? I don't see what kind of
> expression would make the current implementation unsafe.
It took me a while working with container_of_const() to get it right
with the (), so be careful removing any.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] container_of: remove useless pair of parentheses
2026-07-15 14:57 ` Greg Kroah-Hartman
@ 2026-07-15 16:27 ` Vincent Mailhol
2026-07-16 14:09 ` Greg Kroah-Hartman
0 siblings, 1 reply; 11+ messages in thread
From: Vincent Mailhol @ 2026-07-15 16:27 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Alexey Dobriyan, Andrew Morton, Rasmus Villemoes, Sakari Ailus,
Nick Desaulniers, Alexander Lobakin, Arnd Bergmann, David Sterba,
Ivo van Doorn, linux-kernel
On 15/07/2026 at 16:57, Greg Kroah-Hartman wrote:
> On Wed, Jul 15, 2026 at 04:51:28PM +0200, Vincent Mailhol wrote:
>> On 15/07/2026 at 16:30, Alexey Dobriyan wrote:
>>> On Tue, Jul 14, 2026 at 08:18:02PM +0200, Vincent Mailhol wrote:
>>>> The last expression in container_of() doesn't need an extra pair of
>>>> parenthesis. Remove it.
>>>>
>>>> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
>>>> ---
>>>> include/linux/container_of.h | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/linux/container_of.h b/include/linux/container_of.h
>>>> index 28500a62ab7e..68153170db32 100644
>>>> --- a/include/linux/container_of.h
>>>> +++ b/include/linux/container_of.h
>>>> @@ -21,7 +21,7 @@
>>>> static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
>>>> __same_type(*(ptr), void), \
>>>> "pointer type mismatch in container_of()"); \
>>>> - ((type *)(__mptr - offsetof(type, member))); })
>>>> + (type *)(__mptr - offsetof(type, member)); })
>>>
>>> container_of_const() has the same problem.
>>
>> Yes. But scoped I my series to the container_of() macro only.
>>
>> I don't plan to send a follow-up patch to remove the other useless
>> parenthesis. The things which really bothered me was the __mptr
>> variable which I removed in patch #3. And with that, I am done with
>> what I wanted to contribute in this file.
>>
>> But you are welcome to do this clean-up if you feel the need for it.
>>
>>> And it should parenthesize first argument to _Generic.
>>
>> Do you mean like this:
>>
>> #define container_of_const(ptr, type, member) \
>> _Generic((ptr), \
>> const typeof(*(ptr)) *: ((const type *)container_of(ptr, type, member)),\
>> default: ((type *)container_of(ptr, type, member)) \
>> )
>>
>> ?
>>
>> Why would parenthesis be needed here? I don't see what kind of
>> expression would make the current implementation unsafe.
>
> It took me a while working with container_of_const() to get it right
> with the (), so be careful removing any.
I just tested this patch:
---8<---
diff --git a/include/linux/container_of.h b/include/linux/container_of.h
index 1f6ebf27d962..03d5b91534a7 100644
--- a/include/linux/container_of.h
+++ b/include/linux/container_of.h
@@ -34,8 +34,8 @@
*/
#define container_of_const(ptr, type, member) \
_Generic(ptr, \
- const typeof(*(ptr)) *: ((const type *)container_of(ptr, type, member)),\
- default: ((type *)container_of(ptr, type, member)) \
+ const typeof(*(ptr)) *: (const type *)container_of(ptr, type, member),\
+ default: (type *)container_of(ptr, type, member) \
)
#endif /* _LINUX_CONTAINER_OF_H */
---8<---
I could compile and boot it, no problem (I am actually writing this
message from a machine on which this patch is applied).
So, yes, I believe that these two pairs of parenthesis can be removed.
Just not sure if it is worth it. For container_of(), the parenthesis
removal was part of a series with more meaningful changes. I don't
feel exited of sending another series to just remove the
container_of_const() parenthesis.
Yours sincerely,
Vincent Mailhol
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] container_of: remove useless pair of parentheses
2026-07-15 16:27 ` Vincent Mailhol
@ 2026-07-16 14:09 ` Greg Kroah-Hartman
0 siblings, 0 replies; 11+ messages in thread
From: Greg Kroah-Hartman @ 2026-07-16 14:09 UTC (permalink / raw)
To: Vincent Mailhol
Cc: Alexey Dobriyan, Andrew Morton, Rasmus Villemoes, Sakari Ailus,
Nick Desaulniers, Alexander Lobakin, Arnd Bergmann, David Sterba,
Ivo van Doorn, linux-kernel
On Wed, Jul 15, 2026 at 06:27:31PM +0200, Vincent Mailhol wrote:
> On 15/07/2026 at 16:57, Greg Kroah-Hartman wrote:
> > On Wed, Jul 15, 2026 at 04:51:28PM +0200, Vincent Mailhol wrote:
> >> On 15/07/2026 at 16:30, Alexey Dobriyan wrote:
> >>> On Tue, Jul 14, 2026 at 08:18:02PM +0200, Vincent Mailhol wrote:
> >>>> The last expression in container_of() doesn't need an extra pair of
> >>>> parenthesis. Remove it.
> >>>>
> >>>> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
> >>>> ---
> >>>> include/linux/container_of.h | 2 +-
> >>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/include/linux/container_of.h b/include/linux/container_of.h
> >>>> index 28500a62ab7e..68153170db32 100644
> >>>> --- a/include/linux/container_of.h
> >>>> +++ b/include/linux/container_of.h
> >>>> @@ -21,7 +21,7 @@
> >>>> static_assert(__same_type(*(ptr), typeof_member(type, member)) || \
> >>>> __same_type(*(ptr), void), \
> >>>> "pointer type mismatch in container_of()"); \
> >>>> - ((type *)(__mptr - offsetof(type, member))); })
> >>>> + (type *)(__mptr - offsetof(type, member)); })
> >>>
> >>> container_of_const() has the same problem.
> >>
> >> Yes. But scoped I my series to the container_of() macro only.
> >>
> >> I don't plan to send a follow-up patch to remove the other useless
> >> parenthesis. The things which really bothered me was the __mptr
> >> variable which I removed in patch #3. And with that, I am done with
> >> what I wanted to contribute in this file.
> >>
> >> But you are welcome to do this clean-up if you feel the need for it.
> >>
> >>> And it should parenthesize first argument to _Generic.
> >>
> >> Do you mean like this:
> >>
> >> #define container_of_const(ptr, type, member) \
> >> _Generic((ptr), \
> >> const typeof(*(ptr)) *: ((const type *)container_of(ptr, type, member)),\
> >> default: ((type *)container_of(ptr, type, member)) \
> >> )
> >>
> >> ?
> >>
> >> Why would parenthesis be needed here? I don't see what kind of
> >> expression would make the current implementation unsafe.
> >
> > It took me a while working with container_of_const() to get it right
> > with the (), so be careful removing any.
>
> I just tested this patch:
>
> ---8<---
> diff --git a/include/linux/container_of.h b/include/linux/container_of.h
> index 1f6ebf27d962..03d5b91534a7 100644
> --- a/include/linux/container_of.h
> +++ b/include/linux/container_of.h
> @@ -34,8 +34,8 @@
> */
> #define container_of_const(ptr, type, member) \
> _Generic(ptr, \
> - const typeof(*(ptr)) *: ((const type *)container_of(ptr, type, member)),\
> - default: ((type *)container_of(ptr, type, member)) \
> + const typeof(*(ptr)) *: (const type *)container_of(ptr, type, member),\
> + default: (type *)container_of(ptr, type, member) \
> )
> #endif /* _LINUX_CONTAINER_OF_H */
> ---8<---
>
> I could compile and boot it, no problem (I am actually writing this
> message from a machine on which this patch is applied).
>
> So, yes, I believe that these two pairs of parenthesis can be removed.
> Just not sure if it is worth it. For container_of(), the parenthesis
> removal was part of a series with more meaningful changes. I don't
> feel exited of sending another series to just remove the
> container_of_const() parenthesis.
Yeah, if it's not hurting anything, let's just leave it as-is for now.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-07-16 14:35 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-14 18:18 [PATCH 0/3] container_of: refactors Vincent Mailhol
2026-07-14 18:18 ` [PATCH 1/3] container_of: apply typeof_member() to container_of() Vincent Mailhol
2026-07-14 18:18 ` [PATCH 2/3] container_of: remove useless pair of parentheses Vincent Mailhol
2026-07-15 14:30 ` Alexey Dobriyan
2026-07-15 14:51 ` Vincent Mailhol
2026-07-15 14:57 ` Greg Kroah-Hartman
2026-07-15 16:27 ` Vincent Mailhol
2026-07-16 14:09 ` Greg Kroah-Hartman
2026-07-14 18:18 ` [PATCH 3/3] container_of: remove local __mptr variable Vincent Mailhol
2026-07-15 4:54 ` [PATCH 0/3] container_of: refactors Greg Kroah-Hartman
2026-07-15 5:33 ` Vincent Mailhol
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox