Re: [PATCH 5/7] ibmvnic: use a lock to serialize remove/reset

Jakub Kicinski [kuba@kernel.org] wrote:

On Sun, 10 Jan 2021 19:52:25 -0800 Sukadev Bhattiprolu wrote:

quoted

Jakub Kicinski [kuba@kernel.org] wrote:

quoted

On Thu,  7 Jan 2021 23:12:34 -0800 Sukadev Bhattiprolu wrote:  

quoted

Use a separate lock to serialze ibmvnic_reset() and ibmvnic_remove()
functions. ibmvnic_reset() schedules work for the worker thread and
ibmvnic_remove() flushes the work before removing the adapter. We
don't want any work to be scheduled once we start removing the
adapter (i.e after we have already flushed the work).  

Locking based on functions, not on data being accessed is questionable
IMO. If you don't want work to be scheduled isn't it enough to have a
bit / flag that you set to let other flows know not to schedule reset?  

Maybe I could improve the description, but the "data" being protected
is the work queue. Basically don't add to the work queue while/after
it is (being) flushed.

Existing code is checking for the VNIC_REMOVING state before scheduling
the work but without a lock. If state goes to REMOVING after we check,
we could schedule work after the flush?

I see, and you can't just use the state_lock because it has to be a
spin_lock? If that's the case please just update the commit message 
and comments to describe the data protected.

Yes, has to be spin lock. Will update description.

Thanks,

Sukadev