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?)
