All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chengming Zhou <chengming.zhou@linux.dev>
To: Christoph Hellwig <hch@lst.de>
Cc: axboe@kernel.dk, ming.lei@redhat.com,
	linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
	zhouchengming@bytedance.com
Subject: Re: [PATCH 2/2] blk-flush: don't need to end rq twice for non post-flush requests
Date: Tue, 25 Jul 2023 21:15:59 +0800	[thread overview]
Message-ID: <d9792de8-aaea-e848-9108-323e8e66a429@linux.dev> (raw)
In-Reply-To: <20230710133308.GB23157@lst.de>

On 2023/7/10 21:33, Christoph Hellwig wrote:
> On Mon, Jul 10, 2023 at 02:47:05PM +0800, chengming.zhou@linux.dev wrote:
>> From: Chengming Zhou <zhouchengming@bytedance.com>
>>
>> Now we unconditionally blk_rq_init_flush() to replace rq->end_io to
>> make rq return twice back to the flush state machine for post-flush.
>>
>> Obviously, non post-flush requests don't need it, they don't need to
>> end request twice, so they don't need to replace rq->end_io callback.
>> And the same for requests with the FUA bit on hardware with FUA support.
>>
>> So we move blk_rq_init_flush() to REQ_FSEQ_DATA stage and only replace
>> rq->end_io if it needs post-flush. Otherwise, it can end like normal
>> request and doesn't need to return back to the flush state machine.
> 
> I really like the idea behind this optimization, but I kinda hate
> adding more magic to the already way too magic flush sequence.

Yes, agree.

> 
> I wonder if a better idea would be to kill the flush sequence entirely,
> and just split the flush_queue into a preflush and a postflush queue.
> This would remove a field from struct request and lead to more readable
> code as well.

I have thought about this for some time, it seems feasible. So I implement it
today and test it using blktests, it works.

I will send the patchset soon.

Thanks.

  reply	other threads:[~2023-07-25 13:16 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-10  6:47 [PATCH 1/2] blk-flush: fix rq->flush.seq for post-flush requests chengming.zhou
2023-07-10  6:47 ` [PATCH 2/2] blk-flush: don't need to end rq twice for non " chengming.zhou
2023-07-10 13:33   ` Christoph Hellwig
2023-07-25 13:15     ` Chengming Zhou [this message]
2023-07-10 13:30 ` [PATCH 1/2] blk-flush: fix rq->flush.seq for " Christoph Hellwig
2023-07-11 11:06   ` Chengming Zhou
2023-07-11 11:15     ` [External] " Chengming Zhou
2023-07-11 11:31     ` Christoph Hellwig
2023-07-11 11:52       ` Chengming Zhou
2023-07-11 12:09         ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d9792de8-aaea-e848-9108-323e8e66a429@linux.dev \
    --to=chengming.zhou@linux.dev \
    --cc=axboe@kernel.dk \
    --cc=hch@lst.de \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ming.lei@redhat.com \
    --cc=zhouchengming@bytedance.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.