On Thu, Apr 11, 2024 at 03:04:09PM +0100, Al Viro wrote: > On Thu, Apr 11, 2024 at 01:56:03PM +0200, Christian Brauner wrote: > > On Wed, Apr 10, 2024 at 11:34:43PM +0100, Al Viro wrote: > > > 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: > > > > Ok, let's move forward with this. I've applied the first 19 patches. > > Patch 20 is the start of what we all disliked. 21 is clearly a bugfix > > for current code so that'll go separately from the rest. I've replaced > > open-code f_mapping access with file_mapping(). The symmetry between > > file_inode() and file_mapping() is quite nice. > > > > Al, your idea to switch erofs away from buf->inode can go on top of what > > Yu did imho. There's no real reason to throw it away imho. > > > > I've exported bdev_mapping() because it really makes the btrfs change a > > lot slimmer and we don't need to care about messing with a lot of that > > code. I didn't care about making it static inline because that might've > > meant we need to move other stuff into the header as well. Imho, it's > > not that important but if it's a big deal to any of you just do the > > changes on top of it, please. > > > > Pushed to > > https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.super > > > > If I hear no objections that'll show up in -next tomorrow. Al, would be > > nice if you could do your changes on top of this, please. > > Objection: start with adding bdev->bd_mapping, next convert the really > obvious instances to it and most of this series becomes not needed at > all. > > Really. There is no need whatsoever to push struct file down all those > paths. Your series just replaces bd_inode in struct block_device with bd_mapping. In a lot of places we do have immediate access to the bdev file without changing any calling conventions whatsoever. IMO it's perfectly fine to just use file_mapping() there. Sure, let's use bdev_mapping() in instances like btrfs where we'd otherwise have to change function signatures I'm not opposed to that. But there's no good reason to just replace everything with bdev->bd_mapping access. And really, why keep that thing in struct block_device when we can avoid it. > > And yes, erofs and buffer.c stuff belongs on top of that, no arguments here.
