On Wed, Aug 10, 2022 at 6:00 PM Alex Zhu (Kernel) <alexlzhu@xxxxxx> wrote: > > > > Which series are you talking about? I listed two series and they are > > very different on the code level. > > > > I was referring to the second patch: https://lore.kernel.org/all/1635422215-99394-1-git-send-email-ningzhang@xxxxxxxxxxxxxxxxx/. You mean the second patch*set* or series, the link doesn't point to a single patch :) "the second patch" could mean the second patch in that series. > This patchset adds the THP shrinking as part of shrink_lruvec in mm/vmscan.c. We create a new shrinker that shrinks THPs based off the results > of the scanning implemented in this thp_utilization patch. We also do not have any of the additional knobs for controlling THP reclaim that the patchset above has. That seems unnecessary in the initial patch as shrinking THPs that are almost entirely zero pages should only improve performance. > > I believe the resulting implementation we have is simpler and easier to understand than the above patchset. By identifying and freeing underutilized THPs we hope to eventually deprecate madvise entirely and have THP always enabled. > > > The 2nd patch from the first series does exactly this. > > > >> but it’s worth discussing whether to free zero pages immediately or to add to lruvec to free eventually. > > > > And that patch can be omitted if the third link (a single patch, not a > > series) is used, which makes the workflow "add to lruvec to free > > eventually". > > > >> I believe the split_huge_page() changes could be valuable by as a patch by itself though. Will send that out shortly. > > Referring to this patch: https://lore.kernel.org/r/20210731063938.1391602-1-yuzhao@xxxxxxxxxx/. > > We do indeed do something similar to patches 1 and 3. We may be able to make use of this instead, I’ll take a closer look. Please do. Based on what you said ("chose to free within split_huge_page()"), I very much suspect you do something similar to patch 2 as well. IIRC, that location is the best location to free subpages that only contain zeros because it covers multiple scenarios.
