On Sun, Mar 19, 2023 at 10:19:21PM -0700, Hugh Dickins wrote: > I thought this was fine at first, and of course it's good for all the > usual cases; but not for shmem_get_partial_folio()'s awkward cases. > > Two issues with it. > > One, as you highlight above, the possibility of reading more swap > unnecessarily. I do not mind if partial truncation entails reading > a little unnecessary swap; but I don't like the common case of > truncation to 0 to entail that; even less eviction; even less > unmounting, when eviction of all risks reading lots of swap. > The old code behaved well at i_size 0, the new code not so much. True. We could restore that by doing the i_size check for SGP_FIND, though. > Replacing shmem_get_partial_folio() by SGP_FIND was a good direction > to try, but it hasn't worked out. I tried to get SGPs to work right > for it before, when shmem_get_partial_page() was introduced; but I > did not manage to do so. I think we have to go back to how this was. Hmm, would be sad to lose this entirely. One thing I though about but didn't manage to do, is to rework how the SGP_* flags works. Right now they are used as en enum, and we actually do numerical comparisms on them, which is highly confusing. To be it seems like having actual flags that can be combined and have useful names would seem much better. But I did run out patience for finding good names and figuring out what would be granular enough behavior for such flags. e.g. one would be for limiting to i_size, one for allocating new folios if none was found, etc.
