Re: [PATCH] Export current_is_keventd() for libphy

[Linus Torvalds <torvalds@osdl.org>]

On Wed, 6 Dec 2006, Jeff Garzik wrote:
>
> Honestly I wonder if some of these situations really want
> "kill_scheduled_work_unless_it_is_already_running_right_now_if_so_wait_for_it"

We could do that, and the code to do it would be fairly close to what the 
"run it" code is. Just replace the

	if (!test_bit(WORK_STRUCT_NOAUTOREL, &work->management))
		work_release(work);
	f(work);

with an unconditional

	work_release(work);

instead.

However, I think I'd prefer my patch for now, if only because that 
"work_release()" thing kind of worries me. You're supposed to either do 
the release yourself in the work function _after_ you've done all 
book-keeping, or if the thing doesn't need any book-keeping at all, you 
can do the "autorelease" thing. The "kill without running" breaks that 
conceptual model.

So a "kill_work()" usage should almost always end up being something like

	if (kill_work(work))
		release anything that running the work function would release

but then for the (probably common) case where there is nothing that the 
work function releases, that would obviously boil down to just a

	kill_work(work);

However, my point is that the _workqueue_ logic cannot know what the user 
of the workqueue wants, so the "run_scheduled_work()" approach is much 
saner in this respect.

NOTE NOTE NOTE! In neither case can we do anything at all about a 
workqueue entry that is actually _already_ running on another CPU. My 
suggested patch will simply not run it at all synchronously (and return 
0), and a "kill_work()" thing couldn't do anything either. The required 
synchronization for something like that simply doesn't exist.

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