Skip to contentSkip to conversation

knfsd: remove unnecessary BUG_ON's

PATCH In review J. Bruce Fields 2 participants 4 messages Sep 12, 2007 – Sep 14, 2007

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 knfsd: remove unnecessary BUG_ON's

Trivial patch, but--any objection to removing these assertions?

We set svsk->sk_pool = pool just 30-some lines above, and there's no obvious reason this should have changed, or any other particular reason to assert this here.

---
 net/sunrpc/svcsock.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)
net/sunrpc/svcsock.c +0 -2
index 12ff5da..d064879 100644
@@ -298,12 +298,10 @@ svc_sock_enqueue(struct svc_sock *svsk)
 		atomic_inc(&svsk->sk_inuse);
 		rqstp->rq_reserved = serv->sv_max_mesg;
 		atomic_add(rqstp->rq_reserved, &svsk->sk_reserved);
-		BUG_ON(svsk->sk_pool != pool);
 		wake_up(&rqstp->rq_wait);
 	} else {
 		dprintk("svc: socket %p put into queue\n", svsk->sk_sk);
 		list_add_tail(&svsk->sk_ready, &pool->sp_sockets);
-		BUG_ON(svsk->sk_pool != pool);
 	}
 
 out_unlock:
Signature · 10 lines
1.5.3.1.40.g6972


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
NFS maillist  -  [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
#1 lore raw
Quoted 5 lines · J. Bruce Fields

Trivial patch, but--any objection to removing these assertions?

We set svsk->sk_pool = pool just 30-some lines above, and there's no obvious reason this should have changed, or any other particular reason to assert this here.

Yeah, you'd think that. However we saw these BUG_ON()s tripped when attempting to port the 2.6.18-based NFS/RDMA patches to a 2.6.19-like kernel. All it takes is for some transport code to get confused about the proper way to clear SK_BUSY, and *blam*. So I think they're useful.

Greg.

Signature · 11 lines
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: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
NFS maillist  -  [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
#2 lore raw
Quoted 7 lines · Greg Banks
Quoted 5 lines · J. Bruce Fields

Trivial patch, but--any objection to removing these assertions?

We set svsk->sk_pool = pool just 30-some lines above, and there's no obvious reason this should have changed, or any other particular reason to assert this here.

Quoted 5 lines

Yeah, you'd think that. However we saw these BUG_ON()s tripped when attempting to port the 2.6.18-based NFS/RDMA patches to a 2.6.19-like kernel. All it takes is for some transport code to get confused about the proper way to clear SK_BUSY, and *blam*. So I think they're useful.

OK, I'll take your word for it--dropped. (But, I'm just curious--do you remember what the sequence of events was?)

--b.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

#3 lore raw
Quoted 8 lines · J. Bruce Fields
Quoted 7 lines · Greg Banks
Quoted 5 lines · J. Bruce Fields

Trivial patch, but--any objection to removing these assertions?

We set svsk->sk_pool = pool just 30-some lines above, and there's no obvious reason this should have changed, or any other particular reason to assert this here.

Quoted 8 lines
Quoted 5 lines

Yeah, you'd think that. However we saw these BUG_ON()s tripped when attempting to port the 2.6.18-based NFS/RDMA patches to a 2.6.19-like kernel. All it takes is for some transport code to get confused about the proper way to clear SK_BUSY, and *blam*. So I think they're useful.

OK, I'll take your word for it--dropped. (But, I'm just curious--do you remember what the sequence of events was?)

It's been a while, but IIRC the new transport code had copied out the internals of svc_request_received() pre .19, and were manually clearing SK_BUSY and calling svc_sock_enqueue(). In .19 when sk_pool was added, svc_request_received() began setting sk_pool to NULL, and the transport code wasn't doing that. Or maybe that was a different failure mode ;-)

Greg.

Signature · 11 lines
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: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
NFS maillist  -  [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs