Re: [RFC] Use plan9 C extensions

[Matthew Wilcox <willy@infradead.org>]

On Wed, Jan 09, 2019 at 02:07:11PM -0800, Nick Desaulniers wrote:
> On Wed, Jan 9, 2019 at 12:31 PM Matthew Wilcox <willy@infradead.org> wrote:
> > For cyclic allocations, we need to keep a 'next' value somewhere.
> > The IDR had a moderately large root, so they didn't mind the cost to
> > all users of embedding the next value in the root.
> 
> This sounds like some kind of "intrusive data structure?"  Doesn't the
> kernel have macro's for generic intrusive containers?  Would you be
> adding similar macros for xarray (or is that irrelevant to your
> question)?

I think that's irrelevant ...

> Sorry, this doesn't look quite syntactically valid, so I'm having a
> hard time following along.  Would you be able to show me a simple
> reproducer in godbolt (https://godbolt.org/) with GCC what you're
> trying to do (maybe with comments showing before/after)?  (Godbolt is
> a godsend for developers to clearly communicate codegen issues with
> compiler vendors, and its short links can easily be embedded in bug
> reports and commit messages).

Sure!  https://godbolt.org/z/OfHOLI is an example of a client of the API
I want to create.

> > (ie one fewer argument, as well as one fewer thing to track in their struct).
> 
> The difference in the second case from the first looks like the "one
> fewer argument" is the address of another member of `nn`.  In general
> cases of trying to minimize the number of parameters passed, rather
> than pass 2 pointers to 2 members, I would curious if you could just
> pass 1 pointer to parent struct (ie. `nn`) to `xa_foo()` and friends
> and pull out the members there?

Not really; nn is going to be a per-client type, ie hundreds of different
structs.  The struct xarray gets embedded in it.

> > That's what -fplan9-extensions would allow us to do.  Without it,
> > we'd need to name that struct xarray and need extra syntactic sugar;
> > eg later when it calls xa_erase(), it'd look like this:
> >
> >         xa_erase(&nn->s2s_cp_stateids, copy->cp_stateid.si_opaque.so_id);
> >
> > instead of:
> >
> >         xa_erase(&nn->s2s_cp_stateids.xa, copy->cp_stateid.si_opaque.so_id);
> 
> Also hard to spot the intended difference as there's no `xa` member in
> any of your examples (I assume you meant `xarray`, but a godbolt link
> would be appreciated to make sure I'm understanding the problem
> correctly).

Right, what I'm trying to communicate there is that if we need to name
the xarray, we then have to name it everywhere that we use it.

> Thanks for including us (LLVM folks)!  It would be good that new
> additions be made optional if possible if unsupported in Clang; but
> that's not doable in this case.  From the documentation, it seems that
> `-fplan9-extensions` enables a couple of things, and I think with a
> clearer example of what you're trying to do, I would be better
> equipped to comment on which part of the compiler flag you'd like to
> make use of.

I hope the example makes things clearer!