All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luis Chamberlain <mcgrof@kernel.org>
To: Matthew Wilcox <willy@infradead.org>
Cc: Hannes Reinecke <hare@suse.de>,
	Pankaj Raghav <p.raghav@samsung.com>,
	"kbus >> Keith Busch" <kbusch@kernel.org>,
	brauner@kernel.org, viro@zeniv.linux.org.uk,
	akpm@linux-foundation.org, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, gost.dev@samsung.com
Subject: Re: [RFC 0/4] convert create_page_buffers to create_folio_buffers
Date: Sat, 15 Apr 2023 18:28:11 -0700	[thread overview]
Message-ID: <ZDtPK5Qdts19bKY2@bombadil.infradead.org> (raw)
In-Reply-To: <ZDraOHQHqeabyCvN@casper.infradead.org>

On Sat, Apr 15, 2023 at 06:09:12PM +0100, Matthew Wilcox wrote:
> On Sat, Apr 15, 2023 at 03:14:33PM +0200, Hannes Reinecke wrote:
> > On 4/15/23 05:44, Matthew Wilcox wrote:
> > We _could_ upgrade to always do full page I/O; there's a good
> > chance we'll be using the entire page anyway eventually.

*Iff* doing away with buffer head 512 granularity could help block sizes
greater than page size where physical and logical block size > PAGE_SIZE
we whould also be able to see it on 4kn drives (logical and physical
block size == 4k). A projection could be made after.

In so far as experimenting with this, if you already have some
effort on IOMAP for bdev aops one possibility for pure experimentation
for now would be to peg a new set of aops could be set in the path
of __alloc_disk_node() --> bdev_alloc() but that's a wee-bit too early
for us to know if the device is has (lbs = pbs) > 512. For NVMe for
instance this would be nvme_alloc_ns() --> blk_mq_alloc_disk(). We
put together and set the logical and phyiscal block size on NVMe on
nvme_update_ns_info() --> nvme_update_disk_info(), right before we
call device_add_disk(). The only way to override the aops then would
be right before device_add_disk(), or as part of a new device_add_disk_aops()
or whatever.

> > And with storage bandwidth getting larger and larger we might even
> > get a performance boost there.
> 
> I think we need to look at this from the filesystem side.

Before that let's recap the bdev cache current issues.

Today by just adding the disk we move on to partition scanning
immediately unless your block driver has a flag that says otherwise. The
current crash we're evaluating with brd and that we also hit with NVMe
is due to this part.

device_add_disk() -->
  disk_scan_partitions() -->
    blkdev_get_whole() -->
      bdev_disk_changed() -->
        filemap_read_folio() --> filler()

The filler is from aops.

We don't even have a filesystem yet on these devices at this point. The entire
partition core does this partition scanning. Refer to:

disk_scan_partitions() --> block/partitions/core.c : bdev_disk_changed()

And all of that stuff is also under a 512-byte atomic operation assumption,
we could do better if wanted to.

> What do filesystems actually want to do?

So you are suggesting that the early reads of the block device by the
block cache and its use of the page cache cache should be aligned /
perhaps redesigned to assist more clearly with what modern filesystems
might actually would want today?

> The first thing is they want to read
> the superblock.  That's either going to be immediately freed ("Oh,
> this isn't a JFS filesystem after all") or it's going to hang around
> indefinitely.  There's no particular need to keep it in any kind of
> cache (buffer or page). 

And the bdev cache would not be able to know before hand that's the case.

> Except ... we want to probe a dozen different
> filesystems, and half of them keep their superblock at the same offset
> from the start of the block device.  So we do want to keep it cached.
> That's arguing for using the page cache, at least to read it.

Do we currently share anything from the bdev cache with the fs for this?
Let's say that first block device blocksize in memory.

> Now, do we want userspace to be able to dd a new superblock into place
> and have the mounted filesystem see it? 

Not sure I follow this. dd a new super block?

> I suspect that confuses just
> about every filesystem out there.  So I think the right answer is to read
> the page into the bdev's page cache and then copy it into a kmalloc'ed
> buffer which the filesystem is then responsible for freeing.  It's also
> responsible for writing it back (so that's another API we need), and for
> a journalled filesystem, it needs to fit into the journalling scheme.
> Also, we may need to write back multiple copies of the superblock,
> possibly with slight modifications.

Are you considering these as extentions to the bdev cache?

  Luis

  reply	other threads:[~2023-04-16  1:28 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20230414110825eucas1p1ed4d16627889ef8542dfa31b1183063d@eucas1p1.samsung.com>
2023-04-14 11:08 ` [RFC 0/4] convert create_page_buffers to create_folio_buffers Pankaj Raghav
2023-04-14 11:08   ` [RFC 1/4] fs/buffer: add set_bh_folio helper Pankaj Raghav
2023-04-14 11:08   ` [RFC 2/4] buffer: add alloc_folio_buffers() helper Pankaj Raghav
2023-04-14 13:06     ` Matthew Wilcox
2023-04-14 15:01       ` Pankaj Raghav
2023-04-14 14:04     ` kernel test robot
2023-04-14 14:45     ` kernel test robot
2023-04-14 11:08   ` [RFC 3/4] fs/buffer: add folio_create_empty_buffers helper Pankaj Raghav
2023-04-14 13:16     ` Matthew Wilcox
2023-04-14 11:08   ` [RFC 4/4] fs/buffer: convert create_page_buffers to create_folio_buffers Pankaj Raghav
2023-04-14 13:21     ` Matthew Wilcox
2023-04-14 13:47   ` [RFC 0/4] " Hannes Reinecke
2023-04-14 13:51     ` Matthew Wilcox
2023-04-14 13:56       ` Hannes Reinecke
2023-04-14 15:00     ` Pankaj Raghav
2023-04-15  1:01     ` Luis Chamberlain
2023-04-15  2:31       ` Matthew Wilcox
2023-04-15  3:24         ` Luis Chamberlain
2023-04-15  3:44           ` Matthew Wilcox
2023-04-15 13:14             ` Hannes Reinecke
2023-04-15 17:09               ` Matthew Wilcox
2023-04-16  1:28                 ` Luis Chamberlain [this message]
2023-04-16  3:40                   ` Matthew Wilcox
2023-04-16  5:26                     ` Luis Chamberlain
2023-04-16 14:07                       ` Matthew Wilcox
2023-04-17 15:40                         ` Darrick J. Wong
2023-04-16 22:57                       ` Dave Chinner
2023-04-17  2:27     ` Luis Chamberlain
2023-04-17  6:04       ` Hannes Reinecke

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=ZDtPK5Qdts19bKY2@bombadil.infradead.org \
    --to=mcgrof@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=brauner@kernel.org \
    --cc=gost.dev@samsung.com \
    --cc=hare@suse.de \
    --cc=kbusch@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=p.raghav@samsung.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    /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.