linux-kernel - Re: [PATCH 3/3] make kthread

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20070414184956.GA615@tv-sign.ru>
Date:	Sat, 14 Apr 2007 22:50:12 +0400
From:	Oleg Nesterov <oleg@...sign.ru>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Davide Libenzi <davidel@...ilserver.org>,
	Ingo Molnar <mingo@...e.hu>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	Roland McGrath <roland@...hat.com>,
	Rusty Russell <rusty@...tcorp.com.au>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] make kthread_stop() scalable

On 04/14, Eric W. Biederman wrote:
>
> This is where I was going beyond what you were doing.  I needed a flag to say
> that this a kthread that is stopping to test in recalc_sigpending.  To be certain
> of terminating interruptible sleeps.  I could not get at your struct kthread
> in that case.
> 
> If it wasn't for the wait_event_interruptible thing I likely would
> have just thrown a union in struct task_struct.
> 
> I also got lucky in that vfork_done is designed to point a completion
> just where I need it (when a task exits).  The name is now a little
> abused but otherwise it does just what I want it to.
> 
> >> It also doesn't solve the biggest problem with the current kthread interface
> >> in that calling kthread_stop does not cause the code to break out of
> >> interruptible sleeps.
> >
> > Hm? kthread_stop() does wake_up_process(), it wakes up TASK_INTERRUPTIBLE tasks.
> 
> Yes. But if they are looping, unless signal_pending is set it is quite possible
> they will go back to sleep.
> 
> Take for example:
> 
> > #define __wait_event_interruptible(wq, condition, ret)		\
> > do {									\
> > 	DEFINE_WAIT(__wait);						\
> > 									\
> > 	for (;;) {							\
> > 		prepare_to_wait(&wq, &__wait, TASK_INTERRUPTIBLE);	\
> > 		if (condition)						\
> > 			break;						\
> > 		if (!signal_pending(current)) {				\
> > 			schedule();					\
> > 			continue;					\
> > 		}							\
> > 		ret = -ERESTARTSYS;					\
> > 		break;							\
> > 	}								\
> > 	finish_wait(&wq, &__wait);					\
> > } while (0)
> 
> We don't break out until either condition is true or signal_pending(current)
> is true.
> 
> Loops that do that are very common in the kernel.  I counted about 500
> calls of signal pending in places that otherwise care nothing about signals.
> Several kernel threads call into functions that use loops like
> wait_event_interruptible.  So I need a more forceful kthread_stop.  If
> I don't want to continue to use signals.

Yes, I got it reading your next patches. Ok, probably this change is good.
My question was: do we really want to force a kernel thread to exit if it
waits for event in TASK_INTERRUPTIBLE state? probably yes.

> > Yes, thanks... Can't understand how I was soooo stupid!!! thanks...
> >
> > Damn. We don't need 2 completions! just one.
> 
> Yep.  My second patch in this last round implements that.

Yes, I have read it. It is clearly better then mine, and I think correct.

Oleg.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/