From: Jim Newsome <jnewsome@torproject.org>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
"Eric W . Biederman" <ebiederm@xmission.com>,
Christian Brauner <christian@brauner.io>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] do_wait: make PIDTYPE_PID case O(1) instead of O(n)
Date: Tue, 9 Mar 2021 14:42:00 -0600 [thread overview]
Message-ID: <7f5508ef-dbaa-fe5c-9826-cce0122eb2f8@torproject.org> (raw)
In-Reply-To: <20210309171539.GA32475@redhat.com>
On 3/9/21 11:15, Oleg Nesterov wrote:
> Jim,
>
> Thanks, the patch looks good to me. Yet I think you need to send V3 even
> if I personally do not care ;) Please consider ./scripts/checkpatch.pl,
> it reports all the coding-style problems I was going to mention.
Thanks! I'd thought clang-format with the included configuration would
be sufficient, but apparently not :)
> Both if's use "int retval", to me it would be better to declare this variable
> at the start of do_wait_pid(). But again, I won't insist this is up to you.
My usual inclination is to avoid uninitialized variables and prefer
putting them in tighter scopes. I don't think it's really much of an
issue in this relatively short function though; happy to go with the
prevailing style.
> I am wondering if something like
>
> static inline bool is_parent(struct task_struct *tsk, struct task_struct *p, int flags)
> {
> return tsk == p || !(flags & __WNOTHREAD)) && same_thread_group(tsk, p);
> }
>
> makes any sense to make do_wait_pid() more clear... probably not.
Yeah, I lean slightly towards the extra level of indirection not being
worth the deduplication.
I made a couple other small changes as well:
* No need for do_wait_pid to take the parameter `tsk` since it's only
ever called with `current`
* With that change, the declaration of `tsk` in `do_wait` can be moved
into a tighter scope of where it's used in the loop.
v3: https://lkml.org/lkml/2021/3/9/1134
prev parent reply other threads:[~2021-03-09 20:43 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-09 16:15 Jim Newsome
2021-03-09 17:15 ` Oleg Nesterov
2021-03-09 20:42 ` Jim Newsome [this message]
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=7f5508ef-dbaa-fe5c-9826-cce0122eb2f8@torproject.org \
--to=jnewsome@torproject.org \
--cc=akpm@linux-foundation.org \
--cc=christian@brauner.io \
--cc=ebiederm@xmission.com \
--cc=linux-kernel@vger.kernel.org \
--cc=oleg@redhat.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
Powered by JetHome