All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shakeel Butt <shakeelb@google.com>
To: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Minchan Kim <minchan@kernel.org>,
	Huang Ying <ying.huang@intel.com>,
	Mel Gorman <mgorman@techsingularity.net>,
	Vladimir Davydov <vdavydov.dev@gmail.com>,
	Michal Hocko <mhocko@kernel.org>,
	Greg Thelen <gthelen@google.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux MM <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] mm, shrinker: make shrinker_list lockless
Date: Fri, 10 Nov 2017 10:16:14 -0800	[thread overview]
Message-ID: <CALvZod4HAVpmPyE7k_Brme=mzic8SDO8gyAFtDhW9r0oBurN4w@mail.gmail.com> (raw)
In-Reply-To: <201711100646.IJH39597.HOtMLJVSFOQFOF@I-love.SAKURA.ne.jp>

On Thu, Nov 9, 2017 at 1:46 PM, Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
> Shakeel Butt wrote:
>> > If you can accept serialized register_shrinker()/unregister_shrinker(),
>> > I think that something like shown below can do it.
>>
>> If we assume that we will never do register_shrinker and
>> unregister_shrinker on the same object in parallel then do we still
>> need to do msleep & synchronize_rcu() within mutex?
>
> Doing register_shrinker() and unregister_shrinker() on the same object
> in parallel is wrong. This mutex is to ensure that we do not need to
> worry about ->list.next field. synchronize_rcu() should not be slow.
> If you want to avoid msleep() with mutex held, you can also apply
>
>> > If you want parallel register_shrinker()/unregister_shrinker(), something like
>> > shown below on top of shown above will do it.
>
> change.

Thanks for the explanation. Can you post the patch for others to
review without parallel register/unregister and SHRINKER_PERMANENT (we
can add when we need them)? You can use the motivation for the patch I
mentioned in my patch instead.

WARNING: multiple messages have this Message-ID (diff)
From: Shakeel Butt <shakeelb@google.com>
To: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Minchan Kim <minchan@kernel.org>,
	Huang Ying <ying.huang@intel.com>,
	Mel Gorman <mgorman@techsingularity.net>,
	Vladimir Davydov <vdavydov.dev@gmail.com>,
	Michal Hocko <mhocko@kernel.org>,
	Greg Thelen <gthelen@google.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux MM <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] mm, shrinker: make shrinker_list lockless
Date: Fri, 10 Nov 2017 10:16:14 -0800	[thread overview]
Message-ID: <CALvZod4HAVpmPyE7k_Brme=mzic8SDO8gyAFtDhW9r0oBurN4w@mail.gmail.com> (raw)
In-Reply-To: <201711100646.IJH39597.HOtMLJVSFOQFOF@I-love.SAKURA.ne.jp>

On Thu, Nov 9, 2017 at 1:46 PM, Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
> Shakeel Butt wrote:
>> > If you can accept serialized register_shrinker()/unregister_shrinker(),
>> > I think that something like shown below can do it.
>>
>> If we assume that we will never do register_shrinker and
>> unregister_shrinker on the same object in parallel then do we still
>> need to do msleep & synchronize_rcu() within mutex?
>
> Doing register_shrinker() and unregister_shrinker() on the same object
> in parallel is wrong. This mutex is to ensure that we do not need to
> worry about ->list.next field. synchronize_rcu() should not be slow.
> If you want to avoid msleep() with mutex held, you can also apply
>
>> > If you want parallel register_shrinker()/unregister_shrinker(), something like
>> > shown below on top of shown above will do it.
>
> change.

Thanks for the explanation. Can you post the patch for others to
review without parallel register/unregister and SHRINKER_PERMANENT (we
can add when we need them)? You can use the motivation for the patch I
mentioned in my patch instead.

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

  reply	other threads:[~2017-11-10 18:16 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-08 17:37 [PATCH v2] mm, shrinker: make shrinker_list lockless Shakeel Butt
2017-11-08 17:37 ` Shakeel Butt
2017-11-08 17:58 ` Greg Thelen
2017-11-08 17:58   ` Greg Thelen
2017-11-09  0:07 ` Minchan Kim
2017-11-09  0:07   ` Minchan Kim
2017-11-09  1:07   ` Shakeel Butt
2017-11-09  1:07     ` Shakeel Butt
2017-11-09  1:40     ` Minchan Kim
2017-11-09  1:40       ` Minchan Kim
2017-11-09 10:26 ` Tetsuo Handa
2017-11-09 10:26   ` Tetsuo Handa
2017-11-09 15:34   ` Shakeel Butt
2017-11-09 15:34     ` Shakeel Butt
2017-11-09 21:46     ` Tetsuo Handa
2017-11-09 21:46       ` Tetsuo Handa
2017-11-10 18:16       ` Shakeel Butt [this message]
2017-11-10 18:16         ` Shakeel Butt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CALvZod4HAVpmPyE7k_Brme=mzic8SDO8gyAFtDhW9r0oBurN4w@mail.gmail.com' \
    --to=shakeelb@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=gthelen@google.com \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@techsingularity.net \
    --cc=mhocko@kernel.org \
    --cc=minchan@kernel.org \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=vdavydov.dev@gmail.com \
    --cc=ying.huang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.