* [PATCH] pcnet_cs: Use proper netdev_*-printouts
From: Wolfram Sang @ 2010-08-08 12:32 UTC (permalink / raw)
To: netdev
Cc: linux-pcmcia, Wolfram Sang, Dominik Brodowski, Joe Perches,
David Miller
To prevent broken messages like:
[ 204.024291] eth%d: pcnet_reset_8390() did not complete.
Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Dominik Brodowski <linux@dominikbrodowski.net>
Cc: Joe Perches <joe@perches.com>
Cc: David Miller <davem@davemloft.net>
---
There are 5 checkpatch-warnings regarding indentation. I did not fix them to
keep the style of the rest of the driver. The code looks correct to me.
drivers/net/pcmcia/pcnet_cs.c | 58 +++++++++++++++++++----------------------
1 files changed, 27 insertions(+), 31 deletions(-)
diff --git a/drivers/net/pcmcia/pcnet_cs.c b/drivers/net/pcmcia/pcnet_cs.c
index bfdef72..e127923 100644
--- a/drivers/net/pcmcia/pcnet_cs.c
+++ b/drivers/net/pcmcia/pcnet_cs.c
@@ -646,12 +646,12 @@ static int pcnet_config(struct pcmcia_device *link)
if (info->flags & (IS_DL10019|IS_DL10022)) {
u_char id = inb(dev->base_addr + 0x1a);
- printk(KERN_INFO "%s: NE2000 (DL100%d rev %02x): ",
- dev->name, ((info->flags & IS_DL10022) ? 22 : 19), id);
+ netdev_info(dev, "NE2000 (DL100%d rev %02x): ",
+ (info->flags & IS_DL10022) ? 22 : 19, id);
if (info->pna_phy)
printk("PNA, ");
} else {
- printk(KERN_INFO "%s: NE2000 Compatible: ", dev->name);
+ netdev_info(dev, "NE2000 Compatible: ");
}
printk("io %#3lx, irq %d,", dev->base_addr, dev->irq);
if (info->flags & USE_SHMEM)
@@ -932,7 +932,7 @@ static void mii_phy_probe(struct net_device *dev)
phyid = tmp << 16;
phyid |= mdio_read(mii_addr, i, MII_PHYID_REG2);
phyid &= MII_PHYID_REV_MASK;
- pr_debug("%s: MII at %d is 0x%08x\n", dev->name, i, phyid);
+ netdev_dbg(dev, "MII at %d is 0x%08x\n", i, phyid);
if (phyid == AM79C9XX_HOME_PHY) {
info->pna_phy = i;
} else if (phyid != AM79C9XX_ETH_PHY) {
@@ -1018,8 +1018,8 @@ static void pcnet_reset_8390(struct net_device *dev)
outb_p(ENISR_RESET, nic_base + EN0_ISR); /* Ack intr. */
if (i == 100)
- printk(KERN_ERR "%s: pcnet_reset_8390() did not complete.\n",
- dev->name);
+ netdev_err(dev, "pcnet_reset_8390() did not complete.\n");
+
set_misc_reg(dev);
} /* pcnet_reset_8390 */
@@ -1035,8 +1035,7 @@ static int set_config(struct net_device *dev, struct ifmap *map)
else if ((map->port < 1) || (map->port > 2))
return -EINVAL;
dev->if_port = map->port;
- printk(KERN_INFO "%s: switched to %s port\n",
- dev->name, if_names[dev->if_port]);
+ netdev_info(dev, "switched to %s port\n", if_names[dev->if_port]);
NS8390_init(dev, 1);
}
return 0;
@@ -1071,7 +1070,7 @@ static void ei_watchdog(u_long arg)
this, we can limp along even if the interrupt is blocked */
if (info->stale++ && (inb_p(nic_base + EN0_ISR) & ENISR_ALL)) {
if (!info->fast_poll)
- printk(KERN_INFO "%s: interrupt(s) dropped!\n", dev->name);
+ netdev_info(dev, "interrupt(s) dropped!\n");
ei_irq_wrapper(dev->irq, dev);
info->fast_poll = HZ;
}
@@ -1091,7 +1090,7 @@ static void ei_watchdog(u_long arg)
if (info->eth_phy) {
info->phy_id = info->eth_phy = 0;
} else {
- printk(KERN_INFO "%s: MII is missing!\n", dev->name);
+ netdev_info(dev, "MII is missing!\n");
info->flags &= ~HAS_MII;
}
goto reschedule;
@@ -1100,8 +1099,7 @@ static void ei_watchdog(u_long arg)
link &= 0x0004;
if (link != info->link_status) {
u_short p = mdio_read(mii_addr, info->phy_id, 5);
- printk(KERN_INFO "%s: %s link beat\n", dev->name,
- (link) ? "found" : "lost");
+ netdev_info(dev, "%s link beat\n", link ? "found" : "lost");
if (link && (info->flags & IS_DL10022)) {
/* Disable collision detection on full duplex links */
outb((p & 0x0140) ? 4 : 0, nic_base + DLINK_DIAG);
@@ -1112,13 +1110,12 @@ static void ei_watchdog(u_long arg)
if (link) {
if (info->phy_id == info->eth_phy) {
if (p)
- printk(KERN_INFO "%s: autonegotiation complete: "
- "%sbaseT-%cD selected\n", dev->name,
+ netdev_info(dev, "autonegotiation complete: "
+ "%sbaseT-%cD selected\n",
((p & 0x0180) ? "100" : "10"),
((p & 0x0140) ? 'F' : 'H'));
else
- printk(KERN_INFO "%s: link partner did not "
- "autonegotiate\n", dev->name);
+ netdev_info(dev, "link partner did not autonegotiate\n");
}
NS8390_init(dev, 1);
}
@@ -1131,7 +1128,7 @@ static void ei_watchdog(u_long arg)
/* isolate this MII and try flipping to the other one */
mdio_write(mii_addr, info->phy_id, 0, 0x0400);
info->phy_id ^= info->pna_phy ^ info->eth_phy;
- printk(KERN_INFO "%s: switched to %s transceiver\n", dev->name,
+ netdev_info(dev, "switched to %s transceiver\n",
(info->phy_id == info->eth_phy) ? "ethernet" : "PNA");
mdio_write(mii_addr, info->phy_id, 0,
(info->phy_id == info->eth_phy) ? 0x1000 : 0);
@@ -1191,9 +1188,9 @@ static void dma_get_8390_hdr(struct net_device *dev,
unsigned int nic_base = dev->base_addr;
if (ei_status.dmaing) {
- printk(KERN_NOTICE "%s: DMAing conflict in dma_block_input."
+ netdev_notice(dev, "DMAing conflict in dma_block_input."
"[DMAstat:%1x][irqlock:%1x]\n",
- dev->name, ei_status.dmaing, ei_status.irqlock);
+ ei_status.dmaing, ei_status.irqlock);
return;
}
@@ -1224,11 +1221,11 @@ static void dma_block_input(struct net_device *dev, int count,
char *buf = skb->data;
if ((ei_debug > 4) && (count != 4))
- pr_debug("%s: [bi=%d]\n", dev->name, count+4);
+ netdev_dbg(dev, "[bi=%d]\n", count+4);
if (ei_status.dmaing) {
- printk(KERN_NOTICE "%s: DMAing conflict in dma_block_input."
+ netdev_notice(dev, "DMAing conflict in dma_block_input."
"[DMAstat:%1x][irqlock:%1x]\n",
- dev->name, ei_status.dmaing, ei_status.irqlock);
+ ei_status.dmaing, ei_status.irqlock);
return;
}
ei_status.dmaing |= 0x01;
@@ -1258,9 +1255,9 @@ static void dma_block_input(struct net_device *dev, int count,
break;
} while (--tries > 0);
if (tries <= 0)
- printk(KERN_NOTICE "%s: RX transfer address mismatch,"
+ netdev_notice(dev, "RX transfer address mismatch,"
"%#4.4x (expected) vs. %#4.4x (actual).\n",
- dev->name, ring_offset + xfer_count, addr);
+ ring_offset + xfer_count, addr);
}
#endif
outb_p(ENISR_RDC, nic_base + EN0_ISR); /* Ack intr. */
@@ -1281,7 +1278,7 @@ static void dma_block_output(struct net_device *dev, int count,
#ifdef PCMCIA_DEBUG
if (ei_debug > 4)
- printk(KERN_DEBUG "%s: [bo=%d]\n", dev->name, count);
+ netdev_dbg(dev, "[bo=%d]\n", count);
#endif
/* Round the count up for word writes. Do we need to do this?
@@ -1290,9 +1287,9 @@ static void dma_block_output(struct net_device *dev, int count,
if (count & 0x01)
count++;
if (ei_status.dmaing) {
- printk(KERN_NOTICE "%s: DMAing conflict in dma_block_output."
+ netdev_notice(dev, "DMAing conflict in dma_block_output."
"[DMAstat:%1x][irqlock:%1x]\n",
- dev->name, ei_status.dmaing, ei_status.irqlock);
+ ei_status.dmaing, ei_status.irqlock);
return;
}
ei_status.dmaing |= 0x01;
@@ -1329,9 +1326,9 @@ static void dma_block_output(struct net_device *dev, int count,
break;
} while (--tries > 0);
if (tries <= 0) {
- printk(KERN_NOTICE "%s: Tx packet transfer address mismatch,"
+ netdev_notice(dev, "Tx packet transfer address mismatch,"
"%#4.4x (expected) vs. %#4.4x (actual).\n",
- dev->name, (start_page << 8) + count, addr);
+ (start_page << 8) + count, addr);
if (retries++ == 0)
goto retry;
}
@@ -1340,8 +1337,7 @@ static void dma_block_output(struct net_device *dev, int count,
while ((inb_p(nic_base + EN0_ISR) & ENISR_RDC) == 0)
if (time_after(jiffies, dma_start + PCNET_RDC_TIMEOUT)) {
- printk(KERN_NOTICE "%s: timeout waiting for Tx RDC.\n",
- dev->name);
+ netdev_notice(dev, "timeout waiting for Tx RDC.\n");
pcnet_reset_8390(dev);
NS8390_init(dev, 1);
break;
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] sfq: add dummy bind/unbind handles
From: Jarek Poplawski @ 2010-08-08 7:04 UTC (permalink / raw)
To: David Miller; +Cc: shemminger, netdev, franchoze
In-Reply-To: <20100807.224514.137860816.davem@davemloft.net>
David Miller wrote, On 08.08.2010 07:45:
> From: Jarek Poplawski <jarkao2@gmail.com>
> Date: Sat, 07 Aug 2010 01:17:07 +0200
>
>> Stephen Hemminger wrote, On 07.08.2010 00:23:
>>
>>> Applying a filter to an SFQ qdisc would cause null dereference
>>> in tcf_bind_filter because although SFQ is classful it didn't
>>> have all the necessary equipment.
>>>
>>> Better alternative to changing tcf_bind API is to just fix
>>> SFQ. This should go to net-2.6 and stable.
>>>
>>
>> Hmm... FYI, actually I've sent already a similar patch to the
>> original bug report thread (except .unbind_tcf method which
>> doesn't matter for fixing this bug, so should be rather
>> implemented in a separate patch, if needed at all in this
>> case).
>
> Agreed, I can't see a way that unbind can ever be invoked
> if the bind call always returns zero.
To tell the truth, I think unbind should be implemented anyway,
just for consistency, safety, and easier verification. But, looking
at a similar case of .get and .put in the same driver, Patrick
seemed to do it purposely, so I expected some discussion about the
rules yet, and made it minimal to ease merging to older kernels.
Thanks,
Jarek P.
^ permalink raw reply
* Re: [PATCH] phy: SMSC: fix unused warnings when built into kernel
From: David Miller @ 2010-08-08 6:08 UTC (permalink / raw)
To: bhutchings; +Cc: xiaosuo, vapier, netdev
In-Reply-To: <1281099435.2088.2.camel@achroite.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Fri, 06 Aug 2010 13:57:15 +0100
> On Fri, 2010-08-06 at 14:43 +0800, Changli Gao wrote:
>> On Fri, Aug 6, 2010 at 2:27 PM, Mike Frysinger <vapier@gentoo.org> wrote:
>> > If the SMSC phy is not built as a module, we get the warning:
>> > drivers/net/phy/smsc.c:257: warning: 'smsc_tbl' defined but not used
>> >
>> > So mark the structure as maybe unused to avoid this.
>> >
>> > Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>> > ---
>> > drivers/net/phy/smsc.c | 2 +-
>> > 1 files changed, 1 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
>> > index 78fa988..11944ef 100644
>> > --- a/drivers/net/phy/smsc.c
>> > +++ b/drivers/net/phy/smsc.c
>> > @@ -254,7 +254,7 @@ MODULE_LICENSE("GPL");
>> > module_init(smsc_init);
>> > module_exit(smsc_exit);
>> >
>> > -static struct mdio_device_id smsc_tbl[] = {
>> > +static __maybe_unused struct mdio_device_id smsc_tbl[] = {
>> > { 0x0007c0a0, 0xfffffff0 },
>> > { 0x0007c0b0, 0xfffffff0 },
>> > { 0x0007c0c0, 0xfffffff0 },
>>
>> How about adding a #ifdef CONFIG_SMSC_PHY_MODULE clause around these lines?
>
> By default, gcc will discard unused variables and functions declared as
> static. So this would not provide any run-time benefit.
We never have to do this for any other module device table, because
usually the table is actually used for the probing.
The MDIO ones are special in that they are only used for assisting in
module probing.
I would suggest that, in light of this, we make MODULE_DEVICE_TABLE()
make a dummy reference to the table.
Otherwise we're likely going going to have to add these ugly tags to
every single PHY driver.
^ permalink raw reply
* Re: [PATCH NEXT 1/1] netxen: protect tx timeout recovery by rtnl lock
From: David Miller @ 2010-08-08 6:05 UTC (permalink / raw)
To: amit.salecha; +Cc: netdev, ameen.rahman
In-Reply-To: <1281101611-16893-1-git-send-email-amit.salecha@qlogic.com>
From: Amit Kumar Salecha <amit.salecha@qlogic.com>
Date: Fri, 6 Aug 2010 06:33:31 -0700
> Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
Applied, thanks.
^ permalink raw reply
* Re: [patch] isdn: gigaset: add missing unlock
From: David Miller @ 2010-08-08 6:04 UTC (permalink / raw)
To: error27
Cc: hjlipp, tilman, isdn, ext-andriy.shevchenko, gigaset307x-common,
netdev, kernel-janitors
In-Reply-To: <20100806082323.GU9031@bicker>
From: Dan Carpenter <error27@gmail.com>
Date: Fri, 6 Aug 2010 10:23:23 +0200
> We should unlock here. This is the only place where we return from the
> function with the lock held. The caller isn't expecting it.
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
Applied.
^ permalink raw reply
* Re: [patch] isdn: gigaset: use after free
From: David Miller @ 2010-08-08 6:04 UTC (permalink / raw)
To: error27
Cc: hjlipp, tilman, isdn, tj, joe, gigaset307x-common, netdev,
kernel-janitors
In-Reply-To: <20100806082126.GT9031@bicker>
From: Dan Carpenter <error27@gmail.com>
Date: Fri, 6 Aug 2010 10:21:26 +0200
> I moved the kfree(cb) below the dereferences.
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH] solos-pci: Fix race condition in tasklet RX handling
From: David Miller @ 2010-08-08 6:02 UTC (permalink / raw)
To: dwmw2; +Cc: netdev, nathan, linux-atm-general
In-Reply-To: <1281111456.2715.29.camel@localhost>
From: David Woodhouse <dwmw2@infradead.org>
Date: Fri, 06 Aug 2010 17:17:36 +0100
> We were seeing faults in the solos-pci receive tasklet when packets
> arrived for a VCC which was currently being closed:
...
> This patch uses an RCU-inspired approach to fix it. In the RX tasklet's
> find_vcc() function we first refuse to use a VCC which already has the
> ATM_VF_READY bit cleared. And in the VCC close function, we synchronise
> with the tasklet to ensure that it can't still be using the VCC before
> we continue and allow the VCC to be destroyed.
>
> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
> Tested-by: Nathan Williams <nathan@traverse.com.au>
> Cc: stable@kernel.org
Applied, thanks David.
^ permalink raw reply
* Re: [PATCH 2/2] igbvf.txt: Add igbvf Documentation
From: David Miller @ 2010-08-08 6:01 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, linux-kernel, gospo, bphilips
In-Reply-To: <20100804001615.6571.69837.stgit@localhost.localdomain>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 03 Aug 2010 17:16:17 -0700
> Adds documentation for the igbvf (igb virtual function driver).
>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Tons of trailing whitespace, GIT nearly craps itself when I try
to apply this.
Please fix this, and also submit the fixed up 1/1 ixgbe.txt doc patch
and even more importantly the ixgbe FCOE build fix you promised I'd
have two nights ago :-)
Thanks.
^ permalink raw reply
* Re: [PATCH 1/9] net classifier: dont allow filters on semi-classful qdisc
From: David Miller @ 2010-08-08 5:59 UTC (permalink / raw)
To: jarkao2; +Cc: shemminger, netdev
In-Reply-To: <4C5C8C09.9070206@gmail.com>
From: Jarek Poplawski <jarkao2@gmail.com>
Date: Sat, 07 Aug 2010 00:26:17 +0200
> Stephen Hemminger wrote, On 06.08.2010 23:58:
>
>> On Fri, 06 Aug 2010 23:24:47 +0200
>> Jarek Poplawski <jarkao2@gmail.com> wrote:
>>
>>> Stephen Hemminger wrote, On -10.01.-28163 20:59:
>>>
>>>> There are several qdisc which only support a single class (sfq, mq, tbf)
>>>> and the kernel would dereference a null pointer (bind_tcf), if a user
>>>> attempted to apply a filter one of these classes.
>>>
>>>
>>> mq and tbf can't have this issue because they don't have
>>> .tcf_chain class method. sfq should support it on purpose
>>> after this patch:
>>>
>>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=7d2681a6ff4f9ab5e48d02550b4c6338f1638998
>>> and needs tiny fix only.
>>
>> Probably best to fix both ways. Fix sfq to allow filters to
>> be chained, and fix API to prevent refuse to allow qdisc to
>> register with tcf_chain && !bind_tcf
>>
>
> Yes, but your patch needs a different changelog and I'm not sure
> it's necessary for stable (there should be no other such cases
> for now).
Right.
I'm tossing this series since there are dependencies upon this first
patch for which we've arrived at a different solution (Jarek's
patch).
^ permalink raw reply
* Re: [PATCH] sfq: add dummy bind/unbind handles
From: David Miller @ 2010-08-08 5:45 UTC (permalink / raw)
To: jarkao2; +Cc: shemminger, netdev, franchoze
In-Reply-To: <4C5C97F3.3010604@gmail.com>
From: Jarek Poplawski <jarkao2@gmail.com>
Date: Sat, 07 Aug 2010 01:17:07 +0200
> Stephen Hemminger wrote, On 07.08.2010 00:23:
>
>> Applying a filter to an SFQ qdisc would cause null dereference
>> in tcf_bind_filter because although SFQ is classful it didn't
>> have all the necessary equipment.
>>
>> Better alternative to changing tcf_bind API is to just fix
>> SFQ. This should go to net-2.6 and stable.
>>
>
> Hmm... FYI, actually I've sent already a similar patch to the
> original bug report thread (except .unbind_tcf method which
> doesn't matter for fixing this bug, so should be rather
> implemented in a separate patch, if needed at all in this
> case).
Agreed, I can't see a way that unbind can ever be invoked
if the bind call always returns zero.
Therefore I'll apply Jarek's patch, thanks everyone.
^ permalink raw reply
* Re: [PATCH 07/18] isdn: avm: call disable_pci_device() if pci_probe() failed
From: David Miller @ 2010-08-08 5:34 UTC (permalink / raw)
To: segooon; +Cc: kernel-janitors, isdn, adobriyan, tilman, netdev
In-Reply-To: <1281124303-13537-1-git-send-email-segooon@gmail.com>
From: Kulikov Vasiliy <segooon@gmail.com>
Date: Fri, 6 Aug 2010 23:51:41 +0400
> Driver should call disable_pci_device() if it returns from pci_probe()
"pci_disable_device()"
> with error. Also it must not be called if request_region() fails as
> it means that somebody uses device resources and rules the device.
This interface, frankly, sucks.
If this is what we want then pci_enable_device() and
pci_disable_device() should maintain a reference count,
so that device drivers need not be mindful of what
arbitrary special circumstances they should make the
disable call or not. They can make it unconditionally
and the PCI layer makes sure the device doesn't get
disabled unintentionally.
This is software interface design 101.
The current requirements are beyond unreasonable and
should be fixed before we apply patches like this.
I'm tossing all of these patches, sorry.
^ permalink raw reply
* Re: [PATCH] net: disable preemption before call smp_processor_id()
From: David Miller @ 2010-08-08 5:25 UTC (permalink / raw)
To: paulmck; +Cc: xiaosuo, therbert, netdev
In-Reply-To: <20100808045751.GF19600@linux.vnet.ibm.com>
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Date: Sat, 7 Aug 2010 21:57:51 -0700
> If I cover preemptible RCU's semantics, a first cut comes out like this:
>
> * In non-preemptible RCU implementations (TREE_RCU and TINY_RCU), it
> * is illegal to block while in an RCU read-side critical section. In
> * preemptible RCU implementations (TREE_PREEMPT_RCU and TINY_PREEMPT_RCU)
> * in CONFIG_PREEMPT kernel builds, RCU read-side critical sections may
> * be preempted, but explicit blocking is illegal. Finally, in preemptible
> * RCU implementations in real-time (CONFIG_PREEMPT_RT) kernel builds,
> * RCU read-side critical sections may be preempted and they may also
> * block, but only when acquiring spinlocks that are subject to priority
> * inheritance.
>
> Does that seem reasonable?
Sounds good to me.
^ permalink raw reply
* Re: [PATCH] net: disable preemption before call smp_processor_id()
From: Paul E. McKenney @ 2010-08-08 4:57 UTC (permalink / raw)
To: David Miller; +Cc: xiaosuo, therbert, netdev
In-Reply-To: <20100807.203700.186284252.davem@davemloft.net>
On Sat, Aug 07, 2010 at 08:37:00PM -0700, David Miller wrote:
> From: Changli Gao <xiaosuo@gmail.com>
> Date: Sat, 7 Aug 2010 17:26:02 +0800
>
> > Although netif_rx() isn't expected to be called in process context with
> > preemption enabled, it'd better handle this case. And this is why get_cpu()
> > is used in the non-RPS #ifdef branch. If tree RCU is selected,
> > rcu_read_lock() won't disable preemption, so preempt_disable() should be
> > called explictly.
> >
> > Signed-off-by: Changli Gao <xiaosuo@gmail.com>
>
> Applied, thanks Changli.
>
> Paul, perhaps the comment above rcu_read_lock()'s definition should
> be updated in rcupdate.h? It says blocking is not allowed inside
> of a read side critical section, but obviously with tree-rcu that
> is not the case.
>
> Either we should add a mention of tree-rcu's semantics or just remote
> this part of the comment altogether.
Good point, that last sentence is quite obsolete. It also survived a
recent cleanup. :-/
If I cover preemptible RCU's semantics, a first cut comes out like this:
* In non-preemptible RCU implementations (TREE_RCU and TINY_RCU), it
* is illegal to block while in an RCU read-side critical section. In
* preemptible RCU implementations (TREE_PREEMPT_RCU and TINY_PREEMPT_RCU)
* in CONFIG_PREEMPT kernel builds, RCU read-side critical sections may
* be preempted, but explicit blocking is illegal. Finally, in preemptible
* RCU implementations in real-time (CONFIG_PREEMPT_RT) kernel builds,
* RCU read-side critical sections may be preempted and they may also
* block, but only when acquiring spinlocks that are subject to priority
* inheritance.
Does that seem reasonable?
Thanx, Paul
^ permalink raw reply
* Re: [PATCH] net: disable preemption before call smp_processor_id()
From: David Miller @ 2010-08-08 3:37 UTC (permalink / raw)
To: xiaosuo; +Cc: paulmck, therbert, netdev
In-Reply-To: <1281173162-10587-1-git-send-email-xiaosuo@gmail.com>
From: Changli Gao <xiaosuo@gmail.com>
Date: Sat, 7 Aug 2010 17:26:02 +0800
> Although netif_rx() isn't expected to be called in process context with
> preemption enabled, it'd better handle this case. And this is why get_cpu()
> is used in the non-RPS #ifdef branch. If tree RCU is selected,
> rcu_read_lock() won't disable preemption, so preempt_disable() should be
> called explictly.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
Applied, thanks Changli.
Paul, perhaps the comment above rcu_read_lock()'s definition should
be updated in rcupdate.h? It says blocking is not allowed inside
of a read side critical section, but obviously with tree-rcu that
is not the case.
Either we should add a mention of tree-rcu's semantics or just remote
this part of the comment altogether.
^ permalink raw reply
* Re: [PATCH] tcp: no md5sig option size check bug
From: David Miller @ 2010-08-08 3:24 UTC (permalink / raw)
To: dp
Cc: kuznet, jmorris, kaber, yoshfuji, pekkas, gilad, yony, ori,
ilpo.jarvinen, netdev, linux-kernel
In-Reply-To: <AANLkTimHMAwi158ycRh_nRYPYUTSkww-psdFai-LsTsb@mail.gmail.com>
From: Dmitry Popov <dp@highloadlab.com>
Date: Sat, 7 Aug 2010 23:17:52 +0400
> From: Dmitry Popov <dp@highloadlab.com>
>
> tcp_parse_md5sig_option doesn't check md5sig option (TCPOPT_MD5SIG)
> length, but tcp_v[46]_inbound_md5_hash assume that it's at least 16
> bytes long.
>
> Signed-off-by: Dmitry Popov <dp@highloadlab.com>
I'll apply this, but the memcmp() we do against this pointer is always
safe because there's at least skb_shared_info()'s worth of valid
memory past skb->data guarenteed at all times which is much larger
than 16 bytes.
So at worst we'd access garbage, but never past a valid piece of
allocated memory.
Thanks.
^ permalink raw reply
* [PATCH] tcp: no md5sig option size check bug
From: Dmitry Popov @ 2010-08-07 19:17 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, James Morris, Patrick McHardy,
Hideaki YOSHIFUJI
Cc: Pekka Savola (ipv6), Gilad Ben-Yossef, Yony Amit, Ori Finkelman,
Ilpo Järvinen, netdev, linux-kernel
From: Dmitry Popov <dp@highloadlab.com>
tcp_parse_md5sig_option doesn't check md5sig option (TCPOPT_MD5SIG)
length, but tcp_v[46]_inbound_md5_hash assume that it's at least 16
bytes long.
Signed-off-by: Dmitry Popov <dp@highloadlab.com>
---
net/ipv4/tcp_input.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 3c426cb..e663b78 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3930,7 +3930,7 @@ u8 *tcp_parse_md5sig_option(struct tcphdr *th)
if (opsize < 2 || opsize > length)
return NULL;
if (opcode == TCPOPT_MD5SIG)
- return ptr;
+ return opsize == TCPOLEN_MD5SIG ? ptr : NULL;
}
ptr += opsize - 2;
length -= opsize;
^ permalink raw reply related
* Re: [RFC PATCH] platform: Faciliatate the creation of pseduo-platform busses
From: Grant Likely @ 2010-08-07 17:28 UTC (permalink / raw)
To: Greg KH
Cc: Patrick Pannuto, Patrick Pannuto, linux-kernel@vger.kernel.org,
linux-arm-msm@vger.kernel.org, linux-omap@vger.kernel.org,
damm@opensource.se, lethal@linux-sh.org, rjw@sisk.pl,
dtor@mail.ru, eric.y.miao@gmail.com, netdev@vger.kernel.org,
Kevin Hilman
In-Reply-To: <AANLkTimHbcF+fz=aBMLdhLo2s-rhfNvaPCtqE8nYJ8aT@mail.gmail.com>
On Sat, Aug 7, 2010 at 12:35 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Fri, Aug 6, 2010 at 5:46 PM, Greg KH <gregkh@suse.de> wrote:
>> That would be nice, but take your "standard" PC today:
>> > ls /sys/devices/platform/
>> Fixed MDIO bus.0 i8042 pcspkr power serial8250 uevent vesafb.0
>>
>> There are tty devices below the serial port, which is nice to see, but
>> the others? I don't know what type of bus they would be on, do you?
[...]
> I wouldn't have any problem modifying those specific drivers to
> register under something like /sys/devices/legacy, but I don't really
> think it is in any way necessary.
Or for that matter, make those drivers explicitly use
/sys/devices/platform so that I don't cause churn on PCs. :-) I'd
like to be rid of it as default behaviour for embedded though.
g.
^ permalink raw reply
* Re: [PATCH 1/42] drivers/net/wan: Adjust confusing if indentation
From: Krzysztof Halasa @ 2010-08-07 12:37 UTC (permalink / raw)
To: Julia Lawall; +Cc: netdev, linux-kernel, kernel-janitors
In-Reply-To: <Pine.LNX.4.64.1008052216400.31692@ask.diku.dk>
Julia Lawall <julia@diku.dk> writes:
> drivers/net/wan/c101.c | 2 +-
> drivers/net/wan/n2.c | 2 +-
> drivers/net/wan/pci200syn.c | 2 +-
Fully Acked-by: Krzysztof Hałasa <khc@pm.waw.pl>
Thanks.
--
Krzysztof Halasa
^ permalink raw reply
* [PATCH] net: disable preemption before call smp_processor_id()
From: Changli Gao @ 2010-08-07 9:26 UTC (permalink / raw)
To: David S. Miller; +Cc: Paul E. McKenney, Tom Herbert, netdev, Changli Gao
Although netif_rx() isn't expected to be called in process context with
preemption enabled, it'd better handle this case. And this is why get_cpu()
is used in the non-RPS #ifdef branch. If tree RCU is selected,
rcu_read_lock() won't disable preemption, so preempt_disable() should be
called explictly.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
net/core/dev.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/core/dev.c b/net/core/dev.c
index 2b50896..1ae6543 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2517,6 +2517,7 @@ int netif_rx(struct sk_buff *skb)
struct rps_dev_flow voidflow, *rflow = &voidflow;
int cpu;
+ preempt_disable();
rcu_read_lock();
cpu = get_rps_cpu(skb->dev, skb, &rflow);
@@ -2526,6 +2527,7 @@ int netif_rx(struct sk_buff *skb)
ret = enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
rcu_read_unlock();
+ preempt_enable();
}
#else
{
^ permalink raw reply related
* Re: [PATCH] platform: Facilitate the creation of pseduo-platform busses
From: Grant Likely @ 2010-08-07 6:53 UTC (permalink / raw)
To: Patrick Pannuto
Cc: Kevin Hilman, linux-kernel@vger.kernel.org,
linux-arm-msm@vger.kernel.org, linux-omap@vger.kernel.org,
damm@opensource.se, lethal@linux-sh.org, rjw@sisk.pl,
eric.y.miao@gmail.com, netdev@vger.kernel.org, Greg Kroah-Hartman,
alan, zt.tmzt, magnus.damm
In-Reply-To: <4C5B646C.1030308@codeaurora.org>
On Thu, Aug 5, 2010 at 7:25 PM, Patrick Pannuto <ppannuto@codeaurora.org> wrote:
> On 08/05/2010 04:16 PM, Grant Likely wrote:
>> The relevant question before going down this path is, "Is the
>> omap/sh/other-soc behaviour something fundamentally different from the
>> platform bus? Or is it something complementary that would be better
>> handled with a notifier or some orthogonal method of adding new
>> behaviour?"
>>
>> I don't have a problem with multiple platform_bus instances using the
>> same code (I did suggest it after all), but I do worry about muddying
>> the Linux device model or making it overly complex. Binding single
>> drivers to multiple device types could be messy.
>>
>> Cheers,
>> g.
>
> From your other email, the distinction of /bus_types/ versus /physical
> or logical buses/ was very valuable (thanks). I am becoming less
> convinced that I need this infrastructure or the ability to create
> pseudo-platform buses. Let me outline some thoughts below, which I
> think at least solves my problems ( ;) ), and if some of the OMAP folks
> and Alan could chime in as to whether they can apply something similar,
> or if they still have need of creating actual new bus types?
>
>
> As we are exploring all of this, let me put a concrete (ish) scenario
> out there to discuss:
>
> SUB_BUS1---DEVICE1
> /
> CPU ---
> \
> SUB_BUS2---DEVICE2
>
> DEVICE1 and DEVICE2 are the same device (say, usb host controller, or
> whatever), and they should be driven by the same driver. However,
> physically they are attached to different buses.
>
> SUB_BUS1 and SUB_BUS2 are *different* and let's say require a different
> suspend method. If I understand correctly, the right way to build the
> topology would be:
>
> struct device_type SUB_BUS1_type = {
> .name = "sub-bus1-type",
> .pm = &sub_bus1_pm_ops;
> };
>
> struct platform_device SUB_BUS1 = {
> .init_name = "sub-bus1",
> .dev.type = &sub_bus1_type,
> };
>
> struct platform_device DEVICE1 = {
> .name = "device1",
> .dev.parent = &SUB_BUS1.dev,
> };
>
> platform_device_register(&SUB_BUS1);
> platform_device_register(&DEVICE1);
>
> [analogous for *2]
Not quite. The device_type is intended to collect similar, but
different things (ie, all keyboards, regardless of how they are
attached to the system). Trying to capture the device-specific
behavour really belongs in the specific device driver attached to the
SUB_BUS* device. In fact, the way you've constructed your example,
SUB_BUS1 and SUB_BUS2 don't really need to be platform_devices at all;
you could have used a plain struct device because in this example you
aren't binding them to a device driver).
That being said, because each bus *does* have custom behaviour, it
probably does make sense to either bind each to a device driver, or to
do something to each child device that changes the PM behaviour. I
haven't dug into the problem domain deep enough to give you absolute
answers, but the devres approach looks promising, as does creating a
derived platform_bus_type (although I'm not fond of sharing device
drivers between multiple bus_types). Another option might be giving
the parent struct device some runtime PM operations that it performs
per-child. I'm just throwing out ideas here though.
> which would yield:
>
> /sys/bus/platform/devices/
> |
> |-SUB_BUS1
> | |
> | |-DEVICE1
> |
> |-SUB_BUS2
> | |
> | |-DEVICE2
You're looking in the wrong place. Look in /sys/devices instead of
/sys/bus... (see comments below)
> Which is the correct topology, and logically provides all the correct
> interfaces. The only thing that becomes confusing now, is that SUB_BUS*
> is *actually* a physically different bus, yet it's not in /sys/bus;
> although I suppose this is actually how the world works presently (you
> don't see an individual entry in /sys/bus for each usb host controller,
> which is technically defining a sub-bus...)
/sys/bus is for bus_types, not bus instances. All USB devices will be
listed in /sys/bus/usb/devices regardless of the bus instance that
they attached to.
However, look carefully at the files in /sys/bus/*/devices. Notice
that they are all symlinks? Notice that the all of them point to
directories in /sys/devices? /sys/bus tells you which devices are
registered against each bus type, but /sys/devices is the actual
structure. Always look in /sys/devices when you want to know the
topology of the system.
Cheers,
g.
^ permalink raw reply
* Re: [RFC PATCH] platform: Faciliatate the creation of pseduo-platform busses
From: Grant Likely @ 2010-08-07 6:35 UTC (permalink / raw)
To: Greg KH
Cc: Patrick Pannuto, Patrick Pannuto, linux-kernel@vger.kernel.org,
linux-arm-msm@vger.kernel.org, linux-omap@vger.kernel.org,
damm@opensource.se, lethal@linux-sh.org, rjw@sisk.pl,
dtor@mail.ru, eric.y.miao@gmail.com, netdev@vger.kernel.org,
Kevin Hilman
In-Reply-To: <20100806234635.GC16793@suse.de>
On Fri, Aug 6, 2010 at 5:46 PM, Greg KH <gregkh@suse.de> wrote:
> On Fri, Aug 06, 2010 at 09:12:27AM -0600, Grant Likely wrote:
>> On Fri, Aug 6, 2010 at 8:27 AM, Greg KH <gregkh@suse.de> wrote:
>> > On Thu, Aug 05, 2010 at 04:59:35PM -0600, Grant Likely wrote:
>> >> (On that point Greg, what is the reason for even having the
>> >> /sys/devices/platform/ parent? Why not just let the platform devices
>> >> sit at the root of the device tree? In the OF case (granted, I'm
>> >> biased) all of the platform_device registrations reflect the actual
>> >> device hierarchy expressed in the device tree data.)
>> >
>> > If we sat them at the "root", there would be a bunch of them there. I
>> > don't know, we could drop the parent, I guess whoever created the
>> > platform device oh so long ago, decided that it would look nicer to be
>> > in this type of structure.
>>
>> Personally I'd rather see a meaningful structure used here. Maybe
>> having them all in the root will encourage people to find realistic
>> parents for their platform devices. :-)
>
> That would be nice, but take your "standard" PC today:
> > ls /sys/devices/platform/
> Fixed MDIO bus.0 i8042 pcspkr power serial8250 uevent vesafb.0
>
> There are tty devices below the serial port, which is nice to see, but
> the others? I don't know what type of bus they would be on, do you?
Does it matter? On my PC I count 7 devices. On my servers, I see the
same. I don't personally don't see any disadvantage to having them at
the root.
However, looking at them:
"Fixed MDIO bus" is actually a complete hack that I'm going to try and
get rid of.
i8042 is keyboard/mouse that probably lives on the southbridge. I
imagine hanging off the PCI bus in the ISA IO range.
ditto pcspkr
ditto serial8250
ditto vesafb
I wouldn't have any problem modifying those specific drivers to
register under something like /sys/devices/legacy, but I don't really
think it is in any way necessary.
I suppose one *could* try to figure out the actual PCI topology that
leads to these devices, but that gets into trying to guess what the
BIOS set up to decide where to register those things. Probably a lot
of error-prone work for absolutely no benefit. :-)
However, on the embedded side I'm seeing a lot of devices show up
under /sys/devices/platform. Embedded is generally different. We
*know* what the bus topology is. However, it seems to me that many
developers are under the mistaken assumption that platform devices
must live in /sys/devices/platform, so they don't bother reflecting
the topology in the device model and it seems to be affecting how
embedded PM is being approached (my opinion). Removing the "struct
device platform_bus" may reduce the confusion.
>> Why don't I float a patch to remove this and see if anybody freaks
>> out. Should I wrap it with a CONFIG_ so that it can be configurable
>> for a release or to, or just make it unconditional?
>
> If you can figure out a structure for the desktop/server machines, sure,
> I say just always enable it :)
I've got a patch in my tree now. I'll play with it a bit before
posting it for RFC.
g.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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: [PATCH 1/6] bna: Brocade 10Gb Ethernet device driver
From: Debashis Dutt @ 2010-08-07 3:23 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev@vger.kernel.org
Hi Stephen,
Thanks a lot for your comments.
> + if (likely
> + (wis > BNA_QE_FREE_CNT(tcb, tcb->q_depth) ||
> + vectors > BNA_QE_FREE_CNT(unmap_q, unmap_q->q_depth))) {
> + BNAD_UPDATE_CTR(bnad, netif_queue_stop);
> + return NETDEV_TX_BUSY;
>The transmit routine should check for available space after
> queueing to device, so you can avoid having to return
>TX_BUSY.
However your above comment is not very clear to me.
Our Tx routine already cleans up the Tx buffers at the end, through a tasklet.
Cleaning of Tx buffers happens
1) In the sending context at the end of the Tx routine.
2) In the IRQ context when we get a Tx completion interrupt.
Only thing that we could have done, is do this check again at the end of the Tx routine
and called netif_stop_queue() if required, so that the stack stops sending. Even then I am
not sure that we could avoid the current check and returning TX_BUSY.
It would be nice if you clarify, so that I understand this better.
Thanks
--Debashis
Linux LL Driver Team.
-----Original Message-----
From: Stephen Hemminger [mailto:shemminger@vyatta.com]
Sent: Wednesday, August 04, 2010 10:09 AM
To: Rasesh Mody
Cc: netdev@vger.kernel.org; Debashis Dutt; Jing Huang
Subject: Re: [PATCH 1/6] bna: Brocade 10Gb Ethernet device driver
On Tue, 3 Aug 2010 22:15:36 -0700
Rasesh Mody <rmody@brocade.com> wrote:
> From: Rasesh Mody <rmody@brocade.com>
>
> This is patch 1/6 which contains linux driver source for
> Brocade's BR1010/BR1020 10Gb CEE capable ethernet adapter.
> Source is based against net-next-2.6.
>
> We wish this patch to be considered for inclusion in net-next-2.6
>
> Signed-off-by: Rasesh Mody <rmody@brocade.com>
> ---
> bnad.c | 3326 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> bnad.h | 474 ++++++++
> bnad_ethtool.c | 1269 +++++++++++++++++++++
> 3 files changed, 5069 insertions(+)
>
> diff -ruP net-next-2.6.35-rc1-orig/drivers/net/bna/bnad.c net-next-2.6.35-rc1-mod/drivers/net/bna/bnad.c
> --- net-next-2.6.35-rc1-orig/drivers/net/bna/bnad.c 1969-12-31 16:00:00.000000000 -0800
> +++ net-next-2.6.35-rc1-mod/drivers/net/bna/bnad.c 2010-08-02 17:19:19.447239000 -0700
> @@ -0,0 +1,3326 @@
> +/*
> + * Linux network driver for Brocade Converged Network Adapter.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License (GPL) Version 2 as
> + * published by the Free Software Foundation
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + */
> +/*
> + * Copyright (c) 2005-2009 Brocade Communications Systems, Inc.
> + * All rights reserved
> + * www.brocade.com
> + */
> +#include <linux/netdevice.h>
> +#include <linux/skbuff.h>
> +#include <linux/etherdevice.h>
> +#include <linux/in.h>
> +#include <linux/ethtool.h>
> +#include <linux/if_vlan.h>
> +#include <linux/if_ether.h>
> +#include <linux/ip.h>
> +
> +#include "bnad.h"
> +#include "bna.h"
> +#include "cna.h"
> +
> +DEFINE_MUTEX(bnad_fwimg_mutex);
> +
> +/*
> + * Module params
> + */
> +static uint bnad_msix_disable;
> +module_param(bnad_msix_disable, uint, 0444);
> +MODULE_PARM_DESC(bnad_msix_disable, "Disable MSIX mode");
> +
> +static uint bnad_ioc_auto_recover = 1;
> +module_param(bnad_ioc_auto_recover, uint, 0444);
> +MODULE_PARM_DESC(bnad_ioc_auto_recover, "Enable / Disable auto recovery");
> +
> +/*
> + * Global variables
> + */
> +u32 bna_id;
> +u32 bnad_rxqs_per_cq = 2;
> +
> +DECLARE_MUTEX(bnad_list_sem);
> +LIST_HEAD(bnad_list);
> +
> +const u8 bnad_bcast_addr[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
Surprised this isn't defined somewhere else.
> +
> +/*
> + * Local MACROS
> + */
> +#define BNAD_TX_UNMAPQ_DEPTH (bnad->txq_depth * 2)
> +
> +#define BNAD_RX_UNMAPQ_DEPTH (bnad->rxq_depth)
> +
> +#define BNAD_GET_MBOX_IRQ(_bnad) \
> + (((_bnad)->cfg_flags & BNAD_CF_MSIX) ? \
> + ((_bnad)->msix_table[(_bnad)->msix_num - 1].vector) : \
> + ((_bnad)->pcidev->irq))
> +
> +#define BNAD_FILL_UNMAPQ_MEM_REQ(_res_info, _num, _depth) \
> +do { \
> + (_res_info)->res_type = BNA_RES_T_MEM; \
> + (_res_info)->res_u.mem_info.mem_type = BNA_MEM_T_KVA; \
> + (_res_info)->res_u.mem_info.num = (_num); \
> + (_res_info)->res_u.mem_info.len = \
> + sizeof(struct bnad_unmap_q) + \
> + (sizeof(struct bnad_skb_unmap) * ((_depth) - 1)); \
> +} while (0)
> +
> +void
> +bnad_add_to_list(struct bnad *bnad)
> +{
> + down(&bnad_list_sem);
> + list_add_tail(&bnad->list_entry, &bnad_list);
> + bna_id++;
> + up(&bnad_list_sem);
> +}
Why do you need to list semaphore? Isn't RTNL mutex held
when this is done. If you have to have own exclusion use
a mutex for this.
> +void
> +bnad_remove_from_list(struct bnad *bnad)
> +{
> + down(&bnad_list_sem);
> + list_del(&bnad->list_entry);
> + up(&bnad_list_sem);
> +}
> +
> +const struct pci_device_id bnad_pci_id_table[] = {
> + {
> + PCI_DEVICE(PCI_VENDOR_ID_BROCADE,
> + PCI_DEVICE_ID_BROCADE_CT),
> + .class = PCI_CLASS_NETWORK_ETHERNET << 8,
> + .class_mask = 0xffff00
> + }, {0, }
> +};
Why is this not static?
> +/* TX */
> +/* bnad_start_xmit : Netdev entry point for Transmit */
> +/* Called under lock held by net_device */
> +
> +netdev_tx_t
> +bnad_start_xmit(struct sk_buff *skb, struct net_device *netdev)
Should also be static...
> +{
> + struct bnad *bnad = netdev_priv(netdev);
> +
> + u16 txq_prod, vlan_tag = 0;
> + u32 unmap_prod, wis, wis_used, wi_range;
> + u32 vectors, vect_id, i, acked;
> + u32 tx_id;
> + int err;
> +
> + struct bnad_tx_info *tx_info;
> + struct bna_tcb *tcb;
> + struct bnad_unmap_q *unmap_q;
> + dma_addr_t dma_addr;
> + struct bna_txq_entry *txqent;
> + bna_txq_wi_ctrl_flag_t flags;
> +
> + if (unlikely
> + (skb->len <= ETH_HLEN || skb->len > BFI_TX_MAX_DATA_PER_PKT)) {
> + dev_kfree_skb(skb);
> + return NETDEV_TX_OK;
> + }
> +
> + /*
> + * Takes care of the Tx that is scheduled between clearing the flag
> + * and the netif_stop_queue() call.
> + */
> + if (unlikely(!test_bit(BNAD_RF_TX_STARTED, &bnad->run_flags))) {
> + dev_kfree_skb(skb);
> + return NETDEV_TX_OK;
> + }
> +
> + tx_id = BNAD_GET_TX_ID(skb);
> +
> + tx_info = &bnad->tx_info[tx_id];
> + tcb = tx_info->tcb[tx_id];
> + unmap_q = tcb->unmap_q;
> +
> + vectors = 1 + skb_shinfo(skb)->nr_frags;
> + if (vectors > BFI_TX_MAX_VECTORS_PER_PKT) {
> + dev_kfree_skb(skb);
> + return NETDEV_TX_OK;
> + }
> + wis = BNA_TXQ_WI_NEEDED(vectors); /* 4 vectors per work item */
> + acked = 0;
> + if (unlikely
> + (wis > BNA_QE_FREE_CNT(tcb, tcb->q_depth) ||
> + vectors > BNA_QE_FREE_CNT(unmap_q, unmap_q->q_depth))) {
> + if ((u16) (*tcb->hw_consumer_index) !=
> + tcb->consumer_index &&
> + !test_and_set_bit(BNAD_TXQ_FREE_SENT, &tcb->flags)) {
> + acked = bnad_free_txbufs(bnad, tcb);
> + bna_ib_ack(tcb->i_dbell, acked);
> + smp_mb__before_clear_bit();
> + clear_bit(BNAD_TXQ_FREE_SENT, &tcb->flags);
> + } else {
> + netif_stop_queue(netdev);
> + BNAD_UPDATE_CTR(bnad, netif_queue_stop);
> + }
> +
> + smp_mb();
> + /*
> + * Check again to deal with race condition between
> + * netif_stop_queue here, and netif_wake_queue in
> + * interrupt handler which is not inside netif tx lock.
> + */
> + if (likely
> + (wis > BNA_QE_FREE_CNT(tcb, tcb->q_depth) ||
> + vectors > BNA_QE_FREE_CNT(unmap_q, unmap_q->q_depth))) {
> + BNAD_UPDATE_CTR(bnad, netif_queue_stop);
> + return NETDEV_TX_BUSY;
The transmit routine should check for available space after
queueing to device, so you can avoid having to return
TX_BUSY.
^ permalink raw reply
* Re: [RFC PATCH] platform: Faciliatate the creation of pseduo-platform busses
From: Greg KH @ 2010-08-06 23:46 UTC (permalink / raw)
To: Grant Likely
Cc: Patrick Pannuto, Patrick Pannuto, linux-kernel@vger.kernel.org,
linux-arm-msm@vger.kernel.org, linux-omap@vger.kernel.org,
damm@opensource.se, lethal@linux-sh.org, rjw@sisk.pl,
dtor@mail.ru, eric.y.miao@gmail.com, netdev@vger.kernel.org,
Kevin Hilman
In-Reply-To: <AANLkTi=VbVed09=u-Ea0L8c_CWZ_CLLjWH9ZjD2dyg6Y@mail.gmail.com>
On Fri, Aug 06, 2010 at 09:12:27AM -0600, Grant Likely wrote:
> On Fri, Aug 6, 2010 at 8:27 AM, Greg KH <gregkh@suse.de> wrote:
> > On Thu, Aug 05, 2010 at 04:59:35PM -0600, Grant Likely wrote:
> >> (On that point Greg, what is the reason for even having the
> >> /sys/devices/platform/ parent? Why not just let the platform devices
> >> sit at the root of the device tree? In the OF case (granted, I'm
> >> biased) all of the platform_device registrations reflect the actual
> >> device hierarchy expressed in the device tree data.)
> >
> > If we sat them at the "root", there would be a bunch of them there. I
> > don't know, we could drop the parent, I guess whoever created the
> > platform device oh so long ago, decided that it would look nicer to be
> > in this type of structure.
>
> Personally I'd rather see a meaningful structure used here. Maybe
> having them all in the root will encourage people to find realistic
> parents for their platform devices. :-)
That would be nice, but take your "standard" PC today:
> ls /sys/devices/platform/
Fixed MDIO bus.0 i8042 pcspkr power serial8250 uevent vesafb.0
There are tty devices below the serial port, which is nice to see, but
the others? I don't know what type of bus they would be on, do you?
> Why don't I float a patch to remove this and see if anybody freaks
> out. Should I wrap it with a CONFIG_ so that it can be configurable
> for a release or to, or just make it unconditional?
If you can figure out a structure for the desktop/server machines, sure,
I say just always enable it :)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] sfq: add dummy bind/unbind handles
From: Jarek Poplawski @ 2010-08-06 23:17 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev, Franchoze Eric
In-Reply-To: <20100806152313.43abc30b@nehalam>
Stephen Hemminger wrote, On 07.08.2010 00:23:
> Applying a filter to an SFQ qdisc would cause null dereference
> in tcf_bind_filter because although SFQ is classful it didn't
> have all the necessary equipment.
>
> Better alternative to changing tcf_bind API is to just fix
> SFQ. This should go to net-2.6 and stable.
>
Hmm... FYI, actually I've sent already a similar patch to the
original bug report thread (except .unbind_tcf method which
doesn't matter for fixing this bug, so should be rather
implemented in a separate patch, if needed at all in this
case).
Jarek P.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
> --- a/net/sched/sch_sfq.c 2010-08-06 15:07:26.552820159 -0700
> +++ b/net/sched/sch_sfq.c 2010-08-06 15:14:24.458287452 -0700
> @@ -502,6 +502,10 @@ static unsigned long sfq_get(struct Qdis
> return 0;
> }
>
> +static void sfq_put(struct Qdisc *q, unsigned long cl)
> +{
> +}
> +
> static struct tcf_proto **sfq_find_tcf(struct Qdisc *sch, unsigned long cl)
> {
> struct sfq_sched_data *q = qdisc_priv(sch);
> @@ -511,6 +515,12 @@ static struct tcf_proto **sfq_find_tcf(s
> return &q->filter_list;
> }
>
> +static unsigned long sfq_bind_tcf(struct Qdisc *sch, unsigned long parent,
> + u32 cl)
> +{
> + return 0;
> +}
> +
> static int sfq_dump_class(struct Qdisc *sch, unsigned long cl,
> struct sk_buff *skb, struct tcmsg *tcm)
> {
> @@ -556,6 +566,8 @@ static void sfq_walk(struct Qdisc *sch,
> static const struct Qdisc_class_ops sfq_class_ops = {
> .get = sfq_get,
> .tcf_chain = sfq_find_tcf,
> + .bind_tcf = sfq_bind_tcf,
> + .unbind_tcf = sfq_put,
> .dump = sfq_dump_class,
> .dump_stats = sfq_dump_class_stats,
>
^ permalink raw reply
* [PATCH] sfq: add dummy bind/unbind handles
From: Stephen Hemminger @ 2010-08-06 22:23 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Jarek Poplawski, David Miller, netdev, Franchoze Eric
In-Reply-To: <20100806145818.66d389eb@nehalam>
Applying a filter to an SFQ qdisc would cause null dereference
in tcf_bind_filter because although SFQ is classful it didn't
have all the necessary equipment.
Better alternative to changing tcf_bind API is to just fix
SFQ. This should go to net-2.6 and stable.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/sched/sch_sfq.c 2010-08-06 15:07:26.552820159 -0700
+++ b/net/sched/sch_sfq.c 2010-08-06 15:14:24.458287452 -0700
@@ -502,6 +502,10 @@ static unsigned long sfq_get(struct Qdis
return 0;
}
+static void sfq_put(struct Qdisc *q, unsigned long cl)
+{
+}
+
static struct tcf_proto **sfq_find_tcf(struct Qdisc *sch, unsigned long cl)
{
struct sfq_sched_data *q = qdisc_priv(sch);
@@ -511,6 +515,12 @@ static struct tcf_proto **sfq_find_tcf(s
return &q->filter_list;
}
+static unsigned long sfq_bind_tcf(struct Qdisc *sch, unsigned long parent,
+ u32 cl)
+{
+ return 0;
+}
+
static int sfq_dump_class(struct Qdisc *sch, unsigned long cl,
struct sk_buff *skb, struct tcmsg *tcm)
{
@@ -556,6 +566,8 @@ static void sfq_walk(struct Qdisc *sch,
static const struct Qdisc_class_ops sfq_class_ops = {
.get = sfq_get,
.tcf_chain = sfq_find_tcf,
+ .bind_tcf = sfq_bind_tcf,
+ .unbind_tcf = sfq_put,
.dump = sfq_dump_class,
.dump_stats = sfq_dump_class_stats,
^ 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