From: Greg Banks <gnb@sgi.com>
Subject: Re: [RFC,PATCH 8/14] knfsd: centralise SK_CLOSE handling
Date: Mon, 21 May 2007 09:56:54 +1000
Message-ID: <20070520235654.GA24779@sgi.com>
References: <20070516192509.GN9626@sgi.com>
	<17996.13702.484468.777723@notabene.brown>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Cc: Thomas Talpey <Thomas.Talpey@netapp.com>,
	Linux NFS Mailing List <nfs@lists.sourceforge.net>,
	Peter Leckie <pleckie@melbourne.sgi.com>
To: Neil Brown <neilb@suse.de>
In-Reply-To: <17996.13702.484468.777723@notabene.brown>
Sender: nfs-bounces@lists.sourceforge.net
Errors-To: nfs-bounces@lists.sourceforge.net

On Thu, May 17, 2007 at 08:59:18PM +1000, Neil Brown wrote:
> On Thursday May 17, gnb@sgi.com wrote:
> > 
> >  
> >  		if (svsk) {
> > -			svc_sock_enqueue(svsk);
> > +			/*
> > +			 * We're always called in nfsd context so we
> > +			 * don't have to muck around with SK_CLOSE.
> > +			 */
> > +			svc_delete_socket(svsk);
> >  			svc_sock_put(svsk);
> >  		}
> 
> I'm not convinced that this is right.  

On further thought, you were entirely right and that part of my
patch was bogus.  At that point in the code, my comments about it
being safe to call svc_delete_socket() are true for the `svsk' that
is passed in from the caller.  However the `svsk' I was passing
to svc_delete_socket() is *another* struct svc_sock which was
just chosen as a sacrificial victim, and it is *not* safe to call
svc_delete_socket() directly on that.  Woops.

The code now reads:

                if (svsk) {
                        /*
                         * Although we're always called in nfsd context,
                         * there's no guarantee that the svsk we've just
                         * chosen is not attached to a queue by sk_ready,
                         * so we cannot just svc_delete_socket() directly.
                         */
                        svc_sock_delete_bh(svsk);
                        svc_sock_put(svsk);
                }

I'm glad you caught that one.

Greg.
-- 
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
Apparently, I'm Bedevere.  Which MPHG character are you?
I don't speak for SGI.

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs