On Sat, Mar 11, 2017 at 5:51 AM, Yisheng Xie <ysxie@xxxxxxxxxxx> wrote:
> From: Yisheng Xie <xieyisheng1@xxxxxxxxxx>
>
> When we enter do_try_to_free_pages, the may_thrash is always clear, and
> it will retry shrink zones to tap cgroup's reserves memory by setting
> may_thrash when the former shrink_zones reclaim nothing.
>
> However, when memcg is disabled or on legacy hierarchy, it should not do
> this useless retry at all, for we do not have any cgroup's reserves
> memory to tap, and we have already done hard work but made no progress.
>
> To avoid this time costly and useless retrying, add a stub function
> may_thrash and return true when memcg is disabled or on legacy
> hierarchy.
>
> Signed-off-by: Yisheng Xie <xieyisheng1@xxxxxxxxxx>
> Suggested-by: Shakeel Butt <shakeelb@xxxxxxxxxx>
> ---
> v2:
> - more restrictive condition for retry of shrink_zones (restricting
> cgroup_disabled=memory boot option and cgroup legacy hierarchy) - Shakeel
>
> - add a stub function may_thrash() to avoid compile error or warning.
>
> - rename subject from "donot retry shrink zones when memcg is disable"
> to "more restrictive condition for retry in do_try_to_free_pages"
>
> Any comment is more than welcome!
>
> Thanks
> Yisheng Xie
>
> mm/vmscan.c | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index bc8031e..415f800 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -184,6 +184,19 @@ static bool sane_reclaim(struct scan_control *sc)
> #endif
> return false;
> }
> +
> +static bool may_thrash(struct scan_control *sc)
> +{
> + /*
> + * When memcg is disabled or on legacy hierarchy, there is no cgroup
> + * reserves memory to tap.
> + */
> + if (!cgroup_subsys_enabled(memory_cgrp_subsys) ||
> + !cgroup_subsys_on_dfl(memory_cgrp_subsys))
> + return true;
> +
> + return sc->may_thrash;
> +}
> #else
> static bool global_reclaim(struct scan_control *sc)
> {
> @@ -194,6 +207,11 @@ static bool sane_reclaim(struct scan_control *sc)
> {
> return true;
> }
> +
> +static bool may_thrash(struct scan_control *sc)
> +{
> + return true;
> +}
> #endif
>
> /*
> @@ -2808,7 +2826,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> return 1;
>
> /* Untapped cgroup reserves? Don't OOM, retry. */
> - if (!sc->may_thrash) {
> + if (!may_thrash(sc)) {
Thanks Yisheng. The name of the function may_thrash() is confusing in
the sense that it is returning exactly the opposite of what its name
implies. How about reversing the condition of may_thrash() function
and change the scan_control's field may_thrash to thrashed?
> sc->priority = initial_priority;
> sc->may_thrash = 1;
> goto retry;
> --
> 1.9.1
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
