Hi Tom, On Thu, 28 May 2026 at 20:18, Tom Rini <[email protected]> wrote: > > On Thu, May 28, 2026 at 12:12:09PM -0500, Simon Glass wrote: > > Hi Tom, > > > > On Wed, 27 May 2026 at 21:43, Tom Rini <[email protected]> wrote: > > > > > > On Wed, May 27, 2026 at 08:36:45PM +0200, Heinrich Schuchardt wrote: > > > > On 5/27/26 18:10, Simon Glass wrote: > > > > > Sandbox needs to include system headers in some files, but also wants > > > > > to use alist. Adjust the headers to permit this. > > > > > > > > > > Signed-off-by: Simon Glass <[email protected]> > > > > > --- > > > > > > > > > > include/alist.h | 16 +++++++++++----- > > > > > lib/alist.c | 1 + > > > > > 2 files changed, 12 insertions(+), 5 deletions(-) > > > > > > > > > > diff --git a/include/alist.h b/include/alist.h > > > > > index b00d9ea97d6..69d7cdb722f 100644 > > > > > --- a/include/alist.h > > > > > +++ b/include/alist.h > > > > > @@ -10,8 +10,14 @@ > > > > > #define __ALIST_H > > > > > #include <stdbool.h> > > > > > -#include <linux/bitops.h> > > > > > + > > > > > +#ifdef USE_HOSTCC > > > > > +#include <sys/types.h> > > > > > +#include <stdint.h> > > > > > +#else > > > > > #include <linux/types.h> > > > > > +#endif > > > > > +#define BIT(nr) (1UL << (nr)) > > > > > > > > BIT(nr) is already defined in include/linux/bitops.h. > > > > > > > > We must not assume that no module includes both alist.h and > > > > linux/bitops.h. > > > > E.g. include/expo.h and include/lmb.h include both alist.h and > > > > linux/bitops.h. We should avoid redefinitions. > > > > > > > > The following is an abuse of the enum type: > > > > > > > > enum alist_flags { > > > > ALISTF_FAIL = BIT(0), > > > > }; > > > > > > > > As this is the only use of BIT(), we could simply use the value "1" > > > > here. > > > > > > > > #define ALISTF_FAIL (1) > > > > > > > > But as ALISTF_FAIL is the only bit in flags used, the best solution > > > > would be > > > > replacing flags by a boolean called fail. > > > > > > I see this as another reminder about how frustrating it was that alist > > > was introduced under the guise of "must have this for an x86 feature" > > > but instead is a solution in need of a problem to solve, and whenever > > > it's used for something else, yet another problem with it is shown. > > > > The alist was motivated by its original commit. It allows easy > > iteration, uses less memory (no next/prev pointers in every element), > > uses fewer allocations than linked lists (one malloc() for the whole > > list instead of one for each node) and has much better cache locality > > as a result (i.e. improved performance). > > > > I asked my friendly AI to compare them: > > > > - Reach for alist when you want a growable, index-addressable, > > cache-friendly > > collection of small value objects you mostly append to and iterate (e.g. > > bootflow/bootdev scan results, parsed tables). > > - Reach for list_head when elements need O(1) removal/splice from > > arbitrary > > positions, must keep stable identity while linked, or already exist as > > independently-allocated objects (most driver-model / subsystem object > > lists). > > > > One nuance: alist_for_each is pure pointer arithmetic over the buffer, > > whereas > > list_for_each_entry uses container_of to recover the enclosing struct > > from > > the embedded node — a direct consequence of array-of-values vs. > > intrusive-links. > > This changes absolutely nothing about what I said the last time you > wanted to use alist more widely, and in turn what I said about it a year > ago, or whenever that was. I'm not going to go ask an unfriendly AI to > say what's wrong with alist, I'm just going to say that I already said > it.
I understand that, but it does actually have value. It is less error-prone (although the API could use some improvements) and the performance improvements are worth having too. I have found it very useful in bootstd and wish I had started with it. Not that I expect to change your mind, but this is a technical discussion which could be covered in a future community call. I would like to understand your objection beyond just 'Linux doesn't have it'. Regards, Simon
