| Message ID | 62bcfc44-aef4-2536-a2da-acc8a68286de@kernel.dk (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | 9p/client: don't assume signal_pending() clears on recalc_sigpending() | expand |
On Fri, Feb 03, 2023 at 07:02:42AM -0700, Jens Axboe wrote: > signal_pending() really means that an exit to userspace is required to > clear the condition, as it could be either an actual signal, or it could > be TWA_SIGNAL based task_work that needs processing. The 9p client > does a recalc_sigpending() to take care of the former, but that still > leaves TWA_SIGNAL task_work. The result is that if we do have TWA_SIGNAL > task_work pending, then we'll sit in a tight loop spinning as > signal_pending() remains true even after recalc_sigpending(). > > Move the signal_pending() logic into a helper that deals with both. > > Link: https://lore.kernel.org/lkml/Y9TgUupO5C39V%2FDW@xpf.sh.intel.com/ > Reported-and-tested-by: Pengfei Xu <pengfei.xu@intel.com> > Signed-off-by: Jens Axboe <axboe@kernel.dk> > > --- > > diff --git a/net/9p/client.c b/net/9p/client.c > index 622ec6a586ee..7d9b9c150d47 100644 > --- a/net/9p/client.c > +++ b/net/9p/client.c ... > @@ -652,6 +653,28 @@ static struct p9_req_t *p9_client_prepare_req(struct p9_client *c, > return ERR_PTR(err); > } > > +static bool p9_sigpending(void) > +{ > + if (!signal_pending(current)) > + return false; > + > + /* > + * signal_pending() could mean either a real signal pending, or > + * TWA_SIGNAL based task_work that needs processing. Don't return > + * true for just the latter, run and clear it before a wait. > + */ nit: /* Multi-line comments in networking code, * are like this. */ ...
diff --git a/net/9p/client.c b/net/9p/client.c index 622ec6a586ee..7d9b9c150d47 100644 --- a/net/9p/client.c +++ b/net/9p/client.c @@ -18,6 +18,7 @@ #include <linux/sched/signal.h> #include <linux/uaccess.h> #include <linux/uio.h> +#include <linux/task_work.h> #include <net/9p/9p.h> #include <linux/parser.h> #include <linux/seq_file.h> @@ -652,6 +653,28 @@ static struct p9_req_t *p9_client_prepare_req(struct p9_client *c, return ERR_PTR(err); } +static bool p9_sigpending(void) +{ + if (!signal_pending(current)) + return false; + + /* + * signal_pending() could mean either a real signal pending, or + * TWA_SIGNAL based task_work that needs processing. Don't return + * true for just the latter, run and clear it before a wait. + */ + if (test_thread_flag(TIF_NOTIFY_SIGNAL)) + clear_notify_signal(); + if (task_work_pending(current)) + task_work_run(); + if (signal_pending(current)) { + clear_thread_flag(TIF_SIGPENDING); + return true; + } + + return false; +} + /** * p9_client_rpc - issue a request and wait for a response * @c: client session @@ -687,12 +710,7 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...) req->tc.zc = false; req->rc.zc = false; - if (signal_pending(current)) { - sigpending = 1; - clear_thread_flag(TIF_SIGPENDING); - } else { - sigpending = 0; - } + sigpending = p9_sigpending(); err = c->trans_mod->request(c, req); if (err < 0) { @@ -789,12 +807,7 @@ static struct p9_req_t *p9_client_zc_rpc(struct p9_client *c, int8_t type, req->tc.zc = true; req->rc.zc = true; - if (signal_pending(current)) { - sigpending = 1; - clear_thread_flag(TIF_SIGPENDING); - } else { - sigpending = 0; - } + sigpending = p9_sigpending(); err = c->trans_mod->zc_request(c, req, uidata, uodata, inlen, olen, in_hdrlen);
signal_pending() really means that an exit to userspace is required to clear the condition, as it could be either an actual signal, or it could be TWA_SIGNAL based task_work that needs processing. The 9p client does a recalc_sigpending() to take care of the former, but that still leaves TWA_SIGNAL task_work. The result is that if we do have TWA_SIGNAL task_work pending, then we'll sit in a tight loop spinning as signal_pending() remains true even after recalc_sigpending(). Move the signal_pending() logic into a helper that deals with both. Link: https://lore.kernel.org/lkml/Y9TgUupO5C39V%2FDW@xpf.sh.intel.com/ Reported-and-tested-by: Pengfei Xu <pengfei.xu@intel.com> Signed-off-by: Jens Axboe <axboe@kernel.dk> ---