On Fri 18-03-22 03:49:27, Matthew Wilcox wrote: > On Thu, Mar 17, 2022 at 12:58:46PM +0100, Michal Hocko wrote: > > On Wed 16-03-22 20:31:30, Matthew Wilcox wrote: > > > This is a fairly mechanical change to convert mc_target.page to > > > mc_target.folio. This is a prerequisite for converting > > > find_get_incore_page() to find_get_incore_folio(). But I'm not > > > convinced it's right, and I'm not convinced the existing code is > > > quite right either. > > > > > > In particular, the code in hunk @@ -6036,28 +6041,26 @@ needs > > > careful review. There are also assumptions in here that a memory > > > allocation is never larger than a PMD, which is true today, but I've > > > been asked about larger allocations. > > > > Could you be more specific about those usecases? Are they really > > interested in supporting larger pages for the memcg migration which is > > v1 only feature? Or you are interested merely to have the code more > > generic? > > Ah! I didn't realise memcg migration was a v1-only feature. I think > that makes all of the questions much less interesting. I've done some > more reading, and it seems like all of this is "best effort", so it > doesn't really matter if some folios get skipped. Yes. [...] > That makes sense. I think the case that's currently mishandled is a > THP in tmpfs which is misaligned when mapped to userspace. It's > skipped, even if the entire THP is mapped. But maybe that simply > doesn't matter. > > I suppose the question is: Do we care if mappings of files are not > migrated to the new memcg? I'm getting a sense that the answer is "no", > and if we actually ended up skipping all file mappings, it wouldn't > matter. Yes, I wouldn't lose sleep over that. You are not introducing a new regression. The feature is mostly deprecated (along with the whole v1) so we tend to prefer bug-to-bug compatibility rather than making the code more complex to solve a theoretical problem (or at least a problem that nobody is complaining about). Thanks! -- Michal Hocko SUSE Labs
