From: David Miller <davem@davemloft.net>
To: govindarajulu90@gmail.com
Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org, schwidefsky@de.ibm.com,
linville@tuxdriver.com, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, IvDoorn@gmail.com, sbhatewara@vmware.com,
samuel@sortiz.org, chas@cmf.nrl.navy.mil, roland@kernel.org,
isdn@linux-pingi.de, jcliburn@gmail.com, benve@cisco.com,
ssujith@cisco.com, jeffrey.t.kirsher@intel.com,
jesse.brandeburg@intel.com, shahed.shaikh@qlogic.com,
joe@perches.com, apw@canonical.com
Subject: Re: [PATCH net-next 02/13] driver: net: remove unnecessary skb NULL check before calling dev_kfree_skb_irq
Date: Mon, 04 Nov 2013 15:12:30 -0500 (EST) [thread overview]
Message-ID: <20131104.151230.1978898006990867916.davem@davemloft.net> (raw)
In-Reply-To: <1383400074-30555-3-git-send-email-govindarajulu90@gmail.com>
From: Govindarajulu Varadarajan <govindarajulu90@gmail.com>
Date: Sat, 2 Nov 2013 19:17:43 +0530
> @@ -1030,10 +1030,8 @@ static void ni65_xmit_intr(struct net_device *dev,int csr0)
> }
>
> #ifdef XMT_VIA_SKB
> - if(p->tmd_skb[p->tmdlast]) {
> - dev_kfree_skb_irq(p->tmd_skb[p->tmdlast]);
> - p->tmd_skb[p->tmdlast] = NULL;
> - }
> + dev_kfree_skb_irq(p->tmd_skb[p->tmdlast]);
> + p->tmd_skb[p->tmdlast] = NULL;
> #endif
I absolutely disagree with this kind of change.
There is a non-trivial cost for NULL'ing out that array entry
unconditionally. It's a dirtied cache line and this is in the
fast path of TX SKB reclaim of this driver.
You've made several changes of this kind.
And it sort-of shows that the places that do check for NULL,
are getting something in return for that test, namely avoidance
of an unnecessary cpu store in the fast path of the driver.
I'm throwing away this series, sorry.
next prev parent reply other threads:[~2013-11-04 20:12 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-02 13:47 [PATCH net-next 00/13] Protect dev_kfree_skb_irq from NULL and remove unnecessary NULL checks Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 01/13] net: Check skb for NULL in dev_kfree_skb_irq Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 02/13] driver: net: remove unnecessary skb NULL check before calling dev_kfree_skb_irq Govindarajulu Varadarajan
2013-11-04 20:12 ` David Miller [this message]
[not found] ` <20131104.151230.1978898006990867916.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2013-11-11 10:31 ` Govindarajulu Varadarajan
2013-11-18 19:26 ` Govindarajulu Varadarajan
2013-11-18 19:37 ` Johannes Berg
2013-11-18 20:17 ` David Miller
2013-11-02 13:47 ` [PATCH net-next 03/13] driver: atm: " Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 04/13] driver: staging: " Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 05/13] driver: usb/gadget: " Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 06/13] driver: net: remove unnecessary NULL check before dev_kfree_skb_any Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 07/13] driver: staging: " Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 08/13] driver: isdn: " Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 09/13] driver: s390: " Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 10/13] driver: infiniband: " Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 11/13] driver: usb: " Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 12/13] driver: net: fix space before '(' and remove extra variable Govindarajulu Varadarajan
2013-11-02 13:47 ` [PATCH net-next 13/13] scripts/checkpatch.pl: Add dev_kfree_skb*(NULL) check to checkpatch Govindarajulu Varadarajan
2013-11-04 0:37 ` Joe Perches
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=20131104.151230.1978898006990867916.davem@davemloft.net \
--to=davem@davemloft.net \
--cc=IvDoorn@gmail.com \
--cc=apw@canonical.com \
--cc=benve@cisco.com \
--cc=chas@cmf.nrl.navy.mil \
--cc=govindarajulu90@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=isdn@linux-pingi.de \
--cc=jcliburn@gmail.com \
--cc=jeffrey.t.kirsher@intel.com \
--cc=jesse.brandeburg@intel.com \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=linville@tuxdriver.com \
--cc=netdev@vger.kernel.org \
--cc=roland@kernel.org \
--cc=samuel@sortiz.org \
--cc=sbhatewara@vmware.com \
--cc=schwidefsky@de.ibm.com \
--cc=shahed.shaikh@qlogic.com \
--cc=ssujith@cisco.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).