Hi,
在 2024/04/09 12:26, Al Viro 写道:
On Sun, Apr 07, 2024 at 11:21:56AM +0800, Yu Kuai wrote:
Hi,
在 2024/04/07 11:06, Al Viro 写道:
On Sun, Apr 07, 2024 at 10:34:56AM +0800, Yu Kuai wrote:
Other than raw block_device fops, other filesystems can use the opened
bdev_file directly for iomap and buffer_head, and they actually don't
need to reference block_device anymore. The point here is that whether
What do you mean, "reference"? The counting reference is to opened
file; ->s_bdev is a cached pointer to associated struct block_device,
and neither it nor pointers in buffer_head are valid past the moment
when you close the file. Storing (non-counting) pointers to struct
file in struct buffer_head is not different in that respect - they
are *still* only valid while the "master" reference is held.
Again, what's the point of storing struct file * in struct buffer_head
or struct iomap? In any instances of those structures?
Perhaps this is what you missed, like the title of this set, in order to
remove direct acceess of bdev->bd_inode from fs/buffer, we must store
bdev_file in buffer_head and iomap, and 'bdev->bd_inode' is replaced
with 'file_inode(bdev)' now.
BTW, what does that have to do with iomap? All it passes ->bdev to is
1) bio_alloc()
2) bio_alloc_bioset()
3) bio_init()
4) bdev_logical_block_size()
5) bdev_iter_is_aligned()
6) bdev_fua()
7) bdev_write_cache()
None of those goes anywhere near fs/buffer.c or uses ->bd_inode, AFAICS.
Again, what's the point? It feels like you are trying to replace *all*
uses of struct block_device with struct file, just because.
If that's what's going on, please don't. Using struct file instead
of that bdev_handle crap - sure, makes perfect sense. But shoving it
down into struct bio really, really does not.
I'd suggest to start with adding ->bd_mapping as the first step and
converting the places where mapping is all we want to using that.
Right at the beginning of your series. Then let's see what gets
left.
Thanks so much for your advice, in fact, I totally agree with this that
adding a 'bd_mapping' or expose the helper bdev_mapping().
However, I will let Christoph and Jan to make the decision, when they
get time to take a look at this.
Thanks!
Kuai
And leave ->bd_inode there for now; don't blindly replace it with
->bd_mapping->host everywhere. It's much easier to grep for.
The point of the exercise is to find what do we really need ->bd_inode
for and what primitives are missing, not getting rid of a bad word...
.
