On Tue, Oct 05, 2010 at 10:22:30AM +0200, Menyhart Zoltan wrote:
> Due to some race conditions, the reference count can become 0
> while "xprt" is still on a "pool":
Apologies, your email got buried in my inbox....
>
> WARNING: at lib/kref.c:43 kref_get+0x23/0x2d()
> [] kref_get+0x23/0x2d
> [] svc_xprt_get+0x12/0x14 [sunrpc]
> [] svc_recv+0x2db/0x78a [sunrpc]
Which kernel exactly did you see this on? Is it reproduceable?
> I think we should increase the reference counter before adding "xprt"
> onto any list.
I don't see the xprt added to any list after the svc_xprt_get() you've
added below.
Help, I'm confused!
--b.
>
> Obviously, we keep increasing the reference count before passing "xprt"
> to another thread via "rqstp->rq_xprt".
>
> Thanks,
>
> Zoltan Menyhart
> "svc_xprt_get()" should be invoked before adding "xprt" onto "pool" by
> "svc_xprt_enqueue()".
> Otherwise "xprt->xpt_ref" can drop to 0 due to race conditions.
>
> Signed-off-by: Zoltan Menyhart <Zoltan.Menyhart@xxxxxxxx>
>
> --- old/net/sunrpc/svc_xprt.c 2010-10-05 09:47:51.000000000 +0200
> +++ new/net/sunrpc/svc_xprt.c 2010-10-05 09:47:20.000000000 +0200
> @@ -369,6 +369,7 @@
> }
>
> process:
> + svc_xprt_get(xprt);
> if (!list_empty(&pool->sp_threads)) {
> rqstp = list_entry(pool->sp_threads.next,
> struct svc_rqst,
> @@ -381,7 +382,6 @@
> "svc_xprt_enqueue: server %p, rq_xprt=%p!\n",
> rqstp, rqstp->rq_xprt);
> rqstp->rq_xprt = xprt;
> - svc_xprt_get(xprt);
> rqstp->rq_reserved = serv->sv_max_mesg;
> atomic_add(rqstp->rq_reserved, &xprt->xpt_reserved);
> pool->sp_stats.threads_woken++;
> @@ -655,7 +655,6 @@
> xprt = svc_xprt_dequeue(pool);
> if (xprt) {
> rqstp->rq_xprt = xprt;
> - svc_xprt_get(xprt);
> rqstp->rq_reserved = serv->sv_max_mesg;
> atomic_add(rqstp->rq_reserved, &xprt->xpt_reserved);
> } else {
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
