Date: Sat, 21 Dec 2002 17:57:37 -0800 From: Alfred Perlstein <bright@mu.org> To: Bruce Evans <bde@zeta.org.au> Cc: arch@freebsd.org Subject: Re: pipes and FIONBIO breakage? Message-ID: <20021222015737.GO23663@elvis.mu.org> In-Reply-To: <20021222113905.C7736-100000@gamplex.bde.org> References: <20021221042734.GL23663@elvis.mu.org> <20021222113905.C7736-100000@gamplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
* Bruce Evans <bde@zeta.org.au> [021221 17:24] wrote: > > Things work correctly at the file descriptor level and almost correctly > for ordinary pipes. kern_descrip.c maintains the FNONBLOCK flag in > the file struct and arranges to pass it to read/write/etc. pipe_read() > and pipe_write() ignore their `flags' arg and determine the FNONBLOCK > flag by accessing the file struct direct. This is a layering violation. > Most device drivers can't even think about doing it without it being > a clear violation since they don't have a pointer to the file struct > in scope. However, accessing the current value of the flag may be > best since the value in the flags arg can't be changed by fcntl() or > FIONBIO and it may be right to permit unblocking of blocked i/o by > changing the flag. I don't see the FNONBLOCK being passed into dofileread as 'flags'. Are you suggesting though that perhaps it would make sense for the FIONBIO ioctl to issue a wakeup on the object it is being set on to perhaps unblock already blocked threads after setting the state? I like the flexibility that introduces although the semantics of it are somewhat offbeat. > FIONBIO handling is a little different and uglier. Both > fcntl(... F_SETFL ...) and ioctl(... FIONBIO ...) set the FNONBLOCK > flag in the file struct and call fo_ioctl(... FIONBIO ...) to duplicat > any bogus copies of the flag in lower level structs. Non-broken lower > levels don't have a bogus copy of the flag so FIONBIO is null at > their level. The sys_pipe.c level is one of these, so > pipe_ioctl(... FIONBIO ...) returns successfully after doing nothing > except fiddling with locks. I agree with you, you think that perhaps we could fix this by adding a parameter to the socket read/write functions so they would know if they could block or not? > > > This _would_ > > have been a problem for fifo's, however that code seems to duplicate > > the pipe code somewhat. > > Fifos mostly use the socket code. They get the non-blocking flag > correctly (except it may become stale -- see above) in fifo_read() > and fifo_write() and duplicate it in the socket struct. The second > part of this is broken -- there may be different file structs with > different flags (one for each open()), but there is only 1 socket > struct. The flag doesn't get duplicated in the socket struct by > fifo_ioctl() because fifo_ioctl() knows that fifo_read() and > fifo_write() maintain it and just returns without calling soo_ioctl(). Ok. Some more investigation on my part would have probably been a good idea. > > > There does appear to be an issue where the fifo code temporarity > > ORs in the non-block state of the struct file into the socket behind > > it when performing reads and writes. This might cause someone else > > to block when multiple people open a fifo because of races. > > Yes, it seems to be easy for processes to clobber other processes' > blocking state state either way. fifo_read() and fifo_write() don't > even manage to restore the socket struct's copy of the flag to its > initial state. They always turn it off if they set it, even if it > was initially on. This implements clobbering of the state from > on to off, but not soon enough to help processes that actually want > it off, since the flag is assumed to be initially off and not changed > in the !(ap->a_ioflag & IO_NDELAY) case until after doing the i/o. > > This bug suite seems to mostly not affect normal sockets since open() > doesn't work on them so one copy of the file flags is sufficient. So this is somewhat worse than my second take on the problem. Any suggestions for a fix? Passing the blocking status down into the socket level calls seems like the right thing to do. Same with devices I guess? Or is this just another reason device driver writers shouldn't allow more than one open() at a time? :) -- -Alfred Perlstein [alfred@freebsd.org] 'Instead of asking why a piece of software is using "1970s technology," start asking why software is ignoring 30 years of accumulated wisdom.' To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20021222015737.GO23663>