Re: [PATCH v1 5/5] sbm: SandBox Mode documentation

From: Greg Kroah-Hartman
Date: Thu Feb 15 2024 - 04:11:17 EST


On Wed, Feb 14, 2024 at 08:42:54PM +0100, Petr Tesařík wrote:
> On Wed, 14 Feb 2024 19:48:52 +0100
> Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> > On Wed, Feb 14, 2024 at 05:31:12PM +0100, Petr Tesařík wrote:
> > > On Wed, 14 Feb 2024 16:11:05 +0100
> > > Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > > On Wed, Feb 14, 2024 at 03:55:24PM +0100, Petr Tesařík wrote:
> > > > > OK, so why didn't I send the whole thing?
> > > > >
> > > > > Decomposition of the kernel requires many more changes, e.g. in linker
> > > > > scripts. Some of them depend on this patch series. Before I go and
> > > > > clean up my code into something that can be submitted, I want to get
> > > > > feedback from guys like you, to know if the whole idea would be even
> > > > > considered, aka "Fail Fast".
> > > >
> > > > We can't honestly consider this portion without seeing how it would
> > > > work, as we don't even see a working implementation that uses it to
> > > > verify it at all.
> > > >
> > > > The joy of adding new frameworks is that you need a user before anyone
> > > > can spend the time to review it, sorry.
> > >
> > > Thank your for a quick assessment. Will it be sufficient if I send some
> > > code for illustration (with some quick&dirty hacks to bridge the gaps),
> > > or do you need clean and nice kernel code?
> >
> > We need a real user in the kernel, otherwise why would we even consider
> > it? Would you want to review a new subsystem that does nothing and has
> > no real users? If not, why would you want us to? :)
>
> Greg, please enlighten me on the process. How is something like this
> supposed to get in?

If you were in our shoes, what would you want to see in order to be able
to properly review and judge if a new subsystem was ok to accept?

> Subsystem maintainers will not review code that depends on core features
> not yet reviewed by the respective maintainers. If I add only the API
> and a stub implementation, then it brings no benefit and attempts to
> introduce the API will be dismissed. I would certainly do just that if
> I was a maintainer...

Exactly, you need a real user.

> I could try to pack everything (base infrastructure, arch
> implementations, API users) into one big patch with pretty much
> everybody on the Cc list, but how is that ever going to get reviewed?

How are we supposed to know if any of this even works at all if you
don't show that it actually works and is useful? Has any of that work
even been done yet? I'm guessing it has (otherwise you wouldn't have
posted this), but you are expecting us to just "trust us, stuff in the
future is going to use this and need it" here.

Again, we can not add new infrastructure for things that have no users,
nor do you want us to. Ideally you will have at least 3 different
users, as that seems to be the "magic number" that shows that the
api/interface will actually work well, and is flexible enough. Just
one user is great for proof-of-concept, but that usually isn't good
enough to determine if it will work for others (and so it wouldn't need
to be infrastructure at all, but rather just part of that one feature on
its own.)

> Should I just go and maintain an out-of-tree repo for a few years,
> hoping that it gets merged one day, like bcachefs? Is this the way?

No, show us how this is going to be used.

Again, think about what you would want if you had to review this.

thanks,

greg k-h