On 12/10/24 4:31 AM, Christoph Hellwig wrote: > On Fri, Dec 06, 2024 at 11:22:55AM -0700, Jens Axboe wrote: >>> Honestly, I'm not a fan of foliop_uncached or foliop_is_uncached. >> >> It definitely is what I would elegantly refer to as somewhat of a >> hack... But it's not _that_ bad imho. > > It's pretty horrible actually. Tell us how you really feel :-) >>> I think these two macros are only used for ext4 (or really, !iomap) >>> support, right? And that's only to avoid messing with ->write_begin? >> >> Indeed, ideally we'd change ->write_begin() instead. And that probably >> should still be done, I just did not want to deal with that nightmare in >> terms of managing the patchset. And honestly I think it'd be OK to defer >> that part until ->write_begin() needs to be changed for other reasons, >> it's a lot of churn just for this particular thing and dealing with the >> magic pointer value (at least to me) is liveable. > > ->write_begin() really should just go away, it is a horrible interface. > Note that in that past it actually had a flags argument, but that got > killed a while ago. > >>> What if you dropped ext4 support instead? :D >> >> Hah, yes obviously that'd be a solution, then I'd need to drop btrfs as >> well. And I would kind of prefer not doing that ;-) > > Btrfs doesn't need it. In fact the code would be cleaner and do less > work with out, see the patch below. And for ext4 there already is an > iomap conversion patch series on the list that just needs more review, > so skipping it here and growing the uncached support through that sounds > sensible. I can certainly defer the ext4 series if the below sorts out btrfs, if that iomap conversion series is making progress. Don't have an issue slotting behind that. I'll check and test your btrfs tweak, thanks! -- Jens Axboe
