On Sat, Apr 13, 2024 at 05:25:01PM +0200, Christian Brauner wrote: > > It also simplifies the hell out of the patch series - it's one obviously > > safe automatic change in a single commit. > > It's trivial to fold the simple file_mapping() conversion into a single > patch as well. ... after a bunch of patches that propagate struct file to places where it has no business being. Compared to a variant that doesn't need those patches at all. > It's a pure artifact of splitting the patches per > subsystem/driver. No, it is not. ->bd_mapping conversion can be done without any preliminaries. Note that it doesn't need messing with bdev_read_folio(), it doesn't need this journal->j_fs_dev_file thing, etc. One thing I believe is completely wrong in this series is bdev_inode() existence. It (and equivalent use of file_inode() on struct file is even worse) is papering over the real interface deficiencies. And extra file_inode() uses are just about impossible to catch ;-/ IMO we should *never* use file_inode() on opened block devices. At all. It's brittle, it's asking for trouble as soon as somebody passes a normally opened struct file to one of the functions using it and it papers over the missing primitives. As for the space concerns... With struct device embedded into those things, it's not even funny. Space within the first cacheline - sure, but we can have a pointer in there just fine.
