On Fri 16-10-20 12:48:23, Tianxianting wrote: > Thanks, my understanding is, > In shrink_slab(), do_shrink_slab() will do the real reclaim work, which will > occupy current cpu and consume more cpu time, so we need to trigger a > reschedule after reclaim. > But if it jumps to 'out' label, that means we don't do the reclaim work at > this time, it won't cause other thread getting starvation, so we don't need > to call cond_resched() in this case. > Is it right?
You are almost right. But consider situation when the lock is taken for quite some time. do_shrink_slab cannot make any forward progress and effectivelly busy loop. Unless the caller does cond_resched it might cause soft lockups. Anyway let me try to ask again. Why does would this be any problem that deserves a fix? > > -----Original Message----- > From: Michal Hocko [mailto:[email protected]] > Sent: Friday, October 16, 2020 8:08 PM > To: tianxianting (RD) <[email protected]> > Cc: [email protected]; [email protected]; > [email protected] > Subject: Re: [PATCH] mm: vmscan: avoid a unnecessary reschedule in > shrink_slab() > > On Fri 16-10-20 11:39:52, Xianting Tian wrote: > > In shrink_slab(), it directly goes to 'out' label only when it can't > > get the lock of shrinker_rwsew. In this case, it doesn't do the real > > work of shrinking slab, so we don't need trigger a reschedule by > > cond_resched(). > > Your changelog doesn't explain why this is not needed or undesirable. Do you > see any actual problem? > > The point of this code is to provide a deterministic scheduling point > regardless of the shrinker_rwsew. > > > > > Signed-off-by: Xianting Tian <[email protected]> > > --- > > mm/vmscan.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/mm/vmscan.c b/mm/vmscan.c index 466fc3144..676e97b28 > > 100644 > > --- a/mm/vmscan.c > > +++ b/mm/vmscan.c > > @@ -687,8 +687,9 @@ static unsigned long shrink_slab(gfp_t gfp_mask, int > > nid, > > } > > > > up_read(&shrinker_rwsem); > > -out: > > + > > cond_resched(); > > +out: > > return freed; > > } > > > > -- > > 2.17.1 > > > > -- > Michal Hocko > SUSE Labs -- Michal Hocko SUSE Labs
