From: Coly Li <colyli@suse.de>
To: Dongsheng Yang <dongsheng.yang@easystack.cn>,
linux-bcache@vger.kernel.org, linux-kernel@vger.kernel.org,
mchristi@redhat.com
Subject: Re: [PATCH] bcache: dont reset bio opf in bch_data_insert_start
Date: Mon, 25 Jan 2021 12:53:09 +0800 [thread overview]
Message-ID: <7569abf3-3e54-986e-8307-751fa5e00828@suse.de> (raw)
In-Reply-To: <20210125042942.1087170-1-dongsheng.yang@easystack.cn>
On 1/25/21 12:29 PM, Dongsheng Yang wrote:
> commit ad0d9e76(bcache: use bio op accessors) makes the bi_opf
> modified by bio_set_op_attrs(). But there is a logical
> problem in this commit:
>
> trace_bcache_cache_insert(k);
> bch_keylist_push(&op->insert_keys);
>
> - n->bi_rw |= REQ_WRITE;
> + bio_set_op_attrs(n, REQ_OP_WRITE, 0);
> bch_submit_bbio(n, op->c, k, 0);
> } while (n != bio);
>
> The old code add REQ_WRITE into bio n and keep other flags; the
> new code set REQ_OP_WRITE to bi_opf, but reset all other flags.
>
> This problem is discoverd in our performance testing:
> (1) start a fio with 1M x 128depth for read in /dev/nvme0n1p1
> (2) start a fio with 1M x 128depth for write in /dev/escache0 (cache
> device is /dev/nvme0n1p2)
>
> We found the BW of reading is 2000+M/s, but the BW of writing is
> 0-100M/s. After some debugging, we found the problem is io submit in
> writting is very slow.
>
> bch_data_insert_start() insert a bio to /dev/nvme0n1p1, but as
> cached_dev submit stack bio will be added into current->bio_list, and
> return.Then __submit_bio_noacct() will submit the new bio in bio_list
> into /dev/nvme0n1p1. This operation would be slow in
> blk_mq_submit_bio() -> rq_qos_throttle(q, bio);
>
> The rq_qos_throttle() will call wbt_should_throttle(),
> static inline bool wbt_should_throttle(struct rq_wb *rwb, struct bio *bio)
> {
> switch (bio_op(bio)) {
> case REQ_OP_WRITE:
> /*
> * Don't throttle WRITE_ODIRECT
> */
> if ((bio->bi_opf & (REQ_SYNC | REQ_IDLE)) ==
> (REQ_SYNC | REQ_IDLE))
> return false;
> ... ...
> }
>
> As the bio_set_op_attrs() reset the (REQ_SYNC | REQ_IDLE), so this write
> bio will be considered as non-direct write.
>
> After this fix, bio to nvme will flaged as (REQ_SYNC | REQ_IDLE),
> then fio for writing will get about 1000M/s bandwidth.
>
> Fixes: ad0d9e76a4124708dddd00c04fc4b56fc86c02d6
It should be,
Fixes: ad0d9e76a412 ("bcache: use bio op accessors")
> Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>
Please CC the fixed patch author Mike Christie <mchristi@redhat.com>.
> ---
> drivers/md/bcache/request.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
> index c7cadaafa947..eb734f7ddaac 100644
> --- a/drivers/md/bcache/request.c
> +++ b/drivers/md/bcache/request.c
> @@ -244,7 +244,7 @@ static void bch_data_insert_start(struct closure *cl)
> trace_bcache_cache_insert(k);
> bch_keylist_push(&op->insert_keys);
>
> - bio_set_op_attrs(n, REQ_OP_WRITE, 0);
> + n->bi_opf |= REQ_OP_WRITE;
> bch_submit_bbio(n, op->c, k, 0);
> } while (n != bio);
The fix is OK to me, I'd like to see opinion from Mike Christie too.
Thanks for the catch.
Coly Li
next prev parent reply other threads:[~2021-01-25 4:54 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-25 4:29 [PATCH] bcache: dont reset bio opf in bch_data_insert_start Dongsheng Yang
2021-01-25 4:53 ` Coly Li [this message]
2021-01-26 4:32 ` Dongsheng Yang
2021-01-26 4:34 ` Coly Li
2021-01-26 4:41 ` Dongsheng Yang
2021-01-27 17:37 ` Christoph Hellwig
2021-01-28 9:10 ` Dongsheng Yang
2021-01-28 10:45 ` Dongsheng Yang
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=7569abf3-3e54-986e-8307-751fa5e00828@suse.de \
--to=colyli@suse.de \
--cc=dongsheng.yang@easystack.cn \
--cc=linux-bcache@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mchristi@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).