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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
