On Sun, Apr 07, 2024 at 04:06:10AM +0100, Al Viro wrote: > 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? > > There is a good reason to have it in places that keep a reference to > opened block device - the kind that _keeps_ the device opened. Namely, > there's state that need to be carried from the place where we'd opened > the sucker to the place where we close it, and that state is better > carried by opened file. > > But neither iomap nor buffer_head contain anything of that sort - > the lifetime management of the opened device is not in their > competence. As the matter of fact, the logics around closing > those opened devices (bdev_release()) makes sure that no > instances of buffer_head (or iomap) will outlive them. > And they don't care about any extra state - everything > they use is in block_device and coallocated inode. > > I could've easily missed something in one of the threads around > the earlier iterations of the patchset; if that's the case, > could somebody restate the rationale for that part and/or > post relevant lore.kernel.org links? Christian? hch? > What am I missing here? The original series was a simple RFC/POC to show that struct file could be used to remove bd_inode access in a wide variety of situations. But as I've mentioned in that thread I wasn't happy with various aspects of the approach which is why I never pushed forward with it. The part where we pushed struct file into buffer_header was the most obvious one.