Re: [PATCH v5] cxlflash: Base support for IBM CXL Flash Adapter

[Brian King <brking@linux.vnet.ibm.com>]

On 06/09/2015 11:01 AM, Matthew R. Ochs wrote:
> Brian/Manoj,
> 
> See my alternate proposal below.
> 
> -matt
> 
> 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?

-Brian


-- 
Brian King
Power Linux I/O
IBM Linux Technology Center