Message-ID: <Z921_7-I0bXDN_Si@cassiopeiae>
Date: Fri, 21 Mar 2025 19:54:55 +0100
From: Danilo Krummrich <dakr@...nel.org>
To: Jason Gunthorpe <jgg@...dia.com>
Cc: John Hubbard <jhubbard@...dia.com>,
	Greg KH <gregkh@...uxfoundation.org>,
	Joel Fernandes <joelagnelf@...dia.com>,
	Alexandre Courbot <acourbot@...dia.com>,
	Dave Airlie <airlied@...il.com>, Gary Guo <gary@...yguo.net>,
	Joel Fernandes <joel@...lfernandes.org>,
	Boqun Feng <boqun.feng@...il.com>, Ben Skeggs <bskeggs@...dia.com>,
	linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org,
	nouveau@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
	paulmck@...nel.org
Subject: Re: [RFC PATCH 0/3] gpu: nova-core: add basic timer subdevice
 implementation

On Fri, Mar 21, 2025 at 02:49:20PM -0300, Jason Gunthorpe wrote:
> On Fri, Mar 21, 2025 at 01:12:30PM +0100, Danilo Krummrich wrote:
> 
> > Not all device resources are managed in the context of the subsystem, so
> > subsystem-level revokes do not apply.
> 
> They could, you could say that these rust APIs are only safe to use
> for device drivers with C code providing a fence semantic, eg through
> a subsystem.

Not sure how such an implementation would look like. How would you model such a
requirement through the type system? Or do you propose to just use unsafe {}?

> > For the DMA coherent allocations, please see my comment in [1]. Revoking the
> > device resources associated with a DMA coherent allocation should hence never
> > cause any overhead for accessing DMA memory.
> 
> > [1] https://github.com/Rust-for-Linux/linux/blob/rust-next/rust/kernel/dma.rs#L120
> 
> I don't know what to make of this. You argued so much to support
> revocable for rust ideological reasons and in the end the proposal is
> to just completely gives up on all of that?

And I still do so for certain cases, such as I/O memory.

For the CoherentAllocation though, AFAICS, there is no reason to revoke all the
memory, but just the device resources behind.

You gave me the relevant cause for thought on this. I don't see why you would be
unhappy with the idea. We still correspond with the rules of the driver core,
i.e. ensure that device resources do not out-live driver unbind, but still don't
have to deal with a revocable wrapper.

(One side note, please stop continuously accusing me of things, such as having
"rust ideological reasons", not having gone through proper review with the
driver core abstractions, etc. It won't achieve anything and does not come
across very professional.)

> 
> Not even an optional runtime check? :(

What kind of runtime check do you propose in the meantime? Maybe you can send a
patch? That'd be very much appreciated.

> 
> And I'm not sure about the comment written:
> 
> > // However, it is neither desirable nor necessary to protect the allocated memory of the DMA
> > // allocation from surviving device unbind;
> 
> There are alot of things on this path that depend on the struct
> device, there are two kinds of per-device coherent memory allocators
> and swiotlb as well.
> 
> It looks like there are cases where the actual memory must not outlive
> the driver binding.

I don't see why actual memory allocations would ever be required to be freed
immediately when the driver is unbound. It has nothing to do with the device.

Do you have an example where this would be a requirement?

> 
> So, I'd argue that it is necessary, and changing that on the C side
> looks like a big project.

I don't think that the idea is trivial to implement, but ideally the C side
should benefit as well.

Powered by blists - more mailing lists