On Sun, Jan 04, 2015 at 21:06, Mages Simon wrote:
> I restored the functionality according to the manpage.
>
> That means that read() on bpf is blocking again. If a
> timeout is set read() will block until the timeout is
> over.
>
> Maybe asynchronous is also broken, i will look into that later.
Thanks, but I think your diff is wrong. Here's one that should be
better.
If FIONBIO is set to 0, the timeout is set to 0, which should be
infinite. You're right this needs a special case, but it shouldn't
just wait some amount of time. It should wait forever, as requested.
In the regular timeout case, I'm not sure what you're changing. There
is a problem here though. If we're already close to the timeout
expiring, we shouldn't sleep the full timeout, only the time left
since we began the read.
Looking at the current code, the comparison seems reversed. If we've
just started reading, ticks == rdStart, and so adding the timeout will
always be greater, not less, than the current ticks.
I believe this diff addresses all the above issues. The current code
works correctly for the rtout == 0 case, but sleeping for only the
remaining timeout is much easier done by splitting it.
Anyone else have thoughts? I get confused by this code every time I
read it, perhaps because bugs obscure what it's trying to do and what
is doing. Am I correct?
Index: bpf.c
===================================================================
RCS file: /cvs/src/sys/net/bpf.c,v
retrieving revision 1.113
diff -u -p -r1.113 bpf.c
--- bpf.c 16 Dec 2014 18:30:04 -0000 1.113
+++ bpf.c 5 Jan 2015 18:49:11 -0000
@@ -430,10 +430,13 @@ bpfread(dev_t dev, struct uio *uio, int
if (d->bd_rtout == -1) {
/* User requested non-blocking I/O */
error = EWOULDBLOCK;
+ } else if (d->bd_rtout == 0) {
+ error = tsleep(d, PRINET|PCATCH, "bpf", 0);
} else {
- if ((d->bd_rdStart + d->bd_rtout) < ticks) {
- error = tsleep((caddr_t)d, PRINET|PCATCH, "bpf",
- d->bd_rtout);
+ int elapsed = ticks - d->bd_rdStart;
+ if (elapsed < d->bd_rtout) {
+ error = tsleep(d, PRINET|PCATCH, "bpf",
+ d->bd_rtout - elapsed);
} else
error = EWOULDBLOCK;
}
