On Thu, Nov 30, 2023 at 6:14 PM Huan Yang <11133793@xxxxxxxx> wrote:
>
>
> 在 2023/12/1 10:05, Yosry Ahmed 写道:
> >> @@ -2327,7 +2330,8 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,
> >> struct pglist_data *pgdat = lruvec_pgdat(lruvec);
> >> struct mem_cgroup *memcg = lruvec_memcg(lruvec);
> >> unsigned long anon_cost, file_cost, total_cost;
> >> - int swappiness = mem_cgroup_swappiness(memcg);
> >> + int swappiness = sc->swappiness ?
> >> + *sc->swappiness : mem_cgroup_swappiness(memcg);
> >>
> >> Should we use "unlikely" here to indicate that sc->swappiness is an unexpected behavior?
> >> Due to current use case only apply in proactive reclaim.
> > On a system that is not under memory pressure, the rate of proactive
> > reclaim could be higher than reactive reclaim. We should only use
> > likely/unlikely when it's obvious a scenario will happen most of the
> > time. I don't believe that's the case here.
> Not all vendors will use proactive interfaces, and reactive reclaim are
> a normal
> system behavior. In this regard, I think it is appropriate to add
> "unlikely".
The general guidance is not to use likely/unlikely when it's not
certain, which I believe is the case here. I think the CPU will make
better decisions on its own than if we give it hints that's wrong in
some situations. Others please correct me if I am wrong.
> >
> >> u64 fraction[ANON_AND_FILE];
> >> u64 denominator = 0; /* gcc */
> >> enum scan_balance scan_balance;
> >> @@ -2608,6 +2612,9 @@ static int get_swappiness(struct lruvec *lruvec, struct scan_control *sc)
> >> mem_cgroup_get_nr_swap_pages(memcg) < MIN_LRU_BATCH)
> >> return 0;
> >>
> >> + if (sc->swappiness)
> >> + return *sc->swappiness;
> >>
> >> Also there.
> >>
> >> +
> >> return mem_cgroup_swappiness(memcg);
> >> }
> >>
> >> @@ -6433,7 +6440,8 @@ unsigned long mem_cgroup_shrink_node(struct mem_cgroup *memcg,
> >> unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *memcg,
> >> unsigned long nr_pages,
> >> gfp_t gfp_mask,
> >> - unsigned int reclaim_options)
> >> + unsigned int reclaim_options,
> >> + int *swappiness)
> >> {
> >> unsigned long nr_reclaimed;
> >> unsigned int noreclaim_flag;
> >> @@ -6448,6 +6456,7 @@ unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *memcg,
> >> .may_unmap = 1,
> >> .may_swap = !!(reclaim_options & MEMCG_RECLAIM_MAY_SWAP),
> >> .proactive = !!(reclaim_options & MEMCG_RECLAIM_PROACTIVE),
> >> + .swappiness = swappiness,
> >> };
> >> /*
> >> * Traverse the ZONELIST_FALLBACK zonelist of the current node to put
> >> --
> >> 2.34.1
> >>
> >> My previous patch attempted to ensure fully deterministic semantics under extreme swappiness.
> >> For example, when swappiness is set to 200, only anonymous pages will be reclaimed.
> >> Due to code in MGLRU isolate_folios will try scan anon if no scanned, will try other type.(We do not want
> >> it to attempt this behavior.)
> >> How do you think about extreme swappiness scenarios?
> > I think having different semantics between swappiness passed to
> > proactive reclaim and global swappiness can be confusing. If it's
> > needed to have a swappiness value that says "anon only no matter
> > what", perhaps we should introduce such a new value and make it
> > supported by both global and proactive reclaim swappiness? We could
> > support writing "max" or something similar instead of a special value
> > to mean that.
>
> Yes, use other hint more suitable for this scenario.
>
> However, from this patch, it seems that this feature is not supported.
> Do you have a demand for this scenario?
We do anonymous-only proactive reclaim in some setups, so it would be
nice to have. I am not sure if it's absolutely needed vs. just using
swappiness=200 and living with the possibility of reclaiming some file
pages.
