On Fri, Dec 23, 2022 at 11:04:51PM +0100, Andreas Grünbacher wrote: > > I find the naming very confusing. Any good reason to not follow > > the naming of pagecache_isize_extended an call it > > folio_isize_extended? > > A good reason for a different name is because > folio_may_straddle_isize() requires a locked folio, while > pagecache_isize_extended() will fail if the folio is still locked. So > this doesn't follow the usual "replace 'page' with 'folio'" pattern. pagecache also doesn't say page, it says pagecache. I'd still prepfer to keep the postfix the same. And I think the fact that it needs a locked folio should also have an assert, which both documents this and catches errors. I think that's much better than an arbitrarily different name. > > Should pagecache_isize_extended be rewritten to use this helper, > > i.e. turn this into a factoring out of a helper? > > I'm not really sure about that. The boundary conditions in the two > functions are not identical. I think the logic in > folio_may_straddle_isize() is sufficient for the > extending-write-under-folio-lock case, but I'd still need confirmation > for that. If the same logic would also be enough in > pagecache_isize_extended() is more unclear to me. That's another thing that really needs to into the commit log, why is the condition different and pagecache_isize_extended can't just be extended for it (if it really can't).
