From: akpm@linux-foundation.org
To: ssantosh@kernel.org, davem@davemloft.net,
giovanni.cabiddu@intel.com, gregkh@linuxfoundation.org,
herbert@gondor.apana.org.au, isdn@linux-pingi.de, mingo@elte.hu,
pebolle@tiscali.nl, peterz@infradead.org,
salvatore.benedetto@intel.com, tadeusz.struk@intel.com,
tglx@linutronix.de, mm-commits@vger.kernel.org
Subject: + softirq-fix-tasklet_kill-and-its-users.patch added to -mm tree
Date: Tue, 20 Sep 2016 14:55:13 -0700 [thread overview]
Message-ID: <57e1b041.zRoBcsxStpPQoyeo%akpm@linux-foundation.org> (raw)
The patch titled
Subject: softirq: fix tasklet_kill() and its users
has been added to the -mm tree. Its filename is
softirq-fix-tasklet_kill-and-its-users.patch
This patch should soon appear at
http://ozlabs.org/~akpm/mmots/broken-out/softirq-fix-tasklet_kill-and-its-users.patch
and later at
http://ozlabs.org/~akpm/mmotm/broken-out/softirq-fix-tasklet_kill-and-its-users.patch
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/SubmitChecklist when testing your code ***
The -mm tree is included into linux-next and is updated
there every 3-4 working days
------------------------------------------------------
From: Santosh Shilimkar <ssantosh@kernel.org>
Subject: softirq: fix tasklet_kill() and its users
Semantically the expectation from the tasklet init/kill API should be as
below.
tasklet_init() == Init and Enable scheduling
tasklet_kill() == Disable scheduling and Destroy
The tasklet_init() API exibits the above behavior but tasklet_kill() does
not. The tasklet handler can still get scheduled and run even after
tasklet_kill().
There are 2, 3 places where drivers are working around this issue by
calling tasklet_disable() which will add a usecount and thereby preventing
the handlers from being called.
tasklet_enable/tasklet_disable is a pair API and they are expected to be
used together. Usage of tasklet_disable() *just* to work around tasklet
scheduling after a kill is probably not the correct and intended use of
the API. We also happen to see similar issues where in the shutdown path
the tasklet_handler is getting called even after the tasklet_kill().
We fix this by ensuring that tasklet_kill() does the right thing and
thereby ensures that the tasklet handler won't run after tasklet_kil()
with a very simple change. The patch fixes the tasklet code and also
removes a few driver workarounds.
Link: http://lkml.kernel.org/r/1472089950-2724-1-git-send-email-ssantosh@kernel.org
Signed-off-by: Santosh Shilimkar <ssantosh@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tadeusz Struk <tadeusz.struk@intel.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Paul Bolle <pebolle@tiscali.nl>
Cc: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
Cc: Salvatore Benedetto <salvatore.benedetto@intel.com>
Cc: Karsten Keil <isdn@linux-pingi.de>
Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org>
Cc: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
drivers/crypto/qat/qat_common/adf_isr.c | 1 -
drivers/crypto/qat/qat_common/adf_sriov.c | 1 -
drivers/crypto/qat/qat_common/adf_vf_isr.c | 2 --
drivers/isdn/gigaset/interface.c | 1 -
kernel/softirq.c | 7 ++++---
5 files changed, 4 insertions(+), 8 deletions(-)
diff -puN drivers/crypto/qat/qat_common/adf_isr.c~softirq-fix-tasklet_kill-and-its-users drivers/crypto/qat/qat_common/adf_isr.c
--- a/drivers/crypto/qat/qat_common/adf_isr.c~softirq-fix-tasklet_kill-and-its-users
+++ a/drivers/crypto/qat/qat_common/adf_isr.c
@@ -296,7 +296,6 @@ static void adf_cleanup_bh(struct adf_ac
int i;
for (i = 0; i < hw_data->num_banks; i++) {
- tasklet_disable(&priv_data->banks[i].resp_handler);
tasklet_kill(&priv_data->banks[i].resp_handler);
}
}
diff -puN drivers/crypto/qat/qat_common/adf_sriov.c~softirq-fix-tasklet_kill-and-its-users drivers/crypto/qat/qat_common/adf_sriov.c
--- a/drivers/crypto/qat/qat_common/adf_sriov.c~softirq-fix-tasklet_kill-and-its-users
+++ a/drivers/crypto/qat/qat_common/adf_sriov.c
@@ -204,7 +204,6 @@ void adf_disable_sriov(struct adf_accel_
}
for (i = 0, vf = accel_dev->pf.vf_info; i < totalvfs; i++, vf++) {
- tasklet_disable(&vf->vf2pf_bh_tasklet);
tasklet_kill(&vf->vf2pf_bh_tasklet);
mutex_destroy(&vf->pf2vf_lock);
}
diff -puN drivers/crypto/qat/qat_common/adf_vf_isr.c~softirq-fix-tasklet_kill-and-its-users drivers/crypto/qat/qat_common/adf_vf_isr.c
--- a/drivers/crypto/qat/qat_common/adf_vf_isr.c~softirq-fix-tasklet_kill-and-its-users
+++ a/drivers/crypto/qat/qat_common/adf_vf_isr.c
@@ -191,7 +191,6 @@ static int adf_setup_pf2vf_bh(struct adf
static void adf_cleanup_pf2vf_bh(struct adf_accel_dev *accel_dev)
{
- tasklet_disable(&accel_dev->vf.pf2vf_bh_tasklet);
tasklet_kill(&accel_dev->vf.pf2vf_bh_tasklet);
mutex_destroy(&accel_dev->vf.vf2pf_lock);
}
@@ -268,7 +267,6 @@ static void adf_cleanup_bh(struct adf_ac
{
struct adf_etr_data *priv_data = accel_dev->transport;
- tasklet_disable(&priv_data->banks[0].resp_handler);
tasklet_kill(&priv_data->banks[0].resp_handler);
}
diff -puN drivers/isdn/gigaset/interface.c~softirq-fix-tasklet_kill-and-its-users drivers/isdn/gigaset/interface.c
--- a/drivers/isdn/gigaset/interface.c~softirq-fix-tasklet_kill-and-its-users
+++ a/drivers/isdn/gigaset/interface.c
@@ -524,7 +524,6 @@ void gigaset_if_free(struct cardstate *c
if (!drv->have_tty)
return;
- tasklet_disable(&cs->if_wake_tasklet);
tasklet_kill(&cs->if_wake_tasklet);
cs->tty_dev = NULL;
tty_unregister_device(drv->tty, cs->minor_index);
diff -puN kernel/softirq.c~softirq-fix-tasklet_kill-and-its-users kernel/softirq.c
--- a/kernel/softirq.c~softirq-fix-tasklet_kill-and-its-users
+++ a/kernel/softirq.c
@@ -498,7 +498,7 @@ static void tasklet_action(struct softir
list = list->next;
if (tasklet_trylock(t)) {
- if (!atomic_read(&t->count)) {
+ if (atomic_read(&t->count) == 1) {
if (!test_and_clear_bit(TASKLET_STATE_SCHED,
&t->state))
BUG();
@@ -534,7 +534,7 @@ static void tasklet_hi_action(struct sof
list = list->next;
if (tasklet_trylock(t)) {
- if (!atomic_read(&t->count)) {
+ if (atomic_read(&t->count) == 1) {
if (!test_and_clear_bit(TASKLET_STATE_SCHED,
&t->state))
BUG();
@@ -559,7 +559,7 @@ void tasklet_init(struct tasklet_struct
{
t->next = NULL;
t->state = 0;
- atomic_set(&t->count, 0);
+ atomic_set(&t->count, 1);
t->func = func;
t->data = data;
}
@@ -576,6 +576,7 @@ void tasklet_kill(struct tasklet_struct
} while (test_bit(TASKLET_STATE_SCHED, &t->state));
}
tasklet_unlock_wait(t);
+ atomic_dec(&t->count);
clear_bit(TASKLET_STATE_SCHED, &t->state);
}
EXPORT_SYMBOL(tasklet_kill);
_
Patches currently in -mm which might be from ssantosh@kernel.org are
softirq-fix-tasklet_kill-and-its-users.patch
next reply other threads:[~2016-09-20 21:55 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-20 21:55 akpm [this message]
2016-09-21 5:18 ` + softirq-fix-tasklet_kill-and-its-users.patch added to -mm tree Sergey Senozhatsky
2016-09-21 8:09 ` Sergey Senozhatsky
2016-09-21 17:23 ` Santosh Shilimkar
2016-09-22 0:42 ` Sergey Senozhatsky
2016-09-22 2:31 ` Santosh Shilimkar
2016-09-22 7:05 ` Thomas Gleixner
2016-09-22 16:08 ` Santosh Shilimkar
2016-09-22 23:37 ` Stephen Rothwell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=57e1b041.zRoBcsxStpPQoyeo%akpm@linux-foundation.org \
--to=akpm@linux-foundation.org \
--cc=davem@davemloft.net \
--cc=giovanni.cabiddu@intel.com \
--cc=gregkh@linuxfoundation.org \
--cc=herbert@gondor.apana.org.au \
--cc=isdn@linux-pingi.de \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=mm-commits@vger.kernel.org \
--cc=pebolle@tiscali.nl \
--cc=peterz@infradead.org \
--cc=salvatore.benedetto@intel.com \
--cc=ssantosh@kernel.org \
--cc=tadeusz.struk@intel.com \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.