Re: [PATCH 1/5] dmaengine: Store module owner in dma_device struct

From: Dave Jiang
Date: Fri Nov 22 2019 - 11:53:26 EST

On 11/21/19 10:20 PM, Vinod Koul wrote:
On 14-11-19, 10:03, Logan Gunthorpe wrote:

On 2019-11-13 9:55 p.m., Vinod Koul wrote:
But that's the problem. We can't expect our users to be "nice" and not
unbind when the driver is in use. Killing the kernel if the user
unexpectedly unbinds is not acceptable.

And that is why we review the code and ensure this does not happen and
behaviour is as expected

Yes, but the current code can kill the kernel when the driver is unbound.

I suspect this is less of an issue for most devices as they wouldn't
normally be unbound while in use (for example there's really no reason
to ever unbind IOAT seeing it's built into the system). Though, the fact
is, the user could unbind these devices at anytime and we don't want to
panic if they do.

There are many drivers which do modules so yes I am expecting unbind and
even a bind following that to work

Except they will panic if they unbind while in use, so that's a
questionable definition of "work".

dmaengine core has module reference so while they are being used they
won't be removed (unless I complete misread the driver core behaviour)

Yes, as I mentioned in my other email, holding a module reference does
not prevent the driver from being unbound. Any driver can be unbound by
the user at any time without the module being removed.

That sounds okay then.

I'm actually glad Logan is putting some work in addressing this. I also ran into the same issue as well dealing with unbinds on my new driver.

Essentially, at any time, a user can do this:

echo 0000:83:00.4 > /sys/bus/pci/drivers/plx_dma/unbind

Which will call plx_dma_remove() regardless of whether anyone has a
reference to the module, and regardless of whether the dma channel is
currently in use. I feel it is important that drivers support this
without crashing, and my plx_dma driver does the correct thing here.