On Sun 18-08-13 23:05:25, Hugh Dickins wrote:
> Now that everybody loves memcg, configures it on, and would not dream
> of booting with cgroup_disable=memory, it can pass unnoticed for weeks
> that memcg-less page reclaim is completely broken.
>
> mmotm's "memcg: enhance memcg iterator to support predicates" replaces
> __shrink_zone()'s "do { } while (memcg);" loop by a "while (memcg) {}"
> loop: which is nicer for memcg, but does nothing for !CONFIG_MEMCG or
> cgroup_disable=memory. Page reclaim hangs, making no progress.
Ouch. Very well spotted, Hugh! I have totally missed this...
> Adding mem_cgroup_disabled() and once++ test there is ugly. Ideally,
> even a !CONFIG_MEMCG build might in future have a stub root_mem_cgroup,
> which would get around this: but that's not so at present.
>
> However, it appears that nothing actually dereferences the memcg pointer
> in the mem_cgroup_disabled() case, here or anywhere else that case can
> reach mem_cgroup_iter() (mem_cgroup_iter_break() is not called in
> global reclaim).
>
> So, simply pass back an ordinarily-oopsing non-NULL address the first
> time, and we shall hear about it if I'm wrong.
This is a bit tricky but it seems like the easiest way for now. I will
look at the fake root cgroup for !CONFIG_MEMCG.
> Signed-off-by: Hugh Dickins <hughd@xxxxxxxxxx>
Acked-by: Michal Hocko <mhocko@xxxxxxx>
Thanks!
> ---
> By all means fold in to
> memcg-enhance-memcg-iterator-to-support-predicates.patch
>
> include/linux/memcontrol.h | 3 ++-
> mm/memcontrol.c | 6 ++++--
> 2 files changed, 6 insertions(+), 3 deletions(-)
>
> --- 3.11-rc5-mm1/include/linux/memcontrol.h 2013-08-15 18:10:50.504539510 -0700
> +++ linux/include/linux/memcontrol.h 2013-08-18 12:30:58.116460318 -0700
> @@ -370,7 +370,8 @@ mem_cgroup_iter_cond(struct mem_cgroup *
> struct mem_cgroup_reclaim_cookie *reclaim,
> mem_cgroup_iter_filter cond)
> {
> - return NULL;
> + /* first call must return non-NULL, second return NULL */
> + return (struct mem_cgroup *)(unsigned long)!prev;
> }
>
> static inline struct mem_cgroup *
> --- 3.11-rc5-mm1/mm/memcontrol.c 2013-08-15 18:10:50.720539516 -0700
> +++ linux/mm/memcontrol.c 2013-08-18 12:29:15.352460818 -0700
> @@ -1086,8 +1086,10 @@ struct mem_cgroup *mem_cgroup_iter_cond(
> struct mem_cgroup *memcg = NULL;
> struct mem_cgroup *last_visited = NULL;
>
> - if (mem_cgroup_disabled())
> - return NULL;
> + if (mem_cgroup_disabled()) {
> + /* first call must return non-NULL, second return NULL */
> + return (struct mem_cgroup *)(unsigned long)!prev;
> + }
>
> if (!root)
> root = root_mem_cgroup;
--
Michal Hocko
SUSE Labs
--
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>