mm/mempolicy: Fix weighted interleave auto sysfs name

[PATCH] mm/mempolicy: Fix weighted interleave auto sysfs name

Posted by Joshua Hahn 2 months, 2 weeks ago

The __ATTR macro is a utility that makes defining kobj_attributes easier
by stringfying the name, verifying the mode, and setting the show/store
fields in a single initializer. It takes a raw token as the first value,
rather than a string, so that __ATTR family macros like __ATTR_RW can
token-paste it for inferring the _show / _store function names.

Commit e341f9c3c841 ("mm/mempolicy: Weighted Interleave Auto-tuning")
used the __ATTR macro to define the "auto" sysfs for weighted
interleave. A few months later, commit 2fb6915fa22d ("compiler_types.h:
add "auto" as a macro for "__auto_type"") introduced a #define macro
which expanded auto into __auto_type.

This led to the "auto" token passed into __ATTR to be expanded out into
__auto_type, and the sysfs entry to be displayed as __auto_type as well.

Expand out the __ATTR macro and directly pass a string "auto" instead of
the raw token 'auto' to prevent it from being expanded out. Also bypass
the VERIFY_OCTAL_PERMISSIONS check by triple checking that 0664 is
indeed the intended permissions for this sysfs file.

Before:
$ ls /sys/kernel/mm/mempolicy/weighted_interleave
__auto_type  node0

After:
$ ls /sys/kernel/mm/mempolicy/weighted_interleave/
auto  node0

Based on latest mm-new: 96881c429af1

Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
---
 mm/mempolicy.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 62108a5b74c4e..845458a3c18f4 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -3782,9 +3782,11 @@ static void wi_state_free(void)
 	}
 }
 
-static struct kobj_attribute wi_auto_attr =
-	__ATTR(auto, 0664, weighted_interleave_auto_show,
-			   weighted_interleave_auto_store);
+static struct kobj_attribute wi_auto_attr = {
+	.attr = { .name = "auto", .mode = 0664 },
+	.show = weighted_interleave_auto_show,
+	.store = weighted_interleave_auto_store,
+};
 
 static void wi_cleanup(void) {
 	sysfs_remove_file(&wi_group->wi_kobj, &wi_auto_attr.attr);
-- 
2.52.0

Re: [PATCH] mm/mempolicy: Fix weighted interleave auto sysfs name

Posted by Zi Yan 2 months, 1 week ago

On 7 Apr 2026, at 10:14, Joshua Hahn wrote:

> The __ATTR macro is a utility that makes defining kobj_attributes easier
> by stringfying the name, verifying the mode, and setting the show/store
> fields in a single initializer. It takes a raw token as the first value,
> rather than a string, so that __ATTR family macros like __ATTR_RW can
> token-paste it for inferring the _show / _store function names.
>
> Commit e341f9c3c841 ("mm/mempolicy: Weighted Interleave Auto-tuning")
> used the __ATTR macro to define the "auto" sysfs for weighted
> interleave. A few months later, commit 2fb6915fa22d ("compiler_types.h:
> add "auto" as a macro for "__auto_type"") introduced a #define macro
> which expanded auto into __auto_type.
>
> This led to the "auto" token passed into __ATTR to be expanded out into
> __auto_type, and the sysfs entry to be displayed as __auto_type as well.
>
> Expand out the __ATTR macro and directly pass a string "auto" instead of
> the raw token 'auto' to prevent it from being expanded out. Also bypass
> the VERIFY_OCTAL_PERMISSIONS check by triple checking that 0664 is
> indeed the intended permissions for this sysfs file.
>
> Before:
> $ ls /sys/kernel/mm/mempolicy/weighted_interleave
> __auto_type  node0
>
> After:
> $ ls /sys/kernel/mm/mempolicy/weighted_interleave/
> auto  node0
>
> Based on latest mm-new: 96881c429af1
>
> Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> ---
>  mm/mempolicy.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>

Acked-by: Zi Yan <ziy@nvidia.com>

Best Regards,
Yan, Zi

Re: [PATCH] mm/mempolicy: Fix weighted interleave auto sysfs name

Posted by David Hildenbrand (Arm) 2 months, 1 week ago

On 4/7/26 16:14, Joshua Hahn wrote:
> The __ATTR macro is a utility that makes defining kobj_attributes easier
> by stringfying the name, verifying the mode, and setting the show/store
> fields in a single initializer. It takes a raw token as the first value,
> rather than a string, so that __ATTR family macros like __ATTR_RW can
> token-paste it for inferring the _show / _store function names.
> 
> Commit e341f9c3c841 ("mm/mempolicy: Weighted Interleave Auto-tuning")
> used the __ATTR macro to define the "auto" sysfs for weighted
> interleave. A few months later, commit 2fb6915fa22d ("compiler_types.h:
> add "auto" as a macro for "__auto_type"") introduced a #define macro
> which expanded auto into __auto_type.
> 
> This led to the "auto" token passed into __ATTR to be expanded out into
> __auto_type, and the sysfs entry to be displayed as __auto_type as well.
> 
> Expand out the __ATTR macro and directly pass a string "auto" instead of
> the raw token 'auto' to prevent it from being expanded out. Also bypass
> the VERIFY_OCTAL_PERMISSIONS check by triple checking that 0664 is
> indeed the intended permissions for this sysfs file.
> 
> Before:
> $ ls /sys/kernel/mm/mempolicy/weighted_interleave
> __auto_type  node0
> 
> After:
> $ ls /sys/kernel/mm/mempolicy/weighted_interleave/
> auto  node0
> 
> Based on latest mm-new: 96881c429af1
> 
> Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> ---

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

Re: [PATCH] mm/mempolicy: Fix weighted interleave auto sysfs name

Posted by Rakie Kim 2 months, 1 week ago

On Tue,  7 Apr 2026 07:14:14 -0700 Joshua Hahn <joshua.hahnjy@gmail.com> wrote:
> The __ATTR macro is a utility that makes defining kobj_attributes easier
> by stringfying the name, verifying the mode, and setting the show/store
> fields in a single initializer. It takes a raw token as the first value,
> rather than a string, so that __ATTR family macros like __ATTR_RW can
> token-paste it for inferring the _show / _store function names.
> 
> Commit e341f9c3c841 ("mm/mempolicy: Weighted Interleave Auto-tuning")
> used the __ATTR macro to define the "auto" sysfs for weighted
> interleave. A few months later, commit 2fb6915fa22d ("compiler_types.h:
> add "auto" as a macro for "__auto_type"") introduced a #define macro
> which expanded auto into __auto_type.
> 
> This led to the "auto" token passed into __ATTR to be expanded out into
> __auto_type, and the sysfs entry to be displayed as __auto_type as well.
> 
> Expand out the __ATTR macro and directly pass a string "auto" instead of
> the raw token 'auto' to prevent it from being expanded out. Also bypass
> the VERIFY_OCTAL_PERMISSIONS check by triple checking that 0664 is
> indeed the intended permissions for this sysfs file.
> 
> Before:
> $ ls /sys/kernel/mm/mempolicy/weighted_interleave
> __auto_type  node0
> 
> After:
> $ ls /sys/kernel/mm/mempolicy/weighted_interleave/
> auto  node0

Hello Joshua,

This was indeed an unfortunate and tricky issue to track down, given
that it was caused by an unexpected global macro expansion. great catch,
and thanks for the quick response and clean fix.

Reviewed-by: Rakie Kim <rakie.kim@sk.com>

Re: [PATCH] mm/mempolicy: Fix weighted interleave auto sysfs name

Posted by Gregory Price 2 months, 2 weeks ago

On Tue, Apr 07, 2026 at 07:14:14AM -0700, Joshua Hahn wrote:
> The __ATTR macro is a utility that makes defining kobj_attributes easier
> by stringfying the name, verifying the mode, and setting the show/store
> fields in a single initializer. It takes a raw token as the first value,
> rather than a string, so that __ATTR family macros like __ATTR_RW can
> token-paste it for inferring the _show / _store function names.
> 
> Commit e341f9c3c841 ("mm/mempolicy: Weighted Interleave Auto-tuning")
> used the __ATTR macro to define the "auto" sysfs for weighted
> interleave. A few months later, commit 2fb6915fa22d ("compiler_types.h:
> add "auto" as a macro for "__auto_type"") introduced a #define macro
> which expanded auto into __auto_type.
> 

Well that's really unfortunately.

Is this really the only "auto" in the entire sysfs structure?

> This led to the "auto" token passed into __ATTR to be expanded out into
> __auto_type, and the sysfs entry to be displayed as __auto_type as well.
> 
> Expand out the __ATTR macro and directly pass a string "auto" instead of
> the raw token 'auto' to prevent it from being expanded out. Also bypass
> the VERIFY_OCTAL_PERMISSIONS check by triple checking that 0664 is
> indeed the intended permissions for this sysfs file.
> 
> Before:
> $ ls /sys/kernel/mm/mempolicy/weighted_interleave
> __auto_type  node0
> 
> After:
> $ ls /sys/kernel/mm/mempolicy/weighted_interleave/
> auto  node0
> 
> Based on latest mm-new: 96881c429af1
> 

> Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>

Maybe - 

Fixes: 2fb6915fa22d ("compiler_types.h: add "auto" as a macro for "__auto_type"")


Reviewed-by: Gregory Price <gourry@gourry.net>

~Gregory

Re: [PATCH] mm/mempolicy: Fix weighted interleave auto sysfs name

Posted by Joshua Hahn 2 months, 2 weeks ago

On Tue, 7 Apr 2026 13:09:04 -0400 Gregory Price <gourry@gourry.net> wrote:

> On Tue, Apr 07, 2026 at 07:14:14AM -0700, Joshua Hahn wrote:
> > The __ATTR macro is a utility that makes defining kobj_attributes easier
> > by stringfying the name, verifying the mode, and setting the show/store
> > fields in a single initializer. It takes a raw token as the first value,
> > rather than a string, so that __ATTR family macros like __ATTR_RW can
> > token-paste it for inferring the _show / _store function names.
> > 
> > Commit e341f9c3c841 ("mm/mempolicy: Weighted Interleave Auto-tuning")
> > used the __ATTR macro to define the "auto" sysfs for weighted
> > interleave. A few months later, commit 2fb6915fa22d ("compiler_types.h:
> > add "auto" as a macro for "__auto_type"") introduced a #define macro
> > which expanded auto into __auto_type.
> > 
> 

Hello Gregory, thanks for reviewing my patch!!

> Well that's really unfortunately.
> 
> Is this really the only "auto" in the entire sysfs structure?

Thankfully, it seems so:

$ rg "__ATTR\(auto"

mm/mempolicy.c
3786:   __ATTR(auto, 0664, weighted_interleave_auto_show,

arch/powerpc/platforms/pseries/power.c
44:     __ATTR(auto_poweron, 0644, auto_poweron_show, auto_poweron_store);

Although maybe it is more valuable to check any __ATTR family macro
for a macro as the first parameter.  In hindsight, I should have been more
wary of passing a keyword as a token.

[...snip...]

> > Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> 
> Maybe - 
> 
> Fixes: 2fb6915fa22d ("compiler_types.h: add "auto" as a macro for "__auto_type"")

I thought about adding a Fixes tag, decided against it because it's
purely cosmetic.

... but now that I think about it, I think this could break some workflows
for users who have automated writing to the auto sysfs file.

Maybe I should add a fixes tag for the patch that introduced the 
sysfs file instead? That is,

Fixes: e341f9c3c841 ("mm/mempolicy: Weighted Interleave Auto-tuning")

> Reviewed-by: Gregory Price <gourry@gourry.net>

Thanks again, Gregory!! I hope you have a great day : -)
Joshua

Re: [PATCH] mm/mempolicy: Fix weighted interleave auto sysfs name

Posted by Huang, Ying 2 months, 1 week ago

Joshua Hahn <joshua.hahnjy@gmail.com> writes:

> On Tue, 7 Apr 2026 13:09:04 -0400 Gregory Price <gourry@gourry.net> wrote:
>
>> On Tue, Apr 07, 2026 at 07:14:14AM -0700, Joshua Hahn wrote:
>> > The __ATTR macro is a utility that makes defining kobj_attributes easier
>> > by stringfying the name, verifying the mode, and setting the show/store
>> > fields in a single initializer. It takes a raw token as the first value,
>> > rather than a string, so that __ATTR family macros like __ATTR_RW can
>> > token-paste it for inferring the _show / _store function names.
>> > 
>> > Commit e341f9c3c841 ("mm/mempolicy: Weighted Interleave Auto-tuning")
>> > used the __ATTR macro to define the "auto" sysfs for weighted
>> > interleave. A few months later, commit 2fb6915fa22d ("compiler_types.h:
>> > add "auto" as a macro for "__auto_type"") introduced a #define macro
>> > which expanded auto into __auto_type.
>> > 
>> 
>
> Hello Gregory, thanks for reviewing my patch!!
>
>> Well that's really unfortunately.
>> 
>> Is this really the only "auto" in the entire sysfs structure?
>
> Thankfully, it seems so:
>
> $ rg "__ATTR\(auto"
>
> mm/mempolicy.c
> 3786:   __ATTR(auto, 0664, weighted_interleave_auto_show,
>
> arch/powerpc/platforms/pseries/power.c
> 44:     __ATTR(auto_poweron, 0644, auto_poweron_show, auto_poweron_store);
>
> Although maybe it is more valuable to check any __ATTR family macro
> for a macro as the first parameter.  In hindsight, I should have been more
> wary of passing a keyword as a token.
>
> [...snip...]
>
>> > Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
>> 
>> Maybe - 
>> 
>> Fixes: 2fb6915fa22d ("compiler_types.h: add "auto" as a macro for "__auto_type"")
>
> I thought about adding a Fixes tag, decided against it because it's
> purely cosmetic.
>
> ... but now that I think about it, I think this could break some workflows
> for users who have automated writing to the auto sysfs file.

Fixes tag helps people to decide whether to backport the commit.  So, I
think that we need it anyway.  Feel free to add

Acked-by: Huang Ying <ying.huang@linux.alibaba.com>

in the future versions.

[snip]

---
Best Regards,
Huang, Ying

Re: [PATCH] mm/mempolicy: Fix weighted interleave auto sysfs name

Posted by Gregory Price 2 months, 2 weeks ago

On Tue, Apr 07, 2026 at 10:23:50AM -0700, Joshua Hahn wrote:
> On Tue, 7 Apr 2026 13:09:04 -0400 Gregory Price <gourry@gourry.net> wrote:
> 
> > > Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> > 
> > Maybe - 
> > 
> > Fixes: 2fb6915fa22d ("compiler_types.h: add "auto" as a macro for "__auto_type"")
> 
> I thought about adding a Fixes tag, decided against it because it's
> purely cosmetic.
> 
> ... but now that I think about it, I think this could break some workflows
> for users who have automated writing to the auto sysfs file.
>

Yes, it broke userland software depending on that symbol being there
(which, hopefully, is very little at this point in time).

> Maybe I should add a fixes tag for the patch that introduced the 
> sysfs file instead? That is,
> 
> Fixes: e341f9c3c841 ("mm/mempolicy: Weighted Interleave Auto-tuning")
>

The original commit introduced 'auto' and the compiler_types.h commit
modified that to "__auto_type".  The latter broke userland so that's the
right commit to mark for Fixes.

~Gregory