mbox series

[RFC,0/2] Introduce configfs-based interface for gpio-aggregator

Message ID 20250129155525.663780-1-koichiro.den@canonical.com
Headers show
Series Introduce configfs-based interface for gpio-aggregator | expand

Message

Koichiro Den Jan. 29, 2025, 3:55 p.m. UTC
This RFC patch series proposes adding a configfs-based interface to
gpio-aggregator to address limitations in the existing 'new_device'
interface.

The existing 'new_device' interface has several limitations:

  #1. No way to determine when GPIO aggregator creation is complete.
  #2. No way to retrieve errors when creating a GPIO aggregator.
  #3. No way to trace a GPIO line of an aggregator back to its
      corresponding physical device.
  #4. The 'new_device' echo does not indicate which virtual gpiochip.<N>
      was created.
  #5. No way to assign names to GPIO lines exported through an aggregator.

Although issues #1 to #3 could technically be resolved easily without
configfs, using configfs offers a streamlined, modern, and extensible
approach, especially since gpio-sim and gpio-virtuser already utilize
configfs.

This RFC patch series includes two commits:

* [PATCH 1/2] implements the configfs interface and resolves the above
  issues:
  - #1, Wait for probe completion using a platform bus notifier,
        in the same manner as gpio-virtuser.
  - #2, Introduce a 'live' attribute (like gpio-virtuser/gpio-sim),
        returning -ENXIO when probe fails.
  - #3, Structure configfs directories to clearly map virtual lines to
        physical ones.
  - #4, Add a read-only 'dev_name' attribute exposing the platform bus
        device name.
  - #5, Allow users to set custom line names via a 'name' attribute.

* [PATCH 2/2] provides documentation on using the new interface.


Koichiro Den (2):
  gpio: aggregator: Introduce configfs interface
  Documentation: gpio: document configfs interface for gpio-aggregator

 .../admin-guide/gpio/gpio-aggregator.rst      |  86 +++
 drivers/gpio/gpio-aggregator.c                | 673 +++++++++++++++++-
 2 files changed, 757 insertions(+), 2 deletions(-)

Comments

Bartosz Golaszewski Jan. 30, 2025, 10:30 a.m. UTC | #1
On Wed, Jan 29, 2025 at 4:56 PM Koichiro Den <koichiro.den@canonical.com> wrote:
>
> This RFC patch series proposes adding a configfs-based interface to
> gpio-aggregator to address limitations in the existing 'new_device'
> interface.
>
> The existing 'new_device' interface has several limitations:
>
>   #1. No way to determine when GPIO aggregator creation is complete.
>   #2. No way to retrieve errors when creating a GPIO aggregator.
>   #3. No way to trace a GPIO line of an aggregator back to its
>       corresponding physical device.
>   #4. The 'new_device' echo does not indicate which virtual gpiochip.<N>
>       was created.
>   #5. No way to assign names to GPIO lines exported through an aggregator.
>
> Although issues #1 to #3 could technically be resolved easily without
> configfs, using configfs offers a streamlined, modern, and extensible
> approach, especially since gpio-sim and gpio-virtuser already utilize
> configfs.
>
> This RFC patch series includes two commits:
>
> * [PATCH 1/2] implements the configfs interface and resolves the above
>   issues:
>   - #1, Wait for probe completion using a platform bus notifier,
>         in the same manner as gpio-virtuser.
>   - #2, Introduce a 'live' attribute (like gpio-virtuser/gpio-sim),
>         returning -ENXIO when probe fails.
>   - #3, Structure configfs directories to clearly map virtual lines to
>         physical ones.
>   - #4, Add a read-only 'dev_name' attribute exposing the platform bus
>         device name.
>   - #5, Allow users to set custom line names via a 'name' attribute.
>
> * [PATCH 2/2] provides documentation on using the new interface.
>
>
> Koichiro Den (2):
>   gpio: aggregator: Introduce configfs interface
>   Documentation: gpio: document configfs interface for gpio-aggregator
>
>  .../admin-guide/gpio/gpio-aggregator.rst      |  86 +++
>  drivers/gpio/gpio-aggregator.c                | 673 +++++++++++++++++-
>  2 files changed, 757 insertions(+), 2 deletions(-)
>
> --
> 2.45.2
>

Hi!

I love the idea! In fact I think I floated it in a discussion with
Geert some time ago but never got around to working on it.

I just glanced at the code and it looks nice and clean. I'd love to
see some more improvements like using a common prefix for all internal
symbols but it can be addressed in a separate series.

I played a bit with the module and this is where I noticed some issues:

1. The sysfs interface must keep on working. The same command that
works with mainline, fails for me with your patch. There's no error
propagated to user-space, write() returns success and I only see:

gpio-aggregator.0: probe with driver gpio-aggregator failed with error -12

in the kernel log.

2. I couldn't verify that it's not the case already but the code does
not suggest it: IMO devices created with sysfs should appear in
configfs.

3. I don't think the user should need to specify the number of lines
to aggregate. That information should be automatically inferred from
the number of lineX attributes they created instead. Also: if I create
a line attribute without setting num_lines, the driver just crashes.
In fact it seems any discrepancy between the number of lines specified
and the naming convention of the line attribute causes a crash.

4. Writing 1 to live, when no lines to aggregate were specified, should fail.

There's probably more but I haven't had a lot of time.

In short: I'm very much in favor of adding this but it will require some work.

Thanks,
Bartosz
Koichiro Den Jan. 30, 2025, 4:04 p.m. UTC | #2
On Thu, Jan 30, 2025 at 11:30:59AM GMT, Bartosz Golaszewski wrote:
> On Wed, Jan 29, 2025 at 4:56 PM Koichiro Den <koichiro.den@canonical.com> wrote:
> >
> > This RFC patch series proposes adding a configfs-based interface to
> > gpio-aggregator to address limitations in the existing 'new_device'
> > interface.
> >
> > The existing 'new_device' interface has several limitations:
> >
> >   #1. No way to determine when GPIO aggregator creation is complete.
> >   #2. No way to retrieve errors when creating a GPIO aggregator.
> >   #3. No way to trace a GPIO line of an aggregator back to its
> >       corresponding physical device.
> >   #4. The 'new_device' echo does not indicate which virtual gpiochip.<N>
> >       was created.
> >   #5. No way to assign names to GPIO lines exported through an aggregator.
> >
> > Although issues #1 to #3 could technically be resolved easily without
> > configfs, using configfs offers a streamlined, modern, and extensible
> > approach, especially since gpio-sim and gpio-virtuser already utilize
> > configfs.
> >
> > This RFC patch series includes two commits:
> >
> > * [PATCH 1/2] implements the configfs interface and resolves the above
> >   issues:
> >   - #1, Wait for probe completion using a platform bus notifier,
> >         in the same manner as gpio-virtuser.
> >   - #2, Introduce a 'live' attribute (like gpio-virtuser/gpio-sim),
> >         returning -ENXIO when probe fails.
> >   - #3, Structure configfs directories to clearly map virtual lines to
> >         physical ones.
> >   - #4, Add a read-only 'dev_name' attribute exposing the platform bus
> >         device name.
> >   - #5, Allow users to set custom line names via a 'name' attribute.
> >
> > * [PATCH 2/2] provides documentation on using the new interface.
> >
> >
> > Koichiro Den (2):
> >   gpio: aggregator: Introduce configfs interface
> >   Documentation: gpio: document configfs interface for gpio-aggregator
> >
> >  .../admin-guide/gpio/gpio-aggregator.rst      |  86 +++
> >  drivers/gpio/gpio-aggregator.c                | 673 +++++++++++++++++-
> >  2 files changed, 757 insertions(+), 2 deletions(-)
> >
> > --
> > 2.45.2
> >
> 
> Hi!

Hi, thank you for reviewing.

> 
> I love the idea! In fact I think I floated it in a discussion with
> Geert some time ago but never got around to working on it.
> 
> I just glanced at the code and it looks nice and clean. I'd love to
> see some more improvements like using a common prefix for all internal
> symbols but it can be addressed in a separate series.
> 
> I played a bit with the module and this is where I noticed some issues:
> 
> 1. The sysfs interface must keep on working. The same command that
> works with mainline, fails for me with your patch. There's no error
> propagated to user-space, write() returns success and I only see:
> 
> gpio-aggregator.0: probe with driver gpio-aggregator failed with error -12

It looks like the issue is caused by gpiochip_fwd_line_names(). I'll fix it.

> 
> in the kernel log.
> 
> 2. I couldn't verify that it's not the case already but the code does
> not suggest it: IMO devices created with sysfs should appear in
> configfs.

That makes sense, I'll add the implementation.

> 
> 3. I don't think the user should need to specify the number of lines
> to aggregate. That information should be automatically inferred from
> the number of lineX attributes they created instead. [...]

I agree that it's essentially unnecessary, but considering the current
state of gpio-sim's configfs, having the user set num_lines doesn't seem
too unnatural to me. What do you think?

> [...] Also: if I create
> a line attribute without setting num_lines, the driver just crashes.
> In fact it seems any discrepancy between the number of lines specified
> and the naming convention of the line attribute causes a crash.

My bad.. thanks for pointing it out.

> 
> 4. Writing 1 to live, when no lines to aggregate were specified, should fail.

Agreed, I'll address this.

> 
> There's probably more but I haven't had a lot of time.
> 
> In short: I'm very much in favor of adding this but it will require some work.
> 
> Thanks,
> Bartosz

I'd appreciate a response to one question above. I'll prepare v2 after.

Thanks again!

-Koichiro
Bartosz Golaszewski Jan. 30, 2025, 6:40 p.m. UTC | #3
On Thu, Jan 30, 2025 at 5:04 PM Koichiro Den <koichiro.den@canonical.com> wrote:
>
> On Thu, Jan 30, 2025 at 11:30:59AM GMT, Bartosz Golaszewski wrote:
> >
> > 3. I don't think the user should need to specify the number of lines
> > to aggregate. That information should be automatically inferred from
> > the number of lineX attributes they created instead. [...]
>
> I agree that it's essentially unnecessary, but considering the current
> state of gpio-sim's configfs, having the user set num_lines doesn't seem
> too unnatural to me. What do you think?
>

No, this is completely different. We cannot figure out how many lines
the user wants gpio-sim to have without explicitly providing that
information over the num_lines attribute. For the aggregator, we know
exactly how many lines the user wants - it's determined by the number
of line groups. We can simply wait for the live attribute to be set to
1 and then count them. While at it: there's no reason to impose a
naming convention of lineX, lineY etc., the names don't matter for the
aggregator setup (unlike gpio-sim where they indicate the offset of
the line they concern).

Bart
Bartosz Golaszewski Jan. 30, 2025, 8:47 p.m. UTC | #4
On Thu, Jan 30, 2025 at 7:40 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> While at it: there's no reason to impose a
> naming convention of lineX, lineY etc., the names don't matter for the
> aggregator setup (unlike gpio-sim where they indicate the offset of
> the line they concern).
>

Scratch that part. There's a good reason for that - the ordering of
lines within the aggregator. I'm just not sure whether we should
impose a strict naming where - for an aggregator of 3 lines total - we
expect there to exist groups named line0, line1 and line2 or if we
should be more lenient and possibly sort whatever names the user
provides alphabetically?

Bart
Maciej Borzęcki Jan. 31, 2025, 7:25 a.m. UTC | #5
On Thu, 30 Jan 2025 at 21:48, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> On Thu, Jan 30, 2025 at 7:40 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > While at it: there's no reason to impose a
> > naming convention of lineX, lineY etc., the names don't matter for the
> > aggregator setup (unlike gpio-sim where they indicate the offset of
> > the line they concern).
> >
>
> Scratch that part. There's a good reason for that - the ordering of
> lines within the aggregator. I'm just not sure whether we should
> impose a strict naming where - for an aggregator of 3 lines total - we
> expect there to exist groups named line0, line1 and line2 or if we
> should be more lenient and possibly sort whatever names the user
> provides alphabetically?

If I may jump in quickly (I provided some initial feedback on the
configfs interfaces
for the first internal patches). I think it's preferable to have
strict and explicit, even
If more verbose, line ordering in the aggregator.The motivator for
this is that whoever
sets up a new device through the aggregator does not have to second guess what
the driver will do. Implicit ordering could perhaps be fine if the
consumers were to
impose some set of rules themselves, but I fear there would still be
some ambiguity
left if free form names were for e.g. [1, 02, 10]. In the end they
would probably settle
on some mechanism which would mimic what we could already do in the
driver itself
and avoid any further confusion for the user.

Cheers,
Maciej
Koichiro Den Jan. 31, 2025, 2:36 p.m. UTC | #6
On Thu, Jan 30, 2025 at 09:47:47PM GMT, Bartosz Golaszewski wrote:
> On Thu, Jan 30, 2025 at 7:40 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >

(Small reply to the previous comment:)
Yes, I understand your point about why you think 'num_lines' is
unnecessary. What I meant was more of a UX (User eXperience) consideration.

> > While at it: there's no reason to impose a
> > naming convention of lineX, lineY etc., the names don't matter for the
> > aggregator setup (unlike gpio-sim where they indicate the offset of
> > the line they concern).
> >
> 
> Scratch that part. There's a good reason for that - the ordering of
> lines within the aggregator. [...]

You're right, that's exactly the intention of the strict naming, 'line0',
'line1', ..., 'line<Y>'.

> [...] I'm just not sure whether we should
> impose a strict naming where - for an aggregator of 3 lines total - we
> expect there to exist groups named line0, line1 and line2 or if we
> should be more lenient and possibly sort whatever names the user
> provides alphabetically?

As Maciej pointed out:

  (https://lore.kernel.org/all/CAFGk_a0U=jSQD85UKC1e=pSWr8W9y_MMAFyPVFOcE-fUZry7-Q@mail.gmail.com/#t)
  > [...] if free form names were for e.g. [1, 02, 10].

we would need a well-defined rule to avoid ambiguity, which could
potentially unnecessarily impose burden on users to understand how to
properly use the interface.


Regardless, the point is that we need to make it clear to users which GPIO
line a specific line<Y> of an aggregator forwards operations to. Since
requiring users to explicitly set the offset within the aggregator for each
virtual line (e.g. besides 'key'/'offset'/'name' attributes, by adding
'idx' attribute, which users would need to set explicitly) would be
cumbersome, this RFC implementation instead just makes use of directory
naming. I believe we agree on this approach (i.e., using directory naming
to establish ordering). Correct me if I'm wrong.

So, to move forward, let me outline the possible approaches we can take:

Option (a). Drop 'num_lines' attribute and:

  (a-1). Impose strict naming rule for line directories
  
         Users can only create directories with a predefined naming
	 convention. This could be 'line0', 'line1', ... 'line<Y>' (as in
	 the RFC implementation), or simply '0', '1', ..., '<Y>'.
  
  (a-2). Allow arbitrary naming for line directories

	 This would require a well-defined rule to avoid ambiguity. As
	 Maciej pointed out:

         (from https://lore.kernel.org/all/CAFGk_a0U=jSQD85UKC1e=pSWr8W9y_MMAFyPVFOcE-fUZry7-Q@mail.gmail.com/#t)
         > if free form names were for e.g. [1, 02, 10]

         Users would need to understand these rules, which might impose
	 unnecessary burden on users.

Option (b). Keep 'num_lines' attribute but:

    (b-1). Prohibit manual creation of line directories

	   Users would no longer run 'mkdir line0', etc. Instead, writing
	   <Y+1> (Y >= 0) to 'num_lines' would automatically set up the
	   required directories.  convention. This could be 'line0',
	   'line1', ... 'line<Y>' (as in this RFC implementation), or
	   simply '0', '1', ..., '<Y>'.

    (b-2). Keep manual 'mkdir' for each line, in the same manner as (a-1)
	   (as in the RFC implementation) or (a-2). Seems that no-one is
	   favor of this option.


    Note: (b-1) is a new idea. Considering what really needs to be
    configured by users, this could be the least burdensome and simplest,
    especially when configuring many lines. I'm including it here for
    broader discussion.

Personally, now I'm inclined towards (a-1) with the simplest naming scheme:
non-zero-padded integers ('./0', './1', './2', ..., './<Y>'). Or even (b-1).

Let me know your thoughts.

Thanks.

-Koichiro

> 
> Bart
Bartosz Golaszewski Jan. 31, 2025, 5:07 p.m. UTC | #7
On Fri, Jan 31, 2025 at 8:26 AM Maciej Borzęcki
<maciej.borzecki@canonical.com> wrote:
>
> On Thu, 30 Jan 2025 at 21:48, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > On Thu, Jan 30, 2025 at 7:40 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > >
> > > While at it: there's no reason to impose a
> > > naming convention of lineX, lineY etc., the names don't matter for the
> > > aggregator setup (unlike gpio-sim where they indicate the offset of
> > > the line they concern).
> > >
> >
> > Scratch that part. There's a good reason for that - the ordering of
> > lines within the aggregator. I'm just not sure whether we should
> > impose a strict naming where - for an aggregator of 3 lines total - we
> > expect there to exist groups named line0, line1 and line2 or if we
> > should be more lenient and possibly sort whatever names the user
> > provides alphabetically?
>
> If I may jump in quickly (I provided some initial feedback on the
> configfs interfaces
> for the first internal patches). I think it's preferable to have
> strict and explicit, even
> If more verbose, line ordering in the aggregator.The motivator for
> this is that whoever
> sets up a new device through the aggregator does not have to second guess what
> the driver will do. Implicit ordering could perhaps be fine if the
> consumers were to
> impose some set of rules themselves, but I fear there would still be
> some ambiguity
> left if free form names were for e.g. [1, 02, 10]. In the end they
> would probably settle
> on some mechanism which would mimic what we could already do in the
> driver itself
> and avoid any further confusion for the user.
>
> Cheers,
> Maciej

Fair enough, I was thinking that just sorting the entries
alphabetically would both allow the lineX scheme AND leave some leeway
for non-standard naming but it may indeed end up being confusing with
no apparent advantage.

Bart
Bartosz Golaszewski Jan. 31, 2025, 5:22 p.m. UTC | #8
On Fri, Jan 31, 2025 at 3:36 PM Koichiro Den <koichiro.den@canonical.com> wrote:
>
> On Thu, Jan 30, 2025 at 09:47:47PM GMT, Bartosz Golaszewski wrote:
> > On Thu, Jan 30, 2025 at 7:40 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > >
>
> (Small reply to the previous comment:)
> Yes, I understand your point about why you think 'num_lines' is
> unnecessary. What I meant was more of a UX (User eXperience) consideration.
>

If anything this sounds like worse user experience - having to provide
duplicate information.

> > > While at it: there's no reason to impose a
> > > naming convention of lineX, lineY etc., the names don't matter for the
> > > aggregator setup (unlike gpio-sim where they indicate the offset of
> > > the line they concern).
> > >
> >
> > Scratch that part. There's a good reason for that - the ordering of
> > lines within the aggregator. [...]
>
> You're right, that's exactly the intention of the strict naming, 'line0',
> 'line1', ..., 'line<Y>'.
>
> > [...] I'm just not sure whether we should
> > impose a strict naming where - for an aggregator of 3 lines total - we
> > expect there to exist groups named line0, line1 and line2 or if we
> > should be more lenient and possibly sort whatever names the user
> > provides alphabetically?
>
> As Maciej pointed out:
>
>   (https://lore.kernel.org/all/CAFGk_a0U=jSQD85UKC1e=pSWr8W9y_MMAFyPVFOcE-fUZry7-Q@mail.gmail.com/#t)
>   > [...] if free form names were for e.g. [1, 02, 10].
>
> we would need a well-defined rule to avoid ambiguity, which could
> potentially unnecessarily impose burden on users to understand how to
> properly use the interface.
>
>
> Regardless, the point is that we need to make it clear to users which GPIO
> line a specific line<Y> of an aggregator forwards operations to. Since
> requiring users to explicitly set the offset within the aggregator for each
> virtual line (e.g. besides 'key'/'offset'/'name' attributes, by adding
> 'idx' attribute, which users would need to set explicitly) would be
> cumbersome, this RFC implementation instead just makes use of directory
> naming. I believe we agree on this approach (i.e., using directory naming
> to establish ordering). Correct me if I'm wrong.
>
> So, to move forward, let me outline the possible approaches we can take:
>
> Option (a). Drop 'num_lines' attribute and:
>
>   (a-1). Impose strict naming rule for line directories
>
>          Users can only create directories with a predefined naming
>          convention. This could be 'line0', 'line1', ... 'line<Y>' (as in
>          the RFC implementation), or simply '0', '1', ..., '<Y>'.
>
>   (a-2). Allow arbitrary naming for line directories
>
>          This would require a well-defined rule to avoid ambiguity. As
>          Maciej pointed out:
>
>          (from https://lore.kernel.org/all/CAFGk_a0U=jSQD85UKC1e=pSWr8W9y_MMAFyPVFOcE-fUZry7-Q@mail.gmail.com/#t)
>          > if free form names were for e.g. [1, 02, 10]
>
>          Users would need to understand these rules, which might impose
>          unnecessary burden on users.
>
> Option (b). Keep 'num_lines' attribute but:
>
>     (b-1). Prohibit manual creation of line directories
>
>            Users would no longer run 'mkdir line0', etc. Instead, writing
>            <Y+1> (Y >= 0) to 'num_lines' would automatically set up the
>            required directories.  convention. This could be 'line0',
>            'line1', ... 'line<Y>' (as in this RFC implementation), or
>            simply '0', '1', ..., '<Y>'.
>
>     (b-2). Keep manual 'mkdir' for each line, in the same manner as (a-1)
>            (as in the RFC implementation) or (a-2). Seems that no-one is
>            favor of this option.
>
>
>     Note: (b-1) is a new idea. Considering what really needs to be
>     configured by users, this could be the least burdensome and simplest,
>     especially when configuring many lines. I'm including it here for
>     broader discussion.
>
> Personally, now I'm inclined towards (a-1) with the simplest naming scheme:
> non-zero-padded integers ('./0', './1', './2', ..., './<Y>'). Or even (b-1).
>

I too think a-1 is the best option. However, I'd go for line0, line1
etc. convention as for computers it doesn't make any difference while
for humans it's more readable.

Bartosz

> Let me know your thoughts.
>
> Thanks.
>
> -Koichiro
>
> >
> > Bart
Koichiro Den Feb. 1, 2025, 12:26 p.m. UTC | #9
On Fri, Jan 31, 2025 at 06:22:50PM GMT, Bartosz Golaszewski wrote:
> On Fri, Jan 31, 2025 at 3:36 PM Koichiro Den <koichiro.den@canonical.com> wrote:
> >
> > On Thu, Jan 30, 2025 at 09:47:47PM GMT, Bartosz Golaszewski wrote:
> > > On Thu, Jan 30, 2025 at 7:40 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > >
> >
> > (Small reply to the previous comment:)
> > Yes, I understand your point about why you think 'num_lines' is
> > unnecessary. What I meant was more of a UX (User eXperience) consideration.
> >
> 
> If anything this sounds like worse user experience - having to provide
> duplicate information.
> 
> > > > While at it: there's no reason to impose a
> > > > naming convention of lineX, lineY etc., the names don't matter for the
> > > > aggregator setup (unlike gpio-sim where they indicate the offset of
> > > > the line they concern).
> > > >
> > >
> > > Scratch that part. There's a good reason for that - the ordering of
> > > lines within the aggregator. [...]
> >
> > You're right, that's exactly the intention of the strict naming, 'line0',
> > 'line1', ..., 'line<Y>'.
> >
> > > [...] I'm just not sure whether we should
> > > impose a strict naming where - for an aggregator of 3 lines total - we
> > > expect there to exist groups named line0, line1 and line2 or if we
> > > should be more lenient and possibly sort whatever names the user
> > > provides alphabetically?
> >
> > As Maciej pointed out:
> >
> >   (https://lore.kernel.org/all/CAFGk_a0U=jSQD85UKC1e=pSWr8W9y_MMAFyPVFOcE-fUZry7-Q@mail.gmail.com/#t)
> >   > [...] if free form names were for e.g. [1, 02, 10].
> >
> > we would need a well-defined rule to avoid ambiguity, which could
> > potentially unnecessarily impose burden on users to understand how to
> > properly use the interface.
> >
> >
> > Regardless, the point is that we need to make it clear to users which GPIO
> > line a specific line<Y> of an aggregator forwards operations to. Since
> > requiring users to explicitly set the offset within the aggregator for each
> > virtual line (e.g. besides 'key'/'offset'/'name' attributes, by adding
> > 'idx' attribute, which users would need to set explicitly) would be
> > cumbersome, this RFC implementation instead just makes use of directory
> > naming. I believe we agree on this approach (i.e., using directory naming
> > to establish ordering). Correct me if I'm wrong.
> >
> > So, to move forward, let me outline the possible approaches we can take:
> >
> > Option (a). Drop 'num_lines' attribute and:
> >
> >   (a-1). Impose strict naming rule for line directories
> >
> >          Users can only create directories with a predefined naming
> >          convention. This could be 'line0', 'line1', ... 'line<Y>' (as in
> >          the RFC implementation), or simply '0', '1', ..., '<Y>'.
> >
> >   (a-2). Allow arbitrary naming for line directories
> >
> >          This would require a well-defined rule to avoid ambiguity. As
> >          Maciej pointed out:
> >
> >          (from https://lore.kernel.org/all/CAFGk_a0U=jSQD85UKC1e=pSWr8W9y_MMAFyPVFOcE-fUZry7-Q@mail.gmail.com/#t)
> >          > if free form names were for e.g. [1, 02, 10]
> >
> >          Users would need to understand these rules, which might impose
> >          unnecessary burden on users.
> >
> > Option (b). Keep 'num_lines' attribute but:
> >
> >     (b-1). Prohibit manual creation of line directories
> >
> >            Users would no longer run 'mkdir line0', etc. Instead, writing
> >            <Y+1> (Y >= 0) to 'num_lines' would automatically set up the
> >            required directories.  convention. This could be 'line0',
> >            'line1', ... 'line<Y>' (as in this RFC implementation), or
> >            simply '0', '1', ..., '<Y>'.
> >
> >     (b-2). Keep manual 'mkdir' for each line, in the same manner as (a-1)
> >            (as in the RFC implementation) or (a-2). Seems that no-one is
> >            favor of this option.
> >
> >
> >     Note: (b-1) is a new idea. Considering what really needs to be
> >     configured by users, this could be the least burdensome and simplest,
> >     especially when configuring many lines. I'm including it here for
> >     broader discussion.
> >
> > Personally, now I'm inclined towards (a-1) with the simplest naming scheme:
> > non-zero-padded integers ('./0', './1', './2', ..., './<Y>'). Or even (b-1).
> >
> 
> I too think a-1 is the best option. However, I'd go for line0, line1
> etc. convention as for computers it doesn't make any difference while
> for humans it's more readable.

Thank you for the comments. I’ll address your feedbacks and send "v2"
later, Since we seem to agree on the overall approach, I’ll send it as
[PATCH v1] (i.e., without the "RFC").

-Koichiro

> 
> Bartosz
> 
> > Let me know your thoughts.
> >
> > Thanks.
> >
> > -Koichiro
> >
> > >
> > > Bart
Bartosz Golaszewski Feb. 1, 2025, 4:10 p.m. UTC | #10
On Sat, Feb 1, 2025 at 1:26 PM Koichiro Den <koichiro.den@canonical.com> wrote:
>
> On Fri, Jan 31, 2025 at 06:22:50PM GMT, Bartosz Golaszewski wrote:
> >
> > I too think a-1 is the best option. However, I'd go for line0, line1
> > etc. convention as for computers it doesn't make any difference while
> > for humans it's more readable.
>
> Thank you for the comments. I’ll address your feedbacks and send "v2"
> later, Since we seem to agree on the overall approach, I’ll send it as
> [PATCH v1] (i.e., without the "RFC").
>

No, please don't. It'll be confusing. The RFC WAS the v1. Next
iteration must be v2 with a changelog.

Bartosz