Date: 9 Mar 2005 13:02:51 +0100
Date: Wed, 9 Mar 2005 13:02:51 +0100
From: Andi Kleen <ak@muc.de>
To: torvalds@osdl.org, roland@redhat.com, akpm@osdl.org
Cc: linux-kernel@vger.kernel.org
Subject: Bad patch to schedule()
Message-ID: <20050309120251.GA4374@muc.de>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.4.1i
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1494
Lines: 44


I just see that this patch went into mainline.

[PATCH] posix-timers: high-resolution CPU clocks for POSIX clock_* syscalls

This patch provides support for thread and process CPU time clocks in the
....

 /*
+ * This is called on clock ticks and on context switches.
+ * Bank in p->sched_time the ns elapsed since the last tick or switch.
+ */
+static inline void update_cpu_clock(task_t *p, runqueue_t *rq,
+				    unsigned long long now)
+{
+	unsigned long long last = max(p->timestamp, rq->timestamp_last_tick);
+	p->sched_time += now - last;
+}
+

called from schedule(). The problem with this is that it completely
messes up the register allocation for i386 schedule() because it 
does long long arithmetic. This causes gcc to spill everything
else because it needs four registers, and i386 only has 6 usable
ones.

I think a critical path like the scheduler needs a little bit more
care.  Also it is totally unclear if this obscure POSIX feature
is really worth making schedule() slower. I think not.

In case it is kept it should be done in a way that doesn't impact
i386 unduly e.g. by avoiding long long arithmetic here and 
making sure fast paths stay fast.

I would propose to back this patch out again until this is resolved.

-Andi


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