From: Dominique Martinet <asmadeus@codewreck.org>
To: Christian Brauner <christian.brauner@ubuntu.com>
Cc: Oleg Nesterov <oleg@redhat.com>,
Alexander Kapshuk <alexander.kapshuk@gmail.com>,
linux-kernel@vger.kernel.org, ebiederm@xmission.com,
akpm@linux-foundation.org, liuzhiqiang26@huawei.com,
joel@joelfernandes.org, paulmck@linux.vnet.ibm.com,
kernel test robot <lkp@intel.com>
Subject: Re: [PATCH] kernel/signal.c: Export symbol __lock_task_sighand
Date: Mon, 22 Jun 2020 12:24:01 +0200 [thread overview]
Message-ID: <20200622102401.GA12377@nautica> (raw)
In-Reply-To: <20200622083905.c3nurmkbo5yhd6lj@wittgenstein>
Christian Brauner wrote on Mon, Jun 22, 2020:
> On Mon, Jun 22, 2020 at 08:25:28AM +0200, Oleg Nesterov wrote:
>> current->sighand is stable and can't go away. Unless "current" is exiting and
>> has already passed exit_notify(). So I don't think net/9p needs this helper.
>
> From what I can gather from the thread (cf. [1]) that is linked in the
> commit message the main motivation for all of this is sparse not being
> happy and not some bug. (Maybe I'm not seeing something though.)
>
> The patch itself linked here doesn't seem to buy anything. I agree with
> Oleg. Afaict, lock_task_sighand() would only be needed here if the task
> wouldn't be current. So maybe it should just be dropped from the series.
Sure. I honestly have no idea on what guarantees we have from the task
being current here as opposed to any other task -- I guess that another
thread calling exit for exemple would have to wait?
What about the possibility of sighand being null that the function does
check, is that impossible for current as well?
Honestly not a part of the code I'm much familiar with, this all
predates my involvement with 9p by a fair bit...
Anyway, not particularily fussy on this, it just looked like "the right
way" to lock a task signal handler among the few common patterns I could
see; I think it would make sense to just convert all such locks to a
single pattern for a maintenance pov but it's much more work than I'm
willing to do.
I'll just drop the patch :)
>> However, the games with TIF_SIGPENDING doesn't look right in any
>> case.
I definitely agree with this, hence my comment about an old patchset
that will remove these eventually, but while I did send the patches over
a year ago I never took them up due to lack of proper testing.
It's been something people regularily complained about that it makes the
task unkillable in a weird way and many tools like syzbot don't like it
(and potentially users who try ^C won't either)
I guess I'll try to find some time to finish that instead... Will be
more useful than trying to wrap my head around all of that :P
Thanks!
--
Dominique
next prev parent reply other threads:[~2020-06-22 10:24 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-21 13:37 [PATCH] kernel/signal.c: Export symbol __lock_task_sighand Alexander Kapshuk
2020-06-21 13:54 ` Dominique Martinet
2020-06-22 8:39 ` Christian Brauner
2020-06-22 8:50 ` Alexander Kapshuk
2020-06-22 6:25 ` Oleg Nesterov
2020-06-22 8:39 ` Christian Brauner
2020-06-22 10:24 ` Dominique Martinet [this message]
2020-06-22 11:31 ` Oleg Nesterov
2020-06-22 11:36 ` Christian Brauner
2020-06-22 12:03 ` Oleg Nesterov
2020-06-22 12:29 ` Christian Brauner
2020-06-22 13:01 ` Oleg Nesterov
2020-06-22 13:04 ` Christian Brauner
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=20200622102401.GA12377@nautica \
--to=asmadeus@codewreck.org \
--cc=akpm@linux-foundation.org \
--cc=alexander.kapshuk@gmail.com \
--cc=christian.brauner@ubuntu.com \
--cc=ebiederm@xmission.com \
--cc=joel@joelfernandes.org \
--cc=linux-kernel@vger.kernel.org \
--cc=liuzhiqiang26@huawei.com \
--cc=lkp@intel.com \
--cc=oleg@redhat.com \
--cc=paulmck@linux.vnet.ibm.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