kernelnewbies.kernelnewbies.org archive mirror
 help / color / mirror / Atom feed
* Seeking help addressing maintainer objections
@ 2024-04-02 15:59 Ian Pilcher
  2024-04-03 11:38 ` Bjørn Mork
  0 siblings, 1 reply; 4+ messages in thread
From: Ian Pilcher @ 2024-04-02 15:59 UTC (permalink / raw)
  To: kernelnewbies

I have been maintaining an out of tree module for several years, and I'd
like to take (yet another) run at getting it accepted.  In order to do
so, however, I need some advice/help to meet the requirements of the
relevant subsystem maintainer.

The module in question is an LED trigger for block devices.  It allows
the system administrator to create relationships between LEDS and block
devices, so that activity (reads, writes, etc.) on those devices will
cause the LEDs to blink.  You can see the latest version (for 6.8
kernels) here[1].

The trigger is very configurable.  It supports many-to-many
relationships between block devices and LEDS, so a block device can be
associated with more than one LED (potentially for different types of
activity) and an LED can be associated with one or more block devices.

These many-to-many relationships are part of the problem.  LED triggers
are traditionally configured with sysfs, and sysfs attributes don't
really work all that well for configuring a list of things (such as the
block devices associated with an LED).

In order to avoid a read/modify/write cycle when adding or removing a
device/LED association, and adhere to the one value per file guideline,
the trigger provides write-only attributes for adding and removing
associations.  (Existing associations are represented as symbolic
links.)  The sysfs interface is documented here[2].

The maintainer of the LEDs subsystem considers this interface to be
"crazy", but he has not suggested any alternative.[3]  (I have not been
able to find any existing example of configuring many-to-many
relationships via sysfs.)

The second problem is the use of device file paths (including symbolic
link resolution), rather than kernel names, when creating associations.
This limitation exists because the block subsystem does not provide an
API to open a block device by its kernel name; only device file paths
and device numbers (major & minor) are supported.

Adding such an API to the block subsystem has been proposed many times
over the years, and it has been denied (often vehemently) on every
occasion. I believe it's accurate to say that the maintainers of the
block subsystem feel that device file paths (and to a lesser extent
device numbers) are *the* correct way to refer to block devices, and
they are not willing to add APIs that use kernel names.

Does anyone have any suggestions on how I can move forward with this?

Thanks!

[1] 
https://github.com/ipilcher/ledtrig-blkdev/blob/pre-6.9/drivers/leds/trigger/ledtrig-blkdev.c
[2] 
https://github.com/ipilcher/ledtrig-blkdev/blob/pre-6.9/Documentation/leds/ledtrig-blkdev.rst
[3] 
https://lore.kernel.org/lkml/a7ff3338-3d5e-4402-aaba-16e740f4ed5b@gmail.com/T/#mbcf916e3582ae237a08390f5405822bd85f02c89

-- 
========================================================================
If your user interface is intuitive in retrospect ... it isn't intuitive
========================================================================

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Seeking help addressing maintainer objections
  2024-04-02 15:59 Seeking help addressing maintainer objections Ian Pilcher
@ 2024-04-03 11:38 ` Bjørn Mork
  2024-04-03 13:00   ` Ian Pilcher
  0 siblings, 1 reply; 4+ messages in thread
From: Bjørn Mork @ 2024-04-03 11:38 UTC (permalink / raw)
  To: Ian Pilcher; +Cc: kernelnewbies

Ian Pilcher <arequipeno@gmail.com> writes:

> The maintainer of the LEDs subsystem considers this interface to be
> "crazy", but he has not suggested any alternative.[3] 

I read a strong suggestion that one of the unlink interfaces is
dropped.

You should probably read "crazy" as "unjustified".

> (I have not been able to find any existing example of configuring
> many-to-many relationships via sysfs.)

There's at least one such example, hidden under drivers/usb:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/core/ledtrig-usbport.c

See the initial commit for justification and some of the background for
the choices made:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/core/ledtrig-usbport.c?id=0f247626cbbfa2010d2b86fdee652605e084e248


> The second problem is the use of device file paths (including symbolic
> link resolution), rather than kernel names, when creating associations.
> This limitation exists because the block subsystem does not provide an
> API to open a block device by its kernel name; only device file paths
> and device numbers (major & minor) are supported.

So this explains why you can't have link_dev_by_path.  It still does not
explain why you need unlink_dev_by_name.

And if file name with symlink resolution really is a problem, then why
can't you use the major:minor for link/unlink?  That's easy for
userspace to look up whether the input is a device path or a sysfs path.
And it avoids having to wait for an unrelated and unnecessary device
path creation.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/core/ledtrig-usbport.c

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/core/ledtrig-usbport.c?id=0f247626cbbfa2010d2b86fdee652605e084e248

Bjørn

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Seeking help addressing maintainer objections
  2024-04-03 11:38 ` Bjørn Mork
@ 2024-04-03 13:00   ` Ian Pilcher
  2024-04-03 14:05     ` Bjørn Mork
  0 siblings, 1 reply; 4+ messages in thread
From: Ian Pilcher @ 2024-04-03 13:00 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: kernelnewbies

On 4/3/24 06:38, Bjørn Mork wrote:
> Ian Pilcher <arequipeno@gmail.com> writes:
> 
>> The maintainer of the LEDs subsystem considers this interface to be
>> "crazy", but he has not suggested any alternative.[3]
> 
> I read a strong suggestion that one of the unlink interfaces is
> dropped.
> 
> You should probably read "crazy" as "unjustified".
> 
>> (I have not been able to find any existing example of configuring
>> many-to-many relationships via sysfs.)
> 
> There's at least one such example, hidden under drivers/usb:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/core/ledtrig-usbport.c
> 
> See the initial commit for justification and some of the background for
> the choices made:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/core/ledtrig-usbport.c?id=0f247626cbbfa2010d2b86fdee652605e084e248

Thanks! Will take a look at that.

>> The second problem is the use of device file paths (including symbolic
>> link resolution), rather than kernel names, when creating associations.
>> This limitation exists because the block subsystem does not provide an
>> API to open a block device by its kernel name; only device file paths
>> and device numbers (major & minor) are supported.
> 
> So this explains why you can't have link_dev_by_path.  It still does not
> explain why you need unlink_dev_by_name.

It's not absolutely needed, but it does make it easier to unlink an LED
from all devices by using the names of the symlinks in the LED's
linked_devices directory, which will be kernel names.

> And if file name with symlink resolution really is a problem, then why
> can't you use the major:minor for link/unlink?  That's easy for
> userspace to look up whether the input is a device path or a sysfs path.
> And it avoids having to wait for an unrelated and unnecessary device
> path creation.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/core/ledtrig-usbport.c
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/core/ledtrig-usbport.c?id=0f247626cbbfa2010d2b86fdee652605e084e248

Personally, I don't think that using file paths is a problem, and it
can be useful.  ("/dev/vg_root/lv_root" is probably more useful than
"dm-0".)  OTOH, "sda" is slightly simpler than "/dev/sda", so I think
that the ideal situation would be to have both interfaces available.

I did propose using device numbers.  I never received a response from
the maintainer.

Thanks for taking the time to respond to this!

-- 
========================================================================
If your user interface is intuitive in retrospect ... it isn't intuitive
========================================================================


_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Seeking help addressing maintainer objections
  2024-04-03 13:00   ` Ian Pilcher
@ 2024-04-03 14:05     ` Bjørn Mork
  0 siblings, 0 replies; 4+ messages in thread
From: Bjørn Mork @ 2024-04-03 14:05 UTC (permalink / raw)
  To: Ian Pilcher; +Cc: kernelnewbies

Ian Pilcher <arequipeno@gmail.com> writes:

> It's not absolutely needed, but it does make it easier to unlink an LED
> from all devices by using the names of the symlinks in the LED's
> linked_devices directory, which will be kernel names.

Yes, I agree that things are much easier if those names can be fed
directly into the unlink attribute.  And even better if the names in the
linked_devices directory actually matched what you used to link them.

So why not go for "major:minor" everywhere?  I.e for link, unlink and
also for the symlinks in linked_devices.

>> And if file name with symlink resolution really is a problem, then why
>> can't you use the major:minor for link/unlink?  That's easy for
>> userspace to look up whether the input is a device path or a sysfs path.
>> And it avoids having to wait for an unrelated and unnecessary device
>> path creation.
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/core/ledtrig-usbport.c
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/core/ledtrig-usbport.c?id=0f247626cbbfa2010d2b86fdee652605e084e248
>
> Personally, I don't think that using file paths is a problem, and it
> can be useful.  ("/dev/vg_root/lv_root" is probably more useful than
> "dm-0".)  OTOH, "sda" is slightly simpler than "/dev/sda", so I think
> that the ideal situation would be to have both interfaces available.
>
> I did propose using device numbers.  I never received a response from
> the maintainer.

I believe that's how most maintainers work unless the proposal was in
patch form :-)



Bjørn

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-04-03 14:06 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-02 15:59 Seeking help addressing maintainer objections Ian Pilcher
2024-04-03 11:38 ` Bjørn Mork
2024-04-03 13:00   ` Ian Pilcher
2024-04-03 14:05     ` Bjørn Mork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).