On 9:57 07/10, Darrick J. Wong wrote: > On Sat, Oct 05, 2024 at 03:17:47AM +0100, Matthew Wilcox wrote: > > On Fri, Oct 04, 2024 at 04:04:28PM -0400, Goldwyn Rodrigues wrote: > > > Filesystems such as BTRFS use folio->private so that they receive a > > > callback while releasing folios. Add check if folio size is same as > > > filesystem block size while evaluating iomap_folio_state from > > > folio->private. > > > > > > I am hoping this will be removed when all of btrfs code has moved to > > > iomap and BTRFS uses iomap's subpage. > > > > This seems like a terrible explanation for why you need this patch. > > > > As I understand it, what you're really doing is saying that iomap only > > uses folio->private for block size < folio size. So if you add this > > hack, iomap won't look at folio->private for block size == folio size > > and that means that btrfs can continue to use it. > > > > I don't think this is a good way to start the conversion. I appreciate > > that it's a long, complex procedure, and you can't do the whole > > conversion in a single patchset. > > > > Also, please stop calling this "subpage". That's btrfs terminology, > > it's confusing as hell, and it should be deleted from your brain. > > I've long wondered if 'subpage' is shorthand for 'subpage blocksize'? > If so then the term makes sense to me as a fs developer, but I can also > see how it might not make sense to anyone from the mm side of things. Yes, it is subpage blocksize. > > Wait, is a btrfs sector the same as what ext4/xfs call a fs block? Yup, fs_info->sectorsize. > > > But I don't understand why you need it at all. btrfs doesn't attach > > private data to folios unless block size < page size. Which is precisely > > the case that you're not using. So it seems like you could just drop > > this patch and everything would still work. > > I was also wondering this. Given that the end of struct btrfs_subpage > is an uptodate/dirty/ordered bitmap, maybe iomap_folio_ops should grow a > method to allocate a struct iomap_folio_state object, and then you could > embed one in the btrfs subpage object and provide that custom allocation > function? > > (and yes that makes for an ugly mess of pointer math crud to have two > VLAs inside struct btrfs_subpage, so this might be too ugly to live in > practice) > btrfs does use iomap->private and writes out EXTENT_FOLIO_PRIVATE. This is not ideal, but it requires it to get a callback from mm before folios are released. Refer set_folio_extent_mapped(). BTRFS does it for every folio (for filesystems which is not a subpage blocksize). Perhaps there is a better way to do this? Ideally, after the move to iomap, we should not require btrfs_subpage structures, and most (if not all) folio "handlings" will be done by iomap but that is still far way off. -- Goldwyn
