From patchwork Thu Aug 25 01:52:30 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Santosh Shilimkar X-Patchwork-Id: 9298717 X-Patchwork-Delegate: herbert@gondor.apana.org.au Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 166C6607F0 for ; Thu, 25 Aug 2016 06:02:20 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 058A2291CA for ; Thu, 25 Aug 2016 06:02:20 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id EDB11291CC; Thu, 25 Aug 2016 06:02:19 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B108D291CA for ; Thu, 25 Aug 2016 06:02:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757785AbcHYGBr (ORCPT ); Thu, 25 Aug 2016 02:01:47 -0400 Received: from userp1050.oracle.com ([156.151.31.82]:32886 "EHLO userp1050.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754434AbcHYGBm (ORCPT ); Thu, 25 Aug 2016 02:01:42 -0400 Received: from userp1040.oracle.com (userp1040.oracle.com [156.151.31.81]) by userp1050.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id u7P1sdWS002158 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 25 Aug 2016 01:54:40 GMT Received: from aserv0022.oracle.com (aserv0022.oracle.com [141.146.126.234]) by userp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id u7P1qhVS005360 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 25 Aug 2016 01:52:44 GMT Received: from userv0122.oracle.com (userv0122.oracle.com [156.151.31.75]) by aserv0022.oracle.com (8.14.4/8.14.4) with ESMTP id u7P1qhaA017876 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 25 Aug 2016 01:52:43 GMT Received: from abhmp0011.oracle.com (abhmp0011.oracle.com [141.146.116.17]) by userv0122.oracle.com (8.14.4/8.14.4) with ESMTP id u7P1qcfB027269; Thu, 25 Aug 2016 01:52:39 GMT Received: from localhost.localdomain (/10.159.236.54) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Wed, 24 Aug 2016 18:52:38 -0700 From: Santosh Shilimkar To: linux-kernel@vger.kernel.org Cc: santosh.shilimkar@oracle.com, qat-linux@intel.com, linux-crypto@vger.kernel.org, netdev@vger.kernel.org, Santosh Shilimkar , Greg Kroah-Hartman , Andrew Morton , Thomas Gleixner , Tadeusz Struk , Herbert Xu , "David S. Miller" , Paul Bolle , Giovanni Cabiddu , Salvatore Benedetto , Karsten Keil , "Peter Zijlstra (Intel)" Subject: [PATCH] softirq: fix tasklet_kill() and its users Date: Wed, 24 Aug 2016 18:52:30 -0700 Message-Id: <1472089950-2724-1-git-send-email-ssantosh@kernel.org> X-Mailer: git-send-email 1.9.1 X-Source-IP: userp1040.oracle.com [156.151.31.81] Sender: linux-crypto-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-crypto@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 tasklet_init() API exibit above behavior but not the tasklet_kill(). The tasklet handler can still get scheduled and run even after the tasklet_kill(). There are 2, 3 places where drivers are working around this issue by calling tasklet_disable() which will add an usecount and there by avoiding the handlers being called. tasklet_enable/tasklet_disable is a pair API and expected to be used together. Usage of tasklet_disable() *just* to workround tasklet scheduling after kill is probably not the correct and inteded use of the API as done the API. We also happen to see similar issue where in shutdown path the tasklet_handler was getting called even after the tasklet_kill(). We fix this be making sure tasklet_kill() does right thing and there by ensuring tasklet handler won't run after tasklet_kil() with very simple change. Patch fixes the tasklet code and also few drivers workarounds. Cc: Greg Kroah-Hartman Cc: Andrew Morton Cc: Thomas Gleixner Cc: Tadeusz Struk Cc: Herbert Xu Cc: "David S. Miller" Cc: Paul Bolle Cc: Giovanni Cabiddu Cc: Salvatore Benedetto Cc: Karsten Keil Cc: "Peter Zijlstra (Intel)" Signed-off-by: Santosh Shilimkar --- Removed RFC tag from last post and dropped atmel serial driver which seems to have been fixed in 4.8 https://lkml.org/lkml/2016/8/7/7 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 --git a/drivers/crypto/qat/qat_common/adf_isr.c b/drivers/crypto/qat/qat_common/adf_isr.c index 06d4901..fd5e900 100644 --- a/drivers/crypto/qat/qat_common/adf_isr.c +++ b/drivers/crypto/qat/qat_common/adf_isr.c @@ -296,7 +296,6 @@ static void adf_cleanup_bh(struct adf_accel_dev *accel_dev) 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 --git a/drivers/crypto/qat/qat_common/adf_sriov.c b/drivers/crypto/qat/qat_common/adf_sriov.c index 9320ae1..bc7c2fa 100644 --- a/drivers/crypto/qat/qat_common/adf_sriov.c +++ b/drivers/crypto/qat/qat_common/adf_sriov.c @@ -204,7 +204,6 @@ void adf_disable_sriov(struct adf_accel_dev *accel_dev) } 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 --git a/drivers/crypto/qat/qat_common/adf_vf_isr.c b/drivers/crypto/qat/qat_common/adf_vf_isr.c index bf99e11..6e38bff 100644 --- a/drivers/crypto/qat/qat_common/adf_vf_isr.c +++ b/drivers/crypto/qat/qat_common/adf_vf_isr.c @@ -191,7 +191,6 @@ static int adf_setup_pf2vf_bh(struct adf_accel_dev *accel_dev) 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_accel_dev *accel_dev) { 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 --git a/drivers/isdn/gigaset/interface.c b/drivers/isdn/gigaset/interface.c index 600c79b..2ce63b6 100644 --- a/drivers/isdn/gigaset/interface.c +++ b/drivers/isdn/gigaset/interface.c @@ -524,7 +524,6 @@ void gigaset_if_free(struct cardstate *cs) 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 --git a/kernel/softirq.c b/kernel/softirq.c index 17caf4b..21397eb 100644 --- a/kernel/softirq.c +++ b/kernel/softirq.c @@ -498,7 +498,7 @@ static void tasklet_action(struct softirq_action *a) 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 softirq_action *a) 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, { 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 *t) } 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);