On Wed, Jan 17, 2024 at 05:33:36PM +0100, Jan Kara wrote:
> On Wed 17-01-24 17:24:23, Christoph Hellwig wrote:
> > On Wed, Jan 17, 2024 at 05:15:00PM +0100, Jan Kara wrote:
> > > > > 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.
