Messages in this thread | | | Date | Tue, 3 Nov 2009 00:23:22 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH] block/ps3: Fix slow VRAM IO |
| |
On Mon, 19 Oct 2009 12:58:27 -0700 Geoff Levand <geoffrey.levand@am.sony.com> wrote:
> > From: Hideyuki Sasaki <Hideyuki_Sasaki@hq.scei.sony.co.jp> > > The current PS3 VRAM driver uses msleep() to wait for completion > of RSX DMA transfers between system memory and VRAM. Depending > on the system timing, the processing delay and overhead of this > msleep() call can significantly impact VRAM driver IO. > > To avoid the condition, add a short duration (200 usec max) > udelay() polling loop before entering the msleep() polling > loop. >
When raising a performance-based patch, please always try to include before-and-after performance measurements in the changelog. People want to know the magnitude of the improvement.
> > drivers/block/ps3vram.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > --- a/drivers/block/ps3vram.c > +++ b/drivers/block/ps3vram.c > @@ -123,7 +123,15 @@ static int ps3vram_notifier_wait(struct > { > struct ps3vram_priv *priv = ps3_system_bus_get_drvdata(dev); > u32 *notify = ps3vram_get_notifier(priv->reports, NOTIFIER); > - unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms); > + unsigned long timeout; > + > + for (timeout = 20; timeout; timeout--) {
for (timeout = 0; timeout < 20; timeout++) {
would be simpler.
> + if (!notify[3]) > + return 0; > + udelay(10); > + }
You might as well do a udelay(1) here. The additional cost will be negligible, and it will reduce latency.
> + timeout = jiffies + msecs_to_jiffies(timeout_ms);
The maximum latency is now timout_ms + 200usec.
That's OK with the current constants, but if someone later changes a constant, the error could become significant.
Perhaps that isn't worth bothering about though.
> do { > if (!notify[3])
| |