Netdev List
 help / color / mirror / Atom feed
* [PATCH] rps: keep the old behavior on SMP without rps
From: Changli Gao @ 2010-03-31  6:16 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, xiaosuo

keep the old behavior on SMP without rps

RPS introduces a lock operation to per cpu variable input_pkt_queue on
SMP whenever rps is enabled or not. On SMP without RPS, this lock isn't
needed at all.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
net/core/dev.c | 42 ++++++++++++++++++++++++++++--------------
1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 3e7fa16..d154f3b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -207,6 +207,20 @@ static inline struct hlist_head *dev_index_hash(struct net *net, int ifindex)
 	return &net->dev_index_head[ifindex & (NETDEV_HASHENTRIES - 1)];
 }
 
+static inline void rps_lock(struct softnet_data *queue)
+{
+#ifdef CONFIG_RPS
+	spin_lock(&queue->input_pkt_queue.lock);
+#endif
+}
+
+static inline void rps_unlock(struct softnet_data *queue)
+{
+#ifdef CONFIG_RPS
+	spin_unlock(&queue->input_pkt_queue.lock);
+#endif
+}
+
 /* Device list insertion */
 static int list_netdevice(struct net_device *dev)
 {
@@ -2314,13 +2328,13 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu)
 	local_irq_save(flags);
 	__get_cpu_var(netdev_rx_stat).total++;
 
-	spin_lock(&queue->input_pkt_queue.lock);
+	rps_lock(queue);
 	if (queue->input_pkt_queue.qlen <= netdev_max_backlog) {
 		if (queue->input_pkt_queue.qlen) {
 enqueue:
 			__skb_queue_tail(&queue->input_pkt_queue, skb);
-			spin_unlock_irqrestore(&queue->input_pkt_queue.lock,
-			    flags);
+			rps_unlock(queue);
+			local_irq_restore(flags);
 			return NET_RX_SUCCESS;
 		}
 
@@ -2342,7 +2356,7 @@ enqueue:
 		goto enqueue;
 	}
 
-	spin_unlock(&queue->input_pkt_queue.lock);
+	rps_unlock(queue);
 
 	__get_cpu_var(netdev_rx_stat).dropped++;
 	local_irq_restore(flags);
@@ -2767,19 +2781,19 @@ int netif_receive_skb(struct sk_buff *skb)
 EXPORT_SYMBOL(netif_receive_skb);
 
 /* Network device is going away, flush any packets still pending  */
-static void flush_backlog(struct net_device *dev, int cpu)
+static void flush_backlog(void *arg)
 {
-	struct softnet_data *queue = &per_cpu(softnet_data, cpu);
+	struct net_device *dev = arg;
+	struct softnet_data *queue = &__get_cpu_var(softnet_data);
 	struct sk_buff *skb, *tmp;
-	unsigned long flags;
 
-	spin_lock_irqsave(&queue->input_pkt_queue.lock, flags);
+	rps_lock(queue);
 	skb_queue_walk_safe(&queue->input_pkt_queue, skb, tmp)
 		if (skb->dev == dev) {
 			__skb_unlink(skb, &queue->input_pkt_queue);
 			kfree_skb(skb);
 		}
-	spin_unlock_irqrestore(&queue->input_pkt_queue.lock, flags);
+	rps_unlock(queue);
 }
 
 static int napi_gro_complete(struct sk_buff *skb)
@@ -3092,14 +3106,16 @@ static int process_backlog(struct napi_struct *napi, int quota)
 	do {
 		struct sk_buff *skb;
 
-		spin_lock_irq(&queue->input_pkt_queue.lock);
+		local_irq_disable();
+		rps_lock(queue);
 		skb = __skb_dequeue(&queue->input_pkt_queue);
 		if (!skb) {
 			__napi_complete(napi);
 			spin_unlock_irq(&queue->input_pkt_queue.lock);
 			break;
 		}
-		spin_unlock_irq(&queue->input_pkt_queue.lock);
+		rps_unlock(queue);
+		local_irq_enable();
 
 		__netif_receive_skb(skb);
 	} while (++work < quota && jiffies == start_time);
@@ -5549,7 +5565,6 @@ void netdev_run_todo(void)
 	while (!list_empty(&list)) {
 		struct net_device *dev
 			= list_first_entry(&list, struct net_device, todo_list);
-		int i;
 		list_del(&dev->todo_list);
 
 		if (unlikely(dev->reg_state != NETREG_UNREGISTERING)) {
@@ -5561,8 +5576,7 @@ void netdev_run_todo(void)
 
 		dev->reg_state = NETREG_UNREGISTERED;
 
-		for_each_online_cpu(i)
-			flush_backlog(dev, i);
+		on_each_cpu(flush_backlog, dev, 1);
 
 		netdev_wait_allrefs(dev);
 


^ permalink raw reply related

* Re: [PATCH 0/6] tagged sysfs support
From: Tejun Heo @ 2010-03-31  6:25 UTC (permalink / raw)
  To: Kay Sievers
  Cc: Eric W. Biederman, Greg Kroah-Hartman, Greg KH, linux-kernel,
	Cornelia Huck, linux-fsdevel, Eric Dumazet, Benjamin LaHaise,
	Serge Hallyn, netdev
In-Reply-To: <s2hac3eb2511003302251rcbae8767ne21e9daf1546c849@mail.gmail.com>

On 03/31/2010 02:51 PM, Kay Sievers wrote:
> Another problem to solve is that sysfs does not allow us to symlink
> regular files, only directories, so we can currently not create the
> class-wide attributes as symlinks to the proper file in
> /sys/subsystem/.

Making sysfs allow symlinks to attributes shouldn't be too hard if
it's gonna help make things more logical overall.

Thanks.

-- 
tejun

^ permalink raw reply

* RE: re-submit2 [ANNOUNCEMENT] NET: usb: sierra_net.c driver
From: Rory Filer @ 2010-03-31  6:21 UTC (permalink / raw)
  To: David Miller, Elina Pasheva
  Cc: dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20100330.231051.113008279.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

 -----Original Message-----
> >
> > Applied, thanks.
> 
> Nevermind, reverted, it doesn't even build.
> 
> drivers/net/usb/sierra_net.c: In function 'check_ethip_packet':
> drivers/net/usb/sierra_net.c:221:3: error: implicit declaration of
> function 'deverr'
> 
> Really, this should never ever happen, and there is simply
> no excuse at all for this.

Well actually there is an excuse, but I'm sure you don't want to
know what it is. 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: drivers/net: Add missing unlock
From: David Miller @ 2010-03-31  6:17 UTC (permalink / raw)
  To: julia; +Cc: netdev


Applied, thanks.

^ permalink raw reply

* Re: re-submit2 [ANNOUNCEMENT] NET: usb: sierra_net.c driver
From: David Miller @ 2010-03-31  6:10 UTC (permalink / raw)
  To: epasheva-ywE8TTl5eJHWpu6QEFMNjNBPR1lH4CV8
  Cc: dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f,
	rfiler-ywE8TTl5eJHWpu6QEFMNjNBPR1lH4CV8,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20100330.230932.244208287.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

From: David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Date: Tue, 30 Mar 2010 23:09:32 -0700 (PDT)

> From: Elina Pasheva <epasheva-ywE8TTl5eJHWpu6QEFMNjNBPR1lH4CV8@public.gmane.org>
> Date: Mon, 29 Mar 2010 13:13:16 -0700
> 
>> Subject: re-submit2 [ANNOUNCEMENT] NET: usb: sierra_net.c driver
>> From: Elina Pasheva <epasheva-ywE8TTl5eJHWpu6QEFMNjNBPR1lH4CV8@public.gmane.org>
> 
> Applied, thanks.

Nevermind, reverted, it doesn't even build.

drivers/net/usb/sierra_net.c: In function 'check_ethip_packet':
drivers/net/usb/sierra_net.c:221:3: error: implicit declaration of function 'deverr'

Really, this should never ever happen, and there is simply
no excuse at all for this.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: re-submit2 [ANNOUNCEMENT] NET: usb: sierra_net.c driver
From: David Miller @ 2010-03-31  6:09 UTC (permalink / raw)
  To: epasheva-ywE8TTl5eJHWpu6QEFMNjNBPR1lH4CV8
  Cc: dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f,
	rfiler-ywE8TTl5eJHWpu6QEFMNjNBPR1lH4CV8,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1269893596.9242.7.camel@Linuxdev3>

From: Elina Pasheva <epasheva-ywE8TTl5eJHWpu6QEFMNjNBPR1lH4CV8@public.gmane.org>
Date: Mon, 29 Mar 2010 13:13:16 -0700

> Subject: re-submit2 [ANNOUNCEMENT] NET: usb: sierra_net.c driver
> From: Elina Pasheva <epasheva-ywE8TTl5eJHWpu6QEFMNjNBPR1lH4CV8@public.gmane.org>

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/2] net: gianfar - align BD ring size console messages
From: David Miller @ 2010-03-31  6:09 UTC (permalink / raw)
  To: kim.phillips; +Cc: avorontsov, Sandeep.Kumar, galak, afleming, netdev
In-Reply-To: <1269986062-1697-2-git-send-email-kim.phillips@freescale.com>

From: Kim Phillips <kim.phillips@freescale.com>
Date: Tue, 30 Mar 2010 16:54:22 -0500

> fix this:
> 
> eth2: :RX BD ring size for Q[0]: 256
> eth2:TX BD ring size for Q[0]: 256
> 
> to look like:
> 
> eth2: RX BD ring size for Q[0]: 256
> eth2: TX BD ring size for Q[0]: 256
> 
> Signed-off-by: Kim Phillips <kim.phillips@freescale.com>

Applied.

^ permalink raw reply

* Re: [PATCH 1/2] net: gianfar - initialize per-queue statistics
From: David Miller @ 2010-03-31  6:09 UTC (permalink / raw)
  To: kim.phillips; +Cc: avorontsov, Sandeep.Kumar, galak, afleming, netdev
In-Reply-To: <1269986062-1697-1-git-send-email-kim.phillips@freescale.com>

From: Kim Phillips <kim.phillips@freescale.com>
Date: Tue, 30 Mar 2010 16:54:21 -0500

> Interfaces come up claiming having already received 3.0 GiB.
> Use kzalloc to properly initialize per-queue data.
> 
> Signed-off-by: Kim Phillips <kim.phillips@freescale.com>

Applied.

^ permalink raw reply

* Re: [PATCH] gianfar: Fix a memory leak in gianfar close code
From: David Miller @ 2010-03-31  6:09 UTC (permalink / raw)
  To: afleming; +Cc: netdev, eric.dumazet, Sandeep.Kumar
In-Reply-To: <1269913343-6566-1-git-send-email-afleming@freescale.com>

From: Andy Fleming <afleming@freescale.com>
Date: Mon, 29 Mar 2010 20:42:23 -0500

> gianfar needed to ensure existence of the *skbuff arrays before
> freeing the skbs in them, rather than ensuring their nonexistence.
> 
> Signed-off-by: Andy Fleming <afleming@freescale.com>

Although some refactorings have been suggested, this fixes the bug
straightforwardly and is therefore the most appropriate fix for
net-2.6

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] Allow MACB to connect to a higher addresses PHY.
From: Grant Likely @ 2010-03-31  6:08 UTC (permalink / raw)
  To: Anders Darander
  Cc: David S. Miller, Ralf Baechle, Maxime Bizon, David Daney,
	Sekhar Nori, Anton Vorontsov, Andy Fleming, netdev, linux-kernel
In-Reply-To: <20100331055306.GC4837@datarespons.se>

On Tue, Mar 30, 2010 at 11:53 PM, Anders Darander
<anders.darander@datarespons.se> wrote:
> * Grant Likely <grant.likely@secretlab.ca> [100330 16:42]:
> One question, if I add such an kernel parameter, would it be OK to
> implement something like this in the macb-driver:
>
> if parameter == SOME_DEFAULT
>        phydev = phy_find_first(bp->mii_bus);
> else
>        phydev = bp->mii_bus->phy_map[parameter];
>
> I.e. if the parameter is set to some default value, the old behaviour is
> retained, otherwise the specified parameter is used for the phy_addr?

Absolutely.

g.

^ permalink raw reply

* Re: [PATCH] Allow MACB to connect to a higher addresses PHY.
From: Anders Darander @ 2010-03-31  5:53 UTC (permalink / raw)
  To: Grant Likely
  Cc: David S. Miller, Ralf Baechle, Maxime Bizon, David Daney,
	Sekhar Nori, Anton Vorontsov, Andy Fleming, netdev, linux-kernel
In-Reply-To: <fa686aa41003300741x5ef9bdbpf56133304fe4d6e6@mail.gmail.com>

Hi Grant,

Thanks for the feedback!

* Grant Likely <grant.likely@secretlab.ca> [100330 16:42]:
> On Tue, Mar 30, 2010 at 3:58 AM, Anders Darander
> <anders.darander@gmail.com> wrote:
> > Using the Atmel MACB together with an integrated switch, can make only port 1
> > work. This is caused by macb_mii_probe trying to attach the MAC to the first
> > PHY, which often is on one of the external ports.
> >
> > E.g. the Micrel KSZ8873 connects to the MAC on port 3, thus phy_addr should be
> > set to 3.

> I understand what you are trying to do, but this is the wrong way to
> go about it.  Hard coding it into Kconfig breaks multiplatform
> kernels.  Besides, systems may have more than one physical MDIO bus.
> This patch would make CONFIG_SWITCHING_PHY_ADDR the only address
> accessible on all MDIO busses.

True, it would at least make all addresses < CONFIG_SWITCHING_PHY_ADDR
non-accessible.

We developed the patch to solve this problem on a 2.6.29-kernel, in
which the loop in question is directly in the macb_mii_probe() in the
macb.c, thus, the original patch is at least a _little_ bit less ugly...

> Nak.

I fully agree on the nak; I completely forgot that the functions in
phy_devices should be used from more interfaces than only the macb.

> The right thing to do is to add a runtime configuration option (ie.
> kernel parameter or platform data) to the mac driver to specify
> exactly which PHY address it is supposed to use.

I'll see if I can get some time to do such a thing. Unfortunately, it'll
probably have to be dealt with on my sparetime (the customer is quite
unlikely to let me work on that).

One question, if I add such an kernel parameter, would it be OK to
implement something like this in the macb-driver:

if parameter == SOME_DEFAULT
	phydev = phy_find_first(bp->mii_bus);
else
	phydev = bp->mii_bus->phy_map[parameter];

I.e. if the parameter is set to some default value, the old behaviour is
retained, otherwise the specified parameter is used for the phy_addr?

I'd appreciate your input on such a design.

Best regards,
Anders Darander


^ permalink raw reply

* Re: [PATCH] e1000e: typo corrections
From: David Miller @ 2010-03-31  6:01 UTC (permalink / raw)
  To: joe; +Cc: jeffrey.t.kirsher, netdev, gospo, bruce.w.allan
In-Reply-To: <1269670619.1603.9.camel@Joe-Laptop.home>

From: Joe Perches <joe@perches.com>
Date: Fri, 26 Mar 2010 23:16:59 -0700

> Here are the other miscellaneous corrections
> done by an earlier larger suggested patch now
> made unnecessary by a less invasive change.
> 
> Correct a few missing newlines from logging
> messages and a typo fix.  Fix speed/duplex
> logging message.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

I'll apply this to net-next-2.6, thanks.

^ permalink raw reply

* Re: [PATCH] igb: update hw_debug macro to make use of netdev_dbg call
From: David Miller @ 2010-03-31  6:01 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, gospo, alexander.h.duyck
In-Reply-To: <20100325231007.8991.24570.stgit@localhost.localdomain>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Thu, 25 Mar 2010 16:10:08 -0700

> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> This change updates the igb driver to make use of the netdev_dbg function
> macros now provided in netdevice.h
> 
> This is meant to be provided as an alternative to the patch provided by
> Joe Perches.
> 
> It also removes igb_get_time_str since I found that it is unused code that
> is no longer used even in debug.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

I'll apply this to net-next-2.6, thanks.

^ permalink raw reply

* Re: [PATCH RFC] virtio_net: use NAPI for xmit (UNTESTED)
From: Shirley Ma @ 2010-03-31  5:58 UTC (permalink / raw)
  To: Rusty Russell; +Cc: Michael S. Tsirkin, netdev, Herbert Xu
In-Reply-To: <1270014284.25337.3.camel@localhost.localdomain>

Back ported it and prepared for more tests.

Shirley


^ permalink raw reply

* Re: [PATCH 0/6] tagged sysfs support
From: Kay Sievers @ 2010-03-31  5:51 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Greg Kroah-Hartman, Greg KH, linux-kernel, Tejun Heo,
	Cornelia Huck, linux-fsdevel, Eric Dumazet, Benjamin LaHaise,
	Serge Hallyn, netdev
In-Reply-To: <m14ojxh0mz.fsf@fess.ebiederm.org>

On Wed, Mar 31, 2010 at 01:04, Eric W. Biederman <ebiederm@xmission.com> wrote:
> Kay Sievers <kay.sievers@vrfy.org> writes:
>> On Tue, Mar 30, 2010 at 20:30, Eric W. Biederman <ebiederm@xmission.com> wrote:
>>>
>>> The main short coming of using multiple network namespaces today
>>> is that only network devices for the primary network namespaces
>>> can be put in the kobject layer and sysfs.
>>>
>>> This is essentially the earlier version of this patchset that was
>>> reviewed before, just now on top of a version of sysfs that doesn't
>>> need cleanup patches to support it.
>>
>> Just to check if we are not in conflict with planned changes, and how
>> to possibly handle them:
>>
>> There is the plan and ongoing work to unify classes and buses, export
>> them at /sys/subsystem in the same layout of the current /sys/bus/.
>> The decision to export buses and classes as two different things
>> (which they aren't) is the last major piece in the sysfs layout which
>> needs to be fixed.
>
> Interesting.  We will symlinks ie:
> /sys/class -> /sys/subsystem
> /sys/bus -> /sys/subsystem
> to keep from breaking userspace.

Yeah, /sys/bus/, which is the only sane layout of the needlessly
different 3 versions of the same thing (bus, class, block).

/sys/bus/<subsys> can just be a plain symlinks to the
/sys/subsystem/<subsys> directories.

/sys/class/<subsys> *could* be a symlink to the
/sys/subsystem/<subsys>/devices/ directory, but we really don't want
to continue to stupidly mix subsystem-wide control files with device
lists anymore. The "devices" directory needs to be a strict list of
devices, not some collection of random stuff, that it is today. :)

So we either leave all the conceptually broken class attributes behind
us, and put them at the  /sys/subsystem/<subsys>/ level only, or we
need to create the /sys/class/<subsys>/* stuff all as symlinks like we
do today. I expect, we have to create /sys/class as we do today.

Another problem to solve is that sysfs does not allow us to symlink
regular files, only directories, so we can currently not create the
class-wide attributes as symlinks to the proper file in
/sys/subsystem/.

>> It would mean that /sys/subsystem/net/devices/* would look like
>> /sys/class/net/* today. But at the /sys/subsystem/net/ directory could
>> be global network-subsystem-wide control files which would need to be
>> namespaced too. (The network subsystem does not use subsytem-global
>> files today, but a bunch of other classes do.)
>>
>> This could be modeled into the current way of doing sysfs namespaces?
>> A /sys/bus/<subsystem>/ directory hierarchy would need to be
>> namespaced, not just a single plain directory with symlinks. Would
>> that work?
>
> I'm not entirely clear on what you are doing but it all sounds like it
> will fit within what I am doing.

The goal is to unify the 3 needlessly different versions of "device
lists of the same subsystem". We have /sys/class, /sys/bus,
/sys/block, and all of them will be unified at /sys/subsystem/ leaving
the old names as compat links only. Unlike block and class, the
/sys/subsystem/<subsys> directory can be extended with custom
subdirectories and files, without mixing random files into device
lists.

With /sys/subsystem/, userspace can uniquely identify and find all
devices at /sys/<subsys>/devices/<device-name>/ with only the
subsystem and the device name.

All devices in /sys/devices already have a symlink called "subsystem"
which will point back to the corresponding /sys/subsystem/<subsys>
directory, and the event environment already contains a variable
SUBSYSTEM with the name.

That would be the first time sysfs device interfaces have some idea of
consistency. :)

> Right now I have /sys/class/net,
> /sys/devices/virtual/net and a bunch of other net directories becoming
> tagged and only showing up in the appropriately mounted sysfs.  We
> track them all in the class kset and as long as we extend that capability
> when the subsystem change happens in sysfs all should be well.

Ok, sounds good.

> Today we have /sys/class/net/bonding_master.  For now I have that as
> an untagged but the implementation is aware of which network namespace
> your current process is in.  Thinking about that a little more it
> would be better to make that file tagged so that userspace can see
> different versions for the different network namespaces.  Joy.

Yeah, that might make more sense in the end.

> I expect other control files will be the same.

Sounds like, yes.

> In general it doesn't make sense to add control files for networking.
> as they easily conflict with legal network device names and thus create
> the possibility of breaking someones userspace.

Yeah, it did not makes sense it the first place to mix devices lists
with global attributes. It's a real mess what people do in sysfs.

Thanks,
Kay

^ permalink raw reply

* Re: [PATCH 1/6] sysfs: Basic support for multiple super blocks
From: Eric W. Biederman @ 2010-03-31  5:51 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Greg Kroah-Hartman, Kay Sievers, linux-kernel, Cornelia Huck,
	linux-fsdevel, Eric Dumazet, Benjamin LaHaise, Serge Hallyn,
	netdev
In-Reply-To: <4BB2E098.7030202@kernel.org>

Tejun Heo <tj@kernel.org> writes:

> Hello, Eric.
>
> On 03/31/2010 03:31 AM, Eric W. Biederman wrote:
>> From: Eric W. Biederman <ebiederm@xmission.com>
>> 
>> Add all of the necessary bioler plate to support
>                            boiler :-)
>
>> +static int sysfs_test_super(struct super_block *sb, void *data)
>> +{
>> +	struct sysfs_super_info *sb_info = sysfs_info(sb);
>> +	struct sysfs_super_info *info = data;
>> +	int found = 1;
>> +	return found;
>> +}
>
> Can you please make it return bool?

Nope.  That would mean I could not use it with sget.

>>  static int sysfs_get_sb(struct file_system_type *fs_type,
>>  	int flags, const char *dev_name, void *data, struct vfsmount *mnt)
>>  {
>> -	return get_sb_single(fs_type, flags, data, sysfs_fill_super, mnt);
>> +	struct sysfs_super_info *info;
>> +	struct super_block *sb;
>> +	int error;
>> +
>> +	error = -ENOMEM;
>> +	info = kzalloc(sizeof(*info), GFP_KERNEL);
>> +	if (!info)
>> +		goto out;
>> +	sb = sget(fs_type, sysfs_test_super, sysfs_set_super, info);
>> +	if (IS_ERR(sb) || sb->s_fs_info != info)
>> +		kfree(info);
>> +	if (IS_ERR(sb)) {
>> +		kfree(info);
>> +		error = PTR_ERR(sb);
>> +		goto out;
>> +	}
>> +	if (!sb->s_root) {
>> +		sb->s_flags = flags;
>> +		error = sysfs_fill_super(sb, data, flags & MS_SILENT ? 1 : 0);
>> +		if (error) {
>> +			deactivate_locked_super(sb);
>> +			goto out;
>> +		}
>> +		sb->s_flags |= MS_ACTIVE;
>> +	}
>> +
>> +	simple_set_mnt(mnt, sb);
>> +	error = 0;
>> +out:
>> +	return error;
>> +}
>
> I haven't looked at later patches but I suppose this is gonna be
> filled with more meaningful stuff later. 

Yes it will.

> One (possibly silly) thing
> that stands out compared to get_sb_single() is missing remount
> handling.  Is it intended?

There is nothing for a remount to do so I ignore it.   The only
thing that would possibly be meaningful is a read-only mount,
and nothing I know of sysfs suggests read-only mounts of sysfs
work, or make any sense.

>> index 30f5a44..030a39d 100644
>> --- a/fs/sysfs/sysfs.h
>> +++ b/fs/sysfs/sysfs.h
>> @@ -114,6 +114,9 @@ struct sysfs_addrm_cxt {
>>  /*
>>   * mount.c
>>   */
>> +struct sysfs_super_info {
>> +};
>> +#define sysfs_info(SB) ((struct sysfs_super_info *)(SB->s_fs_info))
>
> Another nit picking.  It would be better to wrap SB in the macro
> definition.  Also, wouldn't an inline function be better?

Good spotting.  That doesn't bite today but it will certainly bite
someday if it isn't fixed.

I wonder how that has slipped through the review all of this time.

Eric

^ permalink raw reply

* Re: [PATCH 5/6] cxgb4: Add main driver file and driver Makefile
From: David Miller @ 2010-03-31  5:50 UTC (permalink / raw)
  To: shemminger; +Cc: dm, netdev
In-Reply-To: <20100330141904.5236fe44@nehalam>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Tue, 30 Mar 2010 14:19:04 -0700

> On Tue, 30 Mar 2010 10:52:21 -0800
> Dimitris Michailidis <dm@chelsio.com> wrote:
> 
>> +static struct cxgb4_proc_entry proc_files[] = {
>> +#ifdef CONFIG_PROC_FS
>> +	{ "l2t", 0444, ADAP_NEED_L2T, 0, &t4_l2t_proc_fops },
>> +#endif
>> +	{ "lb_stats", 0444, 0, 0, &lb_stats_proc_fops },
>> +	{ "path_mtus", 0644, 0, 0, &mtutab_proc_fops },
>> +	{ "qstats", 0444, 0, 0, &sge_stats_proc_fops },
>> +	{ "rss", 0444, 0, 0, &rss_proc_fops },
>> +	{ "tcp_stats", 0444, 0, 0, &tcp_stats_proc_fops },
>> +	{ "tids", 0444, ADAP_NEED_OFLD, 0, &tid_info_proc_fops },
>> +	{ "tp_err_stats", 0444, 0, 0, &tp_err_stats_proc_fops },
>> +	{ "trace0", 0644, 0, 0, &mps_trc_proc_fops },
>> +	{ "trace1", 0644, 0, 1, &mps_trc_proc_fops },
>> +	{ "trace2", 0644, 0, 2, &mps_trc_proc_fops },
>> +	{ "trace3", 0644, 0, 3, &mps_trc_proc_fops },
>> +	{ "uld", 0444, 0, 0, &uld_proc_fops },
>> +};
>> +
> 
> Do you really need this large number of /proc files.
> It creates another stable API to worry about.  If it is just for
> debugging move it to debugfs, or better yet just drop it.

I also find this a bit too much.

We have all kinds of ways to export whatever statistics you
want.  In particular we have ethtool stats of which you
can have as many as you wish.

If necessary, we could add a feature to define "views" of ethtool
stats so that we can have domains of driver specific statistics if the
problem is that you don't want all of these debugging stats to clutter
up the "main" ethtool stats.

^ permalink raw reply

* Re: [PATCH RFC] virtio_net: use NAPI for xmit (UNTESTED)
From: Shirley Ma @ 2010-03-31  5:44 UTC (permalink / raw)
  To: Rusty Russell; +Cc: Michael S. Tsirkin, netdev, Herbert Xu
In-Reply-To: <201003311429.57793.rusty@rustcorp.com.au>

Hello Rusty,

On Wed, 2010-03-31 at 14:29 +1030, Rusty Russell wrote:
> I don't have time to chase this, but it's been sitting in my patch
> queue
> for a while.  Wondered if Michael or Shirley wanted to toy with it
> 

Does this patch build on top of net-next-2.6?

Shirley


^ permalink raw reply

* Re: [PATCH RFC] inetpeer: Support ipv6 addresses.
From: Eric W. Biederman @ 2010-03-31  5:44 UTC (permalink / raw)
  To: Herbert Xu; +Cc: David Miller, netdev
In-Reply-To: <20100328135931.GA16430@gondor.apana.org.au>

Herbert Xu <herbert@gondor.apana.org.au> writes:


> BTW, it appears that the inetpeer cache doesn't take namespaces
> into account.  This means that information could potentially leak
> from one namespace into another.  I'm not sure whether that's a
> big deal or not but it's something for the namespaces folks to
> consider.

Bother. I wrote a patch a while back, I even remember people
commenting on it, but it certainly doesn't look like anything
ever made it in.

I will see if I can make some time to dig that up and post
the patch.

Thanks for noticing,
Eric

^ permalink raw reply

* Re: [PATCH RFC] inetpeer: Support ipv6 addresses.
From: Eric W. Biederman @ 2010-03-31  5:43 UTC (permalink / raw)
  To: Herbert Xu; +Cc: David Miller, netdev
In-Reply-To: <20100328135931.GA16430@gondor.apana.org.au>

Herbert Xu <herbert@gondor.apana.org.au> writes:


> BTW, it appears that the inetpeer cache doesn't take namespaces
> into account.  This means that information could potentially leak
> from one namespace into another.  I'm not sure whether that's a
> big deal or not but it's something for the namespaces folks to
> consider.

Bother. I wrote a patch a while back, I even remember people
commenting on it, but it certainly doesn't look like anything
ever made it in.

I will see if I can make some time to dig that up and post
the patch.

Thanks for noticing,
Eric

^ permalink raw reply

* Re: [PATCH 1/6] sysfs: Basic support for multiple super blocks
From: Tejun Heo @ 2010-03-31  5:41 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Greg Kroah-Hartman, Kay Sievers, linux-kernel, Cornelia Huck,
	linux-fsdevel, Eric Dumazet, Benjamin LaHaise, Serge Hallyn,
	netdev
In-Reply-To: <1269973889-25260-1-git-send-email-ebiederm@xmission.com>

Hello, Eric.

On 03/31/2010 03:31 AM, Eric W. Biederman wrote:
> From: Eric W. Biederman <ebiederm@xmission.com>
> 
> Add all of the necessary bioler plate to support
                           boiler :-)

> +static int sysfs_test_super(struct super_block *sb, void *data)
> +{
> +	struct sysfs_super_info *sb_info = sysfs_info(sb);
> +	struct sysfs_super_info *info = data;
> +	int found = 1;
> +	return found;
> +}

Can you please make it return bool?

>  static int sysfs_get_sb(struct file_system_type *fs_type,
>  	int flags, const char *dev_name, void *data, struct vfsmount *mnt)
>  {
> -	return get_sb_single(fs_type, flags, data, sysfs_fill_super, mnt);
> +	struct sysfs_super_info *info;
> +	struct super_block *sb;
> +	int error;
> +
> +	error = -ENOMEM;
> +	info = kzalloc(sizeof(*info), GFP_KERNEL);
> +	if (!info)
> +		goto out;
> +	sb = sget(fs_type, sysfs_test_super, sysfs_set_super, info);
> +	if (IS_ERR(sb) || sb->s_fs_info != info)
> +		kfree(info);
> +	if (IS_ERR(sb)) {
> +		kfree(info);
> +		error = PTR_ERR(sb);
> +		goto out;
> +	}
> +	if (!sb->s_root) {
> +		sb->s_flags = flags;
> +		error = sysfs_fill_super(sb, data, flags & MS_SILENT ? 1 : 0);
> +		if (error) {
> +			deactivate_locked_super(sb);
> +			goto out;
> +		}
> +		sb->s_flags |= MS_ACTIVE;
> +	}
> +
> +	simple_set_mnt(mnt, sb);
> +	error = 0;
> +out:
> +	return error;
> +}

I haven't looked at later patches but I suppose this is gonna be
filled with more meaningful stuff later.  One (possibly silly) thing
that stands out compared to get_sb_single() is missing remount
handling.  Is it intended?

> index 30f5a44..030a39d 100644
> --- a/fs/sysfs/sysfs.h
> +++ b/fs/sysfs/sysfs.h
> @@ -114,6 +114,9 @@ struct sysfs_addrm_cxt {
>  /*
>   * mount.c
>   */
> +struct sysfs_super_info {
> +};
> +#define sysfs_info(SB) ((struct sysfs_super_info *)(SB->s_fs_info))

Another nit picking.  It would be better to wrap SB in the macro
definition.  Also, wouldn't an inline function be better?

Thanks.

-- 
tejun

^ permalink raw reply

* Re: rps: keep the old behavior on SMP without rps
From: David Miller @ 2010-03-31  5:40 UTC (permalink / raw)
  To: xiaosuo; +Cc: therbert, netdev
In-Reply-To: <4BB2DD95.90402@gmail.com>

From: Changli Gao <xiaosuo@gmail.com>
Date: Wed, 31 Mar 2010 13:28:53 +0800

> keep the old behavior on SMP without rps
> 
> RPS introduces a lock operation to per cpu variable input_pkt_queue on
> SMP whenever rps is enabled or not. On SMP without RPS, this lock isn't
> needed at all.
> 
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>

Instead of peppering the file with lots of ifdefs, encapsulate
the thing thats changing into a set of inline functions.

^ permalink raw reply

* Re: [PATCH] virtio_net: avoid BUG_ON() with large packets when CONFIG_DEBUG_SG=y
From: Shirley Ma @ 2010-03-31  5:39 UTC (permalink / raw)
  To: Rusty Russell; +Cc: David Miller, netdev, mst
In-Reply-To: <201003311105.19014.rusty@rustcorp.com.au>

On Wed, 2010-03-31 at 11:05 +1030, Rusty Russell wrote:
> Shirley, please cc me in future.

I will. I thought you might be on vacation. :)

Shirley


^ permalink raw reply

* rps: keep the old behavior on SMP without rps
From: Changli Gao @ 2010-03-31  5:28 UTC (permalink / raw)
  To: David S. Miller; +Cc: Tom Herbert, xiaosuo, netdev

keep the old behavior on SMP without rps

RPS introduces a lock operation to per cpu variable input_pkt_queue on
SMP whenever rps is enabled or not. On SMP without RPS, this lock isn't
needed at all.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
net/core/dev.c | 34 ++++++++++++++++++++++++++--------
1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 3e7fa16..14ad3b7 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2314,13 +2314,19 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu)
 	local_irq_save(flags);
 	__get_cpu_var(netdev_rx_stat).total++;
 
+#ifdef CONFIG_RPS
 	spin_lock(&queue->input_pkt_queue.lock);
+#endif
 	if (queue->input_pkt_queue.qlen <= netdev_max_backlog) {
 		if (queue->input_pkt_queue.qlen) {
 enqueue:
 			__skb_queue_tail(&queue->input_pkt_queue, skb);
+#ifdef CONFIG_RPS
 			spin_unlock_irqrestore(&queue->input_pkt_queue.lock,
 			    flags);
+#else
+			local_irq_restore(flags);
+#endif
 			return NET_RX_SUCCESS;
 		}
 
@@ -2342,7 +2348,9 @@ enqueue:
 		goto enqueue;
 	}
 
+#ifdef CONFIG_RPS
 	spin_unlock(&queue->input_pkt_queue.lock);
+#endif
 
 	__get_cpu_var(netdev_rx_stat).dropped++;
 	local_irq_restore(flags);
@@ -2767,19 +2775,23 @@ int netif_receive_skb(struct sk_buff *skb)
 EXPORT_SYMBOL(netif_receive_skb);
 
 /* Network device is going away, flush any packets still pending  */
-static void flush_backlog(struct net_device *dev, int cpu)
+static void flush_backlog(void *arg)
 {
-	struct softnet_data *queue = &per_cpu(softnet_data, cpu);
+	struct net_device *dev = arg;
+	struct softnet_data *queue = &__get_cpu_var(softnet_data);
 	struct sk_buff *skb, *tmp;
-	unsigned long flags;
 
-	spin_lock_irqsave(&queue->input_pkt_queue.lock, flags);
+#ifdef CONFIG_RPS
+	spin_lock(&queue->input_pkt_queue.lock);
+#endif
 	skb_queue_walk_safe(&queue->input_pkt_queue, skb, tmp)
 		if (skb->dev == dev) {
 			__skb_unlink(skb, &queue->input_pkt_queue);
 			kfree_skb(skb);
 		}
-	spin_unlock_irqrestore(&queue->input_pkt_queue.lock, flags);
+#ifdef CONFIG_RPS
+	spin_unlock(&queue->input_pkt_queue.lock);
+#endif
 }
 
 static int napi_gro_complete(struct sk_buff *skb)
@@ -3092,14 +3104,22 @@ static int process_backlog(struct napi_struct *napi, int quota)
 	do {
 		struct sk_buff *skb;
 
+#ifdef CONFIG_RPS
 		spin_lock_irq(&queue->input_pkt_queue.lock);
+#else
+		local_irq_disable();
+#endif
 		skb = __skb_dequeue(&queue->input_pkt_queue);
 		if (!skb) {
 			__napi_complete(napi);
 			spin_unlock_irq(&queue->input_pkt_queue.lock);
 			break;
 		}
+#ifdef CONFIG_RPS
 		spin_unlock_irq(&queue->input_pkt_queue.lock);
+#else
+		local_irq_enable();
+#endif
 
 		__netif_receive_skb(skb);
 	} while (++work < quota && jiffies == start_time);
@@ -5549,7 +5569,6 @@ void netdev_run_todo(void)
 	while (!list_empty(&list)) {
 		struct net_device *dev
 			= list_first_entry(&list, struct net_device, todo_list);
-		int i;
 		list_del(&dev->todo_list);
 
 		if (unlikely(dev->reg_state != NETREG_UNREGISTERING)) {
@@ -5561,8 +5580,7 @@ void netdev_run_todo(void)
 
 		dev->reg_state = NETREG_UNREGISTERED;
 
-		for_each_online_cpu(i)
-			flush_backlog(dev, i);
+		on_each_cpu(flush_backlog, dev, 1);
 
 		netdev_wait_allrefs(dev);
 



^ permalink raw reply related

* Re: [PATCH 1/6] sysfs: Basic support for multiple super blocks
From: Serge E. Hallyn @ 2010-03-31  5:01 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Greg Kroah-Hartman, Kay Sievers, linux-kernel, Tejun Heo,
	Cornelia Huck, linux-fsdevel, Eric Dumazet, Benjamin LaHaise,
	netdev
In-Reply-To: <20100331050100.GB10144@us.ibm.com>

Quoting Serge E. Hallyn (serue@us.ibm.com):
> Quoting Eric W. Biederman (ebiederm@xmission.com):
> > From: Eric W. Biederman <ebiederm@xmission.com>
> > 
> > Add all of the necessary bioler plate to support
> > multiple superblocks in sysfs.
> > 
> > Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> 
> Acked-by: Serge Hallyn <serue@us.ibm.com>

(with the patch 7/6 of course :)

^ 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