Message-ID: <ZDtPK5Qdts19bKY2@bombadil.infradead.org>
Date:   Sat, 15 Apr 2023 18:28:11 -0700
From:   Luis Chamberlain <mcgrof@...nel.org>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     Hannes Reinecke <hare@...e.de>,
        Pankaj Raghav <p.raghav@...sung.com>,
        "kbus >> Keith Busch" <kbusch@...nel.org>, brauner@...nel.org,
        viro@...iv.linux.org.uk, akpm@...ux-foundation.org,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        gost.dev@...sung.com
Subject: Re: [RFC 0/4] convert create_page_buffers to create_folio_buffers

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

Powered by blists - more mailing lists