Re: [PATCH] preempt abstraction

[Robert Love <rml@tech9.net>]

On Tue, 2002-01-08 at 12:40, David Howells wrote:
> 
> The following patch abstracts access to need_resched:
> 
> 	ftp://infradead.org/pub/people/dwh/preempt-252p10.diff.bz2
> 
> It replaces most C-source read accesses to it with need_preempt() which
> returns true if rescheduling is necessary.

Nice patch!

A couple of points:

Why not use the more commonly named conditional_schedule instead of
preempt() ?  In addition to being more in-use (low-latency, lock-break,
and Andrea's aa patch all use it) I think it better conveys its meaning,
which is a schedule() but only conditionally.

I'm sure it is just being pedantic, but why not just make need_preempt
and preempt (which I would rename need_schedule and
conditional_schedule, personally) defines?  Example:

	#define need_schedule() (unlikely(current->need_resched))
	#define conditional_schedule() do { \
		if (need_schedule()) \
			schedule(); \
	} while(0);

Next, in kernel/sched.c you wrap need_preempt in an unlikey() but note
it is unlikely by design ... Same in mm/vmscan.c a couple times.

Oh, and the patch is confusingly similar to preempt-kernel in name, but
I guess that is my problem. :-)

Anyhow, I like.  2.5 _and_ 2.4?

	Robert Love