linux-bcache.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Coly Li <colyli@suse.de>
To: Christoph Hellwig <hch@lst.de>
Cc: axboe@kernel.dk, linux-bcache@vger.kernel.org,
	linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
	Alexander Ullrich <ealex1979@gmail.com>,
	Diego Ercolani <diego.ercolani@gmail.com>,
	Jan Szubiak <jan.szubiak@linuxpolska.pl>,
	Marco Rebhan <me@dblsaiko.net>,
	Matthias Ferdinand <bcache@mfedv.net>,
	Victor Westerhuis <victor@westerhu.is>,
	Vojtech Pavlik <vojtech@suse.cz>,
	Rolf Fokkens <rolf@rolffokkens.nl>,
	Thorsten Knabe <linux@thorsten-knabe.de>,
	stable@vger.kernel.org,
	Kent Overstreet <kent.overstreet@gmail.com>,
	Nix <nix@esperi.org.uk>, Takashi Iwai <tiwai@suse.com>
Subject: Re: [PATCH v5 2/2] bcache: avoid oversized read request in cache missing code path
Date: Mon, 7 Jun 2021 20:31:59 +0800	[thread overview]
Message-ID: <d7848f93-4acc-031f-b8ae-6fc2a4742fe7@suse.de> (raw)
In-Reply-To: <20210607120822.GA11665@lst.de>

On 6/7/21 8:08 PM, Christoph Hellwig wrote:
> On Mon, Jun 07, 2021 at 07:55:22PM +0800, Coly Li wrote:
>> On 6/7/21 7:06 PM, Christoph Hellwig wrote:
>>> On Mon, Jun 07, 2021 at 06:35:39PM +0800, Coly Li wrote:
>>>> +	/* Limitation for valid replace key size and cache_bio bvecs number */
>>>> +	size_limit = min_t(unsigned int, bio_max_segs(UINT_MAX) * PAGE_SECTORS,
>>>> +			   (1 << KEY_SIZE_BITS) - 1);
>>> bio_max_segs kaps the argument to BIO_MAX_VECS, so you might as well
>> It was suggested to not directly access BIO_MAX_VECS by you, maybe I
>> misunderstood you.
> Yes, drivers really should not care about it.  But hiding that behind
> a tiny wrapper doesn't help either.

OK, I change back to BIO_MAX_VECS.
>>> directly write BIO_MAX_VECS.  Can you explain the PAGE_SECTORS here a bit
>>> more? Does this code path use discontiguous per-sector allocations?
>>> Preferably in a comment.
>>
>> It is just because bch_bio_map() assume the maximum bio size is 1MB. It
>> was not true since the multiple pages bvecs
>> was merged in mainline kernel.
>>  
>> The PAGE_SECTORS part is legacy for 1MB maximum size bio (256*4KB), it
>> should be fixed/improved later to
>> use multiple pages for bio size > 1MB and replace bch_bio_map().
> bch_bio_map and bch_bio_alloc_pages that poke directly into the bio are
> the root cause of a lot of these problems.
>
> I had a series fixing some of that but Kent did not like it.  Drivers must
> not diretly access bi_vcnt or directly build bios.

Last time when you posted the series, Ming Lei's multipages bvecs were
not merged yet.

But the bcache code has kind of implicit restriction, e.g.
cached_dev_cache_miss(), the refill
cache missing 'cache_bio' must be a single bio and not chained,
otherwise the following
cached_dev_read_done() will have problem. Therefore I give up to fix
such thing in this
series, maybe there is no unique way to fix, and I also need to improve
the bio related stuffs
case by case.


>> Not any more. Now the line limit is 100 characters. Though I still
>> prefer 80 characters, place 86 characters in single line
>> makes the change more obvious.
> It makes it really hard to read in a normal terminal.

I don't know you still use 80 characters terminal for coding. Sure you
have my respect and I will serve your
request in next post.

Thanks.

Coly Li

  reply	other threads:[~2021-06-07 12:32 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-07 10:35 [PATCH 0/2] bcache fixes for Linux v5.13-rc6 Coly Li
2021-06-07 10:35 ` [PATCH v5 1/2] bcache: remove bcache device self-defined readahead Coly Li
2021-06-07 11:00   ` Christoph Hellwig
2021-06-07 10:35 ` [PATCH v5 2/2] bcache: avoid oversized read request in cache missing code path Coly Li
2021-06-07 11:06   ` Christoph Hellwig
2021-06-07 11:55     ` Coly Li
2021-06-07 12:08       ` Christoph Hellwig
2021-06-07 12:31         ` Coly Li [this message]
  -- strict thread matches above, loose matches on Subject: below --
2021-05-31 15:31 [PATCH v5 1/2] bcache: remove bcache device self-defined readahead Coly Li
2021-05-31 15:31 ` [PATCH v5 2/2] bcache: avoid oversized read request in cache missing code path Coly Li

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=d7848f93-4acc-031f-b8ae-6fc2a4742fe7@suse.de \
    --to=colyli@suse.de \
    --cc=axboe@kernel.dk \
    --cc=bcache@mfedv.net \
    --cc=diego.ercolani@gmail.com \
    --cc=ealex1979@gmail.com \
    --cc=hch@lst.de \
    --cc=jan.szubiak@linuxpolska.pl \
    --cc=kent.overstreet@gmail.com \
    --cc=linux-bcache@vger.kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@thorsten-knabe.de \
    --cc=me@dblsaiko.net \
    --cc=nix@esperi.org.uk \
    --cc=rolf@rolffokkens.nl \
    --cc=stable@vger.kernel.org \
    --cc=tiwai@suse.com \
    --cc=victor@westerhu.is \
    --cc=vojtech@suse.cz \
    --subject='Re: [PATCH v5 2/2] bcache: avoid oversized read request in cache missing code path' \
    /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

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).