Linux NFS development
 help / color / mirror / Atom feed
From: Steve Dickson <SteveD@redhat.com>
To: Neil Brown <neilb@suse.de>
Cc: nfs@lists.sourceforge.net
Subject: Re: nfs-utils 1.0.9-rc1 - hopefully -final in a week
Date: Fri, 07 Jul 2006 07:13:28 -0400	[thread overview]
Message-ID: <44AE41D8.5040302@RedHat.com> (raw)
In-Reply-To: <17576.25386.427824.894874@cse.unsw.edu.au>

[-- Attachment #1: Type: text/plain, Size: 2417 bytes --]

Hey Neil,

Neil Brown wrote:
> Hi,
>  I'd like to put out nfs-utils-1.0.9 by the end of the week.
> There is (or will soon be) a -pre1 in
>   http://www.kernel.org/pub/linux/utils/nfs/
> and
>   git://linux-nfs.org/nfs-utils
> 
> Recent changes can been seen at
>    http://linux-nfs.org/cgi-bin/gitweb.cgi?p=nfs-utils;a=log
>   
> If anyone cares to do some testing that would be great.

Well using the nfs-utils-1.0.9-pre1 tarball from kernel.org
and the recent kernel patch set you posted here is what I found.

Starting and stopping nfsds did not kill neither the
TCP or UDP connections. There were two problems with this.
One, rpc.nfsd was incorrectly increasing nfsd_serv->sv_nrthreads
in nfsd_svc() by calling nfsd_create_serv(); (Note: another side effect
of this problem was only 7 nfsd process were start instead of 8)
So the solution I've come up with is to only  have nfsd_create_serv()
called when it needs to... basically:
     if (nfsd_serv == NULL) {
         error = nfsd_create_serv();
         if (error)
             goto out;
     }

The second problem was an oops that occurred when the sv_nrthreads was
incremented correctly... and Neil your going to love this one... ;-)

In svc_delete_socket(), sock_release() was being called with a
socket that was already closed... The socket was closed by the
previous sockfd_put() when svsk->sk_sock->file != NULL... So
the code changed from
     if (svsk->sk_sock->file)
         sockfd_put(svsk->sk_sock);
     sock_release(svsk->sk_sock);
to
     if (svsk->sk_sock->file)
         sockfd_put(svsk->sk_sock);
     else
         sock_release(svsk->sk_sock);

The reason Neil is going to love this is I asked him to
make this change because in my previous testing, the only
way I could get the connections to come down was to
always call sock_release() even after the fput() of the
file pointer... So it appears in Neil's rewrite, fput()
is actually doing the release which is how it should work...

Once I got the connections to come up and down as expected, I
noticed I could not turn off any versions...

In the patches I sent Neil, I had to reverse the order of when
nfssvc_setfds() and nfssvc_versbits() were called in rpc.nfsd.
But in Neil's patch it appears nfssvc_versbits() is expect
to be called before nfssvc_setfds(). After I switching the
order of those calls... things started to work as expected..

That patches are attached....

steved.


[-- Attachment #2: kernel-2.6.17-nfsd109.patch --]
[-- Type: text/x-patch, Size: 1191 bytes --]

Don't let rpc.nfsd incorrectly increase the nfsd_serv->sv_nrthreads
and stop sockets from being released twice by only calling sock_release()
on socket that don't have file pointer associated with them.


Signed-off-by: Steve Dickson <steved@redhat.com>
-------------------------

--- linux-2.6.17.i686/fs/nfsd/nfssvc.c.orig	2006-07-06 17:47:22.000000000 -0400
+++ linux-2.6.17.i686/fs/nfsd/nfssvc.c	2006-07-07 06:15:21.000000000 -0400
@@ -265,9 +265,11 @@ nfsd_svc(unsigned short port, int nrserv
 
 	nfsd_reset_versions();
 
-	error = nfsd_create_serv();
-	if (error)
-		goto out;
+	if (nfsd_serv == NULL) {
+		error = nfsd_create_serv();
+		if (error)
+			goto out;
+	}
 	error = nfsd_init_socks(port);
 	if (error)
 		goto failure;
--- linux-2.6.17.i686/net/sunrpc/svcsock.c.orig	2006-07-06 17:47:22.000000000 -0400
+++ linux-2.6.17.i686/net/sunrpc/svcsock.c	2006-07-07 05:41:04.000000000 -0400
@@ -1525,7 +1525,8 @@ svc_delete_socket(struct svc_sock *svsk)
 		spin_unlock_bh(&serv->sv_lock);
 		if (svsk->sk_sock->file)
 			sockfd_put(svsk->sk_sock);
-		sock_release(svsk->sk_sock);
+		else
+			sock_release(svsk->sk_sock);
 		kfree(svsk);
 	} else {
 		spin_unlock_bh(&serv->sv_lock);

[-- Attachment #3: nfs-utils-1.0.9-pre1-vers.patch --]
[-- Type: text/x-patch, Size: 669 bytes --]

nfssvc_versbits() has to be called before nfssvc_setfds()
for the version processing to work correctly

Signed-off-by: Steve Dickson <steved@redhat.com>
-------------------------

--- nfs-utils-1.0.9-pre1/support/nfs/nfssvc.c.orig	2006-07-02 20:02:03.000000000 -0400
+++ nfs-utils-1.0.9-pre1/support/nfs/nfssvc.c	2006-07-07 06:10:01.000000000 -0400
@@ -135,10 +135,10 @@ nfssvc(int port, int nrservs, unsigned i
 	struct nfsctl_arg	arg;
 	int fd;
 
-	nfssvc_setfds(port, protobits, haddr);
-
 	nfssvc_versbits(versbits);
 
+	nfssvc_setfds(port, protobits, haddr);
+
 	fd = open(NFSD_THREAD_FILE, O_WRONLY);
 	if (fd < 0)
 		fd = open("/proc/fs/nfs/threads", O_WRONLY);

[-- Attachment #4: Type: text/plain, Size: 299 bytes --]

Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

[-- Attachment #5: Type: text/plain, Size: 140 bytes --]

_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

  parent reply	other threads:[~2006-07-07 11:19 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-07-03  0:22 nfs-utils 1.0.9-rc1 - hopefully -final in a week Neil Brown
2006-07-03  7:33 ` Greg Banks
2006-07-04  0:40   ` Neil Brown
2006-07-04  4:17     ` Greg Banks
2006-07-04  7:47       ` Neil Brown
2006-07-04  7:56         ` Greg Banks
2006-07-04  8:36           ` Neil Brown
2006-07-04  9:09             ` Greg Banks
2006-07-04 14:50 ` Shankar Anand
2006-07-05  6:03   ` Neil Brown
2006-07-05  6:19     ` Shankar Anand
2006-07-05  7:06       ` Neil Brown
     [not found] ` <1151901993.3932.18.camel@localhost.localdomain>
     [not found]   ` <17579.22262.255903.309723@cse.unsw.edu.au>
2006-07-06 17:32     ` Wendy Cheng
2006-07-07 11:13 ` Steve Dickson [this message]
2006-07-24  6:02   ` Neil Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=44AE41D8.5040302@RedHat.com \
    --to=steved@redhat.com \
    --cc=neilb@suse.de \
    --cc=nfs@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox