Re: [PATCH 1/4] svcrpc: never clear XPT_BUSY on dead xprt

["J. Bruce Fields" <bfields@fieldses.org>]

On Tue, Oct 26, 2010 at 09:58:36AM +1100, Neil Brown wrote:
> On Mon, 25 Oct 2010 16:21:56 -0400
> "J. Bruce Fields" <bfields@fieldses.org> wrote:
> 
> > On Mon, Oct 25, 2010 at 12:43:57PM +1100, Neil Brown wrote:
> > > On Sun, 24 Oct 2010 21:21:30 -0400
> > > "J. Bruce Fields" <bfields@redhat.com> wrote:
> > > 
> > > > Once an xprt has been deleted, there's no reason to allow it to be
> > > > enqueued--at worst, that might cause the xprt to be re-added to some
> > > > global list, resulting in later corruption.
> > > > 
> > > > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > > 
> > > Yep, this makes svc_close_xprt() behave the same way as svc_recv() which
> > > calls svc_delete_xprt but does not clear XPT_BUSY.  The other branches in
> > > svc_recv call svc_xprt_received, but the XPT_CLOSE branch doesn't
> > > 
> > > Reviewed-by: NeilBrown <neilb@suse.de>
> > 
> > Also, of course:
> > 
> > > >  	svc_xprt_get(xprt);
> > > >  	svc_delete_xprt(xprt);
> > > > -	clear_bit(XPT_BUSY, &xprt->xpt_flags);
> > > >  	svc_xprt_put(xprt);
> > 
> > The get/put is pointless: the only reason I can see for doing that of
> > course was to be able to safely clear the bit afterwards.
> >
> 
> Agreed.
> 
> I like patches that get rid of code!!

Unfortunately, I'm stuck on just one more point: is svc_close_all()
really safe?  It assumes it doesn't need any locking to speak of any
more because the server threads are gone--but the xprt's themselves
could still be producing events, right?  (So data could be arriving that
results in calls to svc_xprt_enqueue, for example?)

If that's right, I'm not sure what to do there....

--b.