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. Thoughts? -- 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
