| Message ID | 201204052329.EBH52139.FFOLQMOJSVHOFt@I-love.SAKURA.ne.jp |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
Tetsuo Handa wrote: > Good. -512 is -ERESTARTSYS, and this hang occurs after -ERESTARTSYS is > returned. It indicates that c->trans_mod->request() is interrupted by signal. > Since c->trans_mod->request is pointing at p9_virtio_request, the location > returning that error would be (...snipped...) > 281 err = wait_event_interruptible(*chan->vc_wq, > 282 chan->ring_bufs_avail); > > here. Oops. Not p9_virtio_request(). It is p9_client_rpc(). I misread the output lines. > > > + printk("%u:req->status = %u\n", current->pid, req->status); > > > err = wait_event_interruptible(*req->wq, > > > req->status >= REQ_STATUS_RCVD); > > > + printk("%u:wait = %d\n", current->pid, err); But anyway, I think this is interupt related bug. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Tetsuo Handa wrote: > Most suspicious change is net/9p/client.c because it is changing handling of > ERESTARTSYS case. > > --- linux-3.3.1/net/9p/client.c > +++ linux-next/net/9p/client.c > @@ -740,10 +740,18 @@ > c->status = Disconnected; > goto reterr; > } > +again: > /* Wait for the response */ > err = wait_event_interruptible(*req->wq, > req->status >= REQ_STATUS_RCVD); > > + if ((err == -ERESTARTSYS) && (c->status == Connected) > + && (type == P9_TFLUSH)) { > + sigpending = 1; > + clear_thread_flag(TIF_SIGPENDING); > + goto again; > + } > + I think this loop is bad with regard to response to SIGKILL. If wait_event_interruptible() was interrupted by SIGKILL, it will spin until req->status >= REQ_STATUS_RCVD becomes true. Rather, if ((c->status == Connected) && (type == P9_TFLUSH)) err = wait_event_killable(*req->wq, req->status >= REQ_STATUS_RCVD); else err = wait_event_interruptible(*req->wq, req->status >= REQ_STATUS_RCVD); would be safer. > error: > /* > * Fid is not valid even after a failed clunk > + * If interrupted, retry once then give up and > + * leak fid until umount. > */ > - p9_fid_destroy(fid); > + if (err == -ERESTARTSYS) { > + if (retries++ == 0) > + goto again; I think it is possible that the process is interrupted again upon retrying. I suspect the handling of err == -ERESTARTSYS case when retries != 0. It is returning without calling p9_fid_destroy(), which will be unexpected behaviour for the various callers. > + } else > + p9_fid_destroy(fid); > return err; > } > EXPORT_SYMBOL(p9_client_clunk); -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- linux-3.3.1/net/9p/client.c +++ linux-next/net/9p/client.c @@ -740,10 +740,18 @@ c->status = Disconnected; goto reterr; } +again: /* Wait for the response */ err = wait_event_interruptible(*req->wq, req->status >= REQ_STATUS_RCVD); + if ((err == -ERESTARTSYS) && (c->status == Connected) + && (type == P9_TFLUSH)) { + sigpending = 1; + clear_thread_flag(TIF_SIGPENDING); + goto again; + } + if (req->status == REQ_STATUS_ERROR) { p9_debug(P9_DEBUG_ERROR, "req_status error %d\n", req->t_err); err = req->t_err; @@ -1420,6 +1428,7 @@ int err; struct p9_client *clnt; struct p9_req_t *req; + int retries = 0; if (!fid) { pr_warn("%s (%d): Trying to clunk with NULL fid\n", @@ -1428,7 +1437,9 @@ return 0; } - p9_debug(P9_DEBUG_9P, ">>> TCLUNK fid %d\n", fid->fid); +again: + p9_debug(P9_DEBUG_9P, ">>> TCLUNK fid %d (try %d)\n", fid->fid, + retries); err = 0; clnt = fid->clnt; @@ -1444,8 +1455,14 @@ error: /* * Fid is not valid even after a failed clunk + * If interrupted, retry once then give up and + * leak fid until umount. */ - p9_fid_destroy(fid); + if (err == -ERESTARTSYS) { + if (retries++ == 0) + goto again; + } else + p9_fid_destroy(fid); return err; } EXPORT_SYMBOL(p9_client_clunk); @@ -1470,7 +1487,10 @@ p9_free_req(clnt, req); error: - p9_fid_destroy(fid); + if (err == -ERESTARTSYS) + p9_client_clunk(fid); + else + p9_fid_destroy(fid); return err; } EXPORT_SYMBOL(p9_client_remove);