Messages in this thread | | | Date | Mon, 31 May 2021 11:55:43 +0200 | From | Daniel Wagner <> | Subject | Re: [EXT] [RFC 0/2] Serialize timeout handling and done callback. |
| |
Hi Arun,
On Mon, May 31, 2021 at 02:06:24AM -0700, Arun Easi wrote: > Thanks Daniel for the report and your effort here. Agree, this needs to be > fixed.
Good to hear!
> If you do not mind, can I take this from here? This touches quite a > number of paths, and I would like to have this go through a full > regression cycle before this is merged.
Sure, that is what I hoped for. It is an invasive change and this needs to be properly tested with a few different setups. Something I can't really do. So I would be glad if you could pick up the patches and fix them up.
> That said, I had some general comments: > > 1. I see sp->lock was introduced, but could not locate where it was > used.
I thought I needed it for serializing the kref operations. The lock itself is not used in the driver. After re-reading the documentation, the lock is not necessary as kref_put() is able to serialize the ref counter inc/dec operation correctly. The lock would only be useful to serialize the kref_put() with something which runs in the driver concurrently.
> 2. I did not see a release of lock after a successful kref_put_lock, maybe > that piece was missed out.
I think you got it right. The lock is not necessary.
> 3. The sp->done should be invoked only once, and I do not see if this is > taken care of.
qla2x00_sp_release() will only be called when the ref counter gets 0. This makes sure we only call sp->done() once.
> 4. sp->cmd_sp could be a SCSI IO too, where sp is not allocated > separately, so qla2x00_sp_release shall not be called for it.
Okay, didn't realize this.
Thanks, Daniel
| |