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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux