Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 29 Oct 2020 10:05:23 -0700
From:   Roman Gushchin <guro@fb.com>
To:     Michal =?iso-8859-1?Q?Koutn=FD?= <mkoutny@suse.com>
CC:     Andrew Morton <akpm@linux-foundation.org>,
        Shakeel Butt <shakeelb@google.com>,
        Johannes Weiner <hannes@cmpxchg.org>,
        Michal Hocko <mhocko@kernel.org>,
        <linux-kernel@vger.kernel.org>, <linux-mm@kvack.org>,
        <kernel-team@fb.com>, <ltp@lists.linux.it>,
        Richard Palethorpe <rpalethorpe@suse.com>,
        <stable@vger.kernel.org>
Subject: Re: [PATCH v2] mm: memcg: link page counters to root if
 use_hierarchy is false
Message-ID: <20201029170523.GH827280@carbon.dhcp.thefacebook.com>
References: <20201026231326.3212225-1-guro@fb.com>
 <20201029153921.GC9928@blackbook>
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20201029153921.GC9928@blackbook>
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
Received: from carbon.dhcp.thefacebook.com (2620:10d:c090:400::4:cadf) by MW2PR16CA0055.namprd16.prod.outlook.com (2603:10b6:907:1::32) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3499.18 via Frontend Transport; Thu, 29 Oct 2020 17:05:26 +0000
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id: 78cfff62-a227-4a7d-5bb3-08d87c2cd5b7
X-MS-TrafficTypeDiagnostic: BYAPR15MB2392:
X-MS-Exchange-Transport-Forked: True
X-Microsoft-Antispam-PRVS: <BYAPR15MB2392E3D46DB699D89C2C745ABE140@BYAPR15MB2392.namprd15.prod.outlook.com>
X-FB-Source: Internal
X-MS-Oob-TLC-OOBClassifiers: OLM:8882;
X-MS-Exchange-SenderADCheck: 1
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: dG9bHpeH+vmT5b48hxAtjnmYSAg65ZhqDTaGJDjJAZU/7GjwGZ5FGI+Afm0oduMylgdB5AJ5X9nXsx6mkEPaNxF67VcNp8ME8HK+Uteud9mWm2eQ/3aZ6gpjU2fXVZHZRTZ/Z+BZkOiXOmQZJsEyyQoZwAuBfQvHaI3ed0yTKTitosEAywLu3neMsm+zL1T5S0GbCwJzw7vVYMg/eKLAppdSBctQLQaTiGHv1IP2NY19vu+W+ExXl98u3x2K7onBOqoswX7PexT3/x4Scep5fHJM7u19QMESpzeo95tW4CmvinZXxSExNZ6xs/4g+EjiIyxta/VmwFCUW+p2zqGH5g==
X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BYAPR15MB4136.namprd15.prod.outlook.com;PTR:;CAT:NONE;SFS:(39860400002)(346002)(376002)(366004)(136003)(396003)(6916009)(7416002)(8676002)(8936002)(2906002)(9686003)(4326008)(33656002)(6666004)(86362001)(66556008)(66476007)(66946007)(478600001)(16526019)(52116002)(7696005)(6506007)(186003)(316002)(1076003)(83380400001)(55016002)(5660300002)(54906003);DIR:OUT;SFP:1102;
X-MS-Exchange-AntiSpam-MessageData: ikqcKQhk6EcI4lZFYdMrLWFQVa7LMtDpMeZoUaGkWthwFMm8ryPejsIqJBHwJwN31wZCqSszacyROuW3syMfOzYf57jyvFpV2anpKjBN4F9Ku8uJEqzNjlA3Eyez9glSdSztLHE0S+9NmvUuAKTAYHSY5LB6Hl/lZDd86jrPk4qwBo/k8hhTxvLXNnZgxQkhQf51MsNMHEd77c5AeV/qXTGG5ikPHEPIWVhKfypeV9EYQRoZhB8VkJv6vxM+nFvNOF8qs06fbdyBRYVO3KoyhtfvZI3c4RPy2HjQWQ9XPaAbePZOVSrpChUY5WHyhg2CsjU9MkieTemdadpVmi6JLcRgW4CGoejDJYXoK8SfnyqIH+TvDYn2sUm9sq8xNYrztakbh/GTojIzFYz5J1Ve+SAgl+RdVWqjq7JqIdZVKSqm3X1gKq2fZ/VPKjdW6vNJfB5CZ6OwOIYQeFQqwkgupRg6tZvktxQr5b46snWFs27YNwmU13Si7ZiMcbTh2MDXOIs+GMalWAO+rUSSYlzCFmHXN8KRtt/qKpIrh8LnRru++qG/iZacM5pIGJF+X39IAozKzP0Bc8KJY42PvRKg8Z4XcmUmbqecMEMrQWWtclySe8OhtneBGDh3FWjXgnB1Yp6we24VJhy4IGCmvB/K7dt3TmJ5nd9GnIJrK6ltZL4=
X-MS-Exchange-CrossTenant-Network-Message-Id: 78cfff62-a227-4a7d-5bb3-08d87c2cd5b7
X-MS-Exchange-CrossTenant-AuthSource: BYAPR15MB4136.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Oct 2020 17:05:28.2082
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: TEiM4LaX6UBTry0FXvnTNR95CnS4gYXM84lBKGTQmdVJYDCpL5lNKUl4u5XcVO11
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR15MB2392
X-OriginatorOrg: fb.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.312,18.0.737
 definitions=2020-10-29_11:2020-10-29,2020-10-29 signatures=0
X-Proofpoint-Spam-Details: rule=fb_default_notspam policy=fb_default score=0 clxscore=1011 mlxscore=0
 adultscore=0 bulkscore=0 lowpriorityscore=0 mlxlogscore=999 spamscore=0
 malwarescore=0 phishscore=0 priorityscore=1501 suspectscore=1
 impostorscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2009150000 definitions=main-2010290118
X-FB-Internal: deliver
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Oct 29, 2020 at 04:39:21PM +0100, Michal Koutny wrote:
> Hi.
> 
> On Mon, Oct 26, 2020 at 04:13:26PM -0700, Roman Gushchin <guro@fb.com> wrote:
> > Please note, that in the non-hierarchical mode all objcgs are always
> > reparented to the root memory cgroup, even if the hierarchy has more
> > than 1 level. This patch doesn't change it.
> > 
> > The patch also doesn't affect how the hierarchical mode is working,
> > which is the only sane and truly supported mode now.
> I agree with the patch and you can add
> Reviewed-by: Michal Koutn? <mkoutny@suse.com>
> 
> However, it effectively switches any users of root.use_hierarchy=0 (if there
> are any, watching the counters of root memcg) into root.use_hierarchy=1.
> So I'd show them the warning even with a single level of cgroups, i.e.
> add this hunk

It's only partially true. The main difference between the hierarchical and
non-hierarchical mode on the following simple example

    /
    |
    A
   / \
  B   C

is whether A's memory limits are applied to B, and this is not gonna change.
However you're right, it will change some root cgroup's numbers.

> 
> @@ -5356,12 +5356,11 @@
>  		page_counter_init(&memcg->kmem, &root_mem_cgroup->kmem);
>  		page_counter_init(&memcg->tcpmem, &root_mem_cgroup->tcpmem);
>  		/*
> -		 * Deeper hierachy with use_hierarchy == false doesn't make
> +		 * Hierachy with use_hierarchy == false doesn't make
>  		 * much sense so let cgroup subsystem know about this
>  		 * unfortunate state in our controller.
>  		 */
> -		if (parent != root_mem_cgroup)
> -			memory_cgrp_subsys.broken_hierarchy = true;
> +		memory_cgrp_subsys.broken_hierarchy = true;
>  	}
>  
>  	/* The following stuff does not apply to the root */
> 
> What do you think?

I think it's in a good direction of deprecating the non-hierarchical mode.
Shakeel did propose it too.

I'd also change the displayed message to something similar to we print
for kmem.limit_in_bytes:
    pr_warn_once("kmem.limit_in_bytes is deprecated and will be removed. "
    		 "Please report your usecase to linux-mm@kvack.org if you "
		 "depend on this functionality.\n");

Thanks!