Re: [PATCH 4/8] rust: add driver abstraction

[Greg KH <gregkh@linuxfoundation.org>]

On Tue, Mar 26, 2024 at 04:02:04PM +1000, David Airlie wrote:
> On Tue, Mar 26, 2024 at 3:37 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Mar 26, 2024 at 05:36:14AM +1000, David Airlie wrote:
> > > > I stopped here.  Please build on existing code in the kernel, and also,
> > > > a real user of these structures is needed to see if/how any of this
> > > > actually works.  Why not implement something that uses them that we can
> > > > actually review?
> > >
> > > Seems like you missed where the cover letter explained that this was
> > > in the context of:
> > > https://lore.kernel.org/dri-devel/Zfsj0_tb-0-tNrJy@cassiopeiae/T/#u
> >
> > Yes I did.
> >
> > > Can we stop feeding the chicken/egg problem? :-) This posting attempts
> > > to focus people on *device* abstraction problem, by posting small
> > > patch series directly affecting things in an effort to get them
> > > reviewed, or we post the full patch series touching every subsystem
> > > and then nobody reviews it because it's 50 patches and it isn't
> > > focused on the one thing they care about.
> >
> > Would you want to review new api additions without ever seeing how they
> > are used at all?  I can't agree to that, and neither would you, that's
> > not how any of this works, sorry.
> >
> > I can provide general "this looks really odd" comments, but I can never
> > give a "acked-by:" for something that doesn't have a user, that's just
> > impossible.
> >
> > I understand your prediciment, so work at it in tiny steps like everyone
> > does when adding new apis, this isn't new.
> >
> 
> These are the tiny steps, unfortunately to even create a driver with
> enough "driver" that you have something to review requires a bunch of
> abstractions to a bunch of subsystems.
> 
> In order to do a stub drm driver, we need at least the device, pci and
> drm device abstractions, they all build on each other, and have
> different sets of expert opinions to gather from different people.
> 
> We just end up losing the reviewers in the noise, because there are 50
> patches to review and nobody wants to look at it. Maybe you would
> prefer things that way, and we can certainly optimise for you, but I
> think there'll be a bunch of other less rust friendly maintainers that
> might be more painful.
> 
> Thanks for the review so far, and I don't think we are at the
> expecting acks stage anyways, just discussion around lifetimes and how
> it integrates with the C side are probably way more valuable.
> 
> Maybe for next sending, Danilo can try the all the patches approach
> and we can see if your expectations of kernel maintainers align with
> mine :-)

I don't mind 50 patch long series, those are fine as we can see how
things are actually used.

But for complex beasts like DRM drivers, perhaps people should stop and
implement the tiny "easy" things first, with working users, to verify
that it is all ok first before biting off the big ones.

Like implement a "simple" bus, which means that you have to have the
driver core bindings all working properly and correctly.  Once that is
done and merged then move to the next thing (i.e. a more complex bus
like PCI or USB or platform).

Drivers consume from almost all of the kernel at once, attempting to
write a driver in rust requires bindings from all of those things at
once, a very daunting and difficult task as people are quickly finding
out.  Break it down into smaller things, like the phy driver did, and
move forward that way if people wish to see rust succeed.  Don't attempt
to go and do the biggest beast first, that's just going to make people
frustrated quickly as you have to interact with too many different parts
of the kernel all at once.

Heck, we don't even have working atomics in rust yet, the basic building
blocks of almost all drivers that have complex data paths...

good luck!

greg k-h