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;
Subject: Re: [RFC PATCH 1/3] misc_cgroup: introduce misc.events and
 misc_events.local
To:     Vipin Sharma <vipinsh@google.com>,
        =?UTF-8?Q?Michal_Koutn=c3=bd?= <mkoutny@suse.com>
Cc:     tj@kernel.org, lizefan.x@bytedance.com, hannes@cmpxchg.org,
        linux-kernel@vger.kernel.org, cgroups@vger.kernel.org
References: <988f340462a1a3c62b7dc2c64ceb89a4c0a00552.1631077837.git.brookxu@tencent.com>
 <20210909143702.GA13761@blackbody.suse.cz>
 <CAHVum0eGN=v1kLqHQg7HBESp8Kg4aGZFNd4XTpxfeyToXPmPVg@mail.gmail.com>
From:   brookxu <brookxu.cn@gmail.com>
Message-ID: <8259b666-f3a4-6788-880c-38d679414bcb@gmail.com>
Date:   Fri, 10 Sep 2021 13:20:37 +0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0)
 Gecko/20100101 Thunderbird/78.13.0
MIME-Version: 1.0
In-Reply-To: <CAHVum0eGN=v1kLqHQg7HBESp8Kg4aGZFNd4XTpxfeyToXPmPVg@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk


Vipin Sharma wrote on 2021/9/10 1:08 上午:
> On Thu, Sep 9, 2021 at 7:37 AM Michal Koutný <mkoutny@suse.com> wrote:
>>
>> Hello Chunguang.
>>
>> The new version looks like a good step generally.
>>
>> My main remark is that I wouldn't make a distinct v1 and v2 interface,
>> it's a new controller so I think the v2 could be exposed in both cases
>> (or in other words, don't create new v1-specific features).
> 
> I agree with Michal. We can have the same interface for v1 otherwise
> there will not be any form of feedback in v1 for failures.

Yeah, this is more reasonable. But there is still one question, whether we
need to be consistent with other cgroup subsystems, events and events.local
under v1 should not support hierarchy？

>>
>> On Wed, Sep 08, 2021 at 01:24:34PM +0800, brookxu <brookxu.cn@gmail.com> wrote:
>>> +static int misc_events_show(struct seq_file *sf, void *v)
>>> +{
>>> +     struct misc_cg *cg = css_misc(seq_css(sf));
>>> +     unsigned long count, i;
>>> +
>>> +     for (i = 0; i < MISC_CG_RES_TYPES; i++) {
>>> +             count = atomic_long_read(&cg->events[i]);
>>> +             if (READ_ONCE(misc_res_capacity[i]) || count)
>>> +                     seq_printf(sf, "%s %lu\n", misc_res_name[i], count);
>>
>> More future-proof key would be
>>                         seq_printf(sf, "%s.max %lu\n", misc_res_name[i], count);
>> or
>>                         seq_printf(sf, "max.%s %lu\n", misc_res_name[i], count);
>>
>> (Which one is a judgement call but I'd include the "name" of event type too.)
>>
> I am inclined more towards "%s.max", it looks nice to see the resource
> name before its corresponding events

I also think %s.max may be more intuitive.