linux-api.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michal Hocko <mhocko@suse.com>
To: Gregory Price <gregory.price@memverge.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Gregory Price <gourry.memverge@gmail.com>,
	linux-mm@kvack.org, linux-doc@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-api@vger.kernel.org,
	linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org,
	arnd@arndb.de, tglx@linutronix.de, luto@kernel.org,
	mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
	x86@kernel.org, hpa@zytor.com, tj@kernel.org,
	ying.huang@intel.com
Subject: Re: [RFC PATCH 00/11] mm/mempolicy: Make task->mempolicy externally modifiable via syscall and procfs
Date: Mon, 27 Nov 2023 16:29:56 +0100	[thread overview]
Message-ID: <ZWS19JFHm_LFSsFd@tiehlicka> (raw)
In-Reply-To: <ZV5/ilfUoqC2PW0D@memverge.com>

Sorry, didn't have much time to do a proper review. Couple of points
here at least.

On Wed 22-11-23 17:24:10, Gregory Price wrote:
> On Wed, Nov 22, 2023 at 01:33:48PM -0800, Andrew Morton wrote:
> > On Wed, 22 Nov 2023 16:11:49 -0500 Gregory Price <gourry.memverge@gmail.com> wrote:
> > 
> > > The patch set changes task->mempolicy to be modifiable by tasks other
> > > than just current.
> > > 
> > > The ultimate goal is to make mempolicy more flexible and extensible,
> > > such as adding interleave weights (which may need to change at runtime
> > > due to hotplug events).  Making mempolicy externally modifiable allows
> > > for userland daemons to make runtime performance adjustments to running
> > > tasks without that software needing to be made numa-aware.
> > 
> > Please add to this [0/N] a full description of the security aspect: who
> > can modify whose mempolicy, along with a full description of the
> > reasoning behind this decision.
> > 
> 
> Will do. For the sake of v0 for now:
> 
> 1) the task itself (task == current)
>    for obvious reasons: it already can
> 
> 2) from external interfaces: CAP_SYS_NICE

Makes sense.

[...]
> > > 3. Add external interfaces which allow for a task mempolicy to be
> > >    modified by another task.  This is implemented in 4 syscalls
> > >    and a procfs interface:
> > >         sys_set_task_mempolicy
> > >         sys_get_task_mempolicy
> > >         sys_set_task_mempolicy_home_node
> > >         sys_task_mbind
> > >         /proc/[pid]/mempolicy
> > 
> > Why is the procfs interface needed?  Doesn't it simply duplicate the
> > syscall interface?  Please update [0/N] with a description of this
> > decision.
> > 
> 
> Honestly I wrote the procfs interface first, and then came back around
> to just implement the syscalls.  mbind is not friendly to being procfs'd
> so if the preference is to have only one, not both, then it should
> probably be the syscalls.
> 
> That said, when I introduce weighted interleave on top of this, having a
> simple procfs interface to those weights would be valuable, so I
> imagined something like `proc/mempolicy` to determine if interleave was
> being used and something like `proc/mpol_interleave_weights` for a clean
> interface to update weights.
> 
> However, in the same breath, I have a prior RFC with set/get_mempolicy2
> which could probably take all future mempolicy extensions and wrap them
> up into one pair of syscalls, instead of us ending up with 200 more
> sys_mempolicy_whatever as memory attached fabrics become more common.
> 
> So... yeah... the is one area I think the community very much needs to
> comment:  set/get_mempolicy2, many new mempolicy syscalls, procfs? All
> of the above?

I think we should actively avoid using proc interface. The most
reasonable way would be to add get_mempolicy2 interface that would allow
extensions and then create a pidfd counterpart to allow acting on a
remote task. The latter would require some changes to make mempolicy
code less current oriented.
-- 
Michal Hocko
SUSE Labs

  reply	other threads:[~2023-11-27 15:29 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-22 21:11 [RFC PATCH 00/11] mm/mempolicy: Make task->mempolicy externally modifiable via syscall and procfs Gregory Price
2023-11-22 21:11 ` [RFC PATCH 01/11] mm/mempolicy: refactor do_set_mempolicy for code re-use Gregory Price
2023-11-22 21:11 ` [RFC PATCH 02/11] mm/mempolicy: swap cond reference counting logic in do_get_mempolicy Gregory Price
2023-11-28 14:07   ` Michal Hocko
     [not found]     ` <ZWX0ytAwmOdooHdZ@memverge.com>
2023-11-28 14:28       ` Michal Hocko
2023-11-22 21:11 ` [RFC PATCH 03/11] mm/mempolicy: refactor set_mempolicy stack to take a task argument Gregory Price
2023-11-22 21:11 ` [RFC PATCH 04/11] mm/mempolicy: modify get_mempolicy call " Gregory Price
2023-11-28 14:07   ` Michal Hocko
     [not found]     ` <ZWX1U1gCTXC+lFXn@memverge.com>
2023-11-28 14:49       ` Michal Hocko
2023-11-22 21:11 ` [RFC PATCH 05/11] mm/mempolicy: modify set_mempolicy_home_node " Gregory Price
2023-11-28 14:07   ` Michal Hocko
2023-11-28 14:14     ` Gregory Price
2023-11-22 21:11 ` [RFC PATCH 06/11] mm/mempolicy: modify do_mbind to operate on task argument instead of current Gregory Price
2023-11-28 14:11   ` Michal Hocko
2023-11-28 14:51     ` Gregory Price
2023-11-28 18:08     ` Gregory Price
2023-11-22 21:11 ` [RFC PATCH 07/11] mm/mempolicy: add task mempolicy syscall variants Gregory Price
2023-11-22 21:11 ` [RFC PATCH 08/11] mm/mempolicy: export replace_mempolicy for use by procfs Gregory Price
2023-11-22 21:11 ` [RFC PATCH 09/11] mm/mempolicy: build mpol_parse_str unconditionally Gregory Price
2023-11-22 21:11 ` [RFC PATCH 10/11] mm/mempolicy: mpol_parse_str should ignore trailing characters in nodelist Gregory Price
2023-11-22 21:12 ` [RFC PATCH 11/11] fs/proc: Add mempolicy attribute to allow read/write of task mempolicy Gregory Price
2023-11-22 21:33 ` [RFC PATCH 00/11] mm/mempolicy: Make task->mempolicy externally modifiable via syscall and procfs Andrew Morton
2023-11-22 21:35   ` Andrew Morton
2023-11-22 22:24   ` Gregory Price
2023-11-27 15:29     ` Michal Hocko [this message]
2023-11-27 16:14       ` Gregory Price
2023-11-28  9:45         ` Michal Hocko
2023-11-28 13:15           ` Gregory Price

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZWS19JFHm_LFSsFd@tiehlicka \
    --to=mhocko@suse.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=gourry.memverge@gmail.com \
    --cc=gregory.price@memverge.com \
    --cc=hpa@zytor.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=x86@kernel.org \
    --cc=ying.huang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).