From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755600AbdGSRJo convert rfc822-to-8bit (ORCPT ); Wed, 19 Jul 2017 13:09:44 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37818 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753729AbdGSRJl (ORCPT ); Wed, 19 Jul 2017 13:09:41 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com CEDED7EBD3 Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=longman@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com CEDED7EBD3 Subject: Re: [PATCH 5/6] cgroup: implement cgroup v2 thread support To: Tejun Heo Cc: Peter Zijlstra , lizefan@huawei.com, hannes@cmpxchg.org, mingo@redhat.com, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@fb.com, pjt@google.com, luto@amacapital.net, efault@gmx.de, torvalds@linux-foundation.org, guro@fb.com References: <20170717020721.3612468-1-tj@kernel.org> <20170717020721.3612468-6-tj@kernel.org> <20170717141409.sqafufjupsiffnri@hirez.programming.kicks-ass.net> <825f2eaa-568a-a156-f04d-9235b20e2614@redhat.com> <5fa00590-551f-85d2-511b-ef4033781228@redhat.com> <20170718171034.GE585283@devbig577.frc2.facebook.com> <20170719162937.GP3365493@devbig577.frc2.facebook.com> From: Waiman Long Organization: Red Hat Message-ID: Date: Wed, 19 Jul 2017 13:09:38 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 MIME-Version: 1.0 In-Reply-To: <20170719162937.GP3365493@devbig577.frc2.facebook.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Content-Language: en-US X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Wed, 19 Jul 2017 17:09:41 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/19/2017 12:29 PM, Tejun Heo wrote: > Hello, > > On Tue, Jul 18, 2017 at 01:23:14PM -0400, Waiman Long wrote: >>> If we could get rid of the invalid state completely that way, I'd >>> completely agree with you but that isn't the case here as you noted >>> yourself, so the choice between the two isn't something trivially >>> clear. Both choices come with their pros and cons. We can absoultely >>> discuss them comparing the pros and cons. >> I am not advocating on removing the invalid state now as I note about > Yeah, removing invalid state would be great but we can't at least yet. > >> sibling cgroups. I am just saying that there is no point in not doing an >> automatic conversion to threaded for newly created children of threaded >> cgroups (not thread root). I don't see any cons in doing that. > So, the cons I see is inconsistency, now and in the future. > > This may seem less clear with system root because we can have both > domain and theraded children below it, which makes a newly created > cgroup being a domain seem natural. More importantly, we can't do it > any other way because we'd break existing users otherwise - creating a > threaded cgroup would cause future first level cgroups to be threaded > which will be very unexpected. > > Let's think about a non-root threaded domain. At least for now, a > non-root threaded domain is terminal - they can't host valid domain > children. As the alternative term "thread root" implies, the threaded > domain can be the root of a threaded subtree and nothing else, so it's > kinda weird to make a new child cgroup there start out as a domain > which can't be used, just like it'd be for the second level descendant > cgroup. > > However, the alternative is even stranger. Let's say we make the > first level child automatically threaded, but that is inconsistent > with when we first enable threaded mode. We either would have to turn > all siblings at the same time or disallow enabling threaded mode if > there are domain siblings, which I fear would be unnecessarily > restrictive. > > Another point is that what if we eventually make non-root threaded > roots able to host domain children? Making children automatically > threaded wouldn't make any sense then, right? I'll come back to this > later. > > So, it looks like if we're gonna automatically turn on threaded mode > for new cgroups, the only thing we can do right now is what you're > suggesting; however, we didn't arrive there through some > straight-forward intuition or overall design. It started as a simple > idea (I want it to be automatic) but the end result is a contorted > destination shaped by constraints and happenstance. > > To me, behaving differently on the first-level threaded children than > on second+ level ones is too strange to be justified by the > convenience of not having to turn on threaded on new cgroups. OK, I get your point of being inconsistent. However, I don't think that is a big deal. > On top of that, what happens if we get to implement PeterZ's idea of > skipping over threaded internal cgroups to allow domains under > threaded subtrees? That'd imply that we'd be able to host domains > under threaded domains too. The end result would be completely > non-sensical. We'd be defaulting to different modes for different > reasons where half of those reasons won't hold anymore. This isn't > surprising given that there's nothing actually consistent about the > suggested default behavior. For me, that is the only good reason why we should keep the current behavior. So I am fine with that. + cgrp->dom_cgrp = cgrp->dom_cgrp; However, I am still puzzled by above line of code, should it be just cgrp->dom_cgrp = cgrp; Cheers, Longman