* Re: [PATCH net-next] tipc: cleanup function namespace
From: Stephen Hemminger @ 2010-10-14 22:13 UTC (permalink / raw)
To: Paul Gortmaker
Cc: Neil Horman, David Miller, netdev, allan.stephens, Jon Maloy
In-Reply-To: <20101014214426.GA9236@windriver.com>
On Thu, 14 Oct 2010 17:44:27 -0400
Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> [Re: [PATCH net-next] tipc: cleanup function namespace] On 14/10/2010 (Thu 11:33) Stephen Hemminger wrote:
>
> > On Thu, 14 Oct 2010 13:53:21 -0400
> > Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> >
> > > On 10-10-13 09:29 PM, Neil Horman wrote:
> > > > On Wed, Oct 13, 2010 at 08:23:24PM -0400, Paul Gortmaker wrote:
> > > >> On 10-10-13 07:20 PM, Stephen Hemminger wrote:
> > > >>> Do some cleanups of TIPC based on make namespacecheck
> > > >>> 1. Don't export unused symbols
> > > >>> 2. Eliminate dead code
> > > >>> 3. Make functions and variables local
> > > >>> 4. Rename buf_acquire to tipc_buf_acquire since it is used in several files
> > > >>>
> > > >>> Compile tested only.
> > > >>> This make break out of tree kernel modules that depend on TIPC routines.
> > > >>
> > > >> Hi Stephen,
> > > >>
> > > >> When I first started looking at TIPC code, I too came to the
> > > >> same conclusion as you did and was about to do #1,2,3 -- but
> > > >> then I was told that the exported symbols were part of an API
> > > >> and might be in use by folks here and there as per this thread:
> > > >>
> > > >> http://www.mail-archive.com/netdev@vger.kernel.org/msg30208.html
> > > >>
> > > > I think its telling the the argument in the above thread for keeping the API
> > > > were that users of it were out there and 'likely to contribute' in the future.
> > > > That thread was 3 years ago. They might be using the API from outside the
> > > > kernel tree, but they're not planning on contributing. As Christoph noted,
> > > > they're freeloaders. The community really doesn't need or want to maintain an
> > > > API like that. If these users are your customers, and removing the API is
> > > > unacceptable, perhaps its time to move the entire TIPC module out of tree.
> > >
> > > As I'd said -- I don't know what the use cases of these API users are,
> > > and so as far as I know they aren't customers either. For what it is
> > > worth, know that I personally wouldn't try and use a business case to
> > > justify a technically wrong decision here on netdev anyway.
> > >
> > > I was just describing the history of the situation, and suggesting
> > > one possible slower approach of phasing it out as a courtesy to those
> > > users, in the same way that the kernel community has extended that
> > > same courtesy with other things in feature-removal.txt
> > >
> > > In the end, since Jon is OK with the removal, and is in the process of
> > > communicating this to the API users he is aware of, I sure don't have
> > > any reason to try and save the API. If folks are good with having it
> > > just go away overnight, then great -- I'll be just as happy to see it
> > > disappear as you and Stephen. So, a long winded way of saying...
> > >
> > > Acked-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> >
> > How about putting an entry in feature-removal.txt with a short (6 month)
> > window?
>
> I'm fine with that too.
>
> P.
>
> From 5a15a26de63a29fcb6cb7a7fb83b6d2fc63cbadb Mon Sep 17 00:00:00 2001
> From: Paul Gortmaker <paul.gortmaker@windriver.com>
> Date: Thu, 14 Oct 2010 17:29:08 -0400
> Subject: [PATCH] TIPC: Document the demise of the Native API for March 2011
>
> The native API in the TIPC code exists as a bunch of functions
> and exported symbols that aren't actually used by any currently
> in-tree kernel code/modules.
>
> Since this code is anomalous to the general guiding principle that
> the kernel should not be libc, coverage tools and people intending
> to do general cleanups keep finding this code and suggesting that
> it be removed.
>
> It seems the right thing to do is to just finally delete it once
> and for all, after giving a reasonable window for any existing
> users to find alternative solutions to their custom use case(s).
>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
> Documentation/feature-removal-schedule.txt | 12 ++++++++++++
> 1 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
> index f456389..1def37e 100644
> --- a/Documentation/feature-removal-schedule.txt
> +++ b/Documentation/feature-removal-schedule.txt
> @@ -573,3 +573,15 @@ Why: Hareware scan is the prefer method for iwlwifi devices for
> Who: Wey-Yi Guy <wey-yi.w.guy@intel.com>
>
> ----------------------------
> +
> +What: TIPC: Delete all code and exported symbols specific to Native API
> +When: March 2011
> +Why: The TIPC Native API, as described here:
> + http://tipc.sourceforge.net/doc/tipc_1.7_prog_guide.html#native_api
> + is implemented by exporting a bunch of otherwise unused functions
> + for possible modular linkage by custom end-user code. This goes
> + against the general concept that the kernel should not be libc.
> +
> +Who: Paul Gortmaker <paul.gortmaker@windriver.com>
> +
> +----------------------------
Acked-by: Stephen Hemminger <shemminger@vyatta.com>
--
^ permalink raw reply
* Re: linux-next: build failure after merge of the net tree
From: Jeff Kirsher @ 2010-10-14 22:07 UTC (permalink / raw)
To: Stephen Rothwell, Harvey Harrison
Cc: David Miller, netdev@vger.kernel.org, linux-next@vger.kernel.org,
linux-kernel@vger.kernel.org, Tantilov, Emil S
In-Reply-To: <20101015113854.ae080284.sfr@canb.auug.org.au>
[-- Attachment #1: Type: text/plain, Size: 631 bytes --]
On Thu, 2010-10-14 at 17:38 -0700, Stephen Rothwell wrote:
> Hi all,
>
> After merging the net tree, today's linux-next build (x86_64 allmodconfig)
> failed like this:
>
> drivers/net/ixgbe/ixgbe_main.c: In function 'ixgbe_configure_dcb':
> drivers/net/ixgbe/ixgbe_main.c:3377: error: implicit declaration of function 'ixgbe_dcb_check_config'
>
> Caused by commit f32f837b75233588cd4f8542214a30915ab7847b ("ixgbe: remove
> unused functions").
>
> Grep is your friend :-)
>
> I have used the net tree from next-20101014 for today.
I apologize, this is my fault. I have a patch coming to resolved the
issue.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH net-next] tipc: cleanup function namespace
From: Paul Gortmaker @ 2010-10-14 21:44 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Neil Horman, David Miller, netdev, allan.stephens, Jon Maloy
In-Reply-To: <20101014113320.7720828f@nehalam>
[Re: [PATCH net-next] tipc: cleanup function namespace] On 14/10/2010 (Thu 11:33) Stephen Hemminger wrote:
> On Thu, 14 Oct 2010 13:53:21 -0400
> Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
>
> > On 10-10-13 09:29 PM, Neil Horman wrote:
> > > On Wed, Oct 13, 2010 at 08:23:24PM -0400, Paul Gortmaker wrote:
> > >> On 10-10-13 07:20 PM, Stephen Hemminger wrote:
> > >>> Do some cleanups of TIPC based on make namespacecheck
> > >>> 1. Don't export unused symbols
> > >>> 2. Eliminate dead code
> > >>> 3. Make functions and variables local
> > >>> 4. Rename buf_acquire to tipc_buf_acquire since it is used in several files
> > >>>
> > >>> Compile tested only.
> > >>> This make break out of tree kernel modules that depend on TIPC routines.
> > >>
> > >> Hi Stephen,
> > >>
> > >> When I first started looking at TIPC code, I too came to the
> > >> same conclusion as you did and was about to do #1,2,3 -- but
> > >> then I was told that the exported symbols were part of an API
> > >> and might be in use by folks here and there as per this thread:
> > >>
> > >> http://www.mail-archive.com/netdev@vger.kernel.org/msg30208.html
> > >>
> > > I think its telling the the argument in the above thread for keeping the API
> > > were that users of it were out there and 'likely to contribute' in the future.
> > > That thread was 3 years ago. They might be using the API from outside the
> > > kernel tree, but they're not planning on contributing. As Christoph noted,
> > > they're freeloaders. The community really doesn't need or want to maintain an
> > > API like that. If these users are your customers, and removing the API is
> > > unacceptable, perhaps its time to move the entire TIPC module out of tree.
> >
> > As I'd said -- I don't know what the use cases of these API users are,
> > and so as far as I know they aren't customers either. For what it is
> > worth, know that I personally wouldn't try and use a business case to
> > justify a technically wrong decision here on netdev anyway.
> >
> > I was just describing the history of the situation, and suggesting
> > one possible slower approach of phasing it out as a courtesy to those
> > users, in the same way that the kernel community has extended that
> > same courtesy with other things in feature-removal.txt
> >
> > In the end, since Jon is OK with the removal, and is in the process of
> > communicating this to the API users he is aware of, I sure don't have
> > any reason to try and save the API. If folks are good with having it
> > just go away overnight, then great -- I'll be just as happy to see it
> > disappear as you and Stephen. So, a long winded way of saying...
> >
> > Acked-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>
> How about putting an entry in feature-removal.txt with a short (6 month)
> window?
I'm fine with that too.
P.
>From 5a15a26de63a29fcb6cb7a7fb83b6d2fc63cbadb Mon Sep 17 00:00:00 2001
From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Thu, 14 Oct 2010 17:29:08 -0400
Subject: [PATCH] TIPC: Document the demise of the Native API for March 2011
The native API in the TIPC code exists as a bunch of functions
and exported symbols that aren't actually used by any currently
in-tree kernel code/modules.
Since this code is anomalous to the general guiding principle that
the kernel should not be libc, coverage tools and people intending
to do general cleanups keep finding this code and suggesting that
it be removed.
It seems the right thing to do is to just finally delete it once
and for all, after giving a reasonable window for any existing
users to find alternative solutions to their custom use case(s).
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
Documentation/feature-removal-schedule.txt | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
index f456389..1def37e 100644
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -573,3 +573,15 @@ Why: Hareware scan is the prefer method for iwlwifi devices for
Who: Wey-Yi Guy <wey-yi.w.guy@intel.com>
----------------------------
+
+What: TIPC: Delete all code and exported symbols specific to Native API
+When: March 2011
+Why: The TIPC Native API, as described here:
+ http://tipc.sourceforge.net/doc/tipc_1.7_prog_guide.html#native_api
+ is implemented by exporting a bunch of otherwise unused functions
+ for possible modular linkage by custom end-user code. This goes
+ against the general concept that the kernel should not be libc.
+
+Who: Paul Gortmaker <paul.gortmaker@windriver.com>
+
+----------------------------
--
1.7.2.1
^ permalink raw reply related
* Re: [RFC PATCH net-next] drivers/net Documentation/networking: Create directory intel_wired_lan
From: Michal Marek @ 2010-10-14 21:37 UTC (permalink / raw)
To: Joe Perches
Cc: linux-kbuild, e1000-devel, Allan, Bruce W, Brandeburg, Jesse,
linux-kernel@vger.kernel.org, Rose, Gregory V, Ronciak, John,
jeffrey.t.kirsher, netdev@vger.kernel.org, Sam Ravnborg
In-Reply-To: <1287084643.1117.602.camel@Joe-Laptop>
On 14.10.2010 21:30, Joe Perches wrote:
> On Thu, 2010-10-14 at 02:34 -0700, Jeff Kirsher wrote:
>> On Wed, 2010-10-13 at 22:57 -0700, Joe Perches wrote:
>>> On Wed, 2010-10-13 at 21:57 -0700, Jeff Kirsher wrote:
>>>> On Wed, 2010-10-13 at 15:28 -0700, Joe Perches wrote:
>>>> Sorry I am not ignoring you, I was taking a closer look at your patch.
>>>>> What regression testing would actually be done?
>>>> The Makefile and Kconfig needs more work. I applied your patch and none
>>>> of the Intel Wired drivers build.
>>> Care to describe the Makefile/Kconfig issues you have seen?
>>> I built it allyesconfig, defconfig, allmodconfig and allnoconfig.
>> Yeah, I found all of those built without errors, but if you build the
>> Intel Wired LAN drivers as modules, you will not find the *.ko files
>> after the build. The Kconfig files look fine, the problem was with the
>> Makefiles. Instead of creating a drivers/net/intel_wired_lan/Makefile,
>> I simply changed the path in drivers/net/Makefile to the updated path
>> and that resolved the issue.
>
> (adding a few cc's and a link for history)
>
> http://lkml.org/lkml/2010/10/10/207
>
> That's the way I had done it originally as well, but I found
> you couldn't build the directory with:
>
> make drivers/net/intel_wired_lan/
>
> so I created a Makefile in the new directory below with
> the elements necessary.
>
> Perhaps there's some missing functionality in the build system
> when the Kconfig file resides in a higher directory and the
> directory being built doesn't have a Kconfig file?
This has nothing to do with Kconfig files, I assume you meant Makefiles.
> I think it'd wrong to duplicate the makefile components in
> 2 places to allow "make subdir/" and I wonder if there's a
> good solution for this.
There is no duplication needed, drivers/net/Makefile needs to contain
obj-y += intel_wired_lan/ and drivers/net/intel_wired_lan/Makefile
similar lines for its subdirs. I see your original patch was lacking an
entry in drivers/net/Makefile, so perhaps that was the problem Jeff was
seeing?
Michal
------------------------------------------------------------------------------
Download new Adobe(R) Flash(R) Builder(TM) 4
The new Adobe(R) Flex(R) 4 and Flash(R) Builder(TM) 4 (formerly
Flex(R) Builder(TM)) enable the development of rich applications that run
across multiple browsers and platforms. Download your free trials today!
http://p.sf.net/sfu/adobe-dev2dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* [PATCH net-next 5/8] tg3: Cleanup tg3_alloc_rx_skb()
From: Matt Carlson @ 2010-10-14 20:37 UTC (permalink / raw)
To: davem; +Cc: netdev, andy, mcarlson
src_map is no longer used in tg3_alloc_rx_skb(). Remove it.
Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
---
drivers/net/tg3.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index f2d96dd..84530d9 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -4573,12 +4573,11 @@ static int tg3_alloc_rx_skb(struct tg3 *tp, struct tg3_rx_prodring_set *tpr,
u32 opaque_key, u32 dest_idx_unmasked)
{
struct tg3_rx_buffer_desc *desc;
- struct ring_info *map, *src_map;
+ struct ring_info *map;
struct sk_buff *skb;
dma_addr_t mapping;
int skb_size, dest_idx;
- src_map = NULL;
switch (opaque_key) {
case RXD_OPAQUE_RING_STD:
dest_idx = dest_idx_unmasked & tp->rx_std_ring_mask;
--
1.7.2.2
^ permalink raw reply related
* [PATCH net-next 7/8] tg3: Report invalid link from tg3_get_settings()
From: Matt Carlson @ 2010-10-14 20:37 UTC (permalink / raw)
To: davem; +Cc: netdev, andy, mcarlson
Currently the tg3 driver leaves the speed and duplex fields
uninitialized in tg3_get_settings() if the device is not up. This can
lead to some strange deductions in certain versions of ethtool. When
the device is up and the link is down, the driver reports SPEED_INVALID
and DUPLEX_INVALID for these fields. This patch makes the presentation
consistent by returning SPEED_INVALID and DUPLEX_INVALID when the device
has not been brought up as well.
Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
---
drivers/net/tg3.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 17ca0a3..5d30e67 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -9807,6 +9807,9 @@ static int tg3_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
if (netif_running(dev)) {
cmd->speed = tp->link_config.active_speed;
cmd->duplex = tp->link_config.active_duplex;
+ } else {
+ cmd->speed = SPEED_INVALID;
+ cmd->duplex = DUPLEX_INVALID;
}
cmd->phy_address = tp->phy_addr;
cmd->transceiver = XCVR_INTERNAL;
--
1.7.2.2
^ permalink raw reply related
* [PATCH net-next 6/8] tg3: Don't allocate jumbo ring for 5780 class devs
From: Matt Carlson @ 2010-10-14 20:37 UTC (permalink / raw)
To: davem; +Cc: netdev, andy, mcarlson
The 5714, 5715, and 5780 devices do not have a separate rx jumbo
producer ring. This patch changes the code so that resources are not
allocated for it.
Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
---
drivers/net/tg3.c | 11 +++++++----
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 84530d9..17ca0a3 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -6219,7 +6219,8 @@ static void tg3_rx_prodring_free(struct tg3 *tp,
tg3_rx_skb_free(tp, &tpr->rx_std_buffers[i],
tp->rx_pkt_map_sz);
- if (tp->tg3_flags & TG3_FLAG_JUMBO_CAPABLE) {
+ if ((tp->tg3_flags & TG3_FLAG_JUMBO_CAPABLE) &&
+ !(tp->tg3_flags2 & TG3_FLG2_5780_CLASS)) {
for (i = 0; i <= tp->rx_jmb_ring_mask; i++)
tg3_rx_skb_free(tp, &tpr->rx_jmb_buffers[i],
TG3_RX_JMB_MAP_SZ);
@@ -6246,7 +6247,7 @@ static int tg3_rx_prodring_alloc(struct tg3 *tp,
if (tpr != &tp->napi[0].prodring) {
memset(&tpr->rx_std_buffers[0], 0,
TG3_RX_STD_BUFF_RING_SIZE(tp));
- if (tp->tg3_flags & TG3_FLAG_JUMBO_CAPABLE)
+ if (tpr->rx_jmb_buffers)
memset(&tpr->rx_jmb_buffers[0], 0,
TG3_RX_JMB_BUFF_RING_SIZE(tp));
goto done;
@@ -6289,7 +6290,8 @@ static int tg3_rx_prodring_alloc(struct tg3 *tp,
}
}
- if (!(tp->tg3_flags & TG3_FLAG_JUMBO_CAPABLE))
+ if (!(tp->tg3_flags & TG3_FLAG_JUMBO_CAPABLE) ||
+ (tp->tg3_flags2 & TG3_FLG2_5780_CLASS))
goto done;
memset(tpr->rx_jmb, 0, TG3_RX_JMB_RING_BYTES(tp));
@@ -6361,7 +6363,8 @@ static int tg3_rx_prodring_init(struct tg3 *tp,
if (!tpr->rx_std)
goto err_out;
- if (tp->tg3_flags & TG3_FLAG_JUMBO_CAPABLE) {
+ if ((tp->tg3_flags & TG3_FLAG_JUMBO_CAPABLE) &&
+ !(tp->tg3_flags2 & TG3_FLG2_5780_CLASS)) {
tpr->rx_jmb_buffers = kzalloc(TG3_RX_JMB_BUFF_RING_SIZE(tp),
GFP_KERNEL);
if (!tpr->rx_jmb_buffers)
--
1.7.2.2
^ permalink raw reply related
* [PATCH net-next 8/8] tg3: Update version to 3.115
From: Matt Carlson @ 2010-10-14 20:37 UTC (permalink / raw)
To: davem; +Cc: netdev, andy, mcarlson
This patch updates the tg3 version to 3.115.
Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
---
drivers/net/tg3.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 5d30e67..5b4c510 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -69,10 +69,10 @@
#define DRV_MODULE_NAME "tg3"
#define TG3_MAJ_NUM 3
-#define TG3_MIN_NUM 114
+#define TG3_MIN_NUM 115
#define DRV_MODULE_VERSION \
__stringify(TG3_MAJ_NUM) "." __stringify(TG3_MIN_NUM)
-#define DRV_MODULE_RELDATE "September 30, 2010"
+#define DRV_MODULE_RELDATE "October 14, 2010"
#define TG3_DEF_MAC_MODE 0
#define TG3_DEF_RX_MODE 0
--
1.7.2.2
^ permalink raw reply related
* [PATCH net-next 3/8] tg3: Add clause 45 register accessor methods
From: Matt Carlson @ 2010-10-14 20:37 UTC (permalink / raw)
To: davem; +Cc: netdev, andy, mcarlson
This patch adds clause 45 register access methods. They will be used in
the following patch.
Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
---
drivers/net/tg3.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
drivers/net/tg3.h | 4 ++++
2 files changed, 50 insertions(+), 0 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 1c680ff..afcc593 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -1162,6 +1162,52 @@ static void tg3_mdio_fini(struct tg3 *tp)
}
}
+static int tg3_phy_cl45_write(struct tg3 *tp, u32 devad, u32 addr, u32 val)
+{
+ int err;
+
+ err = tg3_writephy(tp, MII_TG3_MMD_CTRL, devad);
+ if (err)
+ goto done;
+
+ err = tg3_writephy(tp, MII_TG3_MMD_ADDRESS, addr);
+ if (err)
+ goto done;
+
+ err = tg3_writephy(tp, MII_TG3_MMD_CTRL,
+ MII_TG3_MMD_CTRL_DATA_NOINC | devad);
+ if (err)
+ goto done;
+
+ err = tg3_writephy(tp, MII_TG3_MMD_ADDRESS, val);
+
+done:
+ return err;
+}
+
+static int tg3_phy_cl45_read(struct tg3 *tp, u32 devad, u32 addr, u32 *val)
+{
+ int err;
+
+ err = tg3_writephy(tp, MII_TG3_MMD_CTRL, devad);
+ if (err)
+ goto done;
+
+ err = tg3_writephy(tp, MII_TG3_MMD_ADDRESS, addr);
+ if (err)
+ goto done;
+
+ err = tg3_writephy(tp, MII_TG3_MMD_CTRL,
+ MII_TG3_MMD_CTRL_DATA_NOINC | devad);
+ if (err)
+ goto done;
+
+ err = tg3_readphy(tp, MII_TG3_MMD_ADDRESS, val);
+
+done:
+ return err;
+}
+
/* tp->lock is held. */
static inline void tg3_generate_fw_event(struct tg3 *tp)
{
diff --git a/drivers/net/tg3.h b/drivers/net/tg3.h
index 89553c4..99fc306 100644
--- a/drivers/net/tg3.h
+++ b/drivers/net/tg3.h
@@ -2061,6 +2061,10 @@
#define MII_TG3_CTRL_AS_MASTER 0x0800
#define MII_TG3_CTRL_ENABLE_AS_MASTER 0x1000
+#define MII_TG3_MMD_CTRL 0x0d /* MMD Access Control register */
+#define MII_TG3_MMD_CTRL_DATA_NOINC 0x4000
+#define MII_TG3_MMD_ADDRESS 0x0e /* MMD Address Data register */
+
#define MII_TG3_EXT_CTRL 0x10 /* Extended control register */
#define MII_TG3_EXT_CTRL_FIFO_ELASTIC 0x0001
#define MII_TG3_EXT_CTRL_LNK3_LED_MODE 0x0002
--
1.7.2.2
^ permalink raw reply related
* [PATCH net-next 4/8] tg3: Add EEE support
From: Matt Carlson @ 2010-10-14 20:37 UTC (permalink / raw)
To: davem; +Cc: netdev, andy, mcarlson
This patch adds Energy Efficient Ethernet (EEE) support for the 5718
device ID and the 57765 B0 asic revision.
Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
---
drivers/net/tg3.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++++++++
drivers/net/tg3.h | 33 +++++++++++++++-
2 files changed, 146 insertions(+), 1 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index afcc593..f2d96dd 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -1584,6 +1584,17 @@ static void tg3_phy_fini(struct tg3 *tp)
}
}
+static int tg3_phydsp_read(struct tg3 *tp, u32 reg, u32 *val)
+{
+ int err;
+
+ err = tg3_writephy(tp, MII_TG3_DSP_ADDRESS, reg);
+ if (!err)
+ err = tg3_readphy(tp, MII_TG3_DSP_RW_PORT, val);
+
+ return err;
+}
+
static int tg3_phydsp_write(struct tg3 *tp, u32 reg, u32 val)
{
int err;
@@ -1747,6 +1758,42 @@ static void tg3_phy_apply_otp(struct tg3 *tp)
tg3_writephy(tp, MII_TG3_AUX_CTRL, phy);
}
+static void tg3_phy_eee_adjust(struct tg3 *tp, u32 current_link_up)
+{
+ u32 val;
+
+ if (!(tp->phy_flags & TG3_PHYFLG_EEE_CAP))
+ return;
+
+ tp->setlpicnt = 0;
+
+ if (tp->link_config.autoneg == AUTONEG_ENABLE &&
+ current_link_up == 1 &&
+ (tp->link_config.active_speed == SPEED_1000 ||
+ (tp->link_config.active_speed == SPEED_100 &&
+ tp->link_config.active_duplex == DUPLEX_FULL))) {
+ u32 eeectl;
+
+ if (tp->link_config.active_speed == SPEED_1000)
+ eeectl = TG3_CPMU_EEE_CTRL_EXIT_16_5_US;
+ else
+ eeectl = TG3_CPMU_EEE_CTRL_EXIT_36_US;
+
+ tw32(TG3_CPMU_EEE_CTRL, eeectl);
+
+ tg3_phy_cl45_read(tp, 0x7, TG3_CL45_D7_EEERES_STAT, &val);
+
+ if (val == TG3_CL45_D7_EEERES_STAT_LP_1000T ||
+ val == TG3_CL45_D7_EEERES_STAT_LP_100TX)
+ tp->setlpicnt = 2;
+ }
+
+ if (!tp->setlpicnt) {
+ val = tr32(TG3_CPMU_EEE_MODE);
+ tw32(TG3_CPMU_EEE_MODE, val & ~TG3_CPMU_EEEMD_LPI_ENABLE);
+ }
+}
+
static int tg3_wait_macro_done(struct tg3 *tp)
{
int limit = 100;
@@ -2921,6 +2968,44 @@ static void tg3_phy_copper_begin(struct tg3 *tp)
tg3_writephy(tp, MII_TG3_CTRL, new_adv);
}
+ if (tp->phy_flags & TG3_PHYFLG_EEE_CAP) {
+ u32 val = 0;
+
+ tw32(TG3_CPMU_EEE_MODE,
+ tr32(TG3_CPMU_EEE_MODE) & ~TG3_CPMU_EEEMD_LPI_ENABLE);
+
+ /* Enable SM_DSP clock and tx 6dB coding. */
+ val = MII_TG3_AUXCTL_SHDWSEL_AUXCTL |
+ MII_TG3_AUXCTL_ACTL_SMDSP_ENA |
+ MII_TG3_AUXCTL_ACTL_TX_6DB;
+ tg3_writephy(tp, MII_TG3_AUX_CTRL, val);
+
+ if ((GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5717 ||
+ GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_57765) &&
+ !tg3_phydsp_read(tp, MII_TG3_DSP_CH34TP2, &val))
+ tg3_phydsp_write(tp, MII_TG3_DSP_CH34TP2,
+ val | MII_TG3_DSP_CH34TP2_HIBW01);
+
+ if (tp->link_config.autoneg == AUTONEG_ENABLE) {
+ /* Advertise 100-BaseTX EEE ability */
+ if (tp->link_config.advertising &
+ (ADVERTISED_100baseT_Half |
+ ADVERTISED_100baseT_Full))
+ val |= TG3_CL45_D7_EEEADV_CAP_100TX;
+ /* Advertise 1000-BaseT EEE ability */
+ if (tp->link_config.advertising &
+ (ADVERTISED_1000baseT_Half |
+ ADVERTISED_1000baseT_Full))
+ val |= TG3_CL45_D7_EEEADV_CAP_1000T;
+ }
+ tg3_phy_cl45_write(tp, 0x7, TG3_CL45_D7_EEEADV_CAP, val);
+
+ /* Turn off SM_DSP clock. */
+ val = MII_TG3_AUXCTL_SHDWSEL_AUXCTL |
+ MII_TG3_AUXCTL_ACTL_TX_6DB;
+ tg3_writephy(tp, MII_TG3_AUX_CTRL, val);
+ }
+
if (tp->link_config.autoneg == AUTONEG_DISABLE &&
tp->link_config.speed != SPEED_INVALID) {
u32 bmcr, orig_bmcr;
@@ -3282,6 +3367,8 @@ relink:
tw32_f(MAC_MODE, tp->mac_mode);
udelay(40);
+ tg3_phy_eee_adjust(tp, current_link_up);
+
if (tp->tg3_flags & TG3_FLAG_USE_LINKCHG_REG) {
/* Polled via timer. */
tw32_f(MAC_EVENT, 0);
@@ -7790,6 +7877,22 @@ static int tg3_reset_hw(struct tg3 *tp, int reset_phy)
tw32(TG3_CPMU_LSPD_10MB_CLK, val);
}
+ /* Enable MAC control of LPI */
+ if (tp->phy_flags & TG3_PHYFLG_EEE_CAP) {
+ tw32_f(TG3_CPMU_EEE_LNKIDL_CTRL,
+ TG3_CPMU_EEE_LNKIDL_PCIE_NL0 |
+ TG3_CPMU_EEE_LNKIDL_UART_IDL);
+
+ tw32_f(TG3_CPMU_EEE_CTRL,
+ TG3_CPMU_EEE_CTRL_EXIT_20_1_US);
+
+ tw32_f(TG3_CPMU_EEE_MODE,
+ TG3_CPMU_EEEMD_ERLY_L1_XIT_DET |
+ TG3_CPMU_EEEMD_LPI_IN_TX |
+ TG3_CPMU_EEEMD_LPI_IN_RX |
+ TG3_CPMU_EEEMD_EEE_ENABLE);
+ }
+
/* This works around an issue with Athlon chipsets on
* B3 tigon3 silicon. This bit has no effect on any
* other revision. But do not set this on PCI Express
@@ -8598,6 +8701,12 @@ static void tg3_timer(unsigned long __opaque)
if (tp->tg3_flags2 & TG3_FLG2_5705_PLUS)
tg3_periodic_fetch_stats(tp);
+ if (tp->setlpicnt && !--tp->setlpicnt) {
+ u32 val = tr32(TG3_CPMU_EEE_MODE);
+ tw32(TG3_CPMU_EEE_MODE,
+ val | TG3_CPMU_EEEMD_LPI_ENABLE);
+ }
+
if (tp->tg3_flags & TG3_FLAG_USE_LINKCHG_REG) {
u32 mac_stat;
int phy_event;
@@ -12432,6 +12541,11 @@ static int __devinit tg3_phy_probe(struct tg3 *tp)
}
}
+ if (tp->pdev->device == TG3PCI_DEVICE_TIGON3_5718 ||
+ (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_57765 &&
+ tp->pci_chip_rev_id != CHIPREV_ID_57765_A0))
+ tp->phy_flags |= TG3_PHYFLG_EEE_CAP;
+
if (!(tp->phy_flags & TG3_PHYFLG_ANY_SERDES) &&
!(tp->tg3_flags3 & TG3_FLG3_ENABLE_APE) &&
!(tp->tg3_flags & TG3_FLAG_ENABLE_ASF)) {
diff --git a/drivers/net/tg3.h b/drivers/net/tg3.h
index 99fc306..8342190 100644
--- a/drivers/net/tg3.h
+++ b/drivers/net/tg3.h
@@ -1091,7 +1091,26 @@
#define CPMU_MUTEX_GNT_DRIVER 0x00001000
#define TG3_CPMU_PHY_STRAP 0x00003664
#define TG3_CPMU_PHY_STRAP_IS_SERDES 0x00000020
-/* 0x3664 --> 0x3800 unused */
+/* 0x3664 --> 0x36b0 unused */
+
+#define TG3_CPMU_EEE_MODE 0x000036b0
+#define TG3_CPMU_EEEMD_ERLY_L1_XIT_DET 0x00000008
+#define TG3_CPMU_EEEMD_LPI_ENABLE 0x00000080
+#define TG3_CPMU_EEEMD_LPI_IN_TX 0x00000100
+#define TG3_CPMU_EEEMD_LPI_IN_RX 0x00000200
+#define TG3_CPMU_EEEMD_EEE_ENABLE 0x00100000
+/* 0x36b4 --> 0x36b8 unused */
+
+#define TG3_CPMU_EEE_LNKIDL_CTRL 0x000036bc
+#define TG3_CPMU_EEE_LNKIDL_PCIE_NL0 0x01000000
+#define TG3_CPMU_EEE_LNKIDL_UART_IDL 0x00000004
+/* 0x36c0 --> 0x36d0 unused */
+
+#define TG3_CPMU_EEE_CTRL 0x000036d0
+#define TG3_CPMU_EEE_CTRL_EXIT_16_5_US 0x0000019d
+#define TG3_CPMU_EEE_CTRL_EXIT_36_US 0x00000384
+#define TG3_CPMU_EEE_CTRL_EXIT_20_1_US 0x000001f8
+/* 0x36d4 --> 0x3800 unused */
/* Mbuf cluster free registers */
#define MBFREE_MODE 0x00003800
@@ -2082,6 +2101,8 @@
#define MII_TG3_DSP_TAP1 0x0001
#define MII_TG3_DSP_TAP1_AGCTGT_DFLT 0x0007
#define MII_TG3_DSP_AADJ1CH0 0x001f
+#define MII_TG3_DSP_CH34TP2 0x4022
+#define MII_TG3_DSP_CH34TP2_HIBW01 0x0010
#define MII_TG3_DSP_AADJ1CH3 0x601f
#define MII_TG3_DSP_AADJ1CH3_ADCCKADJ 0x0002
#define MII_TG3_DSP_EXP1_INT_STAT 0x0f01
@@ -2148,6 +2169,14 @@
#define MII_TG3_TEST1_TRIM_EN 0x0010
#define MII_TG3_TEST1_CRC_EN 0x8000
+/* Clause 45 expansion registers */
+#define TG3_CL45_D7_EEEADV_CAP 0x003c
+#define TG3_CL45_D7_EEEADV_CAP_100TX 0x0002
+#define TG3_CL45_D7_EEEADV_CAP_1000T 0x0004
+#define TG3_CL45_D7_EEERES_STAT 0x803e
+#define TG3_CL45_D7_EEERES_STAT_LP_100TX 0x0002
+#define TG3_CL45_D7_EEERES_STAT_LP_1000T 0x0004
+
/* Fast Ethernet Tranceiver definitions */
#define MII_TG3_FET_PTEST 0x17
@@ -2992,9 +3021,11 @@ struct tg3 {
#define TG3_PHYFLG_BER_BUG 0x00008000
#define TG3_PHYFLG_SERDES_PREEMPHASIS 0x00010000
#define TG3_PHYFLG_PARALLEL_DETECT 0x00020000
+#define TG3_PHYFLG_EEE_CAP 0x00040000
u32 led_ctrl;
u32 phy_otp;
+ u32 setlpicnt;
#define TG3_BPN_SIZE 24
char board_part_number[TG3_BPN_SIZE];
--
1.7.2.2
^ permalink raw reply related
* [PATCH net-next 2/8] tg3: Disable unused transmit rings
From: Matt Carlson @ 2010-10-14 20:37 UTC (permalink / raw)
To: davem; +Cc: netdev, andy, mcarlson
This patch allows the driver to disable the additional transmit rings
available on the 5717 and 5719 devices. This is not strictly necessary,
but is done anyways for correctness.
Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
---
drivers/net/tg3.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 978ba5d..1c680ff 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -7536,6 +7536,9 @@ static void tg3_rings_reset(struct tg3 *tp)
/* Disable all transmit rings but the first. */
if (!(tp->tg3_flags2 & TG3_FLG2_5705_PLUS))
limit = NIC_SRAM_SEND_RCB + TG3_BDINFO_SIZE * 16;
+ else if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5717 ||
+ GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5719)
+ limit = NIC_SRAM_SEND_RCB + TG3_BDINFO_SIZE * 4;
else if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_57765)
limit = NIC_SRAM_SEND_RCB + TG3_BDINFO_SIZE * 2;
else
--
1.7.2.2
^ permalink raw reply related
* [PATCH net-next 0/8] tg3: Updates and EEE support
From: Matt Carlson @ 2010-10-14 20:37 UTC (permalink / raw)
To: davem; +Cc: netdev, andy, mcarlson
This patchset integrates a few minor changes and adds EEE support
for the 5717 and 57765 asic revs.
^ permalink raw reply
* [PATCH net-next 1/8] tg3: Add support for selfboot format 1 v6
From: Matt Carlson @ 2010-10-14 20:37 UTC (permalink / raw)
To: davem; +Cc: netdev, andy, mcarlson
5718 B0 and 5719 devices will use a new selfboot firmware format. This
patch adds code to detect the new format so that bootcode versions get
reported correctly.
Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
---
drivers/net/tg3.c | 3 +++
drivers/net/tg3.h | 2 ++
2 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 943c283..978ba5d 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -12702,6 +12702,9 @@ static void __devinit tg3_read_sb_ver(struct tg3 *tp, u32 val)
case TG3_EEPROM_SB_REVISION_5:
offset = TG3_EEPROM_SB_F1R5_EDH_OFF;
break;
+ case TG3_EEPROM_SB_REVISION_6:
+ offset = TG3_EEPROM_SB_F1R6_EDH_OFF;
+ break;
default:
return;
}
diff --git a/drivers/net/tg3.h b/drivers/net/tg3.h
index f6b709a..89553c4 100644
--- a/drivers/net/tg3.h
+++ b/drivers/net/tg3.h
@@ -1915,6 +1915,7 @@
#define TG3_EEPROM_SB_REVISION_3 0x00030000
#define TG3_EEPROM_SB_REVISION_4 0x00040000
#define TG3_EEPROM_SB_REVISION_5 0x00050000
+#define TG3_EEPROM_SB_REVISION_6 0x00060000
#define TG3_EEPROM_MAGIC_HW 0xabcd
#define TG3_EEPROM_MAGIC_HW_MSK 0xffff
@@ -1934,6 +1935,7 @@
#define TG3_EEPROM_SB_F1R3_EDH_OFF 0x18
#define TG3_EEPROM_SB_F1R4_EDH_OFF 0x1c
#define TG3_EEPROM_SB_F1R5_EDH_OFF 0x20
+#define TG3_EEPROM_SB_F1R6_EDH_OFF 0x4c
#define TG3_EEPROM_SB_EDH_MAJ_MASK 0x00000700
#define TG3_EEPROM_SB_EDH_MAJ_SHFT 8
#define TG3_EEPROM_SB_EDH_MIN_MASK 0x000000ff
--
1.7.2.2
^ permalink raw reply related
* Re: [PATCH net-next] net: allocate skbs on local node
From: Eric Dumazet @ 2010-10-14 19:59 UTC (permalink / raw)
To: Andrew Morton
Cc: Tom Herbert, David Miller, netdev, Michael Chan, Eilon Greenstein,
Christoph Hellwig, Christoph Lameter
In-Reply-To: <20101014122752.21dd4eaf.akpm@linux-foundation.org>
Le jeudi 14 octobre 2010 à 12:27 -0700, Andrew Morton a écrit :
> Can we think of any hardware configuration for which the change would
> be harmful? Something with really expensive cross-node DMA maybe?
>
If such hardware exists (yes it does, but not close my hands...), then
NIC IRQS probably should be handled by cpus close to the device, or it
might be very slow. This has nothing to do with skb allocation layer.
I believe we should not try to correct wrong IRQ affinities with NUMA
games. Network stack will wakeup threads and scheduler will run them on
same cpu, if possible.
If skb stay long enough on socket receive queue, application will need
to fetch again remote numa node when reading socket a few milliseconds
later, because cache will be cold : Total : two cross node transferts
per incoming frame.
The more node distances are big, the more speedup we can expect from
this patch.
Thanks
^ permalink raw reply
* [PATCH net-next] drivers/net/pch_gbe: Use DEFINE_PCI_DEVICE_TABLE
From: Joe Perches @ 2010-10-14 19:55 UTC (permalink / raw)
To: Masayuki Ohtake; +Cc: netdev, LKML
Use the standard macro to put this table in __devinitconst.
Compiled, untested.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/pch_gbe/pch_gbe_main.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/pch_gbe/pch_gbe_main.c b/drivers/net/pch_gbe/pch_gbe_main.c
index e44644f..cf4b49d 100644
--- a/drivers/net/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/pch_gbe/pch_gbe_main.c
@@ -2394,7 +2394,7 @@ err_disable_device:
return ret;
}
-static const struct pci_device_id pch_gbe_pcidev_id[] = {
+static DEFINE_PCI_DEVICE_TABLE(pch_gbe_pcidev_id) = {
{.vendor = PCI_VENDOR_ID_INTEL,
.device = PCI_DEVICE_ID_INTEL_IOH1_GBE,
.subvendor = PCI_ANY_ID,
^ permalink raw reply related
* RE: [PATCH net-next] tipc: cleanup function namespace
From: Jon Maloy @ 2010-10-14 19:49 UTC (permalink / raw)
To: Stephen Hemminger, Paul Gortmaker
Cc: Neil Horman, David Miller, netdev@vger.kernel.org,
allan.stephens@windriver.com
In-Reply-To: <20101014113320.7720828f@nehalam>
<...>
> > justify a technically wrong decision here on netdev anyway.
> >
> > I was just describing the history of the situation, and
> suggesting one
> > possible slower approach of phasing it out as a courtesy to those
> > users, in the same way that the kernel community has extended that
> > same courtesy with other things in feature-removal.txt
> >
> > In the end, since Jon is OK with the removal, and is in the
> process of
> > communicating this to the API users he is aware of, I sure
> don't have
> > any reason to try and save the API. If folks are good with
> having it
> > just go away overnight, then great -- I'll be just as happy
> to see it
> > disappear as you and Stephen. So, a long winded way of saying...
> >
> > Acked-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>
> How about putting an entry in feature-removal.txt with a
> short (6 month) window?
I think that is a very good idea. That would give the users a reasonable time
to find other solutions.
>
>
> --
> --
> To unsubscribe from this list: send the line "unsubscribe
> netdev" in the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [RFC PATCH net-next] drivers/net Documentation/networking: Create directory intel_wired_lan
From: Joe Perches @ 2010-10-14 19:30 UTC (permalink / raw)
To: jeffrey.t.kirsher, Michal Marek, Sam Ravnborg, linux-kbuild
Cc: Brandeburg, Jesse, Allan, Bruce W, Wyborny, Carolyn,
Skidmore, Donald C, Rose, Gregory V, Waskiewicz Jr, Peter P,
Duyck, Alexander H, Ronciak, John, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, e1000-devel
In-Reply-To: <1287048849.3319.20.camel@jtkirshe-MOBL1>
On Thu, 2010-10-14 at 02:34 -0700, Jeff Kirsher wrote:
> On Wed, 2010-10-13 at 22:57 -0700, Joe Perches wrote:
> > On Wed, 2010-10-13 at 21:57 -0700, Jeff Kirsher wrote:
> > > On Wed, 2010-10-13 at 15:28 -0700, Joe Perches wrote:
> > > Sorry I am not ignoring you, I was taking a closer look at your patch.
> > > > What regression testing would actually be done?
> > > The Makefile and Kconfig needs more work. I applied your patch and none
> > > of the Intel Wired drivers build.
> > Care to describe the Makefile/Kconfig issues you have seen?
> > I built it allyesconfig, defconfig, allmodconfig and allnoconfig.
> Yeah, I found all of those built without errors, but if you build the
> Intel Wired LAN drivers as modules, you will not find the *.ko files
> after the build. The Kconfig files look fine, the problem was with the
> Makefiles. Instead of creating a drivers/net/intel_wired_lan/Makefile,
> I simply changed the path in drivers/net/Makefile to the updated path
> and that resolved the issue.
(adding a few cc's and a link for history)
http://lkml.org/lkml/2010/10/10/207
That's the way I had done it originally as well, but I found
you couldn't build the directory with:
make drivers/net/intel_wired_lan/
so I created a Makefile in the new directory below with
the elements necessary.
Perhaps there's some missing functionality in the build system
when the Kconfig file resides in a higher directory and the
directory being built doesn't have a Kconfig file?
I think it'd wrong to duplicate the makefile components in
2 places to allow "make subdir/" and I wonder if there's a
good solution for this.
> As far as the sub-directory name "intel_wired_lan", what about "intel"
> or "intel_wired"? Just a thought...
Using "intel" seemed too sweeping because of the wireless drivers.
I think intel_wired_lan isn't overly long, but your choice...
Should the new (OKI?/intel) pch_gbe directory be moved as well?
It's using a PCI_VENDOR_ID_INTEL.
^ permalink raw reply
* Re: [PATCH net-next] fib_hash: embed initial hash table in fn_zone
From: Eric Dumazet @ 2010-10-14 19:22 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <1287073458.2712.100.camel@edumazet-laptop>
Le jeudi 14 octobre 2010 à 18:24 +0200, Eric Dumazet a écrit :
> While looking for false sharing problems, I noticed
> sizeof(struct fn_zone) was small (28 bytes) and possibly sharing a cache
> line with an often written kernel structure.
>
> Most of the time, fn_zone uses its initial hash table of 16 slots.
>
> We can avoid the false sharing problem by embedding this initial hash
> table in fn_zone itself, so that sizeof(fn_zone) > L1_CACHE_BYTES
>
> We did a similar optimization in commit a6501e080c (Reduce memory needs
> and speedup lookups)
>
> Add a fz_revorder field to speedup fn_hash() a bit.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Oops, a last minute change was wrong, I'll resend a V2.
Thanks
^ permalink raw reply
* Re: [PATCH net-next] net: allocate skbs on local node
From: Andrew Morton @ 2010-10-14 19:27 UTC (permalink / raw)
To: Tom Herbert
Cc: Eric Dumazet, David Miller, netdev, Michael Chan,
Eilon Greenstein, Christoph Hellwig, Christoph Lameter
In-Reply-To: <AANLkTin8FczYpPvi79w7iuKOZz3uZtYFfTPitAgQFxzD@mail.gmail.com>
On Thu, 14 Oct 2010 08:31:01 -0700
Tom Herbert <therbert@google.com> wrote:
> > This is all conspicuously hand-wavy and unquantified. __(IOW: prove it!)
> >
> > The mooted effects should be tested for on both slab and slub, I
> > suggest. __They're pretty different beasts.
> > --
>
> Some results running netper TCP_RR test with 200 instances, 1 byte
> request and response on 16 core AMD using bnx2x with one 16 queues,
> one for each CPU.
>
> SLAB
>
> Without patch 553570 tps at 86% CPU
> With patch 791883 tps at 93% CPU
>
> SLUB
>
> Without patch 704879 tps at 95% CPU
> With patch 775278 tps at 92% CPU
>
> I believe both show good benfits with patch, and it actually looks
> like the impact is more pronounced for SLAB. I would also note, that
> we have actually already internally patched __netdev_alloc_skb to do
> local node allocation which we have been running in production for
> quite some time.
>
Yes, that's a solid gain.
Can we think of any hardware configuration for which the change would
be harmful? Something with really expensive cross-node DMA maybe?
^ permalink raw reply
* Re: BUG ? ipip unregister_netdevice_many()
From: Octavian Purdila @ 2010-10-14 19:21 UTC (permalink / raw)
To: Eric W. Biederman, David Miller; +Cc: hans.schillstrom, daniel.lezcano, netdev
> > How can it make a real difference even in this case? We'll obliterate
> > all the entries, and then on subsequent passes we'll find nothing
> > matching that namespace any more.
> >
> > Show me performance tests that show it makes any difference, please.
>
> Octavian did you happen to measure the performance difference when you
> added batching of routing table flushes?
>
Unfortunatelly I dont't have the numbers anymore, but I remember it was noticeable when using a large number of interfaces (10K) - if I remember correctly around 1 second out of 10 for the whole unregister process.
BTW, another bottleneck for mass unregister while interfaces are up is dev_deactivate / dev_close. I experimented with "batchifying" it and for 32K interfaces the time went down from 5mins to 30s.
The patch I have is not pretty, it basically creates another 2 functions for each of dev_close and dev_deactivate for pre and post synchronise_rcu barrier.
^ permalink raw reply
* Re: BUG ? ipip unregister_netdevice_many()
From: Eric W. Biederman @ 2010-10-14 18:35 UTC (permalink / raw)
To: David Miller; +Cc: hans.schillstrom, daniel.lezcano, netdev, Octavian Purdila
In-Reply-To: <20101014.080907.189690627.davem@davemloft.net>
David Miller <davem@davemloft.net> writes:
> From: ebiederm@xmission.com (Eric W. Biederman)
> Date: Wed, 13 Oct 2010 22:20:28 -0700
>
>> With the network namespace support we limit the scope of the test of
>> the invalidate to just a single network namespace, and as such
>> rt_is_expired stops being true for every cache entry. So we cannot
>> unconditionally throw away entire chains.
>>
>> All of which can be either done by network namespace equality or by
>> rt_is_expired(). Although Denis picked rt_is_expired() when he made
>> his change.
>
> Right, and I choose to use namespace equality which will completely
> compile into no code at all when namespace support is not in the
> kernel.
>
> Therefore, making the non-namespace case equivalent and as efficient
> as it always was.
Almost you still have the hash list inversion, which means you have
to at look at the rtable entry even on a one list long hash chain.
Perhaps I am looking at it wrong but once you look at the entries
I don't see the difference in the number of cache line faults
between one variant of the code and the other.
>> The only place it makes a noticable difference in practice is what
>> happens when we do batched deleletes of lots of network devices in
>> different network namespaces.
>>
>> During batched network device deletes in fib_netdev_event we do
>> rt_cache_flush(dev_net(dev), -1) for each network device. and then a
>> final rt_cache_flush_batch() to remove the invalidated entries. These
>> devices can be from multiple network namespaces, so I suspect that is
>> a savings worth having.
>
> How can it make a real difference even in this case? We'll obliterate
> all the entries, and then on subsequent passes we'll find nothing
> matching that namespace any more.
>
> Show me performance tests that show it makes any difference, please.
Octavian did you happen to measure the performance difference when you
added batching of routing table flushes?
>> So if we are going to change the tests we need to do something with
>> rt_cache_flush_batch(). Further I do not see what is confusing about
>> a test that asks if the routing cache entry is unusable. Is
>> rt_cache_expired() a bad name?
>
> It's not a bad name, it's just an unnecessary test that we don't need
> to even make in this specific place.
As long as we do something that is correct in the batched flush case
I am happy either way.
Eric
^ permalink raw reply
* Re: [PATCH net-next] tipc: cleanup function namespace
From: Stephen Hemminger @ 2010-10-14 18:33 UTC (permalink / raw)
To: Paul Gortmaker
Cc: Neil Horman, David Miller, netdev, allan.stephens, Jon Maloy
In-Reply-To: <4CB74391.9020400@windriver.com>
On Thu, 14 Oct 2010 13:53:21 -0400
Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> On 10-10-13 09:29 PM, Neil Horman wrote:
> > On Wed, Oct 13, 2010 at 08:23:24PM -0400, Paul Gortmaker wrote:
> >> On 10-10-13 07:20 PM, Stephen Hemminger wrote:
> >>> Do some cleanups of TIPC based on make namespacecheck
> >>> 1. Don't export unused symbols
> >>> 2. Eliminate dead code
> >>> 3. Make functions and variables local
> >>> 4. Rename buf_acquire to tipc_buf_acquire since it is used in several files
> >>>
> >>> Compile tested only.
> >>> This make break out of tree kernel modules that depend on TIPC routines.
> >>
> >> Hi Stephen,
> >>
> >> When I first started looking at TIPC code, I too came to the
> >> same conclusion as you did and was about to do #1,2,3 -- but
> >> then I was told that the exported symbols were part of an API
> >> and might be in use by folks here and there as per this thread:
> >>
> >> http://www.mail-archive.com/netdev@vger.kernel.org/msg30208.html
> >>
> > I think its telling the the argument in the above thread for keeping the API
> > were that users of it were out there and 'likely to contribute' in the future.
> > That thread was 3 years ago. They might be using the API from outside the
> > kernel tree, but they're not planning on contributing. As Christoph noted,
> > they're freeloaders. The community really doesn't need or want to maintain an
> > API like that. If these users are your customers, and removing the API is
> > unacceptable, perhaps its time to move the entire TIPC module out of tree.
>
> As I'd said -- I don't know what the use cases of these API users are,
> and so as far as I know they aren't customers either. For what it is
> worth, know that I personally wouldn't try and use a business case to
> justify a technically wrong decision here on netdev anyway.
>
> I was just describing the history of the situation, and suggesting
> one possible slower approach of phasing it out as a courtesy to those
> users, in the same way that the kernel community has extended that
> same courtesy with other things in feature-removal.txt
>
> In the end, since Jon is OK with the removal, and is in the process of
> communicating this to the API users he is aware of, I sure don't have
> any reason to try and save the API. If folks are good with having it
> just go away overnight, then great -- I'll be just as happy to see it
> disappear as you and Stephen. So, a long winded way of saying...
>
> Acked-by: Paul Gortmaker <paul.gortmaker@windriver.com>
How about putting an entry in feature-removal.txt with a short (6 month)
window?
--
^ permalink raw reply
* Re: [PATCH net-next] bnx2x: dont use netdev_alloc_skb()
From: David Miller @ 2010-10-14 18:25 UTC (permalink / raw)
To: eric.dumazet; +Cc: eilong, vladz, therbert, netdev, mchan
In-Reply-To: <1287080420.2712.102.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 14 Oct 2010 20:20:20 +0200
> Le jeudi 14 octobre 2010 à 20:17 +0200, Eilon Greenstein a écrit :
>> On Thu, 2010-10-14 at 10:39 -0700, David Miller wrote:
>> > From: "Vladislav Zolotarov" <vladz@broadcom.com>
>> > Date: Tue, 12 Oct 2010 02:12:41 -0700
>> >
>> > > Dave, if it can wait till tomorrow, I'd like to ask u to wait for Eilon's
>> > > final ACK.
>> >
>> > Ok, but it is two days later now :-)
>> >
>>
>> I was under the impression that the current path is to revert
>> b30973f877fea1a3fb84e05599890fcc082a88e5 instead of the original thread.
>> Eric - do you still want to apply the original patch from this thread?
>
> No, this patch is not needed, if the generic one is considered.
Ok.
^ permalink raw reply
* Re: [PATCH net-next] bnx2x: dont use netdev_alloc_skb()
From: Eric Dumazet @ 2010-10-14 18:20 UTC (permalink / raw)
To: eilong
Cc: David Miller, Vladislav Zolotarov, therbert@google.com,
netdev@vger.kernel.org, Michael Chan
In-Reply-To: <1287080255.22035.2.camel@lb-tlvb-eilong.il.broadcom.com>
Le jeudi 14 octobre 2010 à 20:17 +0200, Eilon Greenstein a écrit :
> On Thu, 2010-10-14 at 10:39 -0700, David Miller wrote:
> > From: "Vladislav Zolotarov" <vladz@broadcom.com>
> > Date: Tue, 12 Oct 2010 02:12:41 -0700
> >
> > > Dave, if it can wait till tomorrow, I'd like to ask u to wait for Eilon's
> > > final ACK.
> >
> > Ok, but it is two days later now :-)
> >
>
> I was under the impression that the current path is to revert
> b30973f877fea1a3fb84e05599890fcc082a88e5 instead of the original thread.
> Eric - do you still want to apply the original patch from this thread?
No, this patch is not needed, if the generic one is considered.
We might change copybreak stuff, but not all skb allocations in bnx2x
Thanks
^ permalink raw reply
* Re: [PATCH net-next] bnx2x: dont use netdev_alloc_skb()
From: Tom Herbert @ 2010-10-14 18:17 UTC (permalink / raw)
To: David Miller; +Cc: vladz, eric.dumazet, netdev, mchan, eilong
In-Reply-To: <20101014.103914.193713537.davem@davemloft.net>
On Thu, Oct 14, 2010 at 10:39 AM, David Miller <davem@davemloft.net> wrote:
> From: "Vladislav Zolotarov" <vladz@broadcom.com>
> Date: Tue, 12 Oct 2010 02:12:41 -0700
>
>> Dave, if it can wait till tomorrow, I'd like to ask u to wait for Eilon's
>> final ACK.
>
> Ok, but it is two days later now :-)
If the netdev_alloc_skb is reverted to do local node allocation as
Eric suggested, then this patch against bnx2x (and presumably similar
changes to other drivers) is no longer needed.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox