blk-mq: blk_mq_tag_to_rq should handle flush request

flush request is special, which borrows tag from other request. Need a special handling to get it from tag.

Quoted 3 lines · Shaohua Li

flush request is special, which borrows tag from other request. Need a special handling to get it from tag.

Thanks, we probably need this one. But I think you can simply test REQ_FLUSH_SEQ the passed in request instead of the flush_rq_tag_valid flag/

Quoted 8 lines · Christoph Hellwig

Quoted 3 lines · Shaohua Li

flush request is special, which borrows tag from other request. Need a special handling to get it from tag.

Thanks, we probably need this one. But I think you can simply test REQ_FLUSH_SEQ the passed in request instead of the flush_rq_tag_valid flag/

fair enough. updated one.

Subject: blk-mq: blk_mq_tag_to_rq should handle flush request

flush request is special, which borrows tag from other request. Need a special handling to get it from tag.

Quoted 8 lines · Christoph Hellwig

Quoted 3 lines · Shaohua Li

flush request is special, which borrows tag from other request. Need a special handling to get it from tag.

Thanks, we probably need this one. But I think you can simply test REQ_FLUSH_SEQ the passed in request instead of the flush_rq_tag_valid flag/

fair enough. updated one.

Subject: blk-mq: blk_mq_tag_to_rq should handle flush request

flush request is special, which borrows tag from other request. Need a special handling to get it from tag.

I have applied this one, but it irks me a little bit since we have to touch q->flush_rq->stuff from a potential hot path. I haven't thought much about this yet, but it would be a lot better if we could fold in the flush tag somehow.

Quoted 4 lines · Jens Axboe

I have applied this one, but it irks me a little bit since we have to touch q->flush_rq->stuff from a potential hot path. I haven't thought much about this yet, but it would be a lot better if we could fold in the flush tag somehow.

This one also breaks scsi-mq by changing the first parameter of blk_mq_tag_to_rq. With host-wide tags we won't have a hardware context easily available when looking for a request by tag.

Alright, I'll revert it to pass in the tags instead.

Quoted 1 line · Jens Axboe

Alright, I'll revert it to pass in the tags instead.

It's not as simple as the added code wants to get a queue from the hwctx, which we can't get at. I was planning to look into this, but there are various other regressions in the recent block updates that I need to fix before I can even test a tree with this one reverted.

If you can get to sorting this out soon I'd love you to handle it, otherwise I'll look into it as soon as I can.

Quoted 4 lines

It's not as simple as the added code wants to get a queue from the hwctx, which we can't get at. I was planning to look into this, but there are various other regressions in the recent block updates that I need to fix before I can even test a tree with this one reverted.

Which regressions? Performance or crashes?

Quoted 2 lines

If you can get to sorting this out soon I'd love you to handle it, otherwise I'll look into it as soon as I can.

Just took a look at it, but I don't see the problematic path. I'm looking at wip-9.

Quoted 6 lines · Jens Axboe

Quoted 4 lines

It's not as simple as the added code wants to get a queue from the hwctx, which we can't get at. I was planning to look into this, but there are various other regressions in the recent block updates that I need to fix before I can even test a tree with this one reverted.

Which regressions? Performance or crashes?

Both. I've tracked down the SCSI boot crash and you'll have a patch for that soon, still working on bisecting the performance crawl, but I'm getting close.

Quoted 5 lines

Quoted 2 lines

If you can get to sorting this out soon I'd love you to handle it, otherwise I'll look into it as soon as I can.

Just took a look at it, but I don't see the problematic path. I'm looking at wip-9.

scsi_mq_find_tag only gets the scsi host, which may have multiple queues. When called from scsi_find_tag we actually have a scsi device, so that's not an issue, but when called from scsi_host_find_tag the driver only provides the host.

OK strange, there hasn't been that much churn since the last rebase. In my for-linus, there's a patch for a single queue crash, but that should just hit for the removal case. And then there's the atomic schedule patch, but that issue was actually in the code base for about a month, so not a new one either.

Quoted 5 lines · Jens Axboe

OK strange, there hasn't been that much churn since the last rebase. In my for-linus, there's a patch for a single queue crash, but that should just hit for the removal case. And then there's the atomic schedule patch, but that issue was actually in the code base for about a month, so not a new one either.

You're request initializaion optimization doesn't set up req->cmd and thus causes all BLOCK_PC I/O (including the SCSI LUN scan) to crash and burn. The trivial fix is on your way.

The performance regression is caused by "blk-mq: avoid code duplication", but I don't really understand why yet.

Quoted 2 lines

The performance regression is caused by "blk-mq: avoid code duplication", but I don't really understand why yet.

Gah, looks like this one dropped the tag idling. I bet that is why.

Quoted 4 lines · Jens Axboe

Quoted 2 lines

The performance regression is caused by "blk-mq: avoid code duplication", but I don't really understand why yet.

Gah, looks like this one dropped the tag idling. I bet that is why.

Ah, thanks. It's indeed exactly the same sort of slow down I bisected back then, this should have rung a bell instead of keeping me occupied for hours..

Quoted 4 lines · Christoph Hellwig

Quoted 3 lines · Jens Axboe

Quoted 2 lines

The performance regression is caused by "blk-mq: avoid code duplication", but I don't really understand why yet.

Quoted 5 lines

Quoted 1 line

Gah, looks like this one dropped the tag idling. I bet that is why.

Ah, thanks. It's indeed exactly the same sort of slow down I bisected back then, this should have rung a bell instead of keeping me occupied for hours..

Patch is committed :-)

Quoted 11 lines · Jens Axboe

Quoted 9 lines · Christoph Hellwig

Quoted 4 lines · Jens Axboe

Quoted 2 lines

The performance regression is caused by "blk-mq: avoid code duplication", but I don't really understand why yet.

Gah, looks like this one dropped the tag idling. I bet that is why.

Ah, thanks. It's indeed exactly the same sort of slow down I bisected back then, this should have rung a bell instead of keeping me occupied for hours..

Quoted 1 line

Patch is committed :-)

Oh well, I had just prepared the same one for you, but I want to at least do basic sanity checking of my rebased tree with the fixes applied now that I'm done bisecting.

Quoted 7 lines · Christoph Hellwig

Quoted 5 lines · Jens Axboe

OK strange, there hasn't been that much churn since the last rebase. In my for-linus, there's a patch for a single queue crash, but that should just hit for the removal case. And then there's the atomic schedule patch, but that issue was actually in the code base for about a month, so not a new one either.

Quoted 3 lines

You're request initializaion optimization doesn't set up req->cmd and thus causes all BLOCK_PC I/O (including the SCSI LUN scan) to crash and burn. The trivial fix is on your way.

OK. That'll arguably be a good cleanup as well, getting that initialized in the right place. I hate the 'lets clear all the memory' part of rq init, it's not super cheap to do.

Quoted 12 lines · Jens Axboe

Quoted 10 lines · Christoph Hellwig

Quoted 5 lines · Jens Axboe

OK strange, there hasn't been that much churn since the last rebase. In my for-linus, there's a patch for a single queue crash, but that should just hit for the removal case. And then there's the atomic schedule patch, but that issue was actually in the code base for about a month, so not a new one either.

You're request initializaion optimization doesn't set up req->cmd and thus causes all BLOCK_PC I/O (including the SCSI LUN scan) to crash and burn. The trivial fix is on your way.

Quoted 3 lines

OK. That'll arguably be a good cleanup as well, getting that initialized in the right place. I hate the 'lets clear all the memory' part of rq init, it's not super cheap to do.

What would the right place be? We don't really know if a request is going to be used for BLOCK_PC purposes until it has been returned to the caller.

I also found another issue when just initializing req->cmd instead of blindly reverting the patch due to the lack of req->biotail initialization. For now I'll got back to a revert of that patch for my scsi-mq tree, and I'd prefer to keep that for mainline as well until this has been throughoutly tested.

Quoted 4 lines

scsi_mq_find_tag only gets the scsi host, which may have multiple queues. When called from scsi_find_tag we actually have a scsi device, so that's not an issue, but when called from scsi_host_find_tag the driver only provides the host.

Only solution I see right now is to have the flush_rq in the shared tags, but that would potentially be a regression for multiple devices and heavy flush uses cases. I'll see if I can come up with something better, or maybe Shaohua has an idea.

Quoted 9 lines · Jens Axboe

Quoted 4 lines

scsi_mq_find_tag only gets the scsi host, which may have multiple queues. When called from scsi_find_tag we actually have a scsi device, so that's not an issue, but when called from scsi_host_find_tag the driver only provides the host.

Only solution I see right now is to have the flush_rq in the shared tags, but that would potentially be a regression for multiple devices and heavy flush uses cases. I'll see if I can come up with something better, or maybe Shaohua has an idea.

What about something like the following (untest, uncompiled, maybe pseudo-code):

struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag)

{
	struct request *rq = tags->rqs[tag];

	if ((rq->cmd_flags & REQ_FLUSH_SEQ) && rq->q->flush_rq->tag == tag)
		return rq->q->flush_rq;
	return rq;

Quoted 18 lines · Christoph Hellwig

Quoted 9 lines · Jens Axboe

Quoted 4 lines

scsi_mq_find_tag only gets the scsi host, which may have multiple queues. When called from scsi_find_tag we actually have a scsi device, so that's not an issue, but when called from scsi_host_find_tag the driver only provides the host.

Only solution I see right now is to have the flush_rq in the shared tags, but that would potentially be a regression for multiple devices and heavy flush uses cases. I'll see if I can come up with something better, or maybe Shaohua has an idea.

What about something like the following (untest, uncompiled, maybe pseudo-code):

struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag)

{
	struct request *rq = tags->rqs[tag];

Quoted 3 lines

	if ((rq->cmd_flags & REQ_FLUSH_SEQ) && rq->q->flush_rq->tag == tag)
		return rq->q->flush_rq;
	return rq;

Ah yes, that'll work, the queue is always assigned. I'll make that change.

Quoted 23 lines · Jens Axboe

Quoted 21 lines · Christoph Hellwig

Quoted 9 lines · Jens Axboe

Quoted 4 lines

scsi_mq_find_tag only gets the scsi host, which may have multiple queues. When called from scsi_find_tag we actually have a scsi device, so that's not an issue, but when called from scsi_host_find_tag the driver only provides the host.

Only solution I see right now is to have the flush_rq in the shared tags, but that would potentially be a regression for multiple devices and heavy flush uses cases. I'll see if I can come up with something better, or maybe Shaohua has an idea.

What about something like the following (untest, uncompiled, maybe pseudo-code):

struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag)

{
	struct request *rq = tags->rqs[tag];

	if ((rq->cmd_flags & REQ_FLUSH_SEQ) && rq->q->flush_rq->tag == tag)
		return rq->q->flush_rq;
	return rq;

Quoted 1 line

Ah yes, that'll work, the queue is always assigned. I'll make that change.

Something like this in complete form. Compile tested only, I'll test it on dev box. Probably doesn't matter too much, but I prefer to potentially have the faster path (non-flush) just fall inline.

Quoted 24 lines · Jens Axboe

Quoted 23 lines · Jens Axboe

Quoted 21 lines · Christoph Hellwig

Quoted 9 lines · Jens Axboe

Quoted 4 lines

scsi_mq_find_tag only gets the scsi host, which may have multiple queues. When called from scsi_find_tag we actually have a scsi device, so that's not an issue, but when called from scsi_host_find_tag the driver only provides the host.

Only solution I see right now is to have the flush_rq in the shared tags, but that would potentially be a regression for multiple devices and heavy flush uses cases. I'll see if I can come up with something better, or maybe Shaohua has an idea.

What about something like the following (untest, uncompiled, maybe pseudo-code):

struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag)

{
	struct request *rq = tags->rqs[tag];

	if ((rq->cmd_flags & REQ_FLUSH_SEQ) && rq->q->flush_rq->tag == tag)
		return rq->q->flush_rq;
	return rq;

Quoted 5 lines

Quoted 1 line

Ah yes, that'll work, the queue is always assigned. I'll make that change.

Something like this in complete form. Compile tested only, I'll test it on dev box. Probably doesn't matter too much, but I prefer to potentially have the faster path (non-flush) just fall inline.

Works for me, committed.

Quoted 25 lines · Jens Axboe

Quoted 24 lines · Jens Axboe

Quoted 23 lines · Jens Axboe

Quoted 21 lines · Christoph Hellwig

Quoted 9 lines · Jens Axboe

Quoted 4 lines

scsi_mq_find_tag only gets the scsi host, which may have multiple queues. When called from scsi_find_tag we actually have a scsi device, so that's not an issue, but when called from scsi_host_find_tag the driver only provides the host.

Only solution I see right now is to have the flush_rq in the shared tags, but that would potentially be a regression for multiple devices and heavy flush uses cases. I'll see if I can come up with something better, or maybe Shaohua has an idea.

What about something like the following (untest, uncompiled, maybe pseudo-code):

struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag)

{
	struct request *rq = tags->rqs[tag];

	if ((rq->cmd_flags & REQ_FLUSH_SEQ) && rq->q->flush_rq->tag == tag)
		return rq->q->flush_rq;
	return rq;

Quoted 7 lines

Quoted 5 lines

Quoted 1 line

Ah yes, that'll work, the queue is always assigned. I'll make that change.

Something like this in complete form. Compile tested only, I'll test it on dev box. Probably doesn't matter too much, but I prefer to potentially have the faster path (non-flush) just fall inline.

Works for me, committed.

Sounds there is a small race here. FUA request has REQ_FLUSH_SEQ set too. Assume its tag is 0. we initialize flush_rq. blk_mq_rq_init->blk_rq_init->memset could set flush_rq tag to 0 in a short time. In that short time, blk_mq_tag_to_rq will return wrong request for the FUA request.

we can do (rq->cmd_flags & REQ_FLUSH_SEQ) && !(rq->cmd_flags & REQ_FUA) in is_flush_request to avoid this issue.

Quoted 40 lines · Shaohua Li

Quoted 25 lines · Jens Axboe

Quoted 24 lines · Jens Axboe

Quoted 23 lines · Jens Axboe

Quoted 21 lines · Christoph Hellwig

Quoted 9 lines · Jens Axboe

Quoted 4 lines

scsi_mq_find_tag only gets the scsi host, which may have multiple queues. When called from scsi_find_tag we actually have a scsi device, so that's not an issue, but when called from scsi_host_find_tag the driver only provides the host.

Only solution I see right now is to have the flush_rq in the shared tags, but that would potentially be a regression for multiple devices and heavy flush uses cases. I'll see if I can come up with something better, or maybe Shaohua has an idea.

What about something like the following (untest, uncompiled, maybe pseudo-code):

struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag)

{
	struct request *rq = tags->rqs[tag];

	if ((rq->cmd_flags & REQ_FLUSH_SEQ) && rq->q->flush_rq->tag == tag)
		return rq->q->flush_rq;
	return rq;

Quoted 7 lines

Quoted 5 lines

Quoted 1 line

Ah yes, that'll work, the queue is always assigned. I'll make that change.

Something like this in complete form. Compile tested only, I'll test it on dev box. Probably doesn't matter too much, but I prefer to potentially have the faster path (non-flush) just fall inline.

Works for me, committed.

Sounds there is a small race here. FUA request has REQ_FLUSH_SEQ set too. Assume its tag is 0. we initialize flush_rq. blk_mq_rq_init->blk_rq_init->memset could set flush_rq tag to 0 in a short time. In that short time, blk_mq_tag_to_rq will return wrong request for the FUA request.

Quoted 2 lines

we can do (rq->cmd_flags & REQ_FLUSH_SEQ) && !(rq->cmd_flags & REQ_FUA) in is_flush_request to avoid this issue.

We don't memset the entire request anymore from the rq alloc path.

Quoted 41 lines · Jens Axboe

Quoted 40 lines · Shaohua Li

Quoted 25 lines · Jens Axboe

Quoted 24 lines · Jens Axboe

Quoted 23 lines · Jens Axboe

Quoted 21 lines · Christoph Hellwig

Quoted 9 lines · Jens Axboe

Quoted 4 lines

scsi_mq_find_tag only gets the scsi host, which may have multiple queues. When called from scsi_find_tag we actually have a scsi device, so that's not an issue, but when called from scsi_host_find_tag the driver only provides the host.

Only solution I see right now is to have the flush_rq in the shared tags, but that would potentially be a regression for multiple devices and heavy flush uses cases. I'll see if I can come up with something better, or maybe Shaohua has an idea.

What about something like the following (untest, uncompiled, maybe pseudo-code):

struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag)

{
	struct request *rq = tags->rqs[tag];

	if ((rq->cmd_flags & REQ_FLUSH_SEQ) && rq->q->flush_rq->tag == tag)
		return rq->q->flush_rq;
	return rq;

Quoted 7 lines

Quoted 5 lines

Quoted 1 line

Ah yes, that'll work, the queue is always assigned. I'll make that change.

Something like this in complete form. Compile tested only, I'll test it on dev box. Probably doesn't matter too much, but I prefer to potentially have the faster path (non-flush) just fall inline.

Works for me, committed.

Sounds there is a small race here. FUA request has REQ_FLUSH_SEQ set too. Assume its tag is 0. we initialize flush_rq. blk_mq_rq_init->blk_rq_init->memset could set flush_rq tag to 0 in a short time. In that short time, blk_mq_tag_to_rq will return wrong request for the FUA request.

Quoted 4 lines

Quoted 2 lines

we can do (rq->cmd_flags & REQ_FLUSH_SEQ) && !(rq->cmd_flags & REQ_FUA) in is_flush_request to avoid this issue.

We don't memset the entire request anymore from the rq alloc path.

blk_kick_flush() still calls blk_rq_init()?

Quoted 9 lines · Jens Axboe

Quoted 4 lines

scsi_mq_find_tag only gets the scsi host, which may have multiple queues. When called from scsi_find_tag we actually have a scsi device, so that's not an issue, but when called from scsi_host_find_tag the driver only provides the host.

Only solution I see right now is to have the flush_rq in the shared tags, but that would potentially be a regression for multiple devices and heavy flush uses cases. I'll see if I can come up with something better, or maybe Shaohua has an idea.

What about something like the following (untest, uncompiled, maybe pseudo-code):

struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag)

{
        struct request *rq = tags->rqs[tag];

        if ((rq->cmd_flags & REQ_FLUSH_SEQ) && rq->q->flush_rq->tag == tag)
                return rq->q->flush_rq;
        return rq;
}

Quoted 18 lines · Christoph Hellwig

Quoted 9 lines · Jens Axboe

Quoted 4 lines

scsi_mq_find_tag only gets the scsi host, which may have multiple queues. When called from scsi_find_tag we actually have a scsi device, so that's not an issue, but when called from scsi_host_find_tag the driver only provides the host.

Only solution I see right now is to have the flush_rq in the shared tags, but that would potentially be a regression for multiple devices and heavy flush uses cases. I'll see if I can come up with something better, or maybe Shaohua has an idea.

What about something like the following (untest, uncompiled, maybe pseudo-code):

struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag)

{
        struct request *rq = tags->rqs[tag];

Quoted 4 lines

        if ((rq->cmd_flags & REQ_FLUSH_SEQ) && rq->q->flush_rq->tag == tag)
                return rq->q->flush_rq;
        return rq;
}

Looks we thought it together, :-)

Also maybe the flush_rq->tag need to be cleared in flush_end_io().

Quoted 24 lines · Ming Lei

Quoted 22 lines · Christoph Hellwig

Quoted 9 lines · Jens Axboe

Quoted 4 lines

scsi_mq_find_tag only gets the scsi host, which may have multiple queues. When called from scsi_find_tag we actually have a scsi device, so that's not an issue, but when called from scsi_host_find_tag the driver only provides the host.

Only solution I see right now is to have the flush_rq in the shared tags, but that would potentially be a regression for multiple devices and heavy flush uses cases. I'll see if I can come up with something better, or maybe Shaohua has an idea.

What about something like the following (untest, uncompiled, maybe pseudo-code):

struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag)

{
        struct request *rq = tags->rqs[tag];

        if ((rq->cmd_flags & REQ_FLUSH_SEQ) && rq->q->flush_rq->tag == tag)
                return rq->q->flush_rq;
        return rq;
}

Quoted 3 lines

Looks we thought it together, :-)

Also maybe the flush_rq->tag need to be cleared in flush_end_io().

It clears the command flag, so that should be enough.

Quoted 25 lines · Jens Axboe

Quoted 24 lines · Ming Lei

Quoted 22 lines · Christoph Hellwig

Quoted 9 lines · Jens Axboe

Quoted 4 lines

scsi_mq_find_tag only gets the scsi host, which may have multiple queues. When called from scsi_find_tag we actually have a scsi device, so that's not an issue, but when called from scsi_host_find_tag the driver only provides the host.

Only solution I see right now is to have the flush_rq in the shared tags, but that would potentially be a regression for multiple devices and heavy flush uses cases. I'll see if I can come up with something better, or maybe Shaohua has an idea.

What about something like the following (untest, uncompiled, maybe pseudo-code):

struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag)

{
        struct request *rq = tags->rqs[tag];

        if ((rq->cmd_flags & REQ_FLUSH_SEQ) && rq->q->flush_rq->tag == tag)
                return rq->q->flush_rq;
        return rq;
}

Quoted 5 lines

Quoted 3 lines

Looks we thought it together, :-)

Also maybe the flush_rq->tag need to be cleared in flush_end_io().

It clears the command flag, so that should be enough.

Only the flush_rq's command flag is cleared, and its parent request flag isn't cleared.

Quoted 30 lines · Ming Lei

Quoted 29 lines · Jens Axboe

Quoted 27 lines · Ming Lei

Quoted 14 lines · Christoph Hellwig

Quoted 9 lines · Jens Axboe

Quoted 4 lines

scsi_mq_find_tag only gets the scsi host, which may have multiple queues. When called from scsi_find_tag we actually have a scsi device, so that's not an issue, but when called from scsi_host_find_tag the driver only provides the host.

Only solution I see right now is to have the flush_rq in the shared tags, but that would potentially be a regression for multiple devices and heavy flush uses cases. I'll see if I can come up with something better, or maybe Shaohua has an idea.

What about something like the following (untest, uncompiled, maybe pseudo-code):

Quoted 8 lines

struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag)

{
        struct request *rq = tags->rqs[tag];

        if ((rq->cmd_flags & REQ_FLUSH_SEQ) && rq->q->flush_rq->tag == tag)
                return rq->q->flush_rq;
        return rq;
}

Looks we thought it together, :-)

Also maybe the flush_rq->tag need to be cleared in flush_end_io().

Quoted 4 lines

Quoted 1 line

It clears the command flag, so that should be enough.

Only the flush_rq's command flag is cleared, and its parent request flag isn't cleared.

Good point. Care to send in a patch? We can just clear it to -1U, at least in blk-mq that's defined as an invalid tag.

Quoted 33 lines · Jens Axboe

Quoted 32 lines · Ming Lei

Quoted 30 lines · Jens Axboe

Quoted 19 lines · Ming Lei

Quoted 18 lines · Christoph Hellwig

Quoted 9 lines · Jens Axboe

Quoted 4 lines

scsi_mq_find_tag only gets the scsi host, which may have multiple queues. When called from scsi_find_tag we actually have a scsi device, so that's not an issue, but when called from scsi_host_find_tag the driver only provides the host.

Only solution I see right now is to have the flush_rq in the shared tags, but that would potentially be a regression for multiple devices and heavy flush uses cases. I'll see if I can come up with something better, or maybe Shaohua has an idea.

What about something like the following (untest, uncompiled, maybe pseudo-code):

struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag)

{
        struct request *rq = tags->rqs[tag];

Quoted 8 lines

Quoted 4 lines

        if ((rq->cmd_flags & REQ_FLUSH_SEQ) && rq->q->flush_rq->tag == tag)
                return rq->q->flush_rq;
        return rq;
}

Looks we thought it together, :-)

Also maybe the flush_rq->tag need to be cleared in flush_end_io().

It clears the command flag, so that should be enough.

Quoted 5 lines

Quoted 2 lines

Only the flush_rq's command flag is cleared, and its parent request flag isn't cleared.

Good point. Care to send in a patch? We can just clear it to -1U, at least in blk-mq that's defined as an invalid tag.

Attachment patch should be enough.

Quoted 1 line

Attachment patch should be enough.

Yep, looks fine. Care to send as a proper patch, then I will include it?

Quoted 3 lines · Jens Axboe

Quoted 1 line

Attachment patch should be enough.

Yep, looks fine. Care to send as a proper patch, then I will include it?

I think we an also drop the assignment to q->flush_rq->cmd_flags in flush_end_io now.