On Jun 9, 2015, at 1:30 PM, Brian King wrote:
> On 06/09/2015 11:01 AM, Matthew R. Ochs wrote:
>> 
>> On Jun 9, 2015, at 9:37 AM, Manoj Kumar wrote:
>>> On 6/9/2015 6:29 AM, Brian King wrote:
>>>>> 
>>>>> This was the optimization to avoid the MMIO for both threads. The other 
>>>>> thread that raced should
>>>>> do the atomic set of afu->room to a positive value.
>>>> 
>>>> Let's take the simpler scenario of just one thread.
>>>> 
>>>> Let's start with afu->room = 1
>>>> We call atomic64_dec_if_positive, which results in afu->room going to zero 
>>>> and 0 being returned,
>>>> so we go into the if leg.
>>>> 
>>>> If afu->room is zero every time we read it from the adapter and we exhaust 
>>>> our retries,
>>>> we return SCSI_MLQUEUE_HOST_BUSY. However, the next time we enter 
>>>> cxlflash_send_cmd,
>>>> since afu->cmd is now 0, it will no longer get decremented, but the return 
>>>> value will
>>>> be -1, so we'll go down the else if leg. We'll never get into the if leg 
>>>> again to
>>>> re-read afu->room from the AFU. The simplest fix might just be to set 
>>>> afu->room = 1
>>>> if you ever leave the if leg without having room.
>>> 
>>> Good suggestion. Will atomic64_set(&afu->room, 1), if we exhaust retries in 
>>> both legs.
>> 
>> While I agree this will work it seems a bit of a kludge.
>> 
>> What if we instead take advantage of our existing work queue and create a 
>> new work item that
>> simply MMIO reads and atomically sets afu->room? With this, instead of 
>> slamming in a 1 to
>> satisfy our logic such that a subsequent command will MMIO read, we would 
>> schedule the new
>> work item and let afu->room be updated with a real value from the card.
>> 
>> The only downside I see with this approach is that it has the potential to 
>> relax the window of time
>> that we're 'down' (no room) and sending back busy...although that might not 
>> be such a bad thing
>> if we were to get into this condition.
> 
> Seems reasonable. Would be simpler. I assume you then just schedule the work 
> from cxlflash_send_cmd and
> just return SCSI_MLQUEUE_HOST_BUSY from cxlflash_send_cmd if you don't have 
> room?

Correct. We'll keep things similar to how they are now (try our best to get 
room on the queuecommand thread)
and then if we're unable to get room (udelay/retries exhausted) we'll schedule 
the work and return the busy rc.

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to