From owner-cvs-all Mon Dec 17 10:41:31 2001 Delivered-To: cvs-all@freebsd.org Received: from elvis.mu.org (elvis.mu.org [216.33.66.196]) by hub.freebsd.org (Postfix) with ESMTP id 2DCE637BB21; Mon, 17 Dec 2001 10:31:05 -0800 (PST) Received: by elvis.mu.org (Postfix, from userid 1192) id 43D5C81D03; Mon, 17 Dec 2001 12:29:53 -0600 (CST) Date: Mon, 17 Dec 2001 12:29:53 -0600 From: Alfred Perlstein To: Bruce Evans Cc: wollman@FreeBSD.org, net@FreeBSD.org, cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: fifo fix (Re: cvs commit: src/sys/fs/fifofs fifo_vnops.c) Message-ID: <20011217122953.G82439@elvis.mu.org> References: <20011215151025.A82439@elvis.mu.org> <20011217230720.N13599-100000@gamplex.bde.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: <20011217230720.N13599-100000@gamplex.bde.org>; from bde@zeta.org.au on Tue, Dec 18, 2001 at 12:09:42AM +1100 Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG * Bruce Evans [011217 07:10] wrote: > On Sat, 15 Dec 2001, Alfred Perlstein wrote: > > > Can people take a look at this fix? It seems to dtrt, but I need feedback > > here. > > > > It basically backs out my last two revisions and changes the hacks the > > poll call to seemingly do the right thing. > > > > Index: fifo_vnops.c > > =================================================================== > > RCS file: /home/ncvs/src/sys/fs/fifofs/fifo_vnops.c,v > > retrieving revision 1.57 > > diff -u -r1.57 fifo_vnops.c > > --- fifo_vnops.c 12 Dec 2001 09:35:33 -0000 1.57 > > +++ fifo_vnops.c 15 Dec 2001 21:25:30 -0000 > > ... > > @@ -455,13 +451,23 @@ > > } */ *ap; > > { > > struct file filetmp; > > - int revents = 0; > > + int s, revents = 0; > > New style bug: disordered declaration. (Old style bug: initialized in > auto declaration.) > > > > > if (ap->a_events & (POLLIN | POLLPRI | POLLRDNORM | POLLRDBAND)) { > > + struct socket *so; > > + int oflg; > > Style bugs: nested declarations. I'll take care of these two. > > + > > filetmp.f_data = (caddr_t)ap->a_vp->v_fifoinfo->fi_readsock; > > + so = (struct socket *)filetmp.f_data; > > + s = splnet(); > > + oflg = so->so_state & SS_CANTRCVMORE; > > + if (ap->a_vp->v_fifoinfo->fi_writers == 0) > > + so->so_state &= ~SS_CANTRCVMORE; > > if (filetmp.f_data) > > revents |= soo_poll(&filetmp, ap->a_events, ap->a_cred, > > ap->a_td); > > + so->so_state |= oflg; > > + splx(s); > > I'm not happy with frobbing the socket state. I suggest frobbing the > events mask instead. Either use a flag to tell sopoll() to ignore > SS_CANTRCVMORE, or use new events POLLIN_IGNORE_EOF and > POLLRDNORM_IGNORE_EOF (convert the userland POLLIN/POLLRDNORM to these > and change sopoll() to support them). I'll consider that. There's actually a bug here, I need to make sure 'so' isn't NULL before I do this, sticking the frobbing under the 'if (filetmp.f_data)' should fix that. > > } > > if (ap->a_events & (POLLOUT | POLLWRNORM | POLLWRBAND)) { > > filetmp.f_data = (caddr_t)ap->a_vp->v_fifoinfo->fi_writesock; > > > > Similar changes may be needed for kevent(). kevent() has a way to modify > modify the usual watermark handling for reads, but doesn't have anything > similar for EOFs, although it already has more complete EOF handling > (filt_kqread() sets EV_EOF for EOFs, but sopoll() is missing support for > POLLHUP). I'll talk to jlemon about this. -- -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.' http://www.morons.org/rants/gpl-harmful.php3 To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message