Re: [libgpiod] Rethinking struct gpiod_line_bulk

[Kent Gibson <warthog618@xxxxxxxxx>]

On Mon, Oct 12, 2020 at 05:15:25PM +0200, Bartosz Golaszewski wrote:
> Hi!
> 
> One of the things I'd like to address in libgpiod v2.0 is excessive
> stack usage with struct gpiod_line_bulk. This structure is pretty big
> right now: it's an array 64 pointers + 4 bytes size. That amounts to
> 260 bytes on 32-bit and 516 bytes on 64-bit architectures
> respectively. It's also used everywhere as all functions dealing with
> single lines eventually end up calling bulk counterparts.
> 
> I have some ideas for making this structure smaller and I thought I'd
> run them by you.
> 
> The most obvious approach would be to make struct gpiod_line_bulk
> opaque and dynamically allocated. I don't like this idea due to the
> amount of error checking this would involve and also calling malloc()
> on virtually every value read, event poll etc.
> 
> Another idea is to use embedded list node structs (see include/list.h
> in the kernel) in struct gpiod_line and chain the lines together with
> struct gpiod_line_bulk containing the list head. That would mean only
> being able to store each line in a single bulk object. This is
> obviously too limiting.
> 

I don't think I've ever gotten my head fully around the libgpiod API,
or all its use cases, and I'm not clear on why this is too limiting.

What is the purpose of the gpiod_line_bulk, and how does that differ from the
gpio_v2_line_request?

> An idea I think it relatively straightforward without completely
> changing the current interface is making struct gpiod_line_bulk look
> something like this:
> 
> struct gpiod_line_bulk {
>     unsigned int num_lines;
>     uint64_t lines;
> };
> 
> Where lines would be a bitmap with set bits corresponding to offsets
> of lines that are part of this bulk. We'd then provide a function that
> would allow the user to get the line without it being updated (so
> there's no ioctl() call that could fail). The only limit that we'd
> need to introduce here is making it impossible to store lines from
> different chips in a single line bulk object. This doesn't make sense
> anyway so I'm fine with this.
> 
> What do you think? Do you have any other ideas?
> 

Doesn't that place a strict range limit on offset values, 0-63?
The uAPI limits the number of offsets requested to 64, not their value.
Otherwise I'd've used a bitmap there as well.

Or is there some other mapping happening in the background that I'm
missing?

Cheers,
Kent.