Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
From: Minchan Kim <minchan@kernel.org>
To: Christian Brauner <christian.brauner@ubuntu.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>,
	Michal Hocko <mhocko@suse.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-mm <linux-mm@kvack.org>,
	linux-api@vger.kernel.org, oleksandr@redhat.com,
	Suren Baghdasaryan <surenb@google.com>,
	Tim Murray <timmurray@google.com>,
	Daniel Colascione <dancol@google.com>,
	Sandeep Patil <sspatil@google.com>,
	Sonny Rao <sonnyrao@google.com>,
	Brian Geffon <bgeffon@google.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Shakeel Butt <shakeelb@google.com>,
	John Dias <joaodias@google.com>
Subject: Re: [PATCH 2/4] mm: introduce external memory hinting API
Date: Mon, 13 Jan 2020 10:44:08 -0800
Message-ID: <20200113184408.GD110363@google.com> (raw)
In-Reply-To: <20200113104256.5ujbplyec2sk4onn@wittgenstein>

On Mon, Jan 13, 2020 at 11:42:57AM +0100, Christian Brauner wrote:
> On Mon, Jan 13, 2020 at 11:47:11AM +0300, Kirill Tkhai wrote:

< snip >

> > > +SYSCALL_DEFINE5(process_madvise, int, pidfd, unsigned long, start,
> > > +		size_t, len_in, int, behavior, unsigned long, flags)
> > 
> > I don't like the interface. The fact we have pidfd does not mean,
> > we have to use it for new syscalls always. A user may want to set
> > madvise for specific pid from console and pass pid as argument.
> > pidfd would be an overkill in this case.
> > We usually call "kill -9 pid" from console. Why shouldn't process_madvise()
> > allow this?
> > 
> > I suggent to extend first argument to work with both pid and pidfd.
> > Look at what we have for waitid(idtype, id_t id, ...) for example:
> > 
> >        idtype == P_PID
> >               Wait for the child whose process ID matches id.
> > 
> >        idtype == P_PIDFD (since Linux 5.4)
> >               Wait for the child referred to by the PID file descriptor specified in id.  (See pidfd_open(2) for  further  information  on
> >               PID file descriptors.)
> > 
> > We may use @flags argument for this.
> 
> Sorry for chiming in just a comment. Overall, I don't particularly care
> how or if you integrate pidfd here. One thing I would like to point out
> is that we're working on a patch to place new features under pidfd
> specific flags. This e.g. means a pidfd would be only be able to be used
> for madvise operations (or getfd operations) if it was created with that
> specific flag set making it easier to share them with other processes.
> So if you integrate them here I would be quite thankful if you target
> the patchset for the v5.7 merge window, not for v5.6.

Hi Christian,
Sorry but I couldn't understand your point.
Could you clarify what you meant?

Thanks.


  reply index

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-10 21:34 [PATCH 0/4] introduce memory hinting API for external process Minchan Kim
2020-01-10 21:34 ` [PATCH 1/4] mm: factor out madvise's core functionality Minchan Kim
2020-01-11  7:37   ` SeongJae Park
2020-01-13 18:11     ` Minchan Kim
2020-01-13 18:22       ` SeongJae Park
2020-01-10 21:34 ` [PATCH 2/4] mm: introduce external memory hinting API Minchan Kim
2020-01-11  7:34   ` SeongJae Park
2020-01-13 18:02     ` Minchan Kim
2020-01-13  8:47   ` Kirill Tkhai
2020-01-13 10:42     ` Christian Brauner
2020-01-13 18:44       ` Minchan Kim [this message]
2020-01-13 19:10         ` Christian Brauner
2020-01-13 19:27           ` Daniel Colascione
2020-01-13 20:42             ` Christian Brauner
2020-01-13 21:04               ` Daniel Colascione
2020-01-14 19:20                 ` Christian Brauner
2020-01-14 18:59           ` Minchan Kim
2020-01-14 19:22             ` Christian Brauner
2020-01-13 18:39     ` Minchan Kim
2020-01-13 19:18     ` Daniel Colascione
2020-01-14  8:39       ` Kirill Tkhai
2020-01-14 19:12         ` Minchan Kim
2020-01-15  9:38           ` Kirill Tkhai
2020-01-10 21:34 ` [PATCH 3/4] mm/madvise: employ mmget_still_valid for write lock Minchan Kim
2020-01-10 21:34 ` [PATCH 4/4] mm/madvise: allow KSM hints for remote API Minchan Kim
2020-01-11  7:42   ` SeongJae Park

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=20200113184408.GD110363@google.com \
    --to=minchan@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=bgeffon@google.com \
    --cc=christian.brauner@ubuntu.com \
    --cc=dancol@google.com \
    --cc=hannes@cmpxchg.org \
    --cc=joaodias@google.com \
    --cc=ktkhai@virtuozzo.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=oleksandr@redhat.com \
    --cc=shakeelb@google.com \
    --cc=sonnyrao@google.com \
    --cc=sspatil@google.com \
    --cc=surenb@google.com \
    --cc=timmurray@google.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

Linux-mm Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mm/0 linux-mm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mm linux-mm/ https://lore.kernel.org/linux-mm \
		linux-mm@kvack.org
	public-inbox-index linux-mm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kvack.linux-mm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git