On Thu, 2018-12-20 at 11:33 -0700, Jens Axboe wrote: +AD4 +AFs ... +AF0 +AD4 Something like this, totally untested. If the queue matches, we know it's +AD4 safe to dereference it. +AD4 +AD4 A safer API to export as well... +AD4 +AD4 +AD4 diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c +AD4 index 2089c6c62f44..78192b544fa2 100644 +AD4 --- a/block/blk-mq-tag.c +AD4 +-+-+- b/block/blk-mq-tag.c +AD4 +AEAAQA -228,13 +-228,15 +AEAAQA static bool bt+AF8-iter(struct sbitmap +ACo-bitmap, unsigned int bitnr, void +ACo-data) +AD4 +AD4 if (+ACE-reserved) +AD4 bitnr +-+AD0 tags-+AD4-nr+AF8-reserved+AF8-tags+ADs +AD4 - rq +AD0 tags-+AD4-rqs+AFs-bitnr+AF0AOw +AD4 +- if (tags-+AD4-rqs+AFs-bitnr+AF0.queue +ACEAPQ hctx-+AD4-queue) +AD4 +- return true+ADs +AD4 +AD4 /+ACo +AD4 +ACo We can hit rq +AD0APQ NULL here, because the tagging functions +AD4 +ACo test and set the bit before assigning -+AD4-rqs+AFsAXQ. +AD4 +ACo-/ +AD4 - if (rq +ACYAJg rq-+AD4-q +AD0APQ hctx-+AD4-queue) +AD4 +- rq +AD0 tags-+AD4-rqs+AFs-bitnr+AF0.rq+ADs +AD4 +- if (rq) +AD4 return iter+AF8-data-+AD4-fn(hctx, rq, iter+AF8-data-+AD4-data, reserved)+ADs +AD4 return true+ADs +AD4 +AH0 +AD4 +AEAAQA -268,6 +-270,7 +AEAAQA static void bt+AF8-for+AF8-each(struct blk+AF8-mq+AF8-hw+AF8-ctx +ACo-hctx, struct sbitmap+AF8-queue +ACo-bt, +AD4 +AD4 struct bt+AF8-tags+AF8-iter+AF8-data +AHs +AD4 struct blk+AF8-mq+AF8-tags +ACo-tags+ADs +AD4 +- struct blk+AF8-mq+AF8-hw+AF8-ctx +ACo-hctx+ADs +AD4 busy+AF8-tag+AF8-iter+AF8-fn +ACo-fn+ADs +AD4 void +ACo-data+ADs +AD4 bool reserved+ADs +AD4 +AEAAQA -287,7 +-290,7 +AEAAQA static bool bt+AF8-tags+AF8-iter(struct sbitmap +ACo-bitmap, unsigned int bitnr, void +ACo-data) +AD4 +ACo We can hit rq +AD0APQ NULL here, because the tagging functions +AD4 +ACo test and set the bit before assining -+AD4-rqs+AFsAXQ. +AD4 +ACo-/ +AD4 - rq +AD0 tags-+AD4-rqs+AFs-bitnr+AF0AOw +AD4 +- rq +AD0 tags-+AD4-rqs+AFs-bitnr+AF0.rq+ADs +AD4 if (rq +ACYAJg blk+AF8-mq+AF8-request+AF8-started(rq)) +AD4 return iter+AF8-data-+AD4-fn(rq, iter+AF8-data-+AD4-data, reserved)+ADs +AD4 +AD4 diff --git a/block/blk-mq-tag.h b/block/blk-mq-tag.h +AD4 index 61deab0b5a5a..bb84d1f099c7 100644 +AD4 --- a/block/blk-mq-tag.h +AD4 +-+-+- b/block/blk-mq-tag.h +AD4 +AEAAQA -4,6 +-4,11 +AEAAQA +AD4 +AD4 +ACM-include +ACI-blk-mq.h+ACI +AD4 +AD4 +-struct rq+AF8-tag+AF8-entry +AHs +AD4 +- struct request+AF8-queue +ACo-queue+ADs +AD4 +- struct request +ACo-rq+ADs +AD4 +-+AH0AOw +AD4 +- +AD4 /+ACo +AD4 +ACo Tag address space map. +AD4 +ACo-/ +AD4 +AEAAQA -16,7 +-21,7 +AEAAQA struct blk+AF8-mq+AF8-tags +AHs +AD4 struct sbitmap+AF8-queue bitmap+AF8-tags+ADs +AD4 struct sbitmap+AF8-queue breserved+AF8-tags+ADs +AD4 +AD4 - struct request +ACoAKg-rqs+ADs +AD4 +- struct rq+AF8-tag+AF8-entry +ACo-rqs+ADs +AD4 struct request +ACoAKg-static+AF8-rqs+ADs +AD4 struct list+AF8-head page+AF8-list+ADs +AD4 +AH0AOw +AD4 +AEAAQA -78,7 +-83,8 +AEAAQA static inline void blk+AF8-mq+AF8-tag+AF8-idle(struct blk+AF8-mq+AF8-hw+AF8-ctx +ACo-hctx) +AD4 static inline void blk+AF8-mq+AF8-tag+AF8-set+AF8-rq(struct blk+AF8-mq+AF8-hw+AF8-ctx +ACo-hctx, +AD4 unsigned int tag, struct request +ACo-rq) +AD4 +AHs +AD4 - hctx-+AD4-tags-+AD4-rqs+AFs-tag+AF0 +AD0 rq+ADs +AD4 +- hctx-+AD4-tags-+AD4-rqs+AFs-tag+AF0.queue +AD0 hctx-+AD4-queue+ADs +AD4 +- hctx-+AD4-tags-+AD4-rqs+AFs-tag+AF0.rq +AD0 rq+ADs +AD4 +AH0 +AD4 +AD4 static inline bool blk+AF8-mq+AF8-tag+AF8-is+AF8-reserved(struct blk+AF8-mq+AF8-tags +ACo-tags, +AD4 diff --git a/block/blk-mq.c b/block/blk-mq.c +AD4 index 3ba37b9e15e9..4f194946dbd9 100644 +AD4 --- a/block/blk-mq.c +AD4 +-+-+- b/block/blk-mq.c +AD4 +AEAAQA -298,13 +-298,16 +AEAAQA static struct request +ACo-blk+AF8-mq+AF8-rq+AF8-ctx+AF8-init(struct blk+AF8-mq+AF8-alloc+AF8-data +ACo-data, +AD4 rq-+AD4-tag +AD0 -1+ADs +AD4 rq-+AD4-internal+AF8-tag +AD0 tag+ADs +AD4 +AH0 else +AHs +AD4 - if (data-+AD4-hctx-+AD4-flags +ACY BLK+AF8-MQ+AF8-F+AF8-TAG+AF8-SHARED) +AHs +AD4 +- struct blk+AF8-mq+AF8-hw+AF8-ctx +ACo-hctx +AD0 data-+AD4-hctx+ADs +AD4 +- +AD4 +- if (hctx-+AD4-flags +ACY BLK+AF8-MQ+AF8-F+AF8-TAG+AF8-SHARED) +AHs +AD4 rq+AF8-flags +AD0 RQF+AF8-MQ+AF8-INFLIGHT+ADs +AD4 - atomic+AF8-inc(+ACY-data-+AD4-hctx-+AD4-nr+AF8-active)+ADs +AD4 +- atomic+AF8-inc(+ACY-hctx-+AD4-nr+AF8-active)+ADs +AD4 +AH0 +AD4 rq-+AD4-tag +AD0 tag+ADs +AD4 rq-+AD4-internal+AF8-tag +AD0 -1+ADs +AD4 - data-+AD4-hctx-+AD4-tags-+AD4-rqs+AFs-rq-+AD4-tag+AF0 +AD0 rq+ADs +AD4 +- hctx-+AD4-tags-+AD4-rqs+AFs-rq-+AD4-tag+AF0.queue +AD0 hctx-+AD4-queue+ADs +AD4 +- hctx-+AD4-tags-+AD4-rqs+AFs-rq-+AD4-tag+AF0.rq +AD0 rq+ADs +AD4 +AH0 +AD4 +AD4 /+ACo csd/requeue+AF8-work/fifo+AF8-time is initialized before use +ACo-/ +AD4 +AEAAQA -797,8 +-800,8 +AEAAQA EXPORT+AF8-SYMBOL(blk+AF8-mq+AF8-delay+AF8-kick+AF8-requeue+AF8-list)+ADs +AD4 struct request +ACo-blk+AF8-mq+AF8-tag+AF8-to+AF8-rq(struct blk+AF8-mq+AF8-tags +ACo-tags, unsigned int tag) +AD4 +AHs +AD4 if (tag +ADw tags-+AD4-nr+AF8-tags) +AHs +AD4 - prefetch(tags-+AD4-rqs+AFs-tag+AF0)+ADs +AD4 - return tags-+AD4-rqs+AFs-tag+AF0AOw +AD4 +- prefetch(tags-+AD4-rqs+AFs-tag+AF0.rq)+ADs +AD4 +- return tags-+AD4-rqs+AFs-tag+AF0.rq+ADs +AD4 +AH0 +AD4 +AD4 return NULL+ADs +AD4 +AEAAQA -809,10 +-812,11 +AEAAQA static bool blk+AF8-mq+AF8-rq+AF8-inflight(struct blk+AF8-mq+AF8-hw+AF8-ctx +ACo-hctx, struct request +ACo-rq, +AD4 void +ACo-priv, bool reserved) +AD4 +AHs +AD4 /+ACo +AD4 - +ACo If we find a request that is inflight and the queue matches, +AD4 - +ACo we know the queue is busy. Return false to stop the iteration. +AD4 +- +ACo We're only called here if the queue matches. If the rq state is +AD4 +- +ACo inflight, we know the queue is busy. Return false to stop the +AD4 +- +ACo iteration. +AD4 +ACo-/ +AD4 - if (rq-+AD4-state +AD0APQ MQ+AF8-RQ+AF8-IN+AF8-FLIGHT +ACYAJg rq-+AD4-q +AD0APQ hctx-+AD4-queue) +AHs +AD4 +- if (rq-+AD4-state +AD0APQ MQ+AF8-RQ+AF8-IN+AF8-FLIGHT) +AHs +AD4 bool +ACo-busy +AD0 priv+ADs +AD4 +AD4 +ACo-busy +AD0 true+ADs +AD4 +AEAAQA -1049,11 +-1053,14 +AEAAQA bool blk+AF8-mq+AF8-get+AF8-driver+AF8-tag(struct request +ACo-rq) +AD4 shared +AD0 blk+AF8-mq+AF8-tag+AF8-busy(data.hctx)+ADs +AD4 rq-+AD4-tag +AD0 blk+AF8-mq+AF8-get+AF8-tag(+ACY-data)+ADs +AD4 if (rq-+AD4-tag +AD4APQ 0) +AHs +AD4 +- struct blk+AF8-mq+AF8-hw+AF8-ctx +ACo-hctx +AD0 data.hctx+ADs +AD4 +- +AD4 if (shared) +AHs +AD4 rq-+AD4-rq+AF8-flags +AHwAPQ RQF+AF8-MQ+AF8-INFLIGHT+ADs +AD4 atomic+AF8-inc(+ACY-data.hctx-+AD4-nr+AF8-active)+ADs +AD4 +AH0 +AD4 - data.hctx-+AD4-tags-+AD4-rqs+AFs-rq-+AD4-tag+AF0 +AD0 rq+ADs +AD4 +- hctx-+AD4-tags-+AD4-rqs+AFs-rq-+AD4-tag+AF0.queue +AD0 hctx-+AD4-queue+ADs +AD4 +- hctx-+AD4-tags-+AD4-rqs+AFs-rq-+AD4-tag+AF0.rq +AD0 rq+ADs +AD4 +AH0 +AD4 +AD4 done: +AD4 +AEAAQA -2069,7 +-2076,7 +AEAAQA struct blk+AF8-mq+AF8-tags +ACo-blk+AF8-mq+AF8-alloc+AF8-rq+AF8-map(struct blk+AF8-mq+AF8-tag+AF8-set +ACo-set, +AD4 if (+ACE-tags) +AD4 return NULL+ADs +AD4 +AD4 - tags-+AD4-rqs +AD0 kcalloc+AF8-node(nr+AF8-tags, sizeof(struct request +ACo), +AD4 +- tags-+AD4-rqs +AD0 kcalloc+AF8-node(nr+AF8-tags, sizeof(struct rq+AF8-tag+AF8-entry), +AD4 GFP+AF8-NOIO +AHw +AF8AXw-GFP+AF8-NOWARN +AHw +AF8AXw-GFP+AF8-NORETRY, +AD4 node)+ADs +AD4 if (+ACE-tags-+AD4-rqs) +AHs Hi Jens, How about the patch below? Its behavior should be similar to your patch but I think this patch is simpler. Thanks, Bart. --- block/blk-mq.c +AHw 18 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+--- 1 file changed, 16 insertions(+-), 2 deletions(-) diff --git a/block/blk-mq.c b/block/blk-mq.c index 6a7566244de3..c57611b1f2a8 100644 --- a/block/blk-mq.c +-+-+- b/block/blk-mq.c +AEAAQA -86,6 +-86,7 +AEAAQA static void blk+AF8-mq+AF8-hctx+AF8-clear+AF8-pending(struct blk+AF8-mq+AF8-hw+AF8-ctx +ACo-hctx, +AH0 struct mq+AF8-inflight +AHs +- struct request+AF8-queue +ACo-q+ADs struct hd+AF8-struct +ACo-part+ADs unsigned int +ACo-inflight+ADs +AH0AOw +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, +AHs struct mq+AF8-inflight +ACo-mi +AD0 priv+ADs +- if (rq-+AD4-q +ACEAPQ mi-+AD4-q) +- return+ADs +- /+ACo +ACo index+AFs-0+AF0 counts the specific partition that was asked for. index+AFs-1+AF0 +ACo counts the ones that are active on the whole device, so increment +AEAAQA -110,7 +-114,11 +AEAAQA static void blk+AF8-mq+AF8-check+AF8-inflight(struct blk+AF8-mq+AF8-hw+AF8-ctx +ACo-hctx, void blk+AF8-mq+AF8-in+AF8-flight(struct request+AF8-queue +ACo-q, struct hd+AF8-struct +ACo-part, unsigned int inflight+AFs-2+AF0) +AHs - struct mq+AF8-inflight mi +AD0 +AHs .part +AD0 part, .inflight +AD0 inflight, +AH0AOw +- struct mq+AF8-inflight mi +AD0 +AHs +- .q +AD0 q, +- .part +AD0 part, +- .inflight +AD0 inflight, +- +AH0AOw inflight+AFs-0+AF0 +AD0 inflight+AFs-1+AF0 +AD0 0+ADs blk+AF8-mq+AF8-queue+AF8-tag+AF8-busy+AF8-iter(q, blk+AF8-mq+AF8-check+AF8-inflight, +ACY-mi)+ADs +AEAAQA -129,7 +-137,11 +AEAAQA static void blk+AF8-mq+AF8-check+AF8-inflight+AF8-rw(struct blk+AF8-mq+AF8-hw+AF8-ctx +ACo-hctx, void blk+AF8-mq+AF8-in+AF8-flight+AF8-rw(struct request+AF8-queue +ACo-q, struct hd+AF8-struct +ACo-part, unsigned int inflight+AFs-2+AF0) +AHs - struct mq+AF8-inflight mi +AD0 +AHs .part +AD0 part, .inflight +AD0 inflight, +AH0AOw +- struct mq+AF8-inflight mi +AD0 +AHs +- .q +AD0 q, +- .part +AD0 part, +- .inflight +AD0 inflight, +- +AH0AOw inflight+AFs-0+AF0 +AD0 inflight+AFs-1+AF0 +AD0 0+ADs blk+AF8-mq+AF8-queue+AF8-tag+AF8-busy+AF8-iter(q, blk+AF8-mq+AF8-check+AF8-inflight+AF8-rw, +ACY-mi)+ADs +AEAAQA -477,6 +-489,8 +AEAAQA static void +AF8AXw-blk+AF8-mq+AF8-free+AF8-request(struct request +ACo-rq) const int sched+AF8-tag +AD0 rq-+AD4-internal+AF8-tag+ADs blk+AF8-pm+AF8-mark+AF8-last+AF8-busy(rq)+ADs +- /+ACo Avoid that blk+AF8-mq+AF8-queue+AF8-tag+AF8-busy+AF8-iter() picks up this request. +ACo-/ +- rq-+AD4-q +AD0 NULL+ADs if (rq-+AD4-tag +ACEAPQ -1) blk+AF8-mq+AF8-put+AF8-tag(hctx, hctx-+AD4-tags, ctx, rq-+AD4-tag)+ADs if (sched+AF8-tag +ACEAPQ -1)
