On 2022-02-23 16:03, Greg Kroah-Hartman wrote:
On Wed, Feb 23, 2022 at 10:30:11AM -0400, Jason Gunthorpe wrote:
On Wed, Feb 23, 2022 at 10:09:01AM -0400, Jason Gunthorpe wrote:
On Wed, Feb 23, 2022 at 03:06:35PM +0100, Greg Kroah-Hartman wrote:
On Wed, Feb 23, 2022 at 09:46:27AM -0400, Jason Gunthorpe wrote:
On Wed, Feb 23, 2022 at 01:04:00PM +0000, Robin Murphy wrote:
1 - tmp->driver is non-NULL because tmp is already bound.
1.a - If tmp->driver->driver_managed_dma == 0, the group must currently be
DMA-API-owned as a whole. Regardless of what driver dev has unbound from,
its removal does not release someone else's DMA API (co-)ownership.
This is an uncommon locking pattern, but it does work. It relies on
the mutex being an effective synchronization barrier for an unlocked
store:
WRITE_ONCE(dev->driver, NULL)
Only the driver core should be messing with the dev->driver pointer as
when it does so, it already has the proper locks held. Do I need to
move that to a "private" location so that nothing outside of the driver
core can mess with it?
It would be nice, I've seen a abuse and mislocking of it in drivers
Though to be clear, what Robin is describing is still keeping the
dev->driver stores in dd.c, just reading it in a lockless way from
other modules.
"other modules" should never care if a device has a driver bound to it
because instantly after the check happens, it can change so what ever
logic it wanted to do with that knowledge is gone.
Unless the bus lock is held that the device is on, but that should be
only accessable from within the driver core as it controls that type of
stuff, not any random other part of the kernel.
And in looking at this, ick, there are loads of places in the kernel
that are thinking that this pointer being set to something actually
means something. Sometimes it does, but lots of places, it doesn't as
it can change.
That's fine. In this case we're only talking about the low-level IOMMU
code which has to be in cahoots with the driver core to some degree (via
these new callbacks) anyway, but if you're uncomfortable about relying
on dev->driver even there, I can live with that. There are several
potential places to capture the relevant information in IOMMU API
private data, from the point in really_probe() where it *is* stable, and
then never look at dev->driver ever again - even from .dma_cleanup() or
future equivalent, which is the aspect from whence this whole
proof-of-concept tangent span out.
Cheers,
Robin.
