Thanks for looking into this.
I think the correct change is to test

		if (atomic_read(&svsk->sk_inuse) > 1 || test_bit(SK_BUSY, &svsk->sk_flags))

or even

		if (atomic_read(&svsk->sk_inuse) != 1 || test_bit(SK_BUSY, &svsk->sk_flags))
sk_inuse contains a bias of '1' until SK_DEAD is set.  So a normal,

active socket will have an inuse count of 1 or more.  If it is exactly

1, then either it is SK_DEAD (in which case there is nothing for this

code to do), or it has no users, in which case it is appropriate to

close the socket if it is old.

Note that this test is for "the socket should not be closed", so we

test if it is *not* 1, or > 1.
The tests are needed because we don't want to close a socket that

might be inuse elsewhere.  The SK_BUSY bit combined with the sk_inuse

count combine to check if the socket is in use at all or not.
You change effectively disabled the test, as sk_inuse is never <= 0

(except when SK_DEAD is set).
This bug has been present since

commit aaf68cfbf2241d24d46583423f6bff5c47e088b3

Author: NeilBrown <neilb@suse.de>

Date:   Thu Feb 8 14:20:30 2007 -0800
(i.e. it is my fault).

So it is in 2.6.21 and later and should probably go to .stable for .21

and .22.
Bruce:  for you :-)

---------------------------

Correctly close old nfsd/lockd sockets.
From: NeilBrown <neilb@suse.de>
Commit aaf68cfbf2241d24d46583423f6bff5c47e088b3 added a bias

to sk_inuse, so this test for an unused socket now fails.  So no

sockets gets closed because they are old (they might get closed

if the client closed them).
This bug has existed since 2.6.21-rc1.
Thanks to Wolfgang Walter for finding and reporting the bug.
Cc: Wolfgang Walter <wolfgang.walter@studentenwerk.mhn.de>

Signed-off-by: Neil Brown <neilb@suse.de>
### Diffstat output

 ./net/sunrpc/svcsock.c |    3 ++-

 1 file changed, 2 insertions(+), 1 deletion(-)
diff .prev/net/sunrpc/svcsock.c ./net/sunrpc/svcsock.c

--- .prev/net/sunrpc/svcso...
[ message continues ]

What is it that ensures svsk->sk_inuse isn't incremented or SK_BUSY set

after that test?  Not all the code that does either of those is under

the same serv->sv_lock lock that this code is.
--b.

-

This should not matter - SK_CLOSED may be set at any time.
svc_age_temp_sockets only detaches the socket, sets SK_CLOSED and then

enqueues it. If SK_BUSY is set its already enqueued and svc_sock_enqueue

ensures that it is not enqueued twice.
Regards,

--

Wolfgang Walter

Studentenwerk München

Anstalt des öffentlichen Rechts

-

Oh, got it.  And the list manipulation is safe thanks to sv_lock.  Neat,

thanks.  Can you verify that this solves your problem?
--b.

-

Patch works fine here.
Regards,

--

Wolfgang Walter

Studentenwerk München

Anstalt des öffentlichen Rechts

-

Great, thanks again!
--b.

-

I'll test it tomorrow. So friday morning I'll know and mail you for sure.
Regards,

--

Wolfgang Walter

Studentenwerk München

Anstalt des öffentlichen Rechts

-