Re: [PATCH] preempt abstraction

Robert Love (rml@tech9.net)
08 Jan 2002 13:57:28 -0500


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

-
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/