From: Coly Li <firstname.lastname@example.org> To: Christoph Hellwig <email@example.com> Cc: firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, email@example.com, Alexander Ullrich <firstname.lastname@example.org>, Diego Ercolani <email@example.com>, Jan Szubiak <firstname.lastname@example.org>, Marco Rebhan <email@example.com>, Matthias Ferdinand <firstname.lastname@example.org>, Victor Westerhuis <email@example.com>, Vojtech Pavlik <firstname.lastname@example.org>, Rolf Fokkens <email@example.com>, Thorsten Knabe <firstname.lastname@example.org>, email@example.com, Kent Overstreet <firstname.lastname@example.org>, Nix <email@example.com>, Takashi Iwai <firstname.lastname@example.org> 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: <email@example.com> (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
next prev parent 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 \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --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).