linux-kernel - Re: [RFC PATCH 1/3] misc_cgroup: introduce misc.events and misc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8259b666-f3a4-6788-880c-38d679414bcb@gmail.com>
Date:   Fri, 10 Sep 2021 13:20:37 +0800
From:   brookxu <brookxu.cn@...il.com>
To:     Vipin Sharma <vipinsh@...gle.com>,
        Michal Koutný <mkoutny@...e.com>
Cc:     tj@...nel.org, lizefan.x@...edance.com, hannes@...xchg.org,
        linux-kernel@...r.kernel.org, cgroups@...r.kernel.org
Subject: Re: [RFC PATCH 1/3] misc_cgroup: introduce misc.events and
 misc_events.local



Vipin Sharma wrote on 2021/9/10 1:08 上午:
> On Thu, Sep 9, 2021 at 7:37 AM Michal Koutný <mkoutny@...e.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@...il.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.