On Thu, Jan 12, 2023 at 07:52:41AM +1100, Dave Chinner wrote: > Exposing internal implementation details in the API > that is supposed to abstract away the internal implementation > details from users doesn't seem like a great idea to me. While I somewhat agree with the concern of leaking the xarray internals, at least they are clearly documented and easy to find.. > Exactly what are we trying to fix here? Do we really need to punch > a hole through the abstraction layers like this just to remove half > a dozen lines of -slow path- context specific error handling from a > single caller? While the current code (which is getting worse with your fix) leaks completely undocumented and internal decision making. So what this fixes is a real leak of internatal logic inside of __filemap_get_folio into the callers. So as far as I'm concerned we really do need the helper, and anyone using !GFP_CREATE or FGP_NOWAIT should be using it. The only question to me is if exposing the xarray internals is worth it vs the less optimal calling conventions of needing an extra argument for the error code.