Re: Fwd: sysfs: cannot create duplicate filename .../system76_acpi::kbd_backlight/color

Re: Fwd: sysfs: cannot create duplicate filename .../system76_acpi::kbd_backlight/color

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 4637 bytes --]

On Thu, Oct 26, 2023 at 02:55:06PM +0700, Bagas Sanjaya wrote:
> Hi,
> 
> I notice a regression report on Bugzilla [1]. Quoting from it:
> 
> > Loading the system76-acpi kernel module fails on linux 6.6-rc7. This does not seem to be an issue with system76-acpi itself, because reverting commit #5d36931f0fe51665c04f56c027613d22e6a03411, which is the only change made to this driver across the 6.6 development cycle, does not fix the issue. On 6.5.8, everything works fine. My hardware is a Clevo-based Alder Lake laptop running coreboot, roughly similar to the System76 Darter Pro 8.
> > 
> > backtrace:
> > [  266.399036] sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/17761776:00/leds/system76_acpi::kbd_backlight/color'
> > [  266.399045] CPU: 1 PID: 2896 Comm: modprobe Not tainted 6.6.0-rc7 #1
> > [  266.399050] Hardware name: Notebook NS5x_NS7xPU/NS5x_NS7xPU, BIOS Dasharo (coreboot+UEFI) v1.6.0 03/30/2023
> > [  266.399053] Call Trace:
> > [  266.399057]  <TASK>
> > [  266.399063]  dump_stack_lvl+0x36/0x50
> > [  266.399080]  sysfs_warn_dup+0x5a/0x70
> > [  266.399088]  sysfs_add_file_mode_ns+0x11a/0x130
> > [  266.399094]  internal_create_group+0x125/0x3b0
> > [  266.399101]  internal_create_groups+0x42/0xa0
> > [  266.399107]  device_add+0x5b1/0x8a0
> > [  266.399113]  ? kstrdup+0x4c/0x70
> > [  266.399119]  device_create_groups_vargs+0xce/0xf0
> > [  266.399124]  device_create_with_groups+0x4b/0x70
> > [  266.399129]  led_classdev_register_ext+0x1d2/0x470 [led_class]
> > [  266.399149]  ? devm_led_classdev_register_ext+0x3a/0x90 [led_class]
> > [  266.399162]  devm_led_classdev_register_ext+0x50/0x90 [led_class]
> > [  266.399173]  system76_add+0x18b/0x460 [system76_acpi]
> > [  266.399186]  acpi_device_probe+0x47/0x130
> > [  266.399193]  really_probe+0x19b/0x3e0
> > [  266.399199]  ? __pfx___driver_attach+0x10/0x10
> > [  266.399205]  __driver_probe_device+0x78/0x160
> > [  266.399211]  driver_probe_device+0x1f/0x90
> > [  266.399217]  __driver_attach+0xd2/0x1c0
> > [  266.399222]  bus_for_each_dev+0x85/0xd0
> > [  266.399227]  bus_add_driver+0x116/0x220
> > [  266.399233]  driver_register+0x59/0x100
> > [  266.399242]  ? __pfx_system76_driver_init+0x10/0x10 [system76_acpi]
> > [  266.399252]  do_one_initcall+0x5a/0x300
> > [  266.399260]  do_init_module+0x60/0x240
> > [  266.399267]  init_module_from_file+0x86/0xc0
> > [  266.399275]  __x64_sys_finit_module+0x18a/0x350
> > [  266.399282]  do_syscall_64+0x5d/0x90
> > [  266.399289]  ? syscall_exit_to_user_mode+0x26/0x40
> > [  266.399295]  ? do_syscall_64+0x6c/0x90
> > [  266.399300]  ? do_syscall_64+0x6c/0x90
> > [  266.399305]  entry_SYSCALL_64_after_hwframe+0x6e/0xd8
> > [  266.399314] RIP: 0033:0x7f5c11b38d7d
> > [  266.399360] Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 7b d0 0b 00 f7 d8 64 89 01 48
> > [  266.399364] RSP: 002b:00007ffe30e15b88 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
> > [  266.399370] RAX: ffffffffffffffda RBX: 000055a8d48d6c10 RCX: 00007f5c11b38d7d
> > [  266.399372] RDX: 0000000000000000 RSI: 000055a8d3077d8b RDI: 0000000000000003
> > [  266.399375] RBP: 000055a8d3077d8b R08: 00007f5c11bf6b00 R09: 00007ffe30e15bd0
> > [  266.399376] R10: 0000000000000050 R11: 0000000000000246 R12: 0000000000040000
> > [  266.399378] R13: 000055a8d48d6c90 R14: 000055a8d48d6390 R15: 000055a8d48d7090
> > [  266.399382]  </TASK>
> > [  266.399410] System76 ACPI Driver: probe of 17761776:00 failed with error -17
> 
> See Bugzilla for the full thread and attached dmesg output.
> 
> Anyway, I'm adding this regression to regzbot:
> 
> #regzbot introduced: v6.5..v6.6-rc7 https://bugzilla.kernel.org/show_bug.cgi?id=218045
> 

The reporter had narrowed down the culprit. He said on Bugzilla:

> The culprit seems to be commit c7d80059b086c4986cd994a1973ec7a5d75f8eea, which introduces a new 'color' attribute for led sysfs class devices. The problem is that the system76-acpi platform driver tries to create the exact same sysfs attribute itself for the system76_acpi::kbd_backlight device, leading to the conflict. For testing purposes, I've just rebuilt the kernel with the system76-apci color attribute renamed to kb_color, and that fixes the issue.

Jean-Jacques Hiblot, would you like to take a look on this regression,
since you authored the culprit?

Anyway, telling regzbot:

#regzbot introduced: c7d80059b086c4

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: Fwd: sysfs: cannot create duplicate filename .../system76_acpi::kbd_backlight/color

[Jean-Jacques Hiblot]

On 29/10/2023 02:48, Bagas Sanjaya wrote:
> On Thu, Oct 26, 2023 at 02:55:06PM +0700, Bagas Sanjaya wrote:
>> Hi,
>>
>> I notice a regression report on Bugzilla [1]. Quoting from it:
>>
>>> Loading the system76-acpi kernel module fails on linux 6.6-rc7. This does not seem to be an issue with system76-acpi itself, because reverting commit #5d36931f0fe51665c04f56c027613d22e6a03411, which is the only change made to this driver across the 6.6 development cycle, does not fix the issue. On 6.5.8, everything works fine. My hardware is a Clevo-based Alder Lake laptop running coreboot, roughly similar to the System76 Darter Pro 8.
>>>
>>> backtrace:
>>> [  266.399036] sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/17761776:00/leds/system76_acpi::kbd_backlight/color'
>>> [  266.399045] CPU: 1 PID: 2896 Comm: modprobe Not tainted 6.6.0-rc7 #1
>>> [  266.399050] Hardware name: Notebook NS5x_NS7xPU/NS5x_NS7xPU, BIOS Dasharo (coreboot+UEFI) v1.6.0 03/30/2023
>>> [  266.399053] Call Trace:
>>> [  266.399057]  <TASK>
>>> [  266.399063]  dump_stack_lvl+0x36/0x50
>>> [  266.399080]  sysfs_warn_dup+0x5a/0x70
>>> [  266.399088]  sysfs_add_file_mode_ns+0x11a/0x130
>>> [  266.399094]  internal_create_group+0x125/0x3b0
>>> [  266.399101]  internal_create_groups+0x42/0xa0
>>> [  266.399107]  device_add+0x5b1/0x8a0
>>> [  266.399113]  ? kstrdup+0x4c/0x70
>>> [  266.399119]  device_create_groups_vargs+0xce/0xf0
>>> [  266.399124]  device_create_with_groups+0x4b/0x70
>>> [  266.399129]  led_classdev_register_ext+0x1d2/0x470 [led_class]
>>> [  266.399149]  ? devm_led_classdev_register_ext+0x3a/0x90 [led_class]
>>> [  266.399162]  devm_led_classdev_register_ext+0x50/0x90 [led_class]
>>> [  266.399173]  system76_add+0x18b/0x460 [system76_acpi]
>>> [  266.399186]  acpi_device_probe+0x47/0x130
>>> [  266.399193]  really_probe+0x19b/0x3e0
>>> [  266.399199]  ? __pfx___driver_attach+0x10/0x10
>>> [  266.399205]  __driver_probe_device+0x78/0x160
>>> [  266.399211]  driver_probe_device+0x1f/0x90
>>> [  266.399217]  __driver_attach+0xd2/0x1c0
>>> [  266.399222]  bus_for_each_dev+0x85/0xd0
>>> [  266.399227]  bus_add_driver+0x116/0x220
>>> [  266.399233]  driver_register+0x59/0x100
>>> [  266.399242]  ? __pfx_system76_driver_init+0x10/0x10 [system76_acpi]
>>> [  266.399252]  do_one_initcall+0x5a/0x300
>>> [  266.399260]  do_init_module+0x60/0x240
>>> [  266.399267]  init_module_from_file+0x86/0xc0
>>> [  266.399275]  __x64_sys_finit_module+0x18a/0x350
>>> [  266.399282]  do_syscall_64+0x5d/0x90
>>> [  266.399289]  ? syscall_exit_to_user_mode+0x26/0x40
>>> [  266.399295]  ? do_syscall_64+0x6c/0x90
>>> [  266.399300]  ? do_syscall_64+0x6c/0x90
>>> [  266.399305]  entry_SYSCALL_64_after_hwframe+0x6e/0xd8
>>> [  266.399314] RIP: 0033:0x7f5c11b38d7d
>>> [  266.399360] Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 7b d0 0b 00 f7 d8 64 89 01 48
>>> [  266.399364] RSP: 002b:00007ffe30e15b88 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
>>> [  266.399370] RAX: ffffffffffffffda RBX: 000055a8d48d6c10 RCX: 00007f5c11b38d7d
>>> [  266.399372] RDX: 0000000000000000 RSI: 000055a8d3077d8b RDI: 0000000000000003
>>> [  266.399375] RBP: 000055a8d3077d8b R08: 00007f5c11bf6b00 R09: 00007ffe30e15bd0
>>> [  266.399376] R10: 0000000000000050 R11: 0000000000000246 R12: 0000000000040000
>>> [  266.399378] R13: 000055a8d48d6c90 R14: 000055a8d48d6390 R15: 000055a8d48d7090
>>> [  266.399382]  </TASK>
>>> [  266.399410] System76 ACPI Driver: probe of 17761776:00 failed with error -17
>>
>> See Bugzilla for the full thread and attached dmesg output.
>>
>> Anyway, I'm adding this regression to regzbot:
>>
>> #regzbot introduced: v6.5..v6.6-rc7 https://bugzilla.kernel.org/show_bug.cgi?id=218045
>>
> 
> The reporter had narrowed down the culprit. He said on Bugzilla:
> 
>> The culprit seems to be commit c7d80059b086c4986cd994a1973ec7a5d75f8eea, which introduces a new 'color' attribute for led sysfs class devices. The problem is that the system76-acpi platform driver tries to create the exact same sysfs attribute itself for the system76_acpi::kbd_backlight device, leading to the conflict. For testing purposes, I've just rebuilt the kernel with the system76-apci color attribute renamed to kb_color, and that fixes the issue.
> 
> Jean-Jacques Hiblot, would you like to take a look on this regression,
> since you authored the culprit?
Hi,

The offending commit stores the color in struct led_classdev and exposes 
it via sysfs. It was part of a series that create a RGB leds from 
multiple single-color LEDs. for this series, we need the color 
information but we don't really need to expose it it via sysfs. In order 
to fix the issue, we can remove the 'color' attribute from the sysfs.

JJ

> 
> Anyway, telling regzbot:
> 
> #regzbot introduced: c7d80059b086c4
> 
> Thanks.
> 

Re: Fwd: sysfs: cannot create duplicate filename .../system76_acpi::kbd_backlight/color

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 1241 bytes --]

On Sat, Nov 04, 2023 at 01:01:56PM +0100, Jean-Jacques Hiblot wrote:
> 
> 
> On 29/10/2023 02:48, Bagas Sanjaya wrote:
> > On Thu, Oct 26, 2023 at 02:55:06PM +0700, Bagas Sanjaya wrote:
> > > The culprit seems to be commit c7d80059b086c4986cd994a1973ec7a5d75f8eea, which introduces a new 'color' attribute for led sysfs class devices. The problem is that the system76-acpi platform driver tries to create the exact same sysfs attribute itself for the system76_acpi::kbd_backlight device, leading to the conflict. For testing purposes, I've just rebuilt the kernel with the system76-apci color attribute renamed to kb_color, and that fixes the issue.
> > 
> > Jean-Jacques Hiblot, would you like to take a look on this regression,
> > since you authored the culprit?
> Hi,
> 
> The offending commit stores the color in struct led_classdev and exposes it
> via sysfs. It was part of a series that create a RGB leds from multiple
> single-color LEDs. for this series, we need the color information but we
> don't really need to expose it it via sysfs. In order to fix the issue, we
> can remove the 'color' attribute from the sysfs.
> 

OK, see you in the patch!

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: Fwd: sysfs: cannot create duplicate filename .../system76_acpi::kbd_backlight/color

[Takashi Iwai]

On Mon, 06 Nov 2023 14:19:08 +0100,
Bagas Sanjaya wrote:
> 
> On Sat, Nov 04, 2023 at 01:01:56PM +0100, Jean-Jacques Hiblot wrote:
> > 
> > 
> > On 29/10/2023 02:48, Bagas Sanjaya wrote:
> > > On Thu, Oct 26, 2023 at 02:55:06PM +0700, Bagas Sanjaya wrote:
> > > > The culprit seems to be commit c7d80059b086c4986cd994a1973ec7a5d75f8eea, which introduces a new 'color' attribute for led sysfs class devices. The problem is that the system76-acpi platform driver tries to create the exact same sysfs attribute itself for the system76_acpi::kbd_backlight device, leading to the conflict. For testing purposes, I've just rebuilt the kernel with the system76-apci color attribute renamed to kb_color, and that fixes the issue.
> > > 
> > > Jean-Jacques Hiblot, would you like to take a look on this regression,
> > > since you authored the culprit?
> > Hi,
> > 
> > The offending commit stores the color in struct led_classdev and exposes it
> > via sysfs. It was part of a series that create a RGB leds from multiple
> > single-color LEDs. for this series, we need the color information but we
> > don't really need to expose it it via sysfs. In order to fix the issue, we
> > can remove the 'color' attribute from the sysfs.
> > 
> 
> OK, see you in the patch!

Is there a patch available?

This bug hits for a few Logitech keyboard models, too, and it makes
6.6 kernel unsable for them, as hid-lg-g15 driver probe fails due to
this bug:
  https://bugzilla.kernel.org/show_bug.cgi?id=218155

We need a quick fix for 6.6.x.


Thanks!

Takashi

Re: Fwd: sysfs: cannot create duplicate filename .../system76_acpi::kbd_backlight/color

[Thorsten Leemhuis]

Takashi, Jean-Jacques Hiblot, Lee,

On 20.11.23 14:53, Takashi Iwai wrote:
> On Mon, 06 Nov 2023 14:19:08 +0100,
> Bagas Sanjaya wrote:
>> On Sat, Nov 04, 2023 at 01:01:56PM +0100, Jean-Jacques Hiblot wrote:
>>> On 29/10/2023 02:48, Bagas Sanjaya wrote:
>>>> On Thu, Oct 26, 2023 at 02:55:06PM +0700, Bagas Sanjaya wrote:
>>>>> The culprit seems to be commit c7d80059b086c4986cd994a1973ec7a5d75f8eea, which introduces a new 'color' attribute for led sysfs class devices. The problem is that the system76-acpi platform driver tries to create the exact same sysfs attribute itself for the system76_acpi::kbd_backlight device, leading to the conflict. For testing purposes, I've just rebuilt the kernel with the system76-apci color attribute renamed to kb_color, and that fixes the issue.
>>>>
>>>> Jean-Jacques Hiblot, would you like to take a look on this regression,
>>>> since you authored the culprit?
>
>>> The offending commit stores the color in struct led_classdev and exposes it
>>> via sysfs. It was part of a series that create a RGB leds from multiple
>>> single-color LEDs. for this series, we need the color information but we
>>> don't really need to expose it it via sysfs. In order to fix the issue, we
>>> can remove the 'color' attribute from the sysfs.
>>
>> OK, see you in the patch!
> 
> Is there a patch available?

Not that I know of. Could not find anything on lore either.

> This bug hits for a few Logitech keyboard models, too, and it makes
> 6.6 kernel unsable for them, as hid-lg-g15 driver probe fails due to
> this bug:
>   https://bugzilla.kernel.org/show_bug.cgi?id=218155
> 
> We need a quick fix for 6.6.x.

Given that Jean-Jacques Hiblot (the author of the culprit) and Lee (who
committed it and sent it to Linus) know about this for a while already
without doing anything about it, I wonder if someone should just send a
revert to Linus (unless of course that is likely to introduce a
regression on its own).

Takashi, could you maybe do this, unless a fix shows up real soon?

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: /me should have followed up on this earlier... :-/
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

Re: Fwd: sysfs: cannot create duplicate filename .../system76_acpi::kbd_backlight/color

[Takashi Iwai]

On Tue, 21 Nov 2023 10:19:03 +0100,
Thorsten Leemhuis wrote:
> 
> Takashi, Jean-Jacques Hiblot, Lee,
> 
> On 20.11.23 14:53, Takashi Iwai wrote:
> > On Mon, 06 Nov 2023 14:19:08 +0100,
> > Bagas Sanjaya wrote:
> >> On Sat, Nov 04, 2023 at 01:01:56PM +0100, Jean-Jacques Hiblot wrote:
> >>> On 29/10/2023 02:48, Bagas Sanjaya wrote:
> >>>> On Thu, Oct 26, 2023 at 02:55:06PM +0700, Bagas Sanjaya wrote:
> >>>>> The culprit seems to be commit c7d80059b086c4986cd994a1973ec7a5d75f8eea, which introduces a new 'color' attribute for led sysfs class devices. The problem is that the system76-acpi platform driver tries to create the exact same sysfs attribute itself for the system76_acpi::kbd_backlight device, leading to the conflict. For testing purposes, I've just rebuilt the kernel with the system76-apci color attribute renamed to kb_color, and that fixes the issue.
> >>>>
> >>>> Jean-Jacques Hiblot, would you like to take a look on this regression,
> >>>> since you authored the culprit?
> >
> >>> The offending commit stores the color in struct led_classdev and exposes it
> >>> via sysfs. It was part of a series that create a RGB leds from multiple
> >>> single-color LEDs. for this series, we need the color information but we
> >>> don't really need to expose it it via sysfs. In order to fix the issue, we
> >>> can remove the 'color' attribute from the sysfs.
> >>
> >> OK, see you in the patch!
> > 
> > Is there a patch available?
> 
> Not that I know of. Could not find anything on lore either.
> 
> > This bug hits for a few Logitech keyboard models, too, and it makes
> > 6.6 kernel unsable for them, as hid-lg-g15 driver probe fails due to
> > this bug:
> >   https://bugzilla.kernel.org/show_bug.cgi?id=218155
> > 
> > We need a quick fix for 6.6.x.
> 
> Given that Jean-Jacques Hiblot (the author of the culprit) and Lee (who
> committed it and sent it to Linus) know about this for a while already
> without doing anything about it, I wonder if someone should just send a
> revert to Linus (unless of course that is likely to introduce a
> regression on its own).
> 
> Takashi, could you maybe do this, unless a fix shows up real soon?

I can, but we need to decide which way to go.
There are several options:

1. Revert the commit c7d80059b086;
   this drops led class color sysfs entries.  Also the store of
   led_cdev->color from fwnode is dropped, too.

2. Drop only led class color sysfs entries;
   a partial revert of c7d80059b086 above

3. Rename conflicting sysfs entries in drivers;
   e.g. color -> kb_color for hid-lg-g15 and system76_acpi

In either way, we'd break user-space (sysfs).

IMO, 2 would be the least harm, as the class sysfs entry was
introduced since 6.6.  I guess this is what Jean-Jacques suggested.
But I'm not sure how important this new class sysfs entry is; it has
to be clarified from leds people who actually use / wanted the
feature.

3 was already confirmed to work on both bug reports.  OTOH, it's not
clear whether we can neglect (potentially) already existing users.


Takashi

> 
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> 
> P.S.: /me should have followed up on this earlier... :-/
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
> 

Re: Fwd: sysfs: cannot create duplicate filename .../system76_acpi::kbd_backlight/color

[Takashi Iwai]

On Tue, 21 Nov 2023 10:52:52 +0100,
Takashi Iwai wrote:
> 
> On Tue, 21 Nov 2023 10:19:03 +0100,
> Thorsten Leemhuis wrote:
> > 
> > Takashi, Jean-Jacques Hiblot, Lee,
> > 
> > On 20.11.23 14:53, Takashi Iwai wrote:
> > > On Mon, 06 Nov 2023 14:19:08 +0100,
> > > Bagas Sanjaya wrote:
> > >> On Sat, Nov 04, 2023 at 01:01:56PM +0100, Jean-Jacques Hiblot wrote:
> > >>> On 29/10/2023 02:48, Bagas Sanjaya wrote:
> > >>>> On Thu, Oct 26, 2023 at 02:55:06PM +0700, Bagas Sanjaya wrote:
> > >>>>> The culprit seems to be commit c7d80059b086c4986cd994a1973ec7a5d75f8eea, which introduces a new 'color' attribute for led sysfs class devices. The problem is that the system76-acpi platform driver tries to create the exact same sysfs attribute itself for the system76_acpi::kbd_backlight device, leading to the conflict. For testing purposes, I've just rebuilt the kernel with the system76-apci color attribute renamed to kb_color, and that fixes the issue.
> > >>>>
> > >>>> Jean-Jacques Hiblot, would you like to take a look on this regression,
> > >>>> since you authored the culprit?
> > >
> > >>> The offending commit stores the color in struct led_classdev and exposes it
> > >>> via sysfs. It was part of a series that create a RGB leds from multiple
> > >>> single-color LEDs. for this series, we need the color information but we
> > >>> don't really need to expose it it via sysfs. In order to fix the issue, we
> > >>> can remove the 'color' attribute from the sysfs.
> > >>
> > >> OK, see you in the patch!
> > > 
> > > Is there a patch available?
> > 
> > Not that I know of. Could not find anything on lore either.
> > 
> > > This bug hits for a few Logitech keyboard models, too, and it makes
> > > 6.6 kernel unsable for them, as hid-lg-g15 driver probe fails due to
> > > this bug:
> > >   https://bugzilla.kernel.org/show_bug.cgi?id=218155
> > > 
> > > We need a quick fix for 6.6.x.
> > 
> > Given that Jean-Jacques Hiblot (the author of the culprit) and Lee (who
> > committed it and sent it to Linus) know about this for a while already
> > without doing anything about it, I wonder if someone should just send a
> > revert to Linus (unless of course that is likely to introduce a
> > regression on its own).
> > 
> > Takashi, could you maybe do this, unless a fix shows up real soon?
> 
> I can, but we need to decide which way to go.
> There are several options:
> 
> 1. Revert the commit c7d80059b086;
>    this drops led class color sysfs entries.  Also the store of
>    led_cdev->color from fwnode is dropped, too.
> 
> 2. Drop only led class color sysfs entries;
>    a partial revert of c7d80059b086 above
> 
> 3. Rename conflicting sysfs entries in drivers;
>    e.g. color -> kb_color for hid-lg-g15 and system76_acpi

... and

4. Rename conflicting sysfs class color entry

> In either way, we'd break user-space (sysfs).

It still holds :)


Takashi

Re: Fwd: sysfs: cannot create duplicate filename .../system76_acpi::kbd_backlight/color

[Hans de Goede]

Hi,

On 11/21/23 10:52, Takashi Iwai wrote:
> On Tue, 21 Nov 2023 10:19:03 +0100,
> Thorsten Leemhuis wrote:
>>
>> Takashi, Jean-Jacques Hiblot, Lee,
>>
>> On 20.11.23 14:53, Takashi Iwai wrote:
>>> On Mon, 06 Nov 2023 14:19:08 +0100,
>>> Bagas Sanjaya wrote:
>>>> On Sat, Nov 04, 2023 at 01:01:56PM +0100, Jean-Jacques Hiblot wrote:
>>>>> On 29/10/2023 02:48, Bagas Sanjaya wrote:
>>>>>> On Thu, Oct 26, 2023 at 02:55:06PM +0700, Bagas Sanjaya wrote:
>>>>>>> The culprit seems to be commit c7d80059b086c4986cd994a1973ec7a5d75f8eea, which introduces a new 'color' attribute for led sysfs class devices. The problem is that the system76-acpi platform driver tries to create the exact same sysfs attribute itself for the system76_acpi::kbd_backlight device, leading to the conflict. For testing purposes, I've just rebuilt the kernel with the system76-apci color attribute renamed to kb_color, and that fixes the issue.
>>>>>>
>>>>>> Jean-Jacques Hiblot, would you like to take a look on this regression,
>>>>>> since you authored the culprit?
>>>
>>>>> The offending commit stores the color in struct led_classdev and exposes it
>>>>> via sysfs. It was part of a series that create a RGB leds from multiple
>>>>> single-color LEDs. for this series, we need the color information but we
>>>>> don't really need to expose it it via sysfs. In order to fix the issue, we
>>>>> can remove the 'color' attribute from the sysfs.
>>>>
>>>> OK, see you in the patch!
>>>
>>> Is there a patch available?
>>
>> Not that I know of. Could not find anything on lore either.
>>
>>> This bug hits for a few Logitech keyboard models, too, and it makes
>>> 6.6 kernel unsable for them, as hid-lg-g15 driver probe fails due to
>>> this bug:
>>>   https://bugzilla.kernel.org/show_bug.cgi?id=218155
>>>
>>> We need a quick fix for 6.6.x.
>>
>> Given that Jean-Jacques Hiblot (the author of the culprit) and Lee (who
>> committed it and sent it to Linus) know about this for a while already
>> without doing anything about it, I wonder if someone should just send a
>> revert to Linus (unless of course that is likely to introduce a
>> regression on its own).
>>
>> Takashi, could you maybe do this, unless a fix shows up real soon?
> 
> I can, but we need to decide which way to go.
> There are several options:
> 
> 1. Revert the commit c7d80059b086;
>    this drops led class color sysfs entries.  Also the store of
>    led_cdev->color from fwnode is dropped, too.
> 
> 2. Drop only led class color sysfs entries;
>    a partial revert of c7d80059b086 above

AFAIK further up in the thread (or a related thread) there
already was consensus to do this. Someone just needs to
write the patch.

> 3. Rename conflicting sysfs entries in drivers;
>    e.g. color -> kb_color for hid-lg-g15 and system76_acpi
> 
> In either way, we'd break user-space (sysfs).

The new color attribute causing the conflict has only
been in 6.6 so there likely aren't any users of it yet
and since dropping it would be backported to 6.6.y
there shouldn't be any future users of it either, since
any 6.6 users presumably will use 6.6.y and not 6.6.0

> IMO, 2 would be the least harm, as the class sysfs entry was
> introduced since 6.6.

Ack.

> I guess this is what Jean-Jacques suggested.

Right.

> But I'm not sure how important this new class sysfs entry is; it has
> to be clarified from leds people who actually use / wanted the
> feature.

If I have read the thread correctly it is not important the value
it represents is used internally in the LED subsystem and userspace
does not really need it.

Regards,

Hans

Re: Fwd: sysfs: cannot create duplicate filename .../system76_acpi::kbd_backlight/color

[Takashi Iwai]

On Tue, 21 Nov 2023 11:21:53 +0100,
Hans de Goede wrote:
> 
> Hi,
> 
> On 11/21/23 10:52, Takashi Iwai wrote:
> > On Tue, 21 Nov 2023 10:19:03 +0100,
> > Thorsten Leemhuis wrote:
> >>
> >> Takashi, Jean-Jacques Hiblot, Lee,
> >>
> >> On 20.11.23 14:53, Takashi Iwai wrote:
> >>> On Mon, 06 Nov 2023 14:19:08 +0100,
> >>> Bagas Sanjaya wrote:
> >>>> On Sat, Nov 04, 2023 at 01:01:56PM +0100, Jean-Jacques Hiblot wrote:
> >>>>> On 29/10/2023 02:48, Bagas Sanjaya wrote:
> >>>>>> On Thu, Oct 26, 2023 at 02:55:06PM +0700, Bagas Sanjaya wrote:
> >>>>>>> The culprit seems to be commit c7d80059b086c4986cd994a1973ec7a5d75f8eea, which introduces a new 'color' attribute for led sysfs class devices. The problem is that the system76-acpi platform driver tries to create the exact same sysfs attribute itself for the system76_acpi::kbd_backlight device, leading to the conflict. For testing purposes, I've just rebuilt the kernel with the system76-apci color attribute renamed to kb_color, and that fixes the issue.
> >>>>>>
> >>>>>> Jean-Jacques Hiblot, would you like to take a look on this regression,
> >>>>>> since you authored the culprit?
> >>>
> >>>>> The offending commit stores the color in struct led_classdev and exposes it
> >>>>> via sysfs. It was part of a series that create a RGB leds from multiple
> >>>>> single-color LEDs. for this series, we need the color information but we
> >>>>> don't really need to expose it it via sysfs. In order to fix the issue, we
> >>>>> can remove the 'color' attribute from the sysfs.
> >>>>
> >>>> OK, see you in the patch!
> >>>
> >>> Is there a patch available?
> >>
> >> Not that I know of. Could not find anything on lore either.
> >>
> >>> This bug hits for a few Logitech keyboard models, too, and it makes
> >>> 6.6 kernel unsable for them, as hid-lg-g15 driver probe fails due to
> >>> this bug:
> >>>   https://bugzilla.kernel.org/show_bug.cgi?id=218155
> >>>
> >>> We need a quick fix for 6.6.x.
> >>
> >> Given that Jean-Jacques Hiblot (the author of the culprit) and Lee (who
> >> committed it and sent it to Linus) know about this for a while already
> >> without doing anything about it, I wonder if someone should just send a
> >> revert to Linus (unless of course that is likely to introduce a
> >> regression on its own).
> >>
> >> Takashi, could you maybe do this, unless a fix shows up real soon?
> > 
> > I can, but we need to decide which way to go.
> > There are several options:
> > 
> > 1. Revert the commit c7d80059b086;
> >    this drops led class color sysfs entries.  Also the store of
> >    led_cdev->color from fwnode is dropped, too.
> > 
> > 2. Drop only led class color sysfs entries;
> >    a partial revert of c7d80059b086 above
> 
> AFAIK further up in the thread (or a related thread) there
> already was consensus to do this. Someone just needs to
> write the patch.

Well, is there any user of this new led_classdev.color field?
The value read from fwnode is stored there, but as far as I see, there
seems no real user, so far.  If it's still unused, we can do the whole
revert -- which is cleaner.


Takashi

> > 3. Rename conflicting sysfs entries in drivers;
> >    e.g. color -> kb_color for hid-lg-g15 and system76_acpi
> > 
> > In either way, we'd break user-space (sysfs).
> 
> The new color attribute causing the conflict has only
> been in 6.6 so there likely aren't any users of it yet
> and since dropping it would be backported to 6.6.y
> there shouldn't be any future users of it either, since
> any 6.6 users presumably will use 6.6.y and not 6.6.0
> 
> > IMO, 2 would be the least harm, as the class sysfs entry was
> > introduced since 6.6.
> 
> Ack.
> 
> > I guess this is what Jean-Jacques suggested.
> 
> Right.
> 
> > But I'm not sure how important this new class sysfs entry is; it has
> > to be clarified from leds people who actually use / wanted the
> > feature.
> 
> If I have read the thread correctly it is not important the value
> it represents is used internally in the LED subsystem and userspace
> does not really need it.
> 
> Regards,
> 
> Hans
> 
> 
> 

Re: Fwd: sysfs: cannot create duplicate filename .../system76_acpi::kbd_backlight/color

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 3537 bytes --]

On Tue, Nov 21, 2023 at 11:33:55AM +0100, Takashi Iwai wrote:
> On Tue, 21 Nov 2023 11:21:53 +0100,
> Hans de Goede wrote:
> > 
> > Hi,
> > 
> > On 11/21/23 10:52, Takashi Iwai wrote:
> > > On Tue, 21 Nov 2023 10:19:03 +0100,
> > > Thorsten Leemhuis wrote:
> > >>
> > >> Takashi, Jean-Jacques Hiblot, Lee,
> > >>
> > >> On 20.11.23 14:53, Takashi Iwai wrote:
> > >>> On Mon, 06 Nov 2023 14:19:08 +0100,
> > >>> Bagas Sanjaya wrote:
> > >>>> On Sat, Nov 04, 2023 at 01:01:56PM +0100, Jean-Jacques Hiblot wrote:
> > >>>>> On 29/10/2023 02:48, Bagas Sanjaya wrote:
> > >>>>>> On Thu, Oct 26, 2023 at 02:55:06PM +0700, Bagas Sanjaya wrote:
> > >>>>>>> The culprit seems to be commit c7d80059b086c4986cd994a1973ec7a5d75f8eea, which introduces a new 'color' attribute for led sysfs class devices. The problem is that the system76-acpi platform driver tries to create the exact same sysfs attribute itself for the system76_acpi::kbd_backlight device, leading to the conflict. For testing purposes, I've just rebuilt the kernel with the system76-apci color attribute renamed to kb_color, and that fixes the issue.
> > >>>>>>
> > >>>>>> Jean-Jacques Hiblot, would you like to take a look on this regression,
> > >>>>>> since you authored the culprit?
> > >>>
> > >>>>> The offending commit stores the color in struct led_classdev and exposes it
> > >>>>> via sysfs. It was part of a series that create a RGB leds from multiple
> > >>>>> single-color LEDs. for this series, we need the color information but we
> > >>>>> don't really need to expose it it via sysfs. In order to fix the issue, we
> > >>>>> can remove the 'color' attribute from the sysfs.
> > >>>>
> > >>>> OK, see you in the patch!
> > >>>
> > >>> Is there a patch available?
> > >>
> > >> Not that I know of. Could not find anything on lore either.
> > >>
> > >>> This bug hits for a few Logitech keyboard models, too, and it makes
> > >>> 6.6 kernel unsable for them, as hid-lg-g15 driver probe fails due to
> > >>> this bug:
> > >>>   https://bugzilla.kernel.org/show_bug.cgi?id=218155
> > >>>
> > >>> We need a quick fix for 6.6.x.
> > >>
> > >> Given that Jean-Jacques Hiblot (the author of the culprit) and Lee (who
> > >> committed it and sent it to Linus) know about this for a while already
> > >> without doing anything about it, I wonder if someone should just send a
> > >> revert to Linus (unless of course that is likely to introduce a
> > >> regression on its own).
> > >>
> > >> Takashi, could you maybe do this, unless a fix shows up real soon?
> > > 
> > > I can, but we need to decide which way to go.
> > > There are several options:
> > > 
> > > 1. Revert the commit c7d80059b086;
> > >    this drops led class color sysfs entries.  Also the store of
> > >    led_cdev->color from fwnode is dropped, too.
> > > 
> > > 2. Drop only led class color sysfs entries;
> > >    a partial revert of c7d80059b086 above
> > 
> > AFAIK further up in the thread (or a related thread) there
> > already was consensus to do this. Someone just needs to
> > write the patch.
> 
> Well, is there any user of this new led_classdev.color field?
> The value read from fwnode is stored there, but as far as I see, there
> seems no real user, so far.  If it's still unused, we can do the whole
> revert -- which is cleaner.

ACK for the revert. The original commit should have been designed to
not have sysfs conflict like this, though.

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: Fwd: sysfs: cannot create duplicate filename .../system76_acpi::kbd_backlight/color

[Hans de Goede]

Hi,

On 11/21/23 11:33, Takashi Iwai wrote:
> On Tue, 21 Nov 2023 11:21:53 +0100,
> Hans de Goede wrote:
>>
>> Hi,
>>
>> On 11/21/23 10:52, Takashi Iwai wrote:
>>> On Tue, 21 Nov 2023 10:19:03 +0100,
>>> Thorsten Leemhuis wrote:
>>>>
>>>> Takashi, Jean-Jacques Hiblot, Lee,
>>>>
>>>> On 20.11.23 14:53, Takashi Iwai wrote:
>>>>> On Mon, 06 Nov 2023 14:19:08 +0100,
>>>>> Bagas Sanjaya wrote:
>>>>>> On Sat, Nov 04, 2023 at 01:01:56PM +0100, Jean-Jacques Hiblot wrote:
>>>>>>> On 29/10/2023 02:48, Bagas Sanjaya wrote:
>>>>>>>> On Thu, Oct 26, 2023 at 02:55:06PM +0700, Bagas Sanjaya wrote:
>>>>>>>>> The culprit seems to be commit c7d80059b086c4986cd994a1973ec7a5d75f8eea, which introduces a new 'color' attribute for led sysfs class devices. The problem is that the system76-acpi platform driver tries to create the exact same sysfs attribute itself for the system76_acpi::kbd_backlight device, leading to the conflict. For testing purposes, I've just rebuilt the kernel with the system76-apci color attribute renamed to kb_color, and that fixes the issue.
>>>>>>>>
>>>>>>>> Jean-Jacques Hiblot, would you like to take a look on this regression,
>>>>>>>> since you authored the culprit?
>>>>>
>>>>>>> The offending commit stores the color in struct led_classdev and exposes it
>>>>>>> via sysfs. It was part of a series that create a RGB leds from multiple
>>>>>>> single-color LEDs. for this series, we need the color information but we
>>>>>>> don't really need to expose it it via sysfs. In order to fix the issue, we
>>>>>>> can remove the 'color' attribute from the sysfs.
>>>>>>
>>>>>> OK, see you in the patch!
>>>>>
>>>>> Is there a patch available?
>>>>
>>>> Not that I know of. Could not find anything on lore either.
>>>>
>>>>> This bug hits for a few Logitech keyboard models, too, and it makes
>>>>> 6.6 kernel unsable for them, as hid-lg-g15 driver probe fails due to
>>>>> this bug:
>>>>>   https://bugzilla.kernel.org/show_bug.cgi?id=218155
>>>>>
>>>>> We need a quick fix for 6.6.x.
>>>>
>>>> Given that Jean-Jacques Hiblot (the author of the culprit) and Lee (who
>>>> committed it and sent it to Linus) know about this for a while already
>>>> without doing anything about it, I wonder if someone should just send a
>>>> revert to Linus (unless of course that is likely to introduce a
>>>> regression on its own).
>>>>
>>>> Takashi, could you maybe do this, unless a fix shows up real soon?
>>>
>>> I can, but we need to decide which way to go.
>>> There are several options:
>>>
>>> 1. Revert the commit c7d80059b086;
>>>    this drops led class color sysfs entries.  Also the store of
>>>    led_cdev->color from fwnode is dropped, too.
>>>
>>> 2. Drop only led class color sysfs entries;
>>>    a partial revert of c7d80059b086 above
>>
>> AFAIK further up in the thread (or a related thread) there
>> already was consensus to do this. Someone just needs to
>> write the patch.
> 
> Well, is there any user of this new led_classdev.color field?
> The value read from fwnode is stored there, but as far as I see, there
> seems no real user, so far.  If it's still unused, we can do the whole
> revert -- which is cleaner.

I honestly don't know. I've mostly just been reading along. I think
there may be some future in kernel use planned (not sure at all though).

If there are no current in kernel users then I agree we should just
go with a full revert now to fix the regression. If some later
in kernel users do come along then they can always re-introduce
the change minus the sysfs attr addition.

Regards,

Hans

Re: Fwd: sysfs: cannot create duplicate filename .../system76_acpi::kbd_backlight/color

[Takashi Iwai]

On Tue, 21 Nov 2023 13:23:20 +0100,
Hans de Goede wrote:
> 
> Hi,
> 
> On 11/21/23 11:33, Takashi Iwai wrote:
> > On Tue, 21 Nov 2023 11:21:53 +0100,
> > Hans de Goede wrote:
> >>
> >> Hi,
> >>
> >> On 11/21/23 10:52, Takashi Iwai wrote:
> >>> On Tue, 21 Nov 2023 10:19:03 +0100,
> >>> Thorsten Leemhuis wrote:
> >>>>
> >>>> Takashi, Jean-Jacques Hiblot, Lee,
> >>>>
> >>>> On 20.11.23 14:53, Takashi Iwai wrote:
> >>>>> On Mon, 06 Nov 2023 14:19:08 +0100,
> >>>>> Bagas Sanjaya wrote:
> >>>>>> On Sat, Nov 04, 2023 at 01:01:56PM +0100, Jean-Jacques Hiblot wrote:
> >>>>>>> On 29/10/2023 02:48, Bagas Sanjaya wrote:
> >>>>>>>> On Thu, Oct 26, 2023 at 02:55:06PM +0700, Bagas Sanjaya wrote:
> >>>>>>>>> The culprit seems to be commit c7d80059b086c4986cd994a1973ec7a5d75f8eea, which introduces a new 'color' attribute for led sysfs class devices. The problem is that the system76-acpi platform driver tries to create the exact same sysfs attribute itself for the system76_acpi::kbd_backlight device, leading to the conflict. For testing purposes, I've just rebuilt the kernel with the system76-apci color attribute renamed to kb_color, and that fixes the issue.
> >>>>>>>>
> >>>>>>>> Jean-Jacques Hiblot, would you like to take a look on this regression,
> >>>>>>>> since you authored the culprit?
> >>>>>
> >>>>>>> The offending commit stores the color in struct led_classdev and exposes it
> >>>>>>> via sysfs. It was part of a series that create a RGB leds from multiple
> >>>>>>> single-color LEDs. for this series, we need the color information but we
> >>>>>>> don't really need to expose it it via sysfs. In order to fix the issue, we
> >>>>>>> can remove the 'color' attribute from the sysfs.
> >>>>>>
> >>>>>> OK, see you in the patch!
> >>>>>
> >>>>> Is there a patch available?
> >>>>
> >>>> Not that I know of. Could not find anything on lore either.
> >>>>
> >>>>> This bug hits for a few Logitech keyboard models, too, and it makes
> >>>>> 6.6 kernel unsable for them, as hid-lg-g15 driver probe fails due to
> >>>>> this bug:
> >>>>>   https://bugzilla.kernel.org/show_bug.cgi?id=218155
> >>>>>
> >>>>> We need a quick fix for 6.6.x.
> >>>>
> >>>> Given that Jean-Jacques Hiblot (the author of the culprit) and Lee (who
> >>>> committed it and sent it to Linus) know about this for a while already
> >>>> without doing anything about it, I wonder if someone should just send a
> >>>> revert to Linus (unless of course that is likely to introduce a
> >>>> regression on its own).
> >>>>
> >>>> Takashi, could you maybe do this, unless a fix shows up real soon?
> >>>
> >>> I can, but we need to decide which way to go.
> >>> There are several options:
> >>>
> >>> 1. Revert the commit c7d80059b086;
> >>>    this drops led class color sysfs entries.  Also the store of
> >>>    led_cdev->color from fwnode is dropped, too.
> >>>
> >>> 2. Drop only led class color sysfs entries;
> >>>    a partial revert of c7d80059b086 above
> >>
> >> AFAIK further up in the thread (or a related thread) there
> >> already was consensus to do this. Someone just needs to
> >> write the patch.
> > 
> > Well, is there any user of this new led_classdev.color field?
> > The value read from fwnode is stored there, but as far as I see, there
> > seems no real user, so far.  If it's still unused, we can do the whole
> > revert -- which is cleaner.
> 
> I honestly don't know. I've mostly just been reading along. I think
> there may be some future in kernel use planned (not sure at all though).
> 
> If there are no current in kernel users then I agree we should just
> go with a full revert now to fix the regression. If some later
> in kernel users do come along then they can always re-introduce
> the change minus the sysfs attr addition.

Sounds reasonable.
OK, let me pitch the revert patch.


thanks,

Takashi

Re: Fwd: sysfs: cannot create duplicate filename .../system76_acpi::kbd_backlight/color

[Lee Jones]

On Tue, 21 Nov 2023, Thorsten Leemhuis wrote:

> Takashi, Jean-Jacques Hiblot, Lee,
> 
> On 20.11.23 14:53, Takashi Iwai wrote:
> > On Mon, 06 Nov 2023 14:19:08 +0100,
> > Bagas Sanjaya wrote:
> >> On Sat, Nov 04, 2023 at 01:01:56PM +0100, Jean-Jacques Hiblot wrote:
> >>> On 29/10/2023 02:48, Bagas Sanjaya wrote:
> >>>> On Thu, Oct 26, 2023 at 02:55:06PM +0700, Bagas Sanjaya wrote:
> >>>>> The culprit seems to be commit c7d80059b086c4986cd994a1973ec7a5d75f8eea, which introduces a new 'color' attribute for led sysfs class devices. The problem is that the system76-acpi platform driver tries to create the exact same sysfs attribute itself for the system76_acpi::kbd_backlight device, leading to the conflict. For testing purposes, I've just rebuilt the kernel with the system76-apci color attribute renamed to kb_color, and that fixes the issue.
> >>>>
> >>>> Jean-Jacques Hiblot, would you like to take a look on this regression,
> >>>> since you authored the culprit?
> >
> >>> The offending commit stores the color in struct led_classdev and exposes it
> >>> via sysfs. It was part of a series that create a RGB leds from multiple
> >>> single-color LEDs. for this series, we need the color information but we
> >>> don't really need to expose it it via sysfs. In order to fix the issue, we
> >>> can remove the 'color' attribute from the sysfs.
> >>
> >> OK, see you in the patch!
> > 
> > Is there a patch available?
> 
> Not that I know of. Could not find anything on lore either.
> 
> > This bug hits for a few Logitech keyboard models, too, and it makes
> > 6.6 kernel unsable for them, as hid-lg-g15 driver probe fails due to
> > this bug:
> >   https://bugzilla.kernel.org/show_bug.cgi?id=218155
> > 
> > We need a quick fix for 6.6.x.
> 
> Given that Jean-Jacques Hiblot (the author of the culprit) and Lee (who
> committed it and sent it to Linus) know about this for a while already
> without doing anything about it, I wonder if someone should just send a
> revert to Linus (unless of course that is likely to introduce a
> regression on its own).

You seem to have gone from DEFCON 4 to DEFCON 2 there.  The middle step
is to submit a patch to fix the issue and follow the usual patch flow.

-- 
Lee Jones [李琼斯]