On Thu, 2018-12-20 at 14:26 -0700, Jens Axboe wrote: +AD4 On 12/20/18 2:23 PM, Bart Van Assche wrote: +AD4 +AD4 On Thu, 2018-12-20 at 14:00 -0700, Jens Axboe wrote: +AD4 +AD4 +AD4 On 12/20/18 1:56 PM, Bart Van Assche wrote: +AD4 +AD4 +AD4 +AD4 +AEAAQA -96,6 +-97,9 +AEAAQA static void blk+AF8-mq+AF8-check+AF8-inflight(struct blk+AF8-mq+AF8-hw+AF8-ctx +ACo-hctx, +AD4 +AD4 +AD4 +AD4 +AHs +AD4 +AD4 +AD4 +AD4 struct mq+AF8-inflight +ACo-mi +AD0 priv+ADs +AD4 +AD4 +AD4 +AD4 +AD4 +AD4 +AD4 +AD4 +- if (rq-+AD4-q +ACEAPQ mi-+AD4-q) +AD4 +AD4 +AD4 +AD4 +- return+ADs +AD4 +AD4 +AD4 +AD4 +AD4 +AD4 Aren't you back to square one with this one, if the tags are shared? You +AD4 +AD4 +AD4 can't dereference it before you know it matches. +AD4 +AD4 +AD4 +AD4 My patch can only work if the new rq-+AD4-q +AD0 NULL assignment in +AF8AXw-blk+AF8-mq+AF8-free+AF8-request() +AD4 +AD4 is executed before the request tag is freed and if freeing a tag does not happen +AD4 +AD4 concurrently with any bt+AF8-iter() call. Would you accept that I add a seqlock to avoid +AD4 +AD4 this scenario? +AD4 +AD4 Ugh no, let's not go that far. Why not just use my approach that avoids +AD4 any need to dereference rq, unless we know it belongs to the queue in +AD4 question? I think that's cheaper than resetting -+AD4-queue as well when the +AD4 rq completes, I'm always wary of adding new stores in the completion +AD4 path. I think there is a race condition in bt+AF8-iter() in your approach: tags-+AD4-rqs+AFs-bitnr+AF0.queue can change after it has been read and that can cause a request that is not associated with hctx-+AD4-queue to be passed to iter+AF8-data-+AD4-fn(). Since 'fn' will access '+ACo-rq' I think that with your patch a use-after-free can occur similar to the one reported at the start of this e-mail thread. Your patch may make it harder to trigger that issue though. Bart.
