Netdev List
 help / color / mirror / Atom feed
* [PATCH 4/4] ethtool: Remove support for obsolete string query operations
From: Ben Hutchings @ 2009-10-01 21:33 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

The in-tree implementations have all been converted to
get_sset_count().

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 include/linux/ethtool.h |    4 ---
 net/core/ethtool.c      |   58 ++++++++--------------------------------------
 2 files changed, 10 insertions(+), 52 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 15e4eb7..aa0dcb3 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -495,10 +495,6 @@ struct ethtool_ops {
 	u32     (*get_priv_flags)(struct net_device *);
 	int     (*set_priv_flags)(struct net_device *, u32);
 	int	(*get_sset_count)(struct net_device *, int);
-
-	/* the following hooks are obsolete */
-	int	(*self_test_count)(struct net_device *);/* use get_sset_count */
-	int	(*get_stats_count)(struct net_device *);/* use get_sset_count */
 	int	(*get_rxnfc)(struct net_device *, struct ethtool_rxnfc *, void *);
 	int	(*set_rxnfc)(struct net_device *, struct ethtool_rxnfc *);
 	int     (*flash_device)(struct net_device *, struct ethtool_flash *);
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 4c12ddb..e195108 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -198,13 +198,6 @@ static int ethtool_get_drvinfo(struct net_device *dev, void __user *useraddr)
 		rc = ops->get_sset_count(dev, ETH_SS_PRIV_FLAGS);
 		if (rc >= 0)
 			info.n_priv_flags = rc;
-	} else {
-		/* code path for obsolete hooks */
-
-		if (ops->self_test_count)
-			info.testinfo_len = ops->self_test_count(dev);
-		if (ops->get_stats_count)
-			info.n_stats = ops->get_stats_count(dev);
 	}
 	if (ops->get_regs_len)
 		info.regdump_len = ops->get_regs_len(dev);
@@ -684,16 +677,10 @@ static int ethtool_self_test(struct net_device *dev, char __user *useraddr)
 	u64 *data;
 	int ret, test_len;
 
-	if (!ops->self_test)
-		return -EOPNOTSUPP;
-	if (!ops->get_sset_count && !ops->self_test_count)
+	if (!ops->self_test || !ops->get_sset_count)
 		return -EOPNOTSUPP;
 
-	if (ops->get_sset_count)
-		test_len = ops->get_sset_count(dev, ETH_SS_TEST);
-	else
-		/* code path for obsolete hook */
-		test_len = ops->self_test_count(dev);
+	test_len = ops->get_sset_count(dev, ETH_SS_TEST);
 	if (test_len < 0)
 		return test_len;
 	WARN_ON(test_len == 0);
@@ -728,36 +715,17 @@ static int ethtool_get_strings(struct net_device *dev, void __user *useraddr)
 	u8 *data;
 	int ret;
 
-	if (!ops->get_strings)
+	if (!ops->get_strings || !ops->get_sset_count)
 		return -EOPNOTSUPP;
 
 	if (copy_from_user(&gstrings, useraddr, sizeof(gstrings)))
 		return -EFAULT;
 
-	if (ops->get_sset_count) {
-		ret = ops->get_sset_count(dev, gstrings.string_set);
-		if (ret < 0)
-			return ret;
-
-		gstrings.len = ret;
-	} else {
-		/* code path for obsolete hooks */
-
-		switch (gstrings.string_set) {
-		case ETH_SS_TEST:
-			if (!ops->self_test_count)
-				return -EOPNOTSUPP;
-			gstrings.len = ops->self_test_count(dev);
-			break;
-		case ETH_SS_STATS:
-			if (!ops->get_stats_count)
-				return -EOPNOTSUPP;
-			gstrings.len = ops->get_stats_count(dev);
-			break;
-		default:
-			return -EINVAL;
-		}
-	}
+	ret = ops->get_sset_count(dev, gstrings.string_set);
+	if (ret < 0)
+		return ret;
+
+	gstrings.len = ret;
 
 	data = kmalloc(gstrings.len * ETH_GSTRING_LEN, GFP_USER);
 	if (!data)
@@ -798,16 +766,10 @@ static int ethtool_get_stats(struct net_device *dev, void __user *useraddr)
 	u64 *data;
 	int ret, n_stats;
 
-	if (!ops->get_ethtool_stats)
-		return -EOPNOTSUPP;
-	if (!ops->get_sset_count && !ops->get_stats_count)
+	if (!ops->get_ethtool_stats || !ops->get_sset_count)
 		return -EOPNOTSUPP;
 
-	if (ops->get_sset_count)
-		n_stats = ops->get_sset_count(dev, ETH_SS_STATS);
-	else
-		/* code path for obsolete hook */
-		n_stats = ops->get_stats_count(dev);
+	n_stats = ops->get_sset_count(dev, ETH_SS_STATS);
 	if (n_stats < 0)
 		return n_stats;
 	WARN_ON(n_stats == 0);

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply related

* NET: mkiss: Fix typo
From: Ralf Baechle @ 2009-10-01 21:42 UTC (permalink / raw)
  To: David S. Miller, netdev, Andreas Koensgen; +Cc: Matti Aarnio, Thomas Osterried

This typo was introduced by 5793f4be23f0171b4999ca68a39a9157b44139f3 on
October 14, 2005 ...

Reported-by: Matti Aarnio <matti.aarnio@zmailer.org>
Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

 drivers/net/hamradio/mkiss.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/hamradio/mkiss.c b/drivers/net/hamradio/mkiss.c
index 33b55f7..db4b7f1 100644
--- a/drivers/net/hamradio/mkiss.c
+++ b/drivers/net/hamradio/mkiss.c
@@ -258,7 +258,7 @@ static void ax_bump(struct mkiss *ax)
 			}
 			if (ax->crcmode != CRC_MODE_SMACK && ax->crcauto) {
 				printk(KERN_INFO
-				       "mkiss: %s: Switchting to crc-smack\n",
+				       "mkiss: %s: Switching to crc-smack\n",
 				       ax->dev->name);
 				ax->crcmode = CRC_MODE_SMACK;
 			}
@@ -272,7 +272,7 @@ static void ax_bump(struct mkiss *ax)
 			}
 			if (ax->crcmode != CRC_MODE_FLEX && ax->crcauto) {
 				printk(KERN_INFO
-				       "mkiss: %s: Switchting to crc-flexnet\n",
+				       "mkiss: %s: Switching to crc-flexnet\n",
 				       ax->dev->name);
 				ax->crcmode = CRC_MODE_FLEX;
 			}

^ permalink raw reply related

* Re: [PATCH net-next-2.6] bonding: set primary param via sysfs
From: David Miller @ 2009-10-01 21:39 UTC (permalink / raw)
  To: jpirko; +Cc: netdev, fubar, bonding-devel
In-Reply-To: <20090918121321.GB2801@psychotron.redhat.com>

From: Jiri Pirko <jpirko@redhat.com>
Date: Fri, 18 Sep 2009 14:13:22 +0200

> Primary module parameter passed to bonding is pernament. That means if you
> release the primary slave and enslave it again, it becomes the primary slave
> again. But if you set primary slave via sysfs, the primary slave is only set
> once and it's not remembered in bond->params structure. Therefore the setting is
> lost after releasing the primary slave. This simple one-liner fixes this.
> 
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>

Applied, thanks Jiri.

^ permalink raw reply

* Re: [PATCH] tg3: Remove prev_vlan_tag from struct tx_ring_info
From: David Miller @ 2009-10-01 21:38 UTC (permalink / raw)
  To: mcarlson; +Cc: eric.dumazet, netdev, mchan
In-Reply-To: <20090930172643.GA14439@xw6200.broadcom.net>

From: "Matt Carlson" <mcarlson@broadcom.com>
Date: Wed, 30 Sep 2009 10:26:43 -0700

> On Wed, Sep 30, 2009 at 07:07:52AM -0700, Eric Dumazet wrote:
>> prev_vlan_tag field is not used.
>> 
>> Patch saves 512*8 bytes per tx queue ring on 64bit arches.
>> 
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> 
> Looks good to me.
> 
> Acked-by: Matthew Carlson <mcarlson@broadcom.com>

Applied, thanks.

Eric, I had to apply this by hand because:

>> @@ -2412,7 +2412,6 @@ struct ring_info {
>>  
>>  struct tx_ring_info {
>>  	struct sk_buff                  *skb;
>> -	u32                             prev_vlan_tag;
>>  };

Your email client changed tabs into spaces.

^ permalink raw reply

* [PATCH 2/4] tehuti: Convert ethtool get_stats_count() ops to get_sset_count()
From: Ben Hutchings @ 2009-10-01 21:27 UTC (permalink / raw)
  To: David Miller; +Cc: Andy Gospodarek, Alexander Indenbaum, netdev

This string query operation was supposed to be replaced by the
generic get_sset_count() starting in 2007.  Convert tehuti's
implementation.

Also remove the dummy self-test name which was not used since tehuti
does not advertise any self-tests.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
Compile-tested only.

Ben.

 drivers/net/tehuti.c |   27 ++++++++++++---------------
 1 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/net/tehuti.c b/drivers/net/tehuti.c
index ec9dfb2..79d4868 100644
--- a/drivers/net/tehuti.c
+++ b/drivers/net/tehuti.c
@@ -2105,12 +2105,6 @@ err_pci:
 }
 
 /****************** Ethtool interface *********************/
-/* get strings for tests */
-static const char
- bdx_test_names[][ETH_GSTRING_LEN] = {
-	"No tests defined"
-};
-
 /* get strings for statistics counters */
 static const char
  bdx_stat_names[][ETH_GSTRING_LEN] = {
@@ -2380,9 +2374,6 @@ bdx_set_ringparam(struct net_device *netdev, struct ethtool_ringparam *ring)
 static void bdx_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
 {
 	switch (stringset) {
-	case ETH_SS_TEST:
-		memcpy(data, *bdx_test_names, sizeof(bdx_test_names));
-		break;
 	case ETH_SS_STATS:
 		memcpy(data, *bdx_stat_names, sizeof(bdx_stat_names));
 		break;
@@ -2390,15 +2381,21 @@ static void bdx_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
 }
 
 /*
- * bdx_get_stats_count - return number of 64bit statistics counters
+ * bdx_get_sset_count - return number of statistics or tests
  * @netdev
  */
-static int bdx_get_stats_count(struct net_device *netdev)
+static int bdx_get_sset_count(struct net_device *netdev, int stringset)
 {
 	struct bdx_priv *priv = netdev_priv(netdev);
-	BDX_ASSERT(ARRAY_SIZE(bdx_stat_names)
-		   != sizeof(struct bdx_stats) / sizeof(u64));
-	return ((priv->stats_flag) ? ARRAY_SIZE(bdx_stat_names)	: 0);
+
+	switch (stringset) {
+	case ETH_SS_STATS:
+		BDX_ASSERT(ARRAY_SIZE(bdx_stat_names)
+			   != sizeof(struct bdx_stats) / sizeof(u64));
+		return ((priv->stats_flag) ? ARRAY_SIZE(bdx_stat_names)	: 0);
+	default:
+		return -EINVAL;
+	}
 }
 
 /*
@@ -2441,7 +2438,7 @@ static void bdx_ethtool_ops(struct net_device *netdev)
 		.get_sg = ethtool_op_get_sg,
 		.get_tso = ethtool_op_get_tso,
 		.get_strings = bdx_get_strings,
-		.get_stats_count = bdx_get_stats_count,
+		.get_sset_count = bdx_get_sset_count,
 		.get_ethtool_stats = bdx_get_ethtool_stats,
 	};
 

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply related

* Re: clownix-spy plots any kernel variable history
From: David Miller @ 2009-10-01 21:31 UTC (permalink / raw)
  To: clownix; +Cc: linux-kernel, netdev
In-Reply-To: <1254432239.6772.1.camel@localhost>

From: clownix <clownix@clownix.net>
Date: Thu, 01 Oct 2009 23:23:59 +0200

Netdev CC:'d

> I have made a tool at http://clownix.net which can plot qdisc
> enqueues/dequeues/drops...
> 
> But the qdisc spy is only given as an example, basically any kernel
> variable can be plotted after you have writen only a few lines in a
> module, in this module, you subscribe to the klownix module and then the
> klownix kernel thread periodically gathers the variables and sends them
> through the netlink socket to a user daemon. A gtk interface requests
> the plotting of the history of the variable.
> 
> The package contains a little test of visualization of the hfsc qdisc in
> action.
> 
> If you visit the clownix site, beware, the tool is not the Cloonix-Net
> which is another project altogether.
> 
> Regards
> Vincent Perrier

^ permalink raw reply

* [PATCH 1/4] qeth: Convert ethtool get_stats_count() ops to get_sset_count()
From: Ben Hutchings @ 2009-10-01 21:24 UTC (permalink / raw)
  To: David Miller; +Cc: Ursula Braun, Frank Blaschka, linux-s390, netdev

This string query operation was supposed to be replaced by the
generic get_sset_count() starting in 2007.  Convert qeth's
implementation.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
This is not even compile-tested because I don't have an s390 compiler.
But it's simple enough that I think I got it right...

Ben.

 drivers/s390/net/qeth_core.h      |    2 +-
 drivers/s390/net/qeth_core_main.c |   11 ++++++++---
 drivers/s390/net/qeth_l2_main.c   |    4 ++--
 drivers/s390/net/qeth_l3_main.c   |    2 +-
 4 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 31a2b4e..e8f72d7 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -849,7 +849,7 @@ int qeth_do_send_packet_fast(struct qeth_card *, struct qeth_qdio_out_q *,
 			struct sk_buff *, struct qeth_hdr *, int, int, int);
 int qeth_do_send_packet(struct qeth_card *, struct qeth_qdio_out_q *,
 		    struct sk_buff *, struct qeth_hdr *, int);
-int qeth_core_get_stats_count(struct net_device *);
+int qeth_core_get_sset_count(struct net_device *, int);
 void qeth_core_get_ethtool_stats(struct net_device *,
 				struct ethtool_stats *, u64 *);
 void qeth_core_get_strings(struct net_device *, u32, u8 *);
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index c4a42d9..edee4dc 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -4305,11 +4305,16 @@ static struct {
 	{"tx csum"},
 };
 
-int qeth_core_get_stats_count(struct net_device *dev)
+int qeth_core_get_sset_count(struct net_device *dev, int stringset)
 {
-	return (sizeof(qeth_ethtool_stats_keys) / ETH_GSTRING_LEN);
+	switch (stringset) {
+	case ETH_SS_STATS:
+		return (sizeof(qeth_ethtool_stats_keys) / ETH_GSTRING_LEN);
+	default:
+		return -EINVAL;
+	}
 }
-EXPORT_SYMBOL_GPL(qeth_core_get_stats_count);
+EXPORT_SYMBOL_GPL(qeth_core_get_sset_count);
 
 void qeth_core_get_ethtool_stats(struct net_device *dev,
 		struct ethtool_stats *stats, u64 *data)
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index f4f3ca1..b61d5c7 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -866,7 +866,7 @@ static const struct ethtool_ops qeth_l2_ethtool_ops = {
 	.get_link = ethtool_op_get_link,
 	.get_strings = qeth_core_get_strings,
 	.get_ethtool_stats = qeth_core_get_ethtool_stats,
-	.get_stats_count = qeth_core_get_stats_count,
+	.get_sset_count = qeth_core_get_sset_count,
 	.get_drvinfo = qeth_core_get_drvinfo,
 	.get_settings = qeth_core_ethtool_get_settings,
 };
@@ -874,7 +874,7 @@ static const struct ethtool_ops qeth_l2_ethtool_ops = {
 static const struct ethtool_ops qeth_l2_osn_ops = {
 	.get_strings = qeth_core_get_strings,
 	.get_ethtool_stats = qeth_core_get_ethtool_stats,
-	.get_stats_count = qeth_core_get_stats_count,
+	.get_sset_count = qeth_core_get_sset_count,
 	.get_drvinfo = qeth_core_get_drvinfo,
 };
 
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index 073b6d3..4ca28c1 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -2957,7 +2957,7 @@ static const struct ethtool_ops qeth_l3_ethtool_ops = {
 	.set_tso     = qeth_l3_ethtool_set_tso,
 	.get_strings = qeth_core_get_strings,
 	.get_ethtool_stats = qeth_core_get_ethtool_stats,
-	.get_stats_count = qeth_core_get_stats_count,
+	.get_sset_count = qeth_core_get_sset_count,
 	.get_drvinfo = qeth_core_get_drvinfo,
 	.get_settings = qeth_core_ethtool_get_settings,
 };

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply related

* Re: WOL with flex filters, reflecting it in /sys
From: David Miller @ 2009-10-01 21:22 UTC (permalink / raw)
  To: Martin.Leisner; +Cc: linux-pm, netdev, martyleisner
In-Reply-To: <76FA3B279DD9DA48896E2B404944957203C09587@USA7061MS02.na.xerox.net>

From: "Leisner, Martin" <Martin.Leisner@xerox.com>
Date: Thu, 1 Oct 2009 13:55:03 -0700

> What I'm thinking of is a hierarchy in /sys/classes/net/<device>/wol/
> 
> wol/
>    /flex-filters
>       /0
>            data -- hex data to match
>            mask -- string of 1 and 0 -- if 1, the data is valid in the
> bit 
>                    position
>       ...
>       /n
> 	   data
>          mask
>   /wake_up_packet
>   /traditional --r/w (0/1)
>      /magic_packet
>      /arp
>      /unicast
>      /multicast
>      /broadcast
>      /phy

That's a pretty hairy interface for users to learn.  Are you
really sure this is nicer than adding this facility to ethtool? :-)

^ permalink raw reply

* Re: [RFC] pkt_sched: gen_estimator: Dont report fake rate   estimators
From: Jarek Poplawski @ 2009-10-01 21:21 UTC (permalink / raw)
  To: David Miller; +Cc: eric.dumazet, kaber, netdev
In-Reply-To: <20091001.141406.62574384.davem@davemloft.net>

David Miller wrote, On 10/01/2009 11:14 PM:

> From: Jarek Poplawski <jarkao2@gmail.com>
> Date: Thu, 01 Oct 2009 23:05:53 +0200
> 
>> Since you ask... I wonder about this whole int plus quite a bit of
>> struct unreadability for one flag only. Maybe it could be queried
>> on qdisc level (with a flag if necessary), and additional parameter
>> of gnet_stats_copy_rate_est()? (Qdiscs should have no problem with
>> setting this param for their classes too.)
> 
> Certainly, that's another approach to this problem.
> 
> But logically, just like we wouldn't emit a block of RED scheduler
> data to 'tc' unless RED is actually configured, it seems consistent to
> not emit estimator data when no estimator is even there.

Sure! I've exaggerated with this additional parameter. ;-)

Jarek P.

^ permalink raw reply

* Re: [linux-pm] WOL with flex filters, reflecting it in /sys
From: Rafael J. Wysocki @ 2009-10-01 21:23 UTC (permalink / raw)
  To: linux-pm; +Cc: Leisner, Martin, netdev, martyleisner
In-Reply-To: <76FA3B279DD9DA48896E2B404944957203C09587@USA7061MS02.na.xerox.net>

On Thursday 01 October 2009, Leisner, Martin wrote:
> I'm looking to extend the work of Mitch Williams to support flexible
> filters.
> (http://www.mail-archive.com/netdev@vger.kernel.org/msg60332.html}
> 
> Looking at the problem, would it be a good idea to reflect WOL
> characteristics in the /sys filesystem (instead of requiring ethtool to
> set them)?
> 
> Seems straightforward and easy enough.

I'm not sure about that.

The supported WoL settings seem to depend on the driver and whether WoL is
going to work also depends on the device's power/wakeup setting.

Thanks,
Rafael

^ permalink raw reply

* Performance questions using bridge and macvlan
From: Ira W. Snyder @ 2009-10-01 21:21 UTC (permalink / raw)
  To: netdev

Hello all,

I've got an "interesting" network setup (using bridge), and I wonder if
macvlan might help me get some more performance. Unfortunately, there
isn't much documentation for macvlan, so I'm asking here.

I have a computer acting as an ethernet bridge. NIC A is on a network
with a 1500 byte mtu. NIC B is a point-to-point ethernet device with a
64K mtu.

Adding both devices to a bridge using brctl works as expected. The
computer attached to NIC B can send/recv normal ethernet traffic onto
the outside network through NIC A.

Unfortunately, the bridge code does not fragment packets, and so the 64K
mtu is reduced to a 1500 byte mtu. This kills performance by a factor of
about 5x on the point-to-point device. All of my tests were using
netperf/netserver running on the machine doing the bridging.

Without bridge, using full 64K mtu packets, netperf gives ~600mbit/sec.
With brigde, using 1500 byte mtu packets, netperf gives ~120mbit/sec.

My question is this: is it possible to setup routing or macvlans such
that any traffic from the bridge machine itself travels across the
point-to-point link (at full 64K mtu), but any other traffic goes
through the bridge (using 1500 byte mtu).

I'm aware that either running NAT or routing will fragment packets and
solve my problem, but this introduces some complexity in my network
setup that I would like to avoid.

Thanks,
Ira

^ permalink raw reply

* Re: [PATCH] connector: Fix regression introduced by sid connector
From: Andrew Morton @ 2009-10-01 21:14 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: oleg, scott, zbr, linux-kernel, matthltc, davem, netdev
In-Reply-To: <200909300843.11208.borntraeger@de.ibm.com>

On Wed, 30 Sep 2009 08:43:11 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> Hi Andrew,
> 
> since commit 02b51df1b07b4e9ca823c89284e704cadb323cd1 (proc connector: add event 
> for process becoming session leader) we have the following warning:
> Badness at kernel/softirq.c:143
> [...]
> Krnl PSW : 0404c00180000000 00000000001481d4 (local_bh_enable+0xb0/0xe0)
> [...]
> Call Trace:
> ([<000000013fe04100>] 0x13fe04100)
>  [<000000000048a946>] sk_filter+0x9a/0xd0
>  [<000000000049d938>] netlink_broadcast+0x2c0/0x53c
>  [<00000000003ba9ae>] cn_netlink_send+0x272/0x2b0
>  [<00000000003baef0>] proc_sid_connector+0xc4/0xd4
>  [<0000000000142604>] __set_special_pids+0x58/0x90
>  [<0000000000159938>] sys_setsid+0xb4/0xd8
>  [<00000000001187fe>] sysc_noemu+0x10/0x16
>  [<00000041616cb266>] 0x41616cb266
> 
> The warning is
> --->    WARN_ON_ONCE(in_irq() || irqs_disabled());
> 
> The network code must not be called with disabled interrupts but
> sys_setsid holds the tasklist_lock with spinlock_irq while calling
> the connector. 
> After a discussion we agreed that we can move proc_sid_connector
> from __set_special_pids to sys_setsid.
> We also agreed that it is sufficient to change the check from
> task_session(curr) != pid into err > 0, since if we don't change the
> session, this means we were already the leader and return -EPERM.
> 
> One last thing:
> There is also daemonize(), and some people might want to get a
> notification in that case. Since daemonize() is only needed if a user
> space does kernel_thread this does not look important (and there seems
> to be no consensus if this connector should be called in daemonize). If
> we really want this, we can add proc_sid_connector to daemonize() in an
> additional patch (Scott?)
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> CCed: Scott James Remnant <scott@ubuntu.com>
> CCed: Matt Helsley <matthltc@us.ibm.com>
> CCed: David S. Miller <davem@davemloft.net>
> Acked-by: Oleg Nesterov <oleg@redhat.com>
> Acked-by: Evgeniy Polyakov <zbr@ioremap.net>
> ---
>  kernel/exit.c |    4 +---
>  kernel/sys.c  |    2 ++
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> Index: linux-2.6/kernel/exit.c
> ===================================================================
> --- linux-2.6.orig/kernel/exit.c
> +++ linux-2.6/kernel/exit.c
> @@ -359,10 +359,8 @@ void __set_special_pids(struct pid *pid)
>  {
>  	struct task_struct *curr = current->group_leader;
>  
> -	if (task_session(curr) != pid) {
> +	if (task_session(curr) != pid)
>  		change_pid(curr, PIDTYPE_SID, pid);
> -		proc_sid_connector(curr);
> -	}
>  
>  	if (task_pgrp(curr) != pid)
>  		change_pid(curr, PIDTYPE_PGID, pid);
> Index: linux-2.6/kernel/sys.c
> ===================================================================
> --- linux-2.6.orig/kernel/sys.c
> +++ linux-2.6/kernel/sys.c
> @@ -1110,6 +1110,8 @@ SYSCALL_DEFINE0(setsid)
>  	err = session;
>  out:
>  	write_unlock_irq(&tasklist_lock);
> +	if (err > 0)
> +		proc_sid_connector(sid);
>  	return err;
>  }

kernel/sys.c: In function 'sys_setsid':
kernel/sys.c:1114: warning: passing argument 1 of 'proc_sid_connector' from incompatible pointer type

Pass a `struct pid*' into a function expecting a `struct task_struct*'.
Surely it will crash??

Please redo the patch and test it more carefully.

^ permalink raw reply

* Re: [RFC] pkt_sched: gen_estimator: Dont report fake rate estimators
From: David Miller @ 2009-10-01 21:14 UTC (permalink / raw)
  To: jarkao2; +Cc: eric.dumazet, kaber, netdev
In-Reply-To: <4AC519B1.1090701@gmail.com>

From: Jarek Poplawski <jarkao2@gmail.com>
Date: Thu, 01 Oct 2009 23:05:53 +0200

> Since you ask... I wonder about this whole int plus quite a bit of
> struct unreadability for one flag only. Maybe it could be queried
> on qdisc level (with a flag if necessary), and additional parameter
> of gnet_stats_copy_rate_est()? (Qdiscs should have no problem with
> setting this param for their classes too.)

Certainly, that's another approach to this problem.

But logically, just like we wouldn't emit a block of RED scheduler
data to 'tc' unless RED is actually configured, it seems consistent to
not emit estimator data when no estimator is even there.

^ permalink raw reply

* Re: [PATCH 04/31] mm: tag reseve pages
From: David Rientjes @ 2009-10-01 21:09 UTC (permalink / raw)
  To: Suresh Jayaraman
  Cc: Linus Torvalds, Andrew Morton, linux-kernel, linux-mm, netdev,
	Neil Brown, Miklos Szeredi, Wouter Verhelst, Peter Zijlstra,
	trond.myklebust
In-Reply-To: <1254405917-15796-1-git-send-email-sjayaraman@suse.de>

On Thu, 1 Oct 2009, Suresh Jayaraman wrote:

> Index: mmotm/mm/page_alloc.c
> ===================================================================
> --- mmotm.orig/mm/page_alloc.c
> +++ mmotm/mm/page_alloc.c
> @@ -1501,8 +1501,10 @@ zonelist_scan:
>  try_this_zone:
>  		page = buffered_rmqueue(preferred_zone, zone, order,
>  						gfp_mask, migratetype);
> -		if (page)
> +		if (page) {
> +			page->reserve = !!(alloc_flags & ALLOC_NO_WATERMARKS);
>  			break;
> +		}
>  this_zone_full:
>  		if (NUMA_BUILD)
>  			zlc_mark_zone_full(zonelist, z);

page->reserve won't necessary indicate that access to reserves was 
_necessary_ for the allocation to succeed, though.  This will mark any 
page being allocated under PF_MEMALLOC as reserve when all zones may be 
well above their min watermarks.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [RFC] pkt_sched: gen_estimator: Dont report fake rate   estimators
From: Jarek Poplawski @ 2009-10-01 21:05 UTC (permalink / raw)
  To: David Miller; +Cc: eric.dumazet, kaber, netdev
In-Reply-To: <20091001.123725.02194664.davem@davemloft.net>

David Miller wrote, On 10/01/2009 09:37 PM:

> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Thu, 01 Oct 2009 21:07:51 +0200
> 
>> We currently send TCA_STATS_RATE_EST elements to netlink users, even if no estimator
>> is running.
>>
>> # tc -s -d qdisc
>> qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>>  Sent 112833764978 bytes 1495081739 pkt (dropped 0, overlimits 0 requeues 0)
>>  rate 0bit 0pps backlog 0b 0p requeues 0
>>
>> User has no way to tell if the "rate 0bit 0pps" is a real estimation, or a fake
>> one (because no estimator is active)
>>
>> After this patch, tc command output is :
>> $ tc -s -d qdisc
>> qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>>  Sent 561075 bytes 1196 pkt (dropped 0, overlimits 0 requeues 0)
>>  backlog 0b 0p requeues 0
>>
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> 
> I'm generally fine with this idea.
> 
> The new behavior is certainly more intuitive even to me :-)
> 
> Unless there are other objections I'm ok with this and I'll apply

Since you ask... I wonder about this whole int plus quite a bit of
struct unreadability for one flag only. Maybe it could be queried
on qdisc level (with a flag if necessary), and additional parameter
of gnet_stats_copy_rate_est()? (Qdiscs should have no problem with
setting this param for their classes too.)


Jarek P.

^ permalink raw reply

* Re: [PATCH 03/31] mm: expose gfp_to_alloc_flags()
From: David Rientjes @ 2009-10-01 21:03 UTC (permalink / raw)
  To: Suresh Jayaraman
  Cc: Linus Torvalds, Andrew Morton, linux-kernel, linux-mm, netdev,
	Neil Brown, Miklos Szeredi, Wouter Verhelst, Peter Zijlstra,
	trond.myklebust
In-Reply-To: <1254405903-15760-1-git-send-email-sjayaraman@suse.de>

On Thu, 1 Oct 2009, Suresh Jayaraman wrote:

> From: Peter Zijlstra <a.p.zijlstra@chello.nl> 
> 
> Expose the gfp to alloc_flags mapping, so we can use it in other parts
> of the vm.
> 
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Signed-off-by: Suresh Jayaraman <sjayaraman@suse.de>

Nack, these flags are internal to the page allocator and exporting them to 
generic VM code is unnecessary.

The only bit you actually use in your patchset is ALLOC_NO_WATERMARKS to 
determine whether a particular allocation can use memory reserves.  I'd 
suggest adding a bool function that returns whether the current context is 
given access to reserves including your new __GFP_MEMALLOC flag and 
exporting that instead.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* WOL with flex filters, reflecting it in /sys
From: Leisner, Martin @ 2009-10-01 20:55 UTC (permalink / raw)
  To: linux-pm, netdev; +Cc: martyleisner

I'm looking to extend the work of Mitch Williams to support flexible
filters.
(http://www.mail-archive.com/netdev@vger.kernel.org/msg60332.html}

Looking at the problem, would it be a good idea to reflect WOL
characteristics in the /sys filesystem (instead of requiring ethtool to
set them)?

Seems straightforward and easy enough.

I know the VIA Rhinefet and Intel e1000 support filters -- and some
freescale
seems to have some front end that support filters...and we've also
implemented filters in software...

What I'm thinking of is a hierarchy in /sys/classes/net/<device>/wol/

wol/
   /flex-filters
      /0
           data -- hex data to match
           mask -- string of 1 and 0 -- if 1, the data is valid in the
bit 
                   position
      ...
      /n
	   data
         mask
  /wake_up_packet
  /traditional --r/w (0/1)
     /magic_packet
     /arp
     /unicast
     /multicast
     /broadcast
     /phy
    
marty

^ permalink raw reply

* Re: [PATCH 30/31] Fix use of uninitialized variable in cache_grow()
From: David Rientjes @ 2009-10-01 20:49 UTC (permalink / raw)
  To: Suresh Jayaraman
  Cc: Linus Torvalds, Andrew Morton, linux-kernel, linux-mm, netdev,
	Neil Brown, Miklos Szeredi, Wouter Verhelst, Peter Zijlstra,
	trond.myklebust
In-Reply-To: <1254406257-16735-1-git-send-email-sjayaraman@suse.de>

On Thu, 1 Oct 2009, Suresh Jayaraman wrote:

> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> This fixes a bug in reserve-slub.patch.
> 
> If cache_grow() was called with objp != NULL then the 'reserve' local
> variable wasn't initialized. This resulted in ac->reserve being set to
> a rubbish value.  Due to this in some circumstances huge amounts of
> slab pages were allocated (due to slab_force_alloc() returning true),
> which caused atomic page allocation failures and slowdown of the
> system.
> 
> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
> Signed-off-by: Suresh Jayaraman <sjayaraman@suse.de>
> ---
>  mm/slab.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> Index: mmotm/mm/slab.c
> ===================================================================
> --- mmotm.orig/mm/slab.c
> +++ mmotm/mm/slab.c
> @@ -2760,7 +2760,7 @@ static int cache_grow(struct kmem_cache
>  	size_t offset;
>  	gfp_t local_flags;
>  	struct kmem_list3 *l3;
> -	int reserve;
> +	int reserve = -1;
>  
>  	/*
>  	 * Be lazy and only check for valid flags here,  keeping it out of the
> @@ -2816,7 +2816,8 @@ static int cache_grow(struct kmem_cache
>  	if (local_flags & __GFP_WAIT)
>  		local_irq_disable();
>  	check_irq_off();
> -	slab_set_reserve(cachep, reserve);
> +	if (reserve != -1)
> +		slab_set_reserve(cachep, reserve);
>  	spin_lock(&l3->list_lock);
>  
>  	/* Make slab active. */

Given the patch description, shouldn't this be a test for objp != NULL 
instead, then?

If so, it doesn't make sense because reserve will only be initialized when 
objp == NULL in the call to kmem_getpages() from cache_grow().


The title of the patch suggests this is just dealing with an uninitialized 
auto variable so the anticipated change would be from "int reserve" to 
"int uninitialized_var(result)".

^ permalink raw reply

* Re: [PATCH 01/31] mm: serialize access to min_free_kbytes
From: David Rientjes @ 2009-10-01 20:35 UTC (permalink / raw)
  To: Suresh Jayaraman
  Cc: Linus Torvalds, Andrew Morton, linux-kernel, linux-mm, netdev,
	Neil Brown, Miklos Szeredi, Wouter Verhelst, Peter Zijlstra,
	trond.myklebust
In-Reply-To: <1254405871-15687-1-git-send-email-sjayaraman@suse.de>

On Thu, 1 Oct 2009, Suresh Jayaraman wrote:

> From: Peter Zijlstra <a.p.zijlstra@chello.nl> 
> 
> There is a small race between the procfs caller and the memory hotplug caller
> of setup_per_zone_wmarks(). Not a big deal, but the next patch will add yet
> another caller. Time to close the gap.
> 

By "next patch," you mean "mm: emegency pool" (patch 08/31)?

If so, can't you eliminate var_free_mutex entirely from that patch and 
take min_free_lock in adjust_memalloc_reserve() instead?

 [ __adjust_memalloc_reserve() would call __setup_per_zone_wmarks()
   under lock instead, now. ]

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH 2/7] mlx4: Query for loopback support
From: David Miller @ 2009-10-01 20:34 UTC (permalink / raw)
  To: yevgenyp; +Cc: netdev
In-Reply-To: <4AC4BDB7.2040306@mellanox.co.il>

From: Yevgeny Petrilin <yevgenyp@mellanox.co.il>
Date: Thu, 01 Oct 2009 16:33:27 +0200

> +	int			loopback_support;

You don't need a full 32-bit integer to store what
amounts to a single bit of boolean state.

Please store this in a flags word somewhere, or
if nothing appropriate exists, use a 'bool'.

^ permalink raw reply

* Re: [PATCH v3] skge: use unique IRQ name
From: David Miller @ 2009-10-01 20:31 UTC (permalink / raw)
  To: shemminger; +Cc: mschmidt, netdev
In-Reply-To: <20091001111323.260d49bf@s6510>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Thu, 1 Oct 2009 11:13:23 -0700

> $ grep skge /proc/interrupts
>  17:        169   IO-APIC-fasteoi   skge@0000:00:0a.0, eth0

Please fix this example output to match the code :-)

> +	sprintf(hw->irq_name, DRV_NAME "@pci:%s", pci_name(pdev));

Thanks!

^ permalink raw reply

* Re: setsockopt option struct with an address?
From: David Miller @ 2009-10-01 20:30 UTC (permalink / raw)
  To: andy.grover; +Cc: netdev
In-Reply-To: <c0a09e5c0910011317n76fba7e8q6de7dd35268654b9@mail.gmail.com>

From: Andrew Grover <andy.grover@gmail.com>
Date: Thu, 1 Oct 2009 13:17:03 -0700

> Or should the added member be a struct sockaddr, or a sockaddr_in, or
> a sockaddr_storage, or what? And should it be a pointer to that
> struct, or embedded?

You probably should use sockaddr_storage, I mean you will want to
support ipv6 at some point right? :-)

^ permalink raw reply

* setsockopt option struct with an address?
From: Andrew Grover @ 2009-10-01 20:17 UTC (permalink / raw)
  To: netdev

Hi all, I'm looking for some advice on defining a new interface.

RDS has an existing interface to prepare a memory region for use in an
RDMA operation, called GET_MR. (see include/linux/rds.h). It basically
takes an address and length, and returns a handle that later commands
use to reference the region.

We need to add an additional sockopt that also passes in the remote
host's address, because the current method for defining memory regions
binds their use to a particular remote host only.

Here's the original sockopt's args:

struct rds_get_mr_args {
        struct rds_iovec vec;
        u_int64_t       cookie_addr;
        uint64_t        flags;
};

Here is a swag at the new sockopt's args:

struct rds_get_mr_for_dest_args {
        u_int32_t       dest_addr;   /*   added this */
        struct rds_iovec vec;
        u_int64_t       cookie_addr;
        uint64_t        flags;
};

Or should the added member be a struct sockaddr, or a sockaddr_in, or
a sockaddr_storage, or what? And should it be a pointer to that
struct, or embedded?

Much thanks -- Regards -- Andy

^ permalink raw reply

* Re: [RFC][PATCH] ethtool: Add reset operation
From: David Miller @ 2009-10-01 20:14 UTC (permalink / raw)
  To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1254426195.2735.16.camel@achroite>

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Thu, 01 Oct 2009 20:43:15 +0100

> After updating firmware stored in flash, users may wish to reset the
> relevant hardware and start the new firmware immediately.  This should
> not be completely automatic as it may be disruptive.
> 
> A selective reset may also be useful for debugging or diagnostics.
> 
> This adds a separate reset operation which takes flags indicating the
> components to be reset.  Drivers are allowed to reset only a subset of
> those requested, and must report the actual subset.  This allows the
> use of generic component masks and some future expansion.

This looks fine to me.

^ permalink raw reply

* Re: [PATCH 0/2] cfg80211: firmware and hardware version
From: Inaky Perez-Gonzalez @ 2009-10-01 20:12 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: John W. Linville, Kalle Valo, linux-wireless@vger.kernel.org,
	netdev@vger.kernel.org
In-Reply-To: <43e72e890910011256v18b30e7ck420ce80b5d35fdcb@mail.gmail.com>

On Thu, 2009-10-01 at 13:56 -0600, Luis R. Rodriguez wrote:
> On Thu, Oct 1, 2009 at 5:07 PM, John W. Linville <linville@tuxdriver.com> wrote:
> 
> > I don't predict a huge problem if there are
> > valid extensions required for use by wireless drivers in the future.
> > But for now, I'd like to see us make use of some of the debugging
> > facilities available in the ethtool API -- hopefully the iwlwifi guys
> > are listening... ;-)
> 
> Does the same apply to wimax then? Ethtool for 802.11 and wimax? Eh.

Not really -- WiMAX is not eth-frame based, but IP based.

The WiMAX stack doesn't require any type of framing/network device
typing requirement. That is left up to the device driver writer
(although yes, emulating eth is easier).

-- 
-- Inaky



^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox