* re: Ethernet driver for the WIZnet W5100 chip
From: Dan Carpenter @ 2012-04-10 8:40 UTC (permalink / raw)
To: msink; +Cc: netdev
Hello Mike Sinkovsky,
This is a semi-automatic email about new static checker warnings.
The patch 8b1467a31343: "Ethernet driver for the WIZnet W5100 chip"
from Apr 4, 2012, leads to the following Smatch complaint:
drivers/net/ethernet/wiznet/w5100.c:685 w5100_hw_probe()
error: we previously assumed 'data' could be null (see line 637)
drivers/net/ethernet/wiznet/w5100.c
636
637 if (data && is_valid_ether_addr(data->mac_addr)) {
^^^^
New check.
638 memcpy(ndev->dev_addr, data->mac_addr, ETH_ALEN);
639 } else {
640 random_ether_addr(ndev->dev_addr);
641 ndev->addr_assign_type |= NET_ADDR_RANDOM;
642 }
643
644 mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
645 if (!mem)
646 return -ENXIO;
647 mem_size = resource_size(mem);
648 if (!devm_request_mem_region(&pdev->dev, mem->start, mem_size, name))
649 return -EBUSY;
650 priv->base = devm_ioremap(&pdev->dev, mem->start, mem_size);
651 if (!priv->base)
652 return -EBUSY;
653
654 spin_lock_init(&priv->reg_lock);
655 priv->indirect = mem_size < W5100_BUS_DIRECT_SIZE;
656 if (priv->indirect) {
657 priv->read = w5100_read_indirect;
658 priv->write = w5100_write_indirect;
659 priv->read16 = w5100_read16_indirect;
660 priv->write16 = w5100_write16_indirect;
661 priv->readbuf = w5100_readbuf_indirect;
662 priv->writebuf = w5100_writebuf_indirect;
663 } else {
664 priv->read = w5100_read_direct;
665 priv->write = w5100_write_direct;
666 priv->read16 = w5100_read16_direct;
667 priv->write16 = w5100_write16_direct;
668 priv->readbuf = w5100_readbuf_direct;
669 priv->writebuf = w5100_writebuf_direct;
670 }
671
672 w5100_hw_reset(priv);
673 if (w5100_read16(priv, W5100_RTR) != RTR_DEFAULT)
674 return -ENODEV;
675
676 irq = platform_get_irq(pdev, 0);
677 if (irq < 0)
678 return irq;
679 ret = request_irq(irq, w5100_interrupt,
680 IRQ_TYPE_LEVEL_LOW, name, ndev);
681 if (ret < 0)
682 return ret;
683 priv->irq = irq;
684
685 priv->link_gpio = data->link_gpio;
^^^^^^^^^^^^^^^
New unchecked dereference.
686 if (gpio_is_valid(priv->link_gpio)) {
687 char *link_name = devm_kzalloc(&pdev->dev, 16, GFP_KERNEL);
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] net: orphan queued skbs if device tx can stall
From: Michael S. Tsirkin @ 2012-04-10 8:41 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, linux-kernel, David S. Miller, Jamal Hadi Salim,
Stephen Hemminger, Jason Wang, Neil Horman, Jiri Pirko,
Jeff Kirsher, Michał Mirosław, Ben Hutchings,
Herbert Xu
In-Reply-To: <1334044558.3126.5.camel@edumazet-glaptop>
On Tue, Apr 10, 2012 at 09:55:58AM +0200, Eric Dumazet wrote:
> On Sun, 2012-04-08 at 20:13 +0300, Michael S. Tsirkin wrote:
> > commit 0110d6f22f392f976e84ab49da1b42f85b64a3c5
> > tun: orphan an skb on tx
> > Fixed a configuration where skbs get queued
> > at the tun device forever, blocking senders.
> >
> > However this fix isn't waterproof:
> > userspace can control whether the interface
> > is stopped, and if it is, packets
> > get queued in the qdisc, again potentially forever.
> >
> > Complete the fix by setting a private flag and orphaning
> > at the qdisc level.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > drivers/net/tun.c | 3 +++
> > include/linux/if.h | 1 +
> > net/core/dev.c | 5 +++++
> > net/sched/sch_generic.c | 5 +++++
> > 4 files changed, 14 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > index bb8c72c..15c5bb8 100644
> > --- a/drivers/net/tun.c
> > +++ b/drivers/net/tun.c
> > @@ -535,6 +535,9 @@ static void tun_net_init(struct net_device *dev)
> > dev->tx_queue_len = TUN_READQ_SIZE; /* We prefer our own queue length */
> > break;
> > }
> > + /* Once queue becomes full, we stop tx until userspace
> > + * dequeues some packets, that is potentially forever. */
> > + dev->priv_flags |= IFF_TX_CAN_STALL;
> > }
> >
> > /* Character device part */
> > diff --git a/include/linux/if.h b/include/linux/if.h
> > index f995c66..dd2c7f7 100644
> > --- a/include/linux/if.h
> > +++ b/include/linux/if.h
> > @@ -81,6 +81,7 @@
> > #define IFF_UNICAST_FLT 0x20000 /* Supports unicast filtering */
> > #define IFF_TEAM_PORT 0x40000 /* device used as team port */
> > #define IFF_SUPP_NOFCS 0x80000 /* device supports sending custom FCS */
> > +#define IFF_TX_CAN_STALL 0x100000 /* Device can stop tx forever */
> >
> >
> > #define IF_GET_IFACE 0x0001 /* for querying only */
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 5d59155..e812706 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -2516,6 +2516,11 @@ int dev_queue_xmit(struct sk_buff *skb)
> > struct Qdisc *q;
> > int rc = -ENOMEM;
> >
> > + /* Orphan the skb - required if we might hang on to it
> > + * for indefinite time. */
> > + if (dev->priv_flags & IFF_TX_CAN_STALL)
> > + skb_orphan(skb);
> > +
> > /* Disable soft irqs for various locks below. Also
> > * stops preemption for RCU.
> > */
> > diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
> > index 67fc573..27883d1 100644
> > --- a/net/sched/sch_generic.c
> > +++ b/net/sched/sch_generic.c
> > @@ -120,6 +120,11 @@ int sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
> > /* And release qdisc */
> > spin_unlock(root_lock);
> >
> > + /* Orphan the skb - required if we might hang on to it
> > + * for indefinite time. */
> > + if (dev->priv_flags & IFF_TX_CAN_STALL)
> > + skb_orphan(skb);
> > +
> > HARD_TX_LOCK(dev, txq, smp_processor_id());
> > if (!netif_xmit_frozen_or_stopped(txq))
> > ret = dev_hard_start_xmit(skb, dev, txq);
>
> This slows down the core fastpath for a very specific use.
You are right.
I presume the fastpath is when the device is *not* stalled,
correct? So maybe it's ok to add this logic on slow path
where the queue is stopped and we queue the packet?
When the queue is running tun can orphan the skb itself.
This is what the change at the bottom of this mail on top of my
patch does - untested yet and naturally needs to be combined
and resubmitted properly, assuming it makes sense.
Would this, in your opinion, address this concern adequately?
Thanks!
> In your case I would just not use qdisc at all, like other virtual
> devices.
I think that if we do this, this also disables gso
for the device, doesn't it?
If true that would be a problem as this would
hurt performance of virtualized setups a lot.
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index bb8c72c..fd8c7f0 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -396,7 +396,7 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> sk_filter(tun->socket.sk, skb))
> goto drop;
>
> - if (skb_queue_len(&tun->socket.sk->sk_receive_queue) >= dev->tx_queue_len) {
> + if (skb_queue_len(&tun->socket.sk->sk_receive_queue) >= TUN_READQ_SIZE) {
> if (!(tun->flags & TUN_ONE_QUEUE)) {
> /* Normal queueing mode. */
> /* Packet scheduler handles dropping of further packets. */
tx_queue_len is controllable by SIOCSIFTXQLEN
so we'll need to override SIOCSIFTXQLEN somehow
to avoid breaking userspace that actually uses SIOCSIFTXQLEN, right?
> @@ -521,7 +521,7 @@ static void tun_net_init(struct net_device *dev)
> /* Zero header length */
> dev->type = ARPHRD_NONE;
> dev->flags = IFF_POINTOPOINT | IFF_NOARP | IFF_MULTICAST;
> - dev->tx_queue_len = TUN_READQ_SIZE; /* We prefer our own queue length */
> + dev->tx_queue_len = 0;
> break;
>
> case TUN_TAP_DEV:
> @@ -532,7 +532,7 @@ static void tun_net_init(struct net_device *dev)
>
> eth_hw_addr_random(dev);
>
> - dev->tx_queue_len = TUN_READQ_SIZE; /* We prefer our own queue length */
> + dev->tx_queue_len = 0;
> break;
> }
> }
>
I presume the fastpath is when the device is *not* stalled,
correct? So maybe the following on top of my patch - untested
and naturally would need to be combined and resubmitted properly -
would address the concern?
Thanks for the review!
diff --git a/net/core/dev.c b/net/core/dev.c
index 9d713b8..e42529b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2455,6 +2455,11 @@ static inline int __dev_xmit_skb(struct sk_buff *skb, struct Qdisc *q,
rc = NET_XMIT_SUCCESS;
} else {
skb_dst_force(skb);
+ /* Orphan the skb - required if we might hang on to it
+ * for indefinite time. */
+ if (unlikely(dev->priv_flags & IFF_TX_CAN_STALL))
+ skb_orphan(skb);
+
rc = q->enqueue(skb, q) & NET_XMIT_MASK;
if (qdisc_run_begin(q)) {
if (unlikely(contended)) {
@@ -2517,11 +2522,6 @@ int dev_queue_xmit(struct sk_buff *skb)
struct Qdisc *q;
int rc = -ENOMEM;
- /* Orphan the skb - required if we might hang on to it
- * for indefinite time. */
- if (dev->priv_flags & IFF_TX_CAN_STALL)
- skb_orphan(skb);
-
/* Disable soft irqs for various locks below. Also
* stops preemption for RCU.
*/
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 27883d1..ccc6137 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -120,14 +120,11 @@ int sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
/* And release qdisc */
spin_unlock(root_lock);
- /* Orphan the skb - required if we might hang on to it
- * for indefinite time. */
- if (dev->priv_flags & IFF_TX_CAN_STALL)
- skb_orphan(skb);
-
HARD_TX_LOCK(dev, txq, smp_processor_id());
- if (!netif_xmit_frozen_or_stopped(txq))
+ if (likely(!netif_xmit_frozen_or_stopped(txq)))
ret = dev_hard_start_xmit(skb, dev, txq);
+ else if (dev->priv_flags & IFF_TX_CAN_STALL)
+ skb_orphan(skb);
HARD_TX_UNLOCK(dev, txq);
^ permalink raw reply related
* Re: net: more accurate skb truesize - regression on Microblaze
From: Eric Dumazet @ 2012-04-10 8:45 UTC (permalink / raw)
To: monstr; +Cc: netdev, John Williams, David Miller
In-Reply-To: <4F83F166.4010208@monstr.eu>
On Tue, 2012-04-10 at 10:37 +0200, Michal Simek wrote:
> On 04/10/2012 10:32 AM, Eric Dumazet wrote:
> > On Tue, 2012-04-10 at 10:27 +0200, Eric Dumazet wrote:
> >
> >> BTW, some NIC drivers are known to provide fat skb in their rx path, and
> >> need to be fixed as well. (Some others just lie about skb->truesize to
> >> avoid the tcp slowdown, see my previous iwlwifi patch)
> >>
> >>
> >
> > What is the driver you currently use on your platform ?
>
> Using Xilinx ll_temac(in mainline) and axi_emac.
>
OK thanks, could you please give us the content
of /proc/sys/net/ipv4/tcp_rmem ?
^ permalink raw reply
* Re: [PATCH] net: orphan queued skbs if device tx can stall
From: Eric Dumazet @ 2012-04-10 8:55 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: netdev, linux-kernel, David S. Miller, Jamal Hadi Salim,
Stephen Hemminger, Jason Wang, Neil Horman, Jiri Pirko,
Jeff Kirsher, Michał Mirosław, Ben Hutchings,
Herbert Xu
In-Reply-To: <20120410084151.GA27193@redhat.com>
On Tue, 2012-04-10 at 11:41 +0300, Michael S. Tsirkin wrote:
> On Tue, Apr 10, 2012 at 09:55:58AM +0200, Eric Dumazet wrote:
> > In your case I would just not use qdisc at all, like other virtual
> > devices.
>
> I think that if we do this, this also disables gso
> for the device, doesn't it?
Not at all, thats unrelated.
> If true that would be a problem as this would
> hurt performance of virtualized setups a lot.
In fact, removing qdisc layer will help a lot, removing a contention
point.
Anyway, with a 500 packet limit in TUN queue itself, qdisc layer should
be always empty. Whats the point storing more than 500 packets for a
device ? Thats a latency killer.
>
> > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > index bb8c72c..fd8c7f0 100644
> > --- a/drivers/net/tun.c
> > +++ b/drivers/net/tun.c
> > @@ -396,7 +396,7 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> > sk_filter(tun->socket.sk, skb))
> > goto drop;
> >
> > - if (skb_queue_len(&tun->socket.sk->sk_receive_queue) >= dev->tx_queue_len) {
> > + if (skb_queue_len(&tun->socket.sk->sk_receive_queue) >= TUN_READQ_SIZE) {
> > if (!(tun->flags & TUN_ONE_QUEUE)) {
> > /* Normal queueing mode. */
> > /* Packet scheduler handles dropping of further packets. */
>
> tx_queue_len is controllable by SIOCSIFTXQLEN
> so we'll need to override SIOCSIFTXQLEN somehow
> to avoid breaking userspace that actually uses SIOCSIFTXQLEN, right?
Right now, you control with this tx_queue_len both the qdisc limit (if
pfifo_fast default) and the receive_queue in TUN.
That doesnt seem right to me, and more a hack/side effect.
Maybe you want to introduce a new setting, only controling receive queue
limit, and use tx_queue_len for its original meaning.
Then, setting tx_queue_len to 0 permits to remove qdisc layer, as any
other netdevice.
^ permalink raw reply
* Re: Ethernet driver for the WIZnet W5100 chip
From: Dan Carpenter @ 2012-04-10 9:08 UTC (permalink / raw)
To: msink; +Cc: netdev
The same issue is there in w5300_hw_probe() as well.
regards,
dan carpenter
^ permalink raw reply
* Re: net: more accurate skb truesize - regression on Microblaze
From: Michal Simek @ 2012-04-10 9:11 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, John Williams, David Miller
In-Reply-To: <1334047544.3126.14.camel@edumazet-glaptop>
On 04/10/2012 10:45 AM, Eric Dumazet wrote:
> On Tue, 2012-04-10 at 10:37 +0200, Michal Simek wrote:
>> On 04/10/2012 10:32 AM, Eric Dumazet wrote:
>>> On Tue, 2012-04-10 at 10:27 +0200, Eric Dumazet wrote:
>>>
>>>> BTW, some NIC drivers are known to provide fat skb in their rx path, and
>>>> need to be fixed as well. (Some others just lie about skb->truesize to
>>>> avoid the tcp slowdown, see my previous iwlwifi patch)
>>>>
>>>>
>>>
>>> What is the driver you currently use on your platform ?
>>
>> Using Xilinx ll_temac(in mainline) and axi_emac.
>>
>
> OK thanks, could you please give us the content
> of /proc/sys/net/ipv4/tcp_rmem ?
~ # cat /proc/sys/net/ipv4/tcp_rmem
4096 87380 130048
Let me know if you want to test anything.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
^ permalink raw reply
* Re: net: more accurate skb truesize - regression on Microblaze
From: Eric Dumazet @ 2012-04-10 9:24 UTC (permalink / raw)
To: monstr; +Cc: netdev, John Williams, David Miller
In-Reply-To: <4F83F959.3070302@monstr.eu>
On Tue, 2012-04-10 at 11:11 +0200, Michal Simek wrote:
> ~ # cat /proc/sys/net/ipv4/tcp_rmem
> 4096 87380 130048
Are they default values, or tuned by admin ?
130048 bytes isnt enough to let TCP open its rcv window.
^ permalink raw reply
* Re: net: more accurate skb truesize - regression on Microblaze
From: Michal Simek @ 2012-04-10 9:29 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, John Williams, David Miller
In-Reply-To: <1334049896.3126.23.camel@edumazet-glaptop>
On 04/10/2012 11:24 AM, Eric Dumazet wrote:
> On Tue, 2012-04-10 at 11:11 +0200, Michal Simek wrote:
>
>> ~ # cat /proc/sys/net/ipv4/tcp_rmem
>> 4096 87380 130048
>
> Are they default values, or tuned by admin ?
>
> 130048 bytes isnt enough to let TCP open its rcv window.
yep. Default value after powerup. What's wrong with that?
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
^ permalink raw reply
* Re: [PATCH] net: orphan queued skbs if device tx can stall
From: Michael S. Tsirkin @ 2012-04-10 9:31 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, linux-kernel, David S. Miller, Jamal Hadi Salim,
Stephen Hemminger, Jason Wang, Neil Horman, Jiri Pirko,
Jeff Kirsher, Michał Mirosław, Ben Hutchings,
Herbert Xu
In-Reply-To: <1334048100.3126.21.camel@edumazet-glaptop>
On Tue, Apr 10, 2012 at 10:55:00AM +0200, Eric Dumazet wrote:
> On Tue, 2012-04-10 at 11:41 +0300, Michael S. Tsirkin wrote:
> > On Tue, Apr 10, 2012 at 09:55:58AM +0200, Eric Dumazet wrote:
>
> > > In your case I would just not use qdisc at all, like other virtual
> > > devices.
> >
> > I think that if we do this, this also disables gso
> > for the device, doesn't it?
>
> Not at all, thats unrelated.
>
> > If true that would be a problem as this would
> > hurt performance of virtualized setups a lot.
>
> In fact, removing qdisc layer will help a lot, removing a contention
> point.
>
> Anyway, with a 500 packet limit in TUN queue itself, qdisc layer should
> be always empty. Whats the point storing more than 500 packets for a
> device ? Thats a latency killer.
AKA bufferbloat :)
We could try and reduce the TUN queue so that qdisc can drop packets in
an intelligent manner (e.g. choke) but this would conflict with what you
propose, right?
> >
> > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > > index bb8c72c..fd8c7f0 100644
> > > --- a/drivers/net/tun.c
> > > +++ b/drivers/net/tun.c
> > > @@ -396,7 +396,7 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> > > sk_filter(tun->socket.sk, skb))
> > > goto drop;
> > >
> > > - if (skb_queue_len(&tun->socket.sk->sk_receive_queue) >= dev->tx_queue_len) {
> > > + if (skb_queue_len(&tun->socket.sk->sk_receive_queue) >= TUN_READQ_SIZE) {
> > > if (!(tun->flags & TUN_ONE_QUEUE)) {
> > > /* Normal queueing mode. */
> > > /* Packet scheduler handles dropping of further packets. */
> >
> > tx_queue_len is controllable by SIOCSIFTXQLEN
> > so we'll need to override SIOCSIFTXQLEN somehow
> > to avoid breaking userspace that actually uses SIOCSIFTXQLEN, right?
>
> Right now, you control with this tx_queue_len both the qdisc limit (if
> pfifo_fast default) and the receive_queue in TUN.
>
> That doesnt seem right to me, and more a hack/side effect.
> Maybe you want to introduce a new setting, only controling receive queue
> limit, and use tx_queue_len for its original meaning.
>
> Then, setting tx_queue_len to 0 permits to remove qdisc layer, as any
> other netdevice.
>
>
True. Still this is the only interface we have for controlling
the internal queue length so it seems safe to assume someone
is using it for this purpose.
And if that happens we get the deadlock back since
tx_queue_len will get set to a non-0 value. Right?
--
MST
^ permalink raw reply
* Re: net: more accurate skb truesize - regression on Microblaze
From: Eric Dumazet @ 2012-04-10 9:38 UTC (permalink / raw)
To: monstr; +Cc: netdev, John Williams, David Miller
In-Reply-To: <4F83FD7A.5010602@monstr.eu>
On Tue, 2012-04-10 at 11:29 +0200, Michal Simek wrote:
> On 04/10/2012 11:24 AM, Eric Dumazet wrote:
> > On Tue, 2012-04-10 at 11:11 +0200, Michal Simek wrote:
> >
> >> ~ # cat /proc/sys/net/ipv4/tcp_rmem
> >> 4096 87380 130048
> >
> > Are they default values, or tuned by admin ?
> >
> > 130048 bytes isnt enough to let TCP open its rcv window.
>
> yep. Default value after powerup. What's wrong with that?
>
> Michal
>
I guess your tcp performance is driven by these numbers mostly.
receive window wont grow above 64K in these case. a tcpdump could
confirm the issue.
Do you have 130000 pages of memory ?
Seems the net/ipv4/tcp.c code is wrong (or the comment is wrong)
Since its not 1/128 but 1/1024 ....
/* Set per-socket limits to no more than 1/128 the pressure threshold */
limit = nr_free_buffer_pages() << (PAGE_SHIFT - 10);
limit = max(limit, 128UL);
max_share = min(4UL*1024*1024, limit);
sysctl_tcp_wmem[0] = SK_MEM_QUANTUM;
sysctl_tcp_wmem[1] = 16*1024;
sysctl_tcp_wmem[2] = max(64*1024, max_share);
sysctl_tcp_rmem[0] = SK_MEM_QUANTUM;
sysctl_tcp_rmem[1] = 87380;
sysctl_tcp_rmem[2] = max(87380, max_share);
^ permalink raw reply
* Re: [PATCH v17 08/15] seccomp: add system call filtering using BPF
From: James Morris @ 2012-04-10 9:48 UTC (permalink / raw)
To: Will Drewry
Cc: Indan Zupancic, Andrew Morton, linux-kernel,
linux-security-module, linux-arch, linux-doc, kernel-hardening,
netdev, x86, arnd, davem, hpa, mingo, oleg, peterz, rdunlap,
mcgrathr, tglx, luto, eparis, serge.hallyn, djm, scarybeasts,
pmoore, corbet, eric.dumazet, markus, coreyb, keescook
In-Reply-To: <CABqD9hYE0S4EL6yH-57SfNs+D8GFKJgWVN21VzXpQmRuh2ow=w@mail.gmail.com>
On Mon, 9 Apr 2012, Will Drewry wrote:
> > seccomp_chk_filter() mirrors sk_chk_filter(). So it refers to
> > "chk", not "check".
>
> I can change it to be written out or leave it matching the networking
> code. Any preferences?
check :-)
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: net: more accurate skb truesize - regression on Microblaze
From: Michal Simek @ 2012-04-10 9:50 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, John Williams, David Miller
In-Reply-To: <1334050698.3126.30.camel@edumazet-glaptop>
On 04/10/2012 11:38 AM, Eric Dumazet wrote:
> On Tue, 2012-04-10 at 11:29 +0200, Michal Simek wrote:
>> On 04/10/2012 11:24 AM, Eric Dumazet wrote:
>>> On Tue, 2012-04-10 at 11:11 +0200, Michal Simek wrote:
>>>
>>>> ~ # cat /proc/sys/net/ipv4/tcp_rmem
>>>> 4096 87380 130048
>>>
>>> Are they default values, or tuned by admin ?
>>>
>>> 130048 bytes isnt enough to let TCP open its rcv window.
>>
>> yep. Default value after powerup. What's wrong with that?
>>
>> Michal
>>
>
> I guess your tcp performance is driven by these numbers mostly.
>
> receive window wont grow above 64K in these case. a tcpdump could
> confirm the issue.
>
> Do you have 130000 pages of memory ?
128MB memory and 4k page which is 32k pages.
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
^ permalink raw reply
* Re: net: more accurate skb truesize - regression on Microblaze
From: Eric Dumazet @ 2012-04-10 9:52 UTC (permalink / raw)
To: monstr; +Cc: netdev, John Williams, David Miller
In-Reply-To: <1334050698.3126.30.camel@edumazet-glaptop>
On Tue, 2012-04-10 at 11:38 +0200, Eric Dumazet wrote:
> On Tue, 2012-04-10 at 11:29 +0200, Michal Simek wrote:
> > On 04/10/2012 11:24 AM, Eric Dumazet wrote:
> > > On Tue, 2012-04-10 at 11:11 +0200, Michal Simek wrote:
> > >
> > >> ~ # cat /proc/sys/net/ipv4/tcp_rmem
> > >> 4096 87380 130048
> > >
> > > Are they default values, or tuned by admin ?
> > >
> > > 130048 bytes isnt enough to let TCP open its rcv window.
> >
> > yep. Default value after powerup. What's wrong with that?
> >
> > Michal
> >
>
> I guess your tcp performance is driven by these numbers mostly.
>
> receive window wont grow above 64K in these case. a tcpdump could
> confirm the issue.
>
> Do you have 130000 pages of memory ?
>
> Seems the net/ipv4/tcp.c code is wrong (or the comment is wrong)
>
> Since its not 1/128 but 1/1024 ....
>
>
> /* Set per-socket limits to no more than 1/128 the pressure threshold */
> limit = nr_free_buffer_pages() << (PAGE_SHIFT - 10);
> limit = max(limit, 128UL);
> max_share = min(4UL*1024*1024, limit);
>
> sysctl_tcp_wmem[0] = SK_MEM_QUANTUM;
> sysctl_tcp_wmem[1] = 16*1024;
> sysctl_tcp_wmem[2] = max(64*1024, max_share);
>
> sysctl_tcp_rmem[0] = SK_MEM_QUANTUM;
> sysctl_tcp_rmem[1] = 87380;
> sysctl_tcp_rmem[2] = max(87380, max_share);
>
OK there is a bug introduced in commit
c43b874d5d714f271b80d4c3f49e05d0cbf51ed2
(tcp: properly initialize tcp memory limits)
I'll send a patch to change :
limit = nr_free_buffer_pages() << (PAGE_SHIFT - 10);
back to
limit = nr_free_buffer_pages() << (PAGE_SHIFT - 7);
You could try this :)
^ permalink raw reply
* [PATCH] tcp: restore correct limit
From: Eric Dumazet @ 2012-04-10 10:03 UTC (permalink / raw)
To: monstr; +Cc: netdev, John Williams, David Miller, Jason Wang, Glauber Costa
In-Reply-To: <1334050698.3126.30.camel@edumazet-glaptop>
Commit c43b874d5d714f (tcp: properly initialize tcp memory limits)
added a regression on machines with low amount of memory, since sockets
cant use 1/128 of memory but 1/1024
Fix this to match comment and previous behavior.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Glauber Costa <glommer@parallels.com>
---
net/ipv4/tcp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 5d54ed3..67d726e 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3302,7 +3302,7 @@ void __init tcp_init(void)
tcp_init_mem(&init_net);
/* Set per-socket limits to no more than 1/128 the pressure threshold */
- limit = nr_free_buffer_pages() << (PAGE_SHIFT - 10);
+ limit = nr_free_buffer_pages() << (PAGE_SHIFT - 7);
limit = max(limit, 128UL);
max_share = min(4UL*1024*1024, limit);
^ permalink raw reply related
* Re: net: more accurate skb truesize - regression on Microblaze
From: Michal Simek @ 2012-04-10 10:03 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, John Williams, David Miller
In-Reply-To: <1334051536.3126.45.camel@edumazet-glaptop>
On 04/10/2012 11:52 AM, Eric Dumazet wrote:
> On Tue, 2012-04-10 at 11:38 +0200, Eric Dumazet wrote:
>> On Tue, 2012-04-10 at 11:29 +0200, Michal Simek wrote:
>>> On 04/10/2012 11:24 AM, Eric Dumazet wrote:
>>>> On Tue, 2012-04-10 at 11:11 +0200, Michal Simek wrote:
>>>>
>>>>> ~ # cat /proc/sys/net/ipv4/tcp_rmem
>>>>> 4096 87380 130048
>>>>
>>>> Are they default values, or tuned by admin ?
>>>>
>>>> 130048 bytes isnt enough to let TCP open its rcv window.
>>>
>>> yep. Default value after powerup. What's wrong with that?
>>>
>>> Michal
>>>
>>
>> I guess your tcp performance is driven by these numbers mostly.
>>
>> receive window wont grow above 64K in these case. a tcpdump could
>> confirm the issue.
>>
>> Do you have 130000 pages of memory ?
>>
>> Seems the net/ipv4/tcp.c code is wrong (or the comment is wrong)
>>
>> Since its not 1/128 but 1/1024 ....
>>
>>
>> /* Set per-socket limits to no more than 1/128 the pressure threshold */
>> limit = nr_free_buffer_pages()<< (PAGE_SHIFT - 10);
>> limit = max(limit, 128UL);
>> max_share = min(4UL*1024*1024, limit);
>>
>> sysctl_tcp_wmem[0] = SK_MEM_QUANTUM;
>> sysctl_tcp_wmem[1] = 16*1024;
>> sysctl_tcp_wmem[2] = max(64*1024, max_share);
>>
>> sysctl_tcp_rmem[0] = SK_MEM_QUANTUM;
>> sysctl_tcp_rmem[1] = 87380;
>> sysctl_tcp_rmem[2] = max(87380, max_share);
>>
>
> OK there is a bug introduced in commit
> c43b874d5d714f271b80d4c3f49e05d0cbf51ed2
> (tcp: properly initialize tcp memory limits)
>
> I'll send a patch to change :
>
> limit = nr_free_buffer_pages()<< (PAGE_SHIFT - 10);
>
> back to
>
> limit = nr_free_buffer_pages()<< (PAGE_SHIFT - 7);
>
>
> You could try this :)
sure.
~ # cat /proc/sys/net/ipv4/tcp_rmem
4096 87380 1040384
Regression is till 5% which is much better on hw design without csum support.
I will also test it with csum support and let you know.
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
^ permalink raw reply
* Re: [PATCH] net: orphan queued skbs if device tx can stall
From: Eric Dumazet @ 2012-04-10 10:04 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: netdev, linux-kernel, David S. Miller, Jamal Hadi Salim,
Stephen Hemminger, Jason Wang, Neil Horman, Jiri Pirko,
Jeff Kirsher, Michał Mirosław, Ben Hutchings,
Herbert Xu
In-Reply-To: <20120410093140.GA27651@redhat.com>
On Tue, 2012-04-10 at 12:31 +0300, Michael S. Tsirkin wrote:
> True. Still this is the only interface we have for controlling
> the internal queue length so it seems safe to assume someone
> is using it for this purpose.
>
So to workaround a problem in tun, you want to hack net/core/dev.c :(
Packets in qdisc should not be orphaned.
If you think about it, why do we attach skb to socket in the first
place ?
If its not needed for tun, why should it be needed for other devices ?
If TUN has a problem being stopped forever, maybe it should take
appropriate action to flush all packets in qdisc queue after a while, as
this makes no sense to delay packets forever.
^ permalink raw reply
* [PATCH 1/2] l2tp: fix refcount leak in l2tp_ip sockets
From: James Chapman @ 2012-04-10 10:10 UTC (permalink / raw)
To: netdev; +Cc: James Chapman
In-Reply-To: <1334052643-10910-1-git-send-email-jchapman@katalix.com>
The l2tp_ip socket close handler does not update the module refcount
correctly which prevents module unload after the first bind() call on
an L2TPv3 IP encapulation socket.
Signed-off-by: James Chapman <jchapman@katalix.com>
---
net/l2tp/l2tp_ip.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c
index 55670ec..b56be14 100644
--- a/net/l2tp/l2tp_ip.c
+++ b/net/l2tp/l2tp_ip.c
@@ -232,7 +232,7 @@ static void l2tp_ip_close(struct sock *sk, long timeout)
{
write_lock_bh(&l2tp_ip_lock);
hlist_del_init(&sk->sk_bind_node);
- hlist_del_init(&sk->sk_node);
+ sk_del_node_init(sk);
write_unlock_bh(&l2tp_ip_lock);
sk_common_release(sk);
}
--
1.7.0.4
^ permalink raw reply related
* [PATCH 0/2] l2tp: l2tp_ip socket fixes
From: James Chapman @ 2012-04-10 10:10 UTC (permalink / raw)
To: netdev
A couple of small fixes for the l2tp_ip socket support. This is for
L2TPv3 IP link encapsulation.
[PATCH 1/2] l2tp: fix refcount leak in l2tp_ip sockets
[PATCH 2/2] l2tp: don't overwrite source address in l2tp_ip_bind()
net/l2tp/l2tp_ip.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
^ permalink raw reply
* [PATCH 2/2] l2tp: don't overwrite source address in l2tp_ip_bind()
From: James Chapman @ 2012-04-10 10:10 UTC (permalink / raw)
To: netdev; +Cc: James Chapman
In-Reply-To: <1334052643-10910-1-git-send-email-jchapman@katalix.com>
Applications using L2TP/IP sockets want to be able to bind() an L2TP/IP
socket to set the local tunnel id while leaving the auto-assigned source
address alone. So if no source address is supplied, don't overwrite
the address already stored in the socket.
Signed-off-by: James Chapman <jchapman@katalix.com>
---
net/l2tp/l2tp_ip.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c
index b56be14..585d93e 100644
--- a/net/l2tp/l2tp_ip.c
+++ b/net/l2tp/l2tp_ip.c
@@ -271,7 +271,8 @@ static int l2tp_ip_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
chk_addr_ret != RTN_MULTICAST && chk_addr_ret != RTN_BROADCAST)
goto out;
- inet->inet_rcv_saddr = inet->inet_saddr = addr->l2tp_addr.s_addr;
+ if (addr->l2tp_addr.s_addr)
+ inet->inet_rcv_saddr = inet->inet_saddr = addr->l2tp_addr.s_addr;
if (chk_addr_ret == RTN_MULTICAST || chk_addr_ret == RTN_BROADCAST)
inet->inet_saddr = 0; /* Use device */
sk_dst_reset(sk);
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] tcp: restore correct limit
From: Michal Simek @ 2012-04-10 10:14 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, John Williams, David Miller, Jason Wang, Glauber Costa
In-Reply-To: <1334052194.3126.66.camel@edumazet-glaptop>
On 04/10/2012 12:03 PM, Eric Dumazet wrote:
> Commit c43b874d5d714f (tcp: properly initialize tcp memory limits)
> added a regression on machines with low amount of memory, since sockets
> cant use 1/128 of memory but 1/1024
>
> Fix this to match comment and previous behavior.
>
> Signed-off-by: Eric Dumazet<eric.dumazet@gmail.com>
> Cc: Jason Wang<jasowang@redhat.com>
> Cc: Glauber Costa<glommer@parallels.com>
> ---
> net/ipv4/tcp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 5d54ed3..67d726e 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -3302,7 +3302,7 @@ void __init tcp_init(void)
>
> tcp_init_mem(&init_net);
> /* Set per-socket limits to no more than 1/128 the pressure threshold */
> - limit = nr_free_buffer_pages()<< (PAGE_SHIFT - 10);
> + limit = nr_free_buffer_pages()<< (PAGE_SHIFT - 7);
> limit = max(limit, 128UL);
> max_share = min(4UL*1024*1024, limit);
>
hw design with csum is also much better.
Tested-by: Michal Simek <monstr@monstr.eu>
Thanks for help,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
^ permalink raw reply
* Re: [PATCH v2] wl12xx: fix DMA-API-related warnings
From: Luciano Coelho @ 2012-04-10 10:19 UTC (permalink / raw)
To: Mircea Gherzan
Cc: John W. Linville, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1332006114-30230-1-git-send-email-mgherzan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Sat, 2012-03-17 at 18:41 +0100, Mircea Gherzan wrote:
> On the PandaBoard (omap_hsmmc + wl12xx_sdio) with DMA_API_DEBUG:
>
> WARNING: at lib/dma-debug.c:930 check_for_stack.part.8+0x7c/0xe0()
> omap_hsmmc omap_hsmmc.4: DMA-API: device driver maps memory fromstack
>
> Signed-off-by: Mircea Gherzan <mgherzan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
Pushed, thanks for fixing this!
--
Cheers,
Luca.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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] tcp: restore correct limit
From: Jason Wang @ 2012-04-10 10:29 UTC (permalink / raw)
To: monstr; +Cc: Eric Dumazet, netdev, John Williams, David Miller, Glauber Costa
In-Reply-To: <4F8407F7.8070703@monstr.eu>
On 04/10/2012 06:14 PM, Michal Simek wrote:
> On 04/10/2012 12:03 PM, Eric Dumazet wrote:
>> Commit c43b874d5d714f (tcp: properly initialize tcp memory limits)
>> added a regression on machines with low amount of memory, since sockets
>> cant use 1/128 of memory but 1/1024
>>
>> Fix this to match comment and previous behavior.
>>
>> Signed-off-by: Eric Dumazet<eric.dumazet@gmail.com>
>> Cc: Jason Wang<jasowang@redhat.com>
>> Cc: Glauber Costa<glommer@parallels.com>
>> ---
>> net/ipv4/tcp.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>> index 5d54ed3..67d726e 100644
>> --- a/net/ipv4/tcp.c
>> +++ b/net/ipv4/tcp.c
>> @@ -3302,7 +3302,7 @@ void __init tcp_init(void)
>>
>> tcp_init_mem(&init_net);
>> /* Set per-socket limits to no more than 1/128 the pressure
>> threshold */
>> - limit = nr_free_buffer_pages()<< (PAGE_SHIFT - 10);
>> + limit = nr_free_buffer_pages()<< (PAGE_SHIFT - 7);
>> limit = max(limit, 128UL);
>> max_share = min(4UL*1024*1024, limit);
>>
>
> hw design with csum is also much better.
> Tested-by: Michal Simek <monstr@monstr.eu>
>
> Thanks for help,
> Michal
>
>
>
>
>
Hi Michal and Eric:
Which version of kernel did you test, did you try the newest kernel? The
reason I use (PAGE_SHIFT - 10) is in the commit before 3dc43e3, the
limit were calculated with:
limit = nr_free_buffer_pages() / 8;
limit = max(limit, 128UL);
...
limit = ((unsigned long)sysctl_tcp_mem[1]) << (PAGE_SHIFT - 7);
So the rmem should be ok. But there's a defect (which I think does
affect the regression) of my patch would could cause limit that we
should shift after comparing with 128UL like:
limit = nr_free_buffer_pages() / 8;
limit = max(limit, 128UL) << (PAGE_SHIFT - 7);
Is anything I miss?
Thanks
^ permalink raw reply
* Re: [PATCH] tcp: restore correct limit
From: Michal Simek @ 2012-04-10 10:32 UTC (permalink / raw)
To: Jason Wang
Cc: Eric Dumazet, netdev, John Williams, David Miller, Glauber Costa
In-Reply-To: <4F840B99.8040409@redhat.com>
On 04/10/2012 12:29 PM, Jason Wang wrote:
> On 04/10/2012 06:14 PM, Michal Simek wrote:
>> On 04/10/2012 12:03 PM, Eric Dumazet wrote:
>>> Commit c43b874d5d714f (tcp: properly initialize tcp memory limits)
>>> added a regression on machines with low amount of memory, since sockets
>>> cant use 1/128 of memory but 1/1024
>>>
>>> Fix this to match comment and previous behavior.
>>>
>>> Signed-off-by: Eric Dumazet<eric.dumazet@gmail.com>
>>> Cc: Jason Wang<jasowang@redhat.com>
>>> Cc: Glauber Costa<glommer@parallels.com>
>>> ---
>>> net/ipv4/tcp.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>>> index 5d54ed3..67d726e 100644
>>> --- a/net/ipv4/tcp.c
>>> +++ b/net/ipv4/tcp.c
>>> @@ -3302,7 +3302,7 @@ void __init tcp_init(void)
>>>
>>> tcp_init_mem(&init_net);
>>> /* Set per-socket limits to no more than 1/128 the pressure threshold */
>>> - limit = nr_free_buffer_pages()<< (PAGE_SHIFT - 10);
>>> + limit = nr_free_buffer_pages()<< (PAGE_SHIFT - 7);
>>> limit = max(limit, 128UL);
>>> max_share = min(4UL*1024*1024, limit);
>>>
>>
>> hw design with csum is also much better.
>> Tested-by: Michal Simek <monstr@monstr.eu>
>>
>> Thanks for help,
>> Michal
>>
>>
>>
>>
>>
> Hi Michal and Eric:
>
> Which version of kernel did you test, did you try the newest kernel? The reason I use (PAGE_SHIFT - 10) is in the commit before 3dc43e3, the limit were calculated with:
I have tested it on 3.4.0-rc2.
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
^ permalink raw reply
* Re: [PATCH v17 08/15] seccomp: add system call filtering using BPF
From: Eric Dumazet @ 2012-04-10 10:34 UTC (permalink / raw)
To: Indan Zupancic
Cc: Andrew Morton, Will Drewry, linux-kernel, linux-security-module,
linux-arch, linux-doc, kernel-hardening, netdev, x86, arnd, davem,
hpa, mingo, oleg, peterz, rdunlap, mcgrathr, tglx, luto, eparis,
serge.hallyn, djm, scarybeasts, pmoore, corbet, markus, coreyb,
keescook, jmorris
In-Reply-To: <67e30a0c8655fc53a92e8138bba9de66.squirrel@webmail.greenhost.nl>
On Mon, 2012-04-09 at 04:22 +1000, Indan Zupancic wrote:
> On Sat, April 7, 2012 06:23, Andrew Morton wrote:
> >
> > I think this gives userspace an easy way of causing page allocation
> > failure warnings, by permitting large kmalloc() attempts. Add
> > __GFP_NOWARN?
>
> Max is 32kb. sk_attach_filter() in net/core/filter.c is worse,
> it allocates up to 512kb before even checking the length.
>
I dont think so.
sk_attach_filter() uses sk_malloc() and it does a check.
# cat /proc/sys/net/core/optmem_max
20480
Of course you can change the limit on your machine.
^ permalink raw reply
* Re: [PATCH] tcp: restore correct limit
From: Eric Dumazet @ 2012-04-10 10:40 UTC (permalink / raw)
To: Jason Wang; +Cc: monstr, netdev, John Williams, David Miller, Glauber Costa
In-Reply-To: <4F840B99.8040409@redhat.com>
On Tue, 2012-04-10 at 18:29 +0800, Jason Wang wrote:
> On 04/10/2012 06:14 PM, Michal Simek wrote:
> > On 04/10/2012 12:03 PM, Eric Dumazet wrote:
> >> Commit c43b874d5d714f (tcp: properly initialize tcp memory limits)
> >> added a regression on machines with low amount of memory, since sockets
> >> cant use 1/128 of memory but 1/1024
> >>
> >> Fix this to match comment and previous behavior.
> >>
> >> Signed-off-by: Eric Dumazet<eric.dumazet@gmail.com>
> >> Cc: Jason Wang<jasowang@redhat.com>
> >> Cc: Glauber Costa<glommer@parallels.com>
> >> ---
> >> net/ipv4/tcp.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> >> index 5d54ed3..67d726e 100644
> >> --- a/net/ipv4/tcp.c
> >> +++ b/net/ipv4/tcp.c
> >> @@ -3302,7 +3302,7 @@ void __init tcp_init(void)
> >>
> >> tcp_init_mem(&init_net);
> >> /* Set per-socket limits to no more than 1/128 the pressure
> >> threshold */
> >> - limit = nr_free_buffer_pages()<< (PAGE_SHIFT - 10);
> >> + limit = nr_free_buffer_pages()<< (PAGE_SHIFT - 7);
> >> limit = max(limit, 128UL);
> >> max_share = min(4UL*1024*1024, limit);
> >>
> >
> > hw design with csum is also much better.
> > Tested-by: Michal Simek <monstr@monstr.eu>
> >
> > Thanks for help,
> > Michal
> >
> >
> >
> >
> >
> Hi Michal and Eric:
>
> Which version of kernel did you test, did you try the newest kernel? The
> reason I use (PAGE_SHIFT - 10) is in the commit before 3dc43e3, the
> limit were calculated with:
>
> limit = nr_free_buffer_pages() / 8;
> limit = max(limit, 128UL);
> ...
> limit = ((unsigned long)sysctl_tcp_mem[1]) << (PAGE_SHIFT - 7);
>
> So the rmem should be ok. But there's a defect (which I think does
> affect the regression) of my patch would could cause limit that we
> should shift after comparing with 128UL like:
>
> limit = nr_free_buffer_pages() / 8;
> limit = max(limit, 128UL) << (PAGE_SHIFT - 7);
>
> Is anything I miss?
>
Yes, probably.
Maybe you should check what was the situation on 2.6 kernels.
Your commit did not completely fix the 4acb41903b2 one
I dont feel its necessary to put in the changelog the complete bug
history, since your commit does the needed tracking.
^ 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