On Fri, 2019-02-15 at 10:29 -0800, Evan Green wrote: +AD4 I got all turned around while trying to understand this fix, and I'll +AD4 admit it's probably just me. It looks like you're trying to use an rcu +AD4 lock to prevent the iter functions from racing with free. Is that +AD4 true? But then the race at least as I understand it wasn't that there +AD4 was a free in-flight, it's that the free had happened a long time ago, +AD4 and there was still a stale value in tags-+AD4-rqs+AFs-bitnr+AF0. So maybe +AD4 there's some other issue that part is solving? +AD4 +AD4 And then it looks like you added a new struct where tags-+AD4-rqs was so +AD4 that you could compare hctx-+AD4-queue without reaching through rq. I have +AD4 no idea if that's sufficient to prevent stale accesses through rq. +AD4 Since you're changing multiple values I think where you populate that +AD4 structure you'd at least need to do something like: clear rq, barrier, +AD4 set hctx, barrier, set rq. But like Bart said, that's probably not the +AD4 right way to go. +AD4 +AD4 Finally, I didn't really get why the conditional in +AD4 blk+AF8-mq+AF8-rq+AF8-inflight() changed. Is that guarding against some other +AD4 identified problem, or just an additional check you can do when you +AD4 convert rqs into a struct? +AD4 +AD4 It looks like blk+AF8-mq+AF8-free+AF8-rqs() might be the magic function I was +AD4 looking for earlier. Would it be possible to just clear tags+AFs-rq-+AD4-tag+AF0 +AD4 for each static+AF8-rq? Or is it possible for rqs from one set to end up +AD4 in the tags array of another set? (Which would make what I just +AD4 suggested insufficient). Hi Evan, Multiple request queues can share a single tag set. Examples of use cases are NVMe namespaces associated with the same NVMe controller and SCSI LUNs associated with the same SCSI host. The code that allocates a request not only marks a tag as allocated but also updates the rqs+AFsAXQ array in the tag set (see also blk+AF8-mq+AF8-get+AF8-request()). The code that iterates over a tag set examines both the tag bitmap and the rqs+AFsAXQ array. The code that allocates requests and the code that iterates over requests are not serialized against each other. That is why the association of a tag with a request queue can change while iterating over a tag set. Another race condition is that a request can be allocated or freed while iterating over a tag set. Fixing these race conditions would require a locking mechanism and I think the performance overhead of such a locking mechanism is larger than acceptable. So code that iterates over requests either must be able to handle such race conditions or must suspend request processing before it iterates over requests. This is why I'm in favor of only modifying blk+AF8-mq+AF8-free+AF8-rqs() such that the memory in which the tags are stored is delayed until a grace period has occurred. Bart.
