[PATCH 7/7] accel/qaic: Add QAIC_DETACH_SLICE_BO IOCTL

[Jeffrey Hugo]

On 9/17/2023 2:56 AM, Stanislaw Gruszka wrote:
> On Fri, Sep 01, 2023 at 11:22:47AM -0600, Jeffrey Hugo wrote:
>> From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy at quicinc.com>
>>
>> Once a BO is attached with slicing configuration that BO can only be used
>> for that particular setting. With this new feature user can detach slicing
>> configuration off an already sliced BO and attach new slicing configuration
>> using QAIC_ATTACH_SLICE_BO.
>>
>> This will support BO recycling.
>>
>> detach_slice_bo() detaches slicing configuration from a BO. This new
>> helper function can also be used in release_dbc() as we are doing the
>> exact same thing.
>>
>> Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy at quicinc.com>
>> Reviewed-by: Jeffrey Hugo <quic_jhugo at quicinc.com>
>> [jhugo: add documentation for new ioctl]
>> Signed-off-by: Jeffrey Hugo <quic_jhugo at quicinc.com>
> <snip>
> 
>> +	/* Check if BO is committed to H/W for DMA */
>> +	spin_lock_irqsave(&dbc->xfer_lock, flags);
>> +	if (bo->queued) {
>> +		spin_unlock_irqrestore(&dbc->xfer_lock, flags);
>> +		ret = -EBUSY;
>> +		goto unlock_ch_srcu;
>> +	}
>> +	spin_unlock_irqrestore(&dbc->xfer_lock, flags);
> 
> This looks like race condition. If some other thread will take the xfer_lock
> and set bo->queued (HERE just after _unlock())  we will not return -EBUSY.
> Something seems to be missing here or xfer_lock is not needed to protect
> bo->queued.

The other thread would also need to take the bo->lock, which is held 
here and not released until after detach_slice_bo().  xfer_lock actually 
protects xfer_list, but bo->queued is a quick check to see if the bo is 
in the list, rather than iterating the list.  I can see how this is 
misleading.  I will ponder how to improve it.

-Jeff