Re: [PATCH rc1-mm 2/3] coredump: shutdown current process first

[Roland McGrath]

> This patch optimize zap_threads() for the case when there are
> no ->mm users except the current's thread group. In that case
> we can avoid 'for_each_process()' loop.

This is a very good optimization.  Please don't use a goto here when a
simple if block and some reindenting works just fine.

I would be inclined to restructure the inner loop something like this:

		p = g;
		while (unlikely(p->mm == NULL)) {
			p = next_thread(p);
			if (p == g)
				break;
		}
		if (p->mm == mm) {
			/*
			 * p->sighand can't disappear, but
			 * may be changed by de_thread()
			 */
			lock_task_sighand(p, &flags);
			zap_process(p);
			unlock_task_sighand(p, &flags);
		}

But that is just taste.

> It also adds a useful invariant: SIGNAL_GROUP_EXIT (if checked
> under ->siglock) always implies that all threads (except may be
> current) have pending SIGKILL.

I agree that's a sensible thing to be able to rely on (though I don't know
of a practical difference it makes atm).  If this is merged with by
SIGNAL_GROUP_EXEC change, then the invariant is that SIGNAL_GROUP_EXIT
always means that all threads (including current) either have pending
SIGKILL or are already calling do_group_exit/do_exit.


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