[RFC v7 11/21] um: nommu: kernel thread support

[Octavian Purdila]

On Thu, Oct 8, 2020 at 10:39 PM Johannes Berg <johannes at sipsolutions.net> wrote:
>
> On Thu, 2020-10-08 at 21:54 +0300, Octavian Purdila wrote:
> > On Wed, Oct 7, 2020 at 9:57 PM Johannes Berg <johannes at sipsolutions.net> wrote:
> > > On Tue, 2020-10-06 at 18:44 +0900, Hajime Tazaki wrote:
> > > > nommu mode does not support user processes
> > >
> > > I find this really confusing. I'm not sure why you ended up calling this
> > > "nommu mode", but there *are* (still) (other) nommu arches, and they
> > > *do* support userspace processes.
> > >
> > > Isn't this really just "LKL mode" or something like that?
> > >
> >
> > This is a very good point, while some other patches make sense in the
> > nommu mode, this one does not - it is rather needed because of the
> > "library mode".
> >
> > Not sure what we can do other than creating a new "library mode" in
> > addition to the "nommu mode". Any suggestions?
>
> Well there's no "nommu mode" in UML other than what you're doing here,
> so as I said on some other patch, it sort of makes sense to have "LKL ==
> NOMMU", but the equation doesn't make sense everywhere, since it's not
> fundamentally NOMMU that drives the need for things (like here no
> userspace, elsewhere the ifdefs, etc.), but LKL-mode.
>
> So I don't think it would be *in addition* to "nommu mode" since such a
> thing doesn't exist on UML (only on other architectures), but mostly be
> a rename of "nommu mode" to "lkl mode" or so?
>
> Don't really have any other suggestions, or maybe I'm not understanding
> your question right.

OK, I agree, renaming "nommu mode" to "lkl mode" looks like the right
thing to do for now.

>
> > > IOW, why isn't this just
> > >
> > > void lkl_sem_free(struct lkl_sem *sem);
> > > void lkl_sem_up(struct lkl_sem *sem);
> > > ...
> > >
> > > and then posix-host.c just includes the header file and implements those
> > > functions?
> > >
> > > I don't see any reason for this to be allowed to have multiple variants
> > > linked and then picking them at runtime?
> > >
> >
> > We could try that and see how it goes. This was baked liked this long
> > time ago, when we wanted to support Windows and there was no proper
> > support for weak functions in mingw for PE/COFF (it still not
> > supported but at least we do have a few patches that fix that).
>
> You've required weak functions elsewhere, but in this case you don't
> even need them since you don't need things to link without an
> implementation? At least I don't see why you'd want to be able to link a
> binary that doesn't have an implementation of the ops required to run?

Yeah, all good points :) I'll discuss it more with Hajime to make sure
I haven't missed anything and we will try it in the next patch series.


> > > Yeah, what? That's an incomprehensible piece of code. At least add
> > > comments, if it _really_ is necessary?
> > >
> >
> > Yeah, sorry about that. We missed adding a bunch of comments in the
> > commit message. It got this complicated because of optimizations to
> > avoid context switching between the native thread running the
> > application and the kernel thread running the system call or interrupt
> > handler.
> >
> > Maybe we should revert to the initial simpler implementation for now
> > and add it later?
>
> Perhaps? Not really sure. Could the optimisations be added in steps so
> they're something that can be explained/followed? If not, well, perhaps
> to ease review for now it'd make sense to start simpler, but I guess
> eventually it'd still want some better explanation of what's going on.
>

OK, I'll discuss more with Hajime, at this point I think we might want
to focus on getting the basics merged first. In either case we will
make sure to have it properly explained.