[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <24add543-e230-7eca-e96b-7253f620b570@kernel.dk>
Date: Mon, 6 Feb 2023 14:58:26 -0700
From: Jens Axboe <axboe@...nel.dk>
To: Dominique Martinet <asmadeus@...ewreck.org>
Cc: Christian Schoenebeck <linux_oss@...debyte.com>,
Eric Van Hensbergen <ericvh@...nel.org>,
netdev <netdev@...r.kernel.org>,
Jakub Kicinski <kuba@...nel.org>,
Pengfei Xu <pengfei.xu@...el.com>,
v9fs-developer@...ts.sourceforge.net
Subject: Re: [PATCH v2] 9p/client: don't assume signal_pending() clears on
recalc_sigpending()
>> Sorry I didn't develop that idea; the signal path resets the pending
>> signal when we're done, I assumed we could also reset the TWA_SIGNAL
>> flag when we're done flushing. That might take a while though, so it's
>> far from optimal.
>
> Sure, if you set it again when done, then it will probably work just
> fine. But you need to treat TIF_NOTIFY_SIGNAL and TIF_SIGPENDING
> separately. An attempt at that at the end of this email, totally
> untested, and I'm not certain it's a good idea at all (see below). Is
> there a reason why we can't exit and get the task_work processed
> instead? That'd be greatly preferable.
Forgot to include it, but as mentioned, don't think it's a sane idea...
diff --git a/net/9p/client.c b/net/9p/client.c
index 622ec6a586ee..e4ff2773e00b 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -652,6 +652,33 @@ static struct p9_req_t *p9_client_prepare_req(struct p9_client *c,
return ERR_PTR(err);
}
+static void p9_clear_sigpending(int *sigpending, int *notifypending)
+{
+ if (signal_pending(current)) {
+ *sigpending = test_thread_flag(TIF_SIGPENDING);
+ if (*sigpending)
+ clear_thread_flag(TIF_SIGPENDING);
+ *notifypending = test_thread_flag(TIF_NOTIFY_SIGNAL);
+ if (*notifypending)
+ clear_thread_flag(TIF_NOTIFY_SIGNAL);
+ } else {
+ *sigpending = *notifypending = 0;
+ }
+}
+
+static void p9_reset_sigpending(int sigpending, int notifypending)
+{
+ unsigned long flags;
+
+ if (sigpending) {
+ spin_lock_irqsave(¤t->sighand->siglock, flags);
+ recalc_sigpending();
+ spin_unlock_irqrestore(¤t->sighand->siglock, flags);
+ }
+ if (notifypending)
+ set_tsk_thread_flag(current, TIF_NOTIFY_SIGNAL);
+}
+
/**
* p9_client_rpc - issue a request and wait for a response
* @c: client session
@@ -665,8 +692,7 @@ static struct p9_req_t *
p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
{
va_list ap;
- int sigpending, err;
- unsigned long flags;
+ int sigpending, notifypending, err;
struct p9_req_t *req;
/* Passing zero for tsize/rsize to p9_client_prepare_req() tells it to
* auto determine an appropriate (small) request/response size
@@ -687,12 +713,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;
- }
+ p9_clear_sigpending(&sigpending, ¬ifypending);
err = c->trans_mod->request(c, req);
if (err < 0) {
@@ -714,8 +735,7 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
if (err == -ERESTARTSYS && c->status == Connected &&
type == P9_TFLUSH) {
- sigpending = 1;
- clear_thread_flag(TIF_SIGPENDING);
+ p9_clear_sigpending(&sigpending, ¬ifypending);
goto again;
}
@@ -725,8 +745,7 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
}
if (err == -ERESTARTSYS && c->status == Connected) {
p9_debug(P9_DEBUG_MUX, "flushing\n");
- sigpending = 1;
- clear_thread_flag(TIF_SIGPENDING);
+ p9_clear_sigpending(&sigpending, ¬ifypending);
if (c->trans_mod->cancel(c, req))
p9_client_flush(c, req);
@@ -736,11 +755,7 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
err = 0;
}
recalc_sigpending:
- if (sigpending) {
- spin_lock_irqsave(¤t->sighand->siglock, flags);
- recalc_sigpending();
- spin_unlock_irqrestore(¤t->sighand->siglock, flags);
- }
+ p9_reset_sigpending(sigpending, notifypending);
if (err < 0)
goto reterr;
@@ -773,8 +788,7 @@ static struct p9_req_t *p9_client_zc_rpc(struct p9_client *c, int8_t type,
const char *fmt, ...)
{
va_list ap;
- int sigpending, err;
- unsigned long flags;
+ int sigpending, notifypending, err;
struct p9_req_t *req;
va_start(ap, fmt);
@@ -789,12 +803,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;
- }
+ p9_clear_sigpending(&sigpending, ¬ifypending);
err = c->trans_mod->zc_request(c, req, uidata, uodata,
inlen, olen, in_hdrlen);
@@ -810,8 +819,7 @@ static struct p9_req_t *p9_client_zc_rpc(struct p9_client *c, int8_t type,
}
if (err == -ERESTARTSYS && c->status == Connected) {
p9_debug(P9_DEBUG_MUX, "flushing\n");
- sigpending = 1;
- clear_thread_flag(TIF_SIGPENDING);
+ p9_clear_sigpending(&sigpending, ¬ifypending);
if (c->trans_mod->cancel(c, req))
p9_client_flush(c, req);
@@ -821,11 +829,7 @@ static struct p9_req_t *p9_client_zc_rpc(struct p9_client *c, int8_t type,
err = 0;
}
recalc_sigpending:
- if (sigpending) {
- spin_lock_irqsave(¤t->sighand->siglock, flags);
- recalc_sigpending();
- spin_unlock_irqrestore(¤t->sighand->siglock, flags);
- }
+ p9_reset_sigpending(sigpending, notifypending);
if (err < 0)
goto reterr;
--
Jens Axboe
Powered by blists - more mailing lists