On Thu, 2018-12-06 at 10:49 -0700, Jason Gunthorpe wrote:
> On Thu, Dec 06, 2018 at 10:47:33AM +0200, Gal Pressman wrote:
>
> > > > +static void efa_destroy_ah_id(struct efa_dev *dev, u8 *id) {
> > > > + u16 device_ah;
> > > > + u32 ref_count;
> > > > + int err;
> > > > +
> > > > + mutex_lock(&dev->ah_list_lock);
> > >
> > > Create and destroy ah for kernel consumers cannot sleep.
> > > Though I understand that currently you don't have kernel consumers.
> > > This should be changed to spin lock along with other allocation to GFP_ATOMIC.
> >
> > Create/destroy AH must be communicated with our device (in the form of an admin
> > command), this requires us to sleep regardless of the mutex/memory allocation
> > (we shouldn't busy wait while waiting for the device to complete the command).
> > None of the currently supported flows are done in an atomic context, we will
> > have to figure that out when we add support for those.
>
> You don't get to make up your own rules. The function pointer this is
> being assigned to required atomic support.
The core code itself treats this as non-atomic for user space calls, and
only atomic for kernel calls. So this isn't making up their own rules,
this is following *exactly* the same rules as the current core kernel
code, they simply don't provide a kernel verbs API implementation, only
a user space implementation.
> That the driver doesn't
> happen to trigger that call path today doesn't matter at all.
Of course it does.
> We can't maintain the core code properly if drivers randomly decide to
> not follow the rules because it is inconvenient.
How often do we tell people that we only accept kernel code that has a
user, no dead code? Well, putting worthless spinlock spinning in here
would be exactly that, dead code.
Besides, they had something ugly in there and I told them to drop it, so
you can blame me for that.
--
Doug Ledford <dledford@xxxxxxxxxx>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
Attachment:
signature.asc
Description: This is a digitally signed message part
