linux-kernel - Re: [External] Re: [PATCH v2] mm: add new syscall pidfd_set

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: <87mt8qw0tr.fsf@yhuang6-desk2.ccr.corp.intel.com>
Date:   Thu, 17 Nov 2022 14:29:36 +0800
From:   "Huang, Ying" <ying.huang@...el.com>
To:     Michal Hocko <mhocko@...e.com>
Cc:     Zhongkun He <hezhongkun.hzk@...edance.com>, corbet@....net,
        akpm@...ux-foundation.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, linux-api@...r.kernel.org,
        linux-doc@...r.kernel.org
Subject: Re: [External] Re: [PATCH v2] mm: add new syscall
 pidfd_set_mempolicy().

Michal Hocko <mhocko@...e.com> writes:

> On Wed 16-11-22 17:38:09, Zhongkun He wrote:
>> Hi Ying, thanks for your replay and suggestions.
>> 
>> > 
>> > I suggest to move the flags in "mode" parameter (MPOL_F_STATIC_NODES,
>> > MPOL_F_RELATIVE_NODES, MPOL_F_NUMA_BALANCING, etc.) to "flags"
>> > parameter, otherwise, why add it?
>> 
>> The "flags" is used for future extension if any, just like
>> process_madvise() and set_mempolicy_home_node().
>> Maybe it should be removed.
>
> No, please! Even if there is no use for the flags now we are usually
> terrible at predicting future and potential extensions. MPOL_F* is kinda
> flags but for historical reasons it is a separate mode and we shouldn't
> create a new confusion when this is treated differently for pidfd based
> APIs.
>
>> > And, how about add a "home_node" parameter?  I don't think that it's a
>> > good idea to add another new syscall for pidfd_set_mempolicy_home_node()
>> > in the future.
>
> Why would this be a bad idea?
>
>> Good idea, but "home_node" is used for vma policy, not task policy.
>> It is possible to use it in pidfd_mbind() in the future.
>
> I woould go with pidfd_set_mempolicy_home_node to counterpart an
> existing syscall.

Yes.  I understand that it's important to make API consistent.

Just my two cents.

>From another point of view, the new API gives us an opportunity to fix
the issues in existing API too?  For example, moving all flags into
"flags" parameter, add another parameter "home_node"?  Maybe we can
switch to this new API in the future completely after finding a way to
indicate "current" task in "pidfd" parameter.

Best Regards,
Huang, Ying