Re: [PATCH vfs.all 22/26] block: stash a bdev_file to read/write raw blcok_device

[Al Viro <viro@zeniv.linux.org.uk>]

On Wed, Apr 10, 2024 at 12:59:11PM +0200, Jan Kara wrote:

> I agree with Christian and Al - and I think I've expressed that already in
> the previous version of the series [1] but I guess I was not explicit
> enough :). I think the initial part of the series (upto patch 21, perhaps
> excluding patch 20) is a nice cleanup but the latter part playing with
> stashing struct file is not an improvement and seems pointless to me. So
> I'd separate the initial part cleaning up the obvious places and let
> Christian merge it and then we can figure out what (if anything) to do with
> remaining bd_inode uses in fs/buffer.c etc. E.g. what Al suggests with
> bd_mapping makes sense to me but I didn't check what's left after your
> initial patches...

FWIW, experimental on top of -next:
Al Viro (7):
      block_device: add a pointer to struct address_space (page cache of bdev)
      use ->bd_mapping instead of ->bd_inode->i_mapping
      grow_dev_folio(): we only want ->bd_inode->i_mapping there
      gfs2: more obvious initializations of mapping->host
      blkdev_write_iter(): saner way to get inode and bdev
      blk_ioctl_{discard,zeroout}(): we only want ->bd_inode->i_mapping here...
      dm-vdo: use bdev_nr_bytes(bdev) instead of i_size_read(bdev->bd_inode)

Yu Kuai (4):
      ext4: remove block_device_ejected()
      block: move two helpers into bdev.c
      bcachefs: remove dead function bdev_sectors()
      block2mtd: prevent direct access of bd_inode	[slightly modified]

leaves only this:
block/bdev.c:60:        struct inode *inode = bdev->bd_inode;
block/bdev.c:137:       loff_t size = i_size_read(bdev->bd_inode);
block/bdev.c:144:       bdev->bd_inode->i_blkbits = blksize_bits(bsize);
block/bdev.c:158:       if (bdev->bd_inode->i_blkbits != blksize_bits(size)) {
block/bdev.c:160:               bdev->bd_inode->i_blkbits = blksize_bits(size);
block/bdev.c:415:       bdev->bd_inode = inode;
block/bdev.c:434:       i_size_write(bdev->bd_inode, (loff_t)sectors << SECTOR_SHIFT);
block/bdev.c:444:       bdev->bd_inode->i_rdev = dev;
block/bdev.c:445:       bdev->bd_inode->i_ino = dev;
block/bdev.c:446:       insert_inode_hash(bdev->bd_inode);
block/bdev.c:974:       bdev_file = alloc_file_pseudo_noaccount(bdev->bd_inode,
block/bdev.c:980:       ihold(bdev->bd_inode);
block/bdev.c:1257:      return !inode_unhashed(disk->part0->bd_inode);
block/bdev.c:1263:      return 1 << bdev->bd_inode->i_blkbits;
block/genhd.c:659:              remove_inode_hash(part->bd_inode);
block/genhd.c:1194:     iput(disk->part0->bd_inode);    /* frees the disk */
block/genhd.c:1384:     iput(disk->part0->bd_inode);
block/partitions/core.c:246:    iput(dev_to_bdev(dev)->bd_inode);
block/partitions/core.c:472:    remove_inode_hash(part->bd_inode);
block/partitions/core.c:658:            remove_inode_hash(part->bd_inode);
drivers/s390/block/dasd_ioctl.c:218:            block->gdp->part0->bd_inode->i_blkbits =
fs/buffer.c:192:        struct inode *bd_inode = bdev->bd_inode;
fs/buffer.c:1699:       struct inode *bd_inode = bdev->bd_inode;
fs/erofs/data.c:73:             buf->inode = sb->s_bdev->bd_inode;
fs/nilfs2/segment.c:2793:       inode_attach_wb(nilfs->ns_bdev->bd_inode, NULL);

I've got erofs patches that get rid of that instance; bdev.c is obviously priveleged
since it sees coallocated inode directly.  Other than those we have
	* 3 callers of remove_inode_hash()
	* 3 callers of iput()
	* one caller of inode_attach_wb() (nilfs2)
	* weird shit in DASD (redundant, that; incidentally, I don't see anything
	  that might prevent DASD format requested with mounted partitions on that
	  disk - and won't that be fun and joy for an admin to step into...)
	* two places in fs/buffer.c that want to convert block numbers to positions
	  in bytes.  Either the function itself or its caller has the block size
	  as argument; replacing that to passing block _shift_ instead of size
	  would reduce those two to ->bd_mapping.
And that's it.  iput() and remove_inode_hash() are obvious candidates for
helpers (internal to block/*; no exporting those, it's private to bdev.c,
genhd.c and paritions/core.c).

fs/buffer.c ones need a bit more code audit (not quite done with that), but
it looks at least plausible.  Which would leave us with whatever nilfs2 is
doing and that weirdness in dasd_format() (why set ->i_blkbits but not
->i_blocksize?  why not use set_blocksize(), for that matter?  where the
hell is check for exclusive open?)