From: Michael Buesch <mb@bu3sch.de>
To: Valdis.Kletnieks@vt.edu
Subject: Re: 2.6.17-rc4-mm3
Date: Mon, 22 May 2006 13:49:28 +0200
User-Agent: KMail/1.9.1
References: <20060522022709.633a7a7f.akpm@osdl.org> <200605221325.10761.mb@bu3sch.de> <200605221138.k4MBcgd2006492@turing-police.cc.vt.edu>
In-Reply-To: <200605221138.k4MBcgd2006492@turing-police.cc.vt.edu>
Cc: Andrew Morton <akpm@osdl.org>, linux-kernel@vger.kernel.org
MIME-Version: 1.0
Content-Type: text/plain;
  charset="iso-8859-15"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
Message-Id: <200605221349.28329.mb@bu3sch.de>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2293
Lines: 54

On Monday 22 May 2006 13:38, you wrote:
> On Mon, 22 May 2006 13:25:10 +0200, Michael Buesch said:
> > On Monday 22 May 2006 13:15, you wrote:
> 
> > > It looks to me line the old code stayed in a while() loop in rng_dev_read
> > > until it had fulfilled the read request (including possibly multiple
> > > calls to need_resched() and friends).  The new code will bail on an -EAGAIN
> > > as soon as the *first* poll fails, rather than waiting until something
> > > is available - even if it is NOT flagged O_NONBLOCK.
> > 
> > Yeah. That is how it works. I am wondering why userspace doesn't
> > simply retry, if it receives an EAGAIN.
> > Should we return ERESTARTSYS or something like that instead?
> 
> That's not the way it worked in previous kernels, and it's not the way that
> the current rng-utils RPM in Fedora expects it to work.
> 
> Here's a patch that makes it work the way it used to.  Adding the test
> for O_NONBLOCK is the biggie - the old code did a resched test at that
> point in the loop, so I added it here too.
> 
> --- linux-2.6.17-rc4-mm3/drivers/char/hw_random/core.c.rnd_fix	2006-05-22 07:23:34.000000000 -0400
> +++ linux-2.6.17-rc4-mm3/drivers/char/hw_random/core.c	2006-05-22 07:22:29.000000000 -0400
> @@ -125,7 +125,7 @@ static ssize_t rng_dev_read(struct file 
>  		mutex_unlock(&rng_mutex);
>  
>  		err = -EAGAIN;
> -		if (!bytes_read)
> +		if (filp->f_flags & O_NONBLOCK && !bytes_read)
>  			goto out;
>  
>  		err = -EFAULT;
> @@ -138,6 +138,9 @@ static ssize_t rng_dev_read(struct file 
>  			data >>= 8;
>  		}
>  
> +		if (need_resched())
> +                        schedule_timeout_interruptible(1);
> +

Andrew's comment on this:

	What's going on with the need_resched() tricks in there?  (Unobvious, needs
	a comment).  From my reading, it'll cause a caller to this function to hang
	for arbitrary periods of time if something if causing heavy scheduling
	pressure.

So I decided to remove it and return -EAGAIN, so userspace can retry.
But seems like it it does not. I thought glibc would handle that.
-
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/