[{"id":1760996,"web_url":"http://patchwork.ozlabs.org/comment/1760996/","msgid":"<20170831133353.GB22739@kroah.com>","list_archive_url":null,"date":"2017-08-31T13:33:53","subject":"Re: [PATCH 24/25] net/cdc_ncm: Replace tasklet with softirq hrtimer","submitter":{"id":11800,"url":"http://patchwork.ozlabs.org/api/people/11800/","name":"Greg Kroah-Hartman","email":"gregkh@linuxfoundation.org"},"content":"On Thu, Aug 31, 2017 at 12:23:46PM -0000, Anna-Maria Gleixner wrote:\n> From: Thomas Gleixner \n> \n> The bh tasklet is used in invoke the hrtimer (cdc_ncm_tx_timer_cb) in\n> softirq context. This can be also achieved without the tasklet but with\n> CLOCK_MONOTONIC_SOFT as hrtimer base.\n> \n> Signed-off-by: Thomas Gleixner \n> Signed-off-by: Anna-Maria Gleixner \n> Cc: Oliver Neukum \n> Cc: Greg Kroah-Hartman \n> Cc: linux-usb@vger.kernel.org\n> Cc: netdev@vger.kernel.org\n\n\nAcked-by: Greg Kroah-Hartman ","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xjjyN3CCpz9s75\n\tfor ;\n\tThu, 31 Aug 2017 23:34:04 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751242AbdHaNdv (ORCPT );\n\tThu, 31 Aug 2017 09:33:51 -0400","from mail.linuxfoundation.org ([140.211.169.12]:51912 \"EHLO\n\tmail.linuxfoundation.org\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750895AbdHaNdu (ORCPT\n\t); Thu, 31 Aug 2017 09:33:50 -0400","from localhost (LFbn-1-12253-150.w90-92.abo.wanadoo.fr\n\t[90.92.67.150])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPSA id CF913258;\n\tThu, 31 Aug 2017 13:33:48 +0000 (UTC)"],"Date":"Thu, 31 Aug 2017 15:33:53 +0200","From":"Greg Kroah-Hartman ","To":"Anna-Maria Gleixner ","Cc":"LKML ,\n\tPeter Zijlstra , Ingo Molnar ,\n\tChristoph Hellwig , keescook@chromium.org,\n\tJohn Stultz ,\n\tThomas Gleixner ,\n\tOliver Neukum , linux-usb@vger.kernel.org,\n\tnetdev@vger.kernel.org","Subject":"Re: [PATCH 24/25] net/cdc_ncm: Replace tasklet with softirq hrtimer","Message-ID":"<20170831133353.GB22739@kroah.com>","References":"<20170831105725.809317030@linutronix.de>\n\t<20170831105827.479650817@linutronix.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170831105827.479650817@linutronix.de>","User-Agent":"Mutt/1.8.3 (2017-05-23)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1761027,"web_url":"http://patchwork.ozlabs.org/comment/1761027/","msgid":"<87mv6fzvpr.fsf@miraculix.mork.no>","list_archive_url":null,"date":"2017-08-31T13:57:04","subject":"Re: [PATCH 24/25] net/cdc_ncm: Replace tasklet with softirq hrtimer","submitter":{"id":2061,"url":"http://patchwork.ozlabs.org/api/people/2061/","name":"Bjørn Mork","email":"bjorn@mork.no"},"content":"Anna-Maria Gleixner writes:\n\n> From: Thomas Gleixner \n>\n> The bh tasklet is used in invoke the hrtimer (cdc_ncm_tx_timer_cb) in\n> softirq context. This can be also achieved without the tasklet but with\n> CLOCK_MONOTONIC_SOFT as hrtimer base.\n>\n> Signed-off-by: Thomas Gleixner \n> Signed-off-by: Anna-Maria Gleixner \n> Cc: Oliver Neukum \n> Cc: Greg Kroah-Hartman \n> Cc: linux-usb@vger.kernel.org\n> Cc: netdev@vger.kernel.org\n> ---\n> drivers/net/usb/cdc_ncm.c | 37 ++++++++++++++++---------------------\n> include/linux/usb/cdc_ncm.h | 2 +-\n> 2 files changed, 17 insertions(+), 22 deletions(-)\n>\n> --- a/drivers/net/usb/cdc_ncm.c\n> +++ b/drivers/net/usb/cdc_ncm.c\n> @@ -61,7 +61,6 @@ static bool prefer_mbim;\n> module_param(prefer_mbim, bool, S_IRUGO | S_IWUSR);\n> MODULE_PARM_DESC(prefer_mbim, \"Prefer MBIM setting on dual NCM/MBIM functions\");\n> \n> -static void cdc_ncm_txpath_bh(unsigned long param);\n> static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx);\n> static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *hr_timer);\n> static struct usb_driver cdc_ncm_driver;\n> @@ -777,10 +776,9 @@ int cdc_ncm_bind_common(struct usbnet *d\n> \tif (!ctx)\n> \t\treturn -ENOMEM;\n> \n> -\thrtimer_init(&ctx->tx_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);\n> +\thrtimer_init(&ctx->tx_timer, CLOCK_MONOTONIC_SOFT, HRTIMER_MODE_REL);\n> \tctx->tx_timer.function = &cdc_ncm_tx_timer_cb;\n> -\tctx->bh.data = (unsigned long)dev;\n> -\tctx->bh.func = cdc_ncm_txpath_bh;\n> +\tctx->usbnet = dev;\n> \tatomic_set(&ctx->stop, 0);\n> \tspin_lock_init(&ctx->mtx);\n> \n> @@ -967,10 +965,7 @@ void cdc_ncm_unbind(struct usbnet *dev,\n> \n> \tatomic_set(&ctx->stop, 1);\n> \n> -\tif (hrtimer_active(&ctx->tx_timer))\n> -\t\thrtimer_cancel(&ctx->tx_timer);\n> -\n> -\ttasklet_kill(&ctx->bh);\n> +\thrtimer_cancel(&ctx->tx_timer);\n> \n> \t/* handle devices with combined control and data interface */\n> \tif (ctx->control == ctx->data)\n> @@ -1348,20 +1343,9 @@ static void cdc_ncm_tx_timeout_start(str\n> \t\t\t\tHRTIMER_MODE_REL);\n> }\n> \n> -static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *timer)\n> +static void cdc_ncm_txpath_bh(struct cdc_ncm_ctx *ctx)\n> {\n> -\tstruct cdc_ncm_ctx *ctx =\n> -\t\t\tcontainer_of(timer, struct cdc_ncm_ctx, tx_timer);\n> -\n> -\tif (!atomic_read(&ctx->stop))\n> -\t\ttasklet_schedule(&ctx->bh);\n> -\treturn HRTIMER_NORESTART;\n> -}\n> -\n> -static void cdc_ncm_txpath_bh(unsigned long param)\n> -{\n> -\tstruct usbnet *dev = (struct usbnet *)param;\n> -\tstruct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];\n> +\tstruct usbnet *dev = ctx->usbnet;\n> \n> \tspin_lock_bh(&ctx->mtx);\n> \tif (ctx->tx_timer_pending != 0) {\n> @@ -1379,6 +1363,17 @@ static void cdc_ncm_txpath_bh(unsigned l\n> \t}\n> }\n> \n> +static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *timer)\n> +{\n> +\tstruct cdc_ncm_ctx *ctx =\n> +\t\t\tcontainer_of(timer, struct cdc_ncm_ctx, tx_timer);\n> +\n> +\tif (!atomic_read(&ctx->stop))\n> +\t\tcdc_ncm_txpath_bh(ctx);\n> +\n> +\treturn HRTIMER_NORESTART;\n> +}\n> +\n> struct sk_buff *\n> cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)\n> {\n> --- a/include/linux/usb/cdc_ncm.h\n> +++ b/include/linux/usb/cdc_ncm.h\n> @@ -92,7 +92,6 @@\n> struct cdc_ncm_ctx {\n> \tstruct usb_cdc_ncm_ntb_parameters ncm_parm;\n> \tstruct hrtimer tx_timer;\n> -\tstruct tasklet_struct bh;\n> \n> \tconst struct usb_cdc_ncm_desc *func_desc;\n> \tconst struct usb_cdc_mbim_desc *mbim_desc;\n> @@ -101,6 +100,7 @@ struct cdc_ncm_ctx {\n> \n> \tstruct usb_interface *control;\n> \tstruct usb_interface *data;\n> +\tstruct usbnet *usbnet;\n> \n> \tstruct sk_buff *tx_curr_skb;\n> \tstruct sk_buff *tx_rem_skb;\n\n\n\n\nI believe the struct usbnet pointer is redundant. We already have lots\nof pointers back and forth here. This should work, but is not tested:\n\n\tstruct usbnet *dev = usb_get_intfdata(ctx->control):\n\n\n\n\nBjørn","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (1024-bit key;\n\tsecure) header.d=mork.no header.i=@mork.no header.b=\"P2AszCxX\";\n\tdkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xjkV758vPz9sPm\n\tfor ;\n\tThu, 31 Aug 2017 23:58:07 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751662AbdHaN5z (ORCPT );\n\tThu, 31 Aug 2017 09:57:55 -0400","from canardo.mork.no ([148.122.252.1]:39921 \"EHLO canardo.mork.no\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751011AbdHaN5x (ORCPT );\n\tThu, 31 Aug 2017 09:57:53 -0400","from miraculix.mork.no (miraculix.mork.no\n\t[IPv6:2001:4641:0:2:7627:374e:db74:e353]) (authenticated bits=0)\n\tby canardo.mork.no (8.15.2/8.15.2) with ESMTPSA id v7VDv5va020038\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=NO); Thu, 31 Aug 2017 15:57:05 +0200","from bjorn by miraculix.mork.no with local (Exim 4.89)\n\t(envelope-from )\n\tid 1dnPxw-0006gv-H7; Thu, 31 Aug 2017 15:57:04 +0200"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=mork.no; s=b;\n\tt=1504187826; bh=sRZdbuCMvUgkM5961tOsV2VwywrZjBtIGfI47YaUJyw=;\n\th=From:To:Cc:Subject:References:Date:Message-ID:From;\n\tb=P2AszCxXYhK4QOXcpjSehO15CwUASeKpJXhrTG+QHZztKwacOBJgx8gI4jJH6wqdL\n\tJXCm4zsHUUfmn/J1YWpnL66WACRaZT/LPBZHiazZL22M+pOHIGjGQqHrnsWZ7KMO8M\n\tONd8J+0h18FIoy6Yw3r6uRyRM24LuqNGlXYa9UlE=","From":"=?utf-8?q?Bj=C3=B8rn_Mork?= ","To":"Anna-Maria Gleixner ","Cc":"LKML ,\n\tPeter Zijlstra , Ingo Molnar ,\n\tChristoph Hellwig , keescook@chromium.org,\n\tJohn Stultz ,\n\tThomas Gleixner , Oliver Neukum ,\n\tGreg Kroah-Hartman ,\n\tlinux-usb@vger.kernel.org, netdev@vger.kernel.org","Subject":"Re: [PATCH 24/25] net/cdc_ncm: Replace tasklet with softirq hrtimer","Organization":"m","References":"<20170831105725.809317030@linutronix.de>\n\t<20170831105827.479650817@linutronix.de>","Date":"Thu, 31 Aug 2017 15:57:04 +0200","In-Reply-To":"<20170831105827.479650817@linutronix.de> (Anna-Maria Gleixner's\n\tmessage of \"Thu, 31 Aug 2017 12:23:46 -0000\")","Message-ID":"<87mv6fzvpr.fsf@miraculix.mork.no>","User-Agent":"Gnus/5.130015 (Ma Gnus v0.15) Emacs/24.5 (gnu/linux)","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-Virus-Scanned":"clamav-milter 0.99.2 at canardo","X-Virus-Status":"Clean","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}}]