lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 9 Nov 2017 10:40:41 +0900
From:   Minchan Kim <>
To:     Shakeel Butt <>
Cc:     Huang Ying <>,
        Mel Gorman <>,
        Vladimir Davydov <>,
        Michal Hocko <>,
        Greg Thelen <>,
        Johannes Weiner <>,
        Andrew Morton <>,
        Linux MM <>,
        LKML <>
Subject: Re: [PATCH v2] mm, shrinker: make shrinker_list lockless

On Wed, Nov 08, 2017 at 05:07:08PM -0800, Shakeel Butt wrote:
> On Wed, Nov 8, 2017 at 4:07 PM, Minchan Kim <> wrote:
> > Hi,
> >
> > On Wed, Nov 08, 2017 at 09:37:40AM -0800, Shakeel Butt wrote:
> >> In our production, we have observed that the job loader gets stuck for
> >> 10s of seconds while doing mount operation. It turns out that it was
> >> stuck in register_shrinker() and some unrelated job was under memory
> >> pressure and spending time in shrink_slab(). Our machines have a lot
> >> of shrinkers registered and jobs under memory pressure has to traverse
> >> all of those memcg-aware shrinkers and do affect unrelated jobs which
> >> want to register their own shrinkers.
> >>
> >> This patch has made the shrinker_list traversal lockless and shrinker
> >> register remain fast. For the shrinker unregister, atomic counter
> >> has been introduced to avoid synchronize_rcu() call. The fields of
> >
> > So, do you want to enhance unregister shrinker path as well as registering?
> >
> Yes, I don't want to add delay to unregister_shrinker for the normal
> case where there isn't any readers (i.e. unconditional
> synchronize_rcu).

Not sure how it makes bad.
It would be better to add opinion in description about why unregister path is
important and how synchronize_rcu might makeA slow for usual cases.

> >> struct shrinker has been rearraged to make sure that the size does
> >> not increase for x86_64.
> >>
> >> The shrinker functions are allowed to reschedule() and thus can not
> >> be called with rcu read lock. One way to resolve that is to use
> >> srcu read lock but then ifdefs has to be used as SRCU is behind
> >> CONFIG_SRCU. Another way is to just release the rcu read lock before
> >> calling the shrinker and reacquire on the return. The atomic counter
> >> will make sure that the shrinker entry will not be freed under us.
> >
> > Instead of adding new lock, could we simply release shrinker_rwsem read-side
> > lock in list traveral periodically to give a chance to hold a write-side
> > lock?
> >
> Greg has already pointed out that this patch is still not right/safe
> and now I am getting to the opinion that without changing the shrinker
> API, it might not be possible to do lockless shrinker traversal and
> unregister shrinker without synchronize_rcu().
> Regarding your suggestion, do you mean to add periodic release lock
> and reacquire using down_read_trylock() or down_read()?

Yub with down_read. Actually, I do not see point of down_read_trylock
when considering write-lock path in reigster_shinker is too short.

The problem in suggested approach is we should traverse list from the
beginning again after reacquiring, which breaks fairness of each

Maybe, we can introduce rwlock_is_contended which checks wait_list
and returns true if wait_list is not empty.


> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to  For more info on Linux MM,
> see: .
> Don't email: <a href=mailto:""> </a>

Powered by blists - more mailing lists