On Wed, Jan 17, 2024 at 05:33:36PM +0100, Jan Kara wrote:

On Wed 17-01-24 17:24:23, Christoph Hellwig wrote:

quoted

On Wed, Jan 17, 2024 at 05:15:00PM +0100, Jan Kara wrote:

quoted

quoted

quoted

 	iomap->bdev = bdev;
+	BUG_ON(true /* TODO(brauner): This is the only place where we don't go from inode->i_sb->s_f_bdev for obvious reasons. Thoughts? */);

Maybe block devices should have their own struct file created when the
block device is instantiated and torn down when the block device is
trashed?

OK, but is there a problem with I_BDEV() which is currently used in
blkdev_iomap_begin()?

Well, blkdev_iomap_begin is always called on the the actual bdev fs
inode that is allocated together with the bdev itself.  So we'll always
be able to get to it using container_of variants.

Yes, that was exactly my point.

So my point is that if we want to have all code pass a file and we have
code in fs/buffer.c like iomap_to_bh():

iomap_to_bh(struct inode *inode, sector_t block, struct buffer_head *bh,
        loff_t offset = block << inode->i_blkbits;

        bh->b_bdev = iomap->bdev;
+       bh->f_b_bdev = iomap->f_bdev;

While that works for every single filesystem that uses block devices
because they stash them somewhere (like s_bdev_file) it doesn't work for
the bdev filesystem itself. So if the bdev filesystem calls into helpers
that expect e.g., buffer_head->s_f_bdev to have been initialized from
iomap->f_bdev this wouldn't work.

So if we want to remove b_bdev from struct buffer_head and fully rely on
f_b_bdev - and similar in iomap - then we need a story for the bdev fs
itself. And I wasn't clear on what that would be. Dave's suggestion
would obviously work iirc. Maybe you're poking my nose at something very
obvious and I don't see it yet?

Fyi, I had planned to leave that patch out of the next revision anyway
because I can merge the generic part without it as it stands on its own.