All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@tv-sign.ru>
To: Satyam Sharma <satyam.sharma@gmail.com>
Cc: Jeff Layton <jlayton@redhat.com>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	"Eric W. Biederman" <ebiederm@xmission.com>
Subject: Re: [PATCH] RFC: have tcp_recvmsg() check kthread_should_stop() and treat it as if it were signalled
Date: Tue, 26 Jun 2007 15:54:49 +0400	[thread overview]
Message-ID: <20070626115449.GA92@tv-sign.ru> (raw)
In-Reply-To: <a781481a0706251241n36126456x7f4b445a86d1a68f@mail.gmail.com>

On 06/26, Satyam Sharma wrote:
> 
> Yes, why not embed a send_sig(SIGKILL) just before the wake_up_process()
> in kthread_stop() itself?

Personally, I don't think we should do this.

kthread_stop() doesn't always mean "kill this thread asap". Suppose that
CPU_DOWN does kthread_stop(workqueue->thread) but doesn't flush the queue
before that (we did so before 2.6.22 and perhaps we will do again). Now
work_struct->func() doing tcp_recvmsg() or wait_event_interruptible() fails,
but this is probably not that we want.

> So could we have signals in _addition_ to kthread_stop_info and change
> kthread_should_stop() to check for both:
> 
> kthread_stop_info.k == current && signal_pending(current)

No, this can't work in general. Some kthreads do flush_signals/dequeue_signal,
so TIF_SIGPENDING can be lost anyway.

I personally think Jeff's idea to use force_sig() is right. kthread_create()
doesn't use CLONE_SIGHAND, so it is safe to change ->sighand->actionp[].


(offtopic)

	cifs_mount:

		send_sig(SIGKILL,srvTcp->tsk,1);
		tsk = srvTcp->tsk;
		if(tsk)
			kthread_stop(tsk);

This "if(tsk)" looks wrong to me. Can srvTcp->tsk be NULL? If yes, send_sig()
is not safe. Can srvTcp->tsk become NULL after send_sig() ? If yes, this
check is racy, and kthread_stop() is not safe.

Oleg.


  parent reply	other threads:[~2007-06-26 11:54 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-06-05 19:23 [PATCH] CIFS: make cifsd (more) signal-safe Jeff Layton
2007-06-06  8:55 ` Christoph Hellwig
2007-06-08 16:35   ` [PATCH] RFC: have tcp_recvmsg() check kthread_should_stop() and treat it as if it were signalled Jeff Layton
2007-06-09  1:30     ` Herbert Xu
2007-06-09 11:08       ` Jeff Layton
2007-06-25 19:41         ` Satyam Sharma
2007-06-25 19:52           ` Jeff Layton
2007-06-25 22:09             ` Satyam Sharma
2007-06-26  0:46               ` Jeff Layton
2007-06-26 11:54           ` Oleg Nesterov [this message]
2007-06-26 22:53             ` Satyam Sharma
2007-06-27  1:29               ` Satyam Sharma
2007-06-27 12:24               ` Oleg Nesterov
2007-06-28  0:44                 ` Satyam Sharma
2007-06-28 14:12                   ` Oleg Nesterov
2007-06-28 15:44                     ` Satyam Sharma
2007-06-28 16:19                       ` Satyam Sharma
2007-06-28 16:24                       ` Satyam Sharma
2007-06-28 17:08                       ` Oleg Nesterov
2007-06-28 18:41                         ` Jeff Layton
2007-06-28 19:22                         ` Satyam Sharma
2007-06-08 17:00   ` Jeff Layton
2007-06-21 14:35   ` [linux-cifs-client] Re: [PATCH] CIFS: make cifsd (more) signal-safe Jeff Layton

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=20070626115449.GA92@tv-sign.ru \
    --to=oleg@tv-sign.ru \
    --cc=ebiederm@xmission.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=jlayton@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=satyam.sharma@gmail.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 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.