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 [thread overview] 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.
WARNING: multiple messages have this Message-ID (diff)
From: Minchan Kim <minchan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> To: Christian Brauner <christian.brauner-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org> Cc: Kirill Tkhai <ktkhai-5HdwGun5lf+gSpxsJD1C4w@public.gmane.org>, Michal Hocko <mhocko-IBi9RG/b67k@public.gmane.org>, Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>, LKML <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, linux-mm <linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org>, linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, oleksandr-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, Suren Baghdasaryan <surenb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>, Tim Murray <timmurray-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>, Daniel Colascione <dancol-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>, Sandeep Patil <sspatil-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>, Sonny Rao <sonnyrao-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>, Brian Geffon <bgeffon-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>, Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org>, Shakeel Butt <shakeelb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>, John Dias <joaodias-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> Subject: Re: [PATCH 2/4] mm: introduce external memory hinting API Date: Mon, 13 Jan 2020 10:44:08 -0800 [thread overview] 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.
next prev parent reply other threads:[~2020-01-13 18:44 UTC|newest] Thread overview: 41+ 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-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-11 7:34 ` SeongJae Park 2020-01-13 18:02 ` Minchan Kim 2020-01-13 8:47 ` Kirill Tkhai 2020-01-13 8:47 ` Kirill Tkhai 2020-01-13 10:42 ` Christian Brauner 2020-01-13 10:42 ` Christian Brauner 2020-01-13 18:44 ` Minchan Kim [this message] 2020-01-13 18:44 ` Minchan Kim 2020-01-13 19:10 ` Christian Brauner 2020-01-13 19:27 ` Daniel Colascione 2020-01-13 19:27 ` Daniel Colascione 2020-01-13 19:27 ` Daniel Colascione 2020-01-13 20:42 ` Christian Brauner 2020-01-13 21:04 ` Daniel Colascione 2020-01-13 21:04 ` Daniel Colascione 2020-01-13 21:04 ` Daniel Colascione 2020-01-14 19:20 ` Christian Brauner 2020-01-14 18:59 ` Minchan Kim 2020-01-14 18:59 ` Minchan Kim 2020-01-14 19:22 ` Christian Brauner 2020-01-14 19:22 ` Christian Brauner 2020-01-13 18:39 ` Minchan Kim 2020-01-13 19:18 ` Daniel Colascione 2020-01-13 19:18 ` Daniel Colascione 2020-01-13 19:18 ` Daniel Colascione 2020-01-14 8:39 ` Kirill Tkhai 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-10 21:34 ` 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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.