| Message ID | 20201202182725.265020-3-shy828301@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Make shrinker's nr_deferred memcg aware | expand |
On 2020/12/3 2:27, Yang Shi wrote: > The tracepoint's nid should show what node the shrink happens on, the start tracepoint > uses nid from shrinkctl, but the nid might be set to 0 before end tracepoint if the > shrinker is not NUMA aware, so the traceing log may show the shrink happens on one > node but end up on the other node. It seems confusing. And the following patch > will remove using nid directly in do_shrink_slab(), this patch also helps cleanup > the code. > > Signed-off-by: Yang Shi <shy828301@gmail.com> > --- > mm/vmscan.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index 7d6186a07daf..457ce04eebf2 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -533,7 +533,7 @@ static unsigned long do_shrink_slab(struct shrink_control *shrinkctl, > new_nr = atomic_long_add_return(next_deferred, > &shrinker->nr_deferred[nid]); > > - trace_mm_shrink_slab_end(shrinker, nid, freed, nr, new_nr, total_scan); > + trace_mm_shrink_slab_end(shrinker, shrinkctl->nid, freed, nr, new_nr, total_scan); Hi， Yang When I read this patch, I wondered why you modified it so much until I read patch6. Could you merge this patch into patch6? Thanks! > return freed; > } >
On Wed, Dec 2, 2020 at 7:13 PM Xiaqing (A) <saberlily.xia@hisilicon.com> wrote: > > > > On 2020/12/3 2:27, Yang Shi wrote: > > The tracepoint's nid should show what node the shrink happens on, the start tracepoint > > uses nid from shrinkctl, but the nid might be set to 0 before end tracepoint if the > > shrinker is not NUMA aware, so the traceing log may show the shrink happens on one > > node but end up on the other node. It seems confusing. And the following patch > > will remove using nid directly in do_shrink_slab(), this patch also helps cleanup > > the code. > > > > Signed-off-by: Yang Shi <shy828301@gmail.com> > > --- > > mm/vmscan.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/mm/vmscan.c b/mm/vmscan.c > > index 7d6186a07daf..457ce04eebf2 100644 > > --- a/mm/vmscan.c > > +++ b/mm/vmscan.c > > @@ -533,7 +533,7 @@ static unsigned long do_shrink_slab(struct shrink_control *shrinkctl, > > new_nr = atomic_long_add_return(next_deferred, > > &shrinker->nr_deferred[nid]); > > > > - trace_mm_shrink_slab_end(shrinker, nid, freed, nr, new_nr, total_scan); > > + trace_mm_shrink_slab_end(shrinker, shrinkctl->nid, freed, nr, new_nr, total_scan); > > Hi， Yang > > When I read this patch, I wondered why you modified it so much until I read patch6. Could you merge > this patch into patch6? Sorry for the late reply. It could be, but I was inclined to think this is a bug and we might need backport it to stable, so I leave it as a standalone patch. > > Thanks! > > > return freed; > > } > > >
diff --git a/mm/vmscan.c b/mm/vmscan.c index 7d6186a07daf..457ce04eebf2 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -533,7 +533,7 @@ static unsigned long do_shrink_slab(struct shrink_control *shrinkctl, new_nr = atomic_long_add_return(next_deferred, &shrinker->nr_deferred[nid]); - trace_mm_shrink_slab_end(shrinker, nid, freed, nr, new_nr, total_scan); + trace_mm_shrink_slab_end(shrinker, shrinkctl->nid, freed, nr, new_nr, total_scan); return freed; }
The tracepoint's nid should show what node the shrink happens on, the start tracepoint uses nid from shrinkctl, but the nid might be set to 0 before end tracepoint if the shrinker is not NUMA aware, so the traceing log may show the shrink happens on one node but end up on the other node. It seems confusing. And the following patch will remove using nid directly in do_shrink_slab(), this patch also helps cleanup the code. Signed-off-by: Yang Shi <shy828301@gmail.com> --- mm/vmscan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)