* [PATCH] Break up the single NBD lock into one per NBD device
From: H.K. Jerry Chu @ 2011-09-26 23:34 UTC (permalink / raw)
To: davem; +Cc: netdev, Jerry Chu
From: Jerry Chu <hkchu@google.com>
This patch breaks up the single NBD lock into one per
disk. The single NBD lock has become a serious performance
bottleneck when multiple NBD disks are being used.
The original comment on why a single lock may be ok no
longer holds for today's much faster NICs.
Signed-off-by: H.K. Jerry Chu <hkchu@google.com>
---
drivers/block/nbd.c | 22 +++++++++-------------
1 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index f533f33..355e15c 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -58,20 +58,9 @@ static unsigned int debugflags;
static unsigned int nbds_max = 16;
static struct nbd_device *nbd_dev;
+static spinlock_t *nbd_locks;
static int max_part;
-/*
- * Use just one lock (or at most 1 per NIC). Two arguments for this:
- * 1. Each NIC is essentially a synchronization point for all servers
- * accessed through that NIC so there's no need to have more locks
- * than NICs anyway.
- * 2. More locks lead to more "Dirty cache line bouncing" which will slow
- * down each lock to the point where they're actually slower than just
- * a single lock.
- * Thanks go to Jens Axboe and Al Viro for their LKML emails explaining this!
- */
-static DEFINE_SPINLOCK(nbd_lock);
-
#ifndef NDEBUG
static const char *ioctl_cmd_to_ascii(int cmd)
{
@@ -753,6 +742,12 @@ static int __init nbd_init(void)
if (!nbd_dev)
return -ENOMEM;
+ nbd_locks = kcalloc(nbds_max, sizeof(*nbd_locks), GFP_KERNEL);
+ if (!nbd_locks) {
+ kfree(nbd_dev);
+ return -ENOMEM;
+ }
+
part_shift = 0;
if (max_part > 0) {
part_shift = fls(max_part);
@@ -784,7 +779,7 @@ static int __init nbd_init(void)
* every gendisk to have its very own request_queue struct.
* These structs are big so we dynamically allocate them.
*/
- disk->queue = blk_init_queue(do_nbd_request, &nbd_lock);
+ disk->queue = blk_init_queue(do_nbd_request, &nbd_locks[i]);
if (!disk->queue) {
put_disk(disk);
goto out;
@@ -832,6 +827,7 @@ out:
put_disk(nbd_dev[i].disk);
}
kfree(nbd_dev);
+ kfree(nbd_locks);
return err;
}
--
1.7.3.1
^ permalink raw reply related
* Re: Bridge stays down until a port is added
From: Sven-Haegar Koch @ 2011-09-27 0:28 UTC (permalink / raw)
To: Nicolas de Pesloüan; +Cc: Stephen Hemminger, Marc Haber, netdev
In-Reply-To: <4E80F8BD.4010401@gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1994 bytes --]
On Tue, 27 Sep 2011, Nicolas de Pesloüan wrote:
> Le 26/09/2011 22:05, Stephen Hemminger a écrit :
> > On Mon, 26 Sep 2011 22:02:21 +0200
> > Nicolas de Pesloüan<nicolas.2p.debian@gmail.com> wrote:
> [...]
> > > Stephen,
> > >
> > > What do you think about a generic per-interface option that would cause
> > > bind() to accept tentative
> > > address hold by a particular interface? This of course violate IPv6
> > > principle, but we are talking
> > > about interfaces that are unable to do DAD, either permanently or until
> > > something happens on the
> > > underlying device.
> > >
> > > echo 1> /sys/class/net/br0/allow_bind_on_tentative_address
> > > echo 1> /sys/class/net/dummy0/allow_bind_on_tentative_address
> > > echo 1> /sys/class/net/wlan0/allow_bind_on_tentative_address
> > > and so on...
> > >
> > > And we may possibly automatically reset this option to 0 if DAD eventually
> > > causes the address to be
> > > considered duplicate.
> >
> > The issue is that if DAD rejects a duplicate, the socket is dead and
> > application is
> > out of luck.
>
> Yes, and this is by design. Setting the option would state "I want to allow
> early bind(), prior to DAD and I assume the fact that a possible duplicate
> address will cause the corresponding socket to be dead and so the using
> application."
How about a setting just completely disabling this
duplicate-address-detection crap?
I do not want my servers to ever give up servicing a configured IP no
matter what else on the network tries to interfere.
If there are two machines using the same IP at a moment, then it may not
work so great, but both should have all their services up and running,
and once the invalid one got disconnected ("switch port down")
everything works - not needing to reboot the real one because some
daemons were not able to bind their service IPs.
c'ya
sven-haegar
--
Three may keep a secret, if two of them are dead.
- Ben F.
^ permalink raw reply
* Re: intel 82599 multi-port performance
From: J.Hwan Kim @ 2011-09-27 0:39 UTC (permalink / raw)
To: Rick Jones; +Cc: netdev
In-Reply-To: <4E80C161.1040204@hp.com>
On 2011년 09월 27일 03:16, Rick Jones wrote:
> On 09/26/2011 08:42 AM, J.Hwan.Kim wrote:
>> On 2011년 09월 26일 23:20, Chris Friesen wrote:
>>> On 09/26/2011 04:26 AM, J.Hwan Kim wrote:
>>>> Hi, everyone
>>>>
>>>> Now, I'm testing a network card including intel 82599.
>>>> In our experiment, with the driver modified with ixgbe and multi-port
>>>> enabled,
>>>
>>> What do you mean by "modified with ixgbe and multi-port enabled"? You
>>> shouldn't need to do anything special to use both ports.
>>>
>>>> rx performance of each port with 10Gbps of 64bytes frame is
>>>> a half than when only 1 port is used.
>>>
>>> Sounds like a cpu limitation. What is your cpu usage? How are your
>>> interrupts routed? Are you using multiple rx queues?
>>>
>>
>> Our server is XEON 2.4GHz with 8 cores.
>> I'm using 4 RSS queues for each port and distributed it's interrupts to
>> different cores respectively.
>> I checked the CPU utilization with TOP, I guess ,it is not cpu imitation
>> problem.
>
> 99 times out of 10, by default top will show the average CPU
> utilization across all the "CPUs" of the system. So I will ask the
> pedantic question - Did you check per-CPU utilization or just overall?
>
I checked CPU utilization per CPU with top. I pressed "1" after
executing top so that I can view the per-cpu utilization.
^ permalink raw reply
* Re: Bridge stays down until a port is added
From: Stephen Hemminger @ 2011-09-27 0:43 UTC (permalink / raw)
To: Sven-Haegar Koch; +Cc: Nicolas de Pesloüan, Marc Haber, netdev
In-Reply-To: <alpine.DEB.2.02.1109270224220.30860@aurora>
On Tue, 27 Sep 2011 02:28:42 +0200 (CEST)
Sven-Haegar Koch <haegar@sdinet.de> wrote:
> On Tue, 27 Sep 2011, Nicolas de Pesloüan wrote:
>
> > Le 26/09/2011 22:05, Stephen Hemminger a écrit :
> > > On Mon, 26 Sep 2011 22:02:21 +0200
> > > Nicolas de Pesloüan<nicolas.2p.debian@gmail.com> wrote:
> > [...]
> > > > Stephen,
> > > >
> > > > What do you think about a generic per-interface option that would cause
> > > > bind() to accept tentative
> > > > address hold by a particular interface? This of course violate IPv6
> > > > principle, but we are talking
> > > > about interfaces that are unable to do DAD, either permanently or until
> > > > something happens on the
> > > > underlying device.
> > > >
> > > > echo 1> /sys/class/net/br0/allow_bind_on_tentative_address
> > > > echo 1> /sys/class/net/dummy0/allow_bind_on_tentative_address
> > > > echo 1> /sys/class/net/wlan0/allow_bind_on_tentative_address
> > > > and so on...
> > > >
> > > > And we may possibly automatically reset this option to 0 if DAD eventually
> > > > causes the address to be
> > > > considered duplicate.
> > >
> > > The issue is that if DAD rejects a duplicate, the socket is dead and
> > > application is
> > > out of luck.
> >
> > Yes, and this is by design. Setting the option would state "I want to allow
> > early bind(), prior to DAD and I assume the fact that a possible duplicate
> > address will cause the corresponding socket to be dead and so the using
> > application."
>
> How about a setting just completely disabling this
> duplicate-address-detection crap?
>
Already multiple ways to disable it:
1. Set /proc/sys/net/ipv6/XXX/accept_dad to 0
2. Set flag when assigning address with 'ip add add ... nodad'
3. Set IFF_NOARP on the interface (done by drivers)
^ permalink raw reply
* Re: intel 82599 multi-port performance
From: J.Hwan Kim @ 2011-09-27 0:45 UTC (permalink / raw)
To: Alexander Duyck; +Cc: netdev
In-Reply-To: <4E80A2AB.2040206@intel.com>
On 2011년 09월 27일 01:04, Alexander Duyck wrote:
> On 09/26/2011 08:42 AM, J.Hwan.Kim wrote:
>> On 2011년 09월 26일 23:20, Chris Friesen wrote:
>>> On 09/26/2011 04:26 AM, J.Hwan Kim wrote:
>>>> Hi, everyone
>>>>
>>>> Now, I'm testing a network card including intel 82599.
>>>> In our experiment, with the driver modified with ixgbe and multi-port
>>>> enabled,
>>>
>>> What do you mean by "modified with ixgbe and multi-port enabled"? You
>>> shouldn't need to do anything special to use both ports.
>>>
>>>> rx performance of each port with 10Gbps of 64bytes frame is
>>>> a half than when only 1 port is used.
>>>
>>> Sounds like a cpu limitation. What is your cpu usage? How are your
>>> interrupts routed? Are you using multiple rx queues?
>>>
>>
>> Our server is XEON 2.4GHz with 8 cores.
>> I'm using 4 RSS queues for each port and distributed it's interrupts
>> to different cores respectively.
>> I checked the CPU utilization with TOP, I guess ,it is not cpu
>> imitation problem.
>
> What kind of rates are you seeing on a single port versus multiple
> ports? There are multiple possibilities in terms of what could be
> limiting your performance.
>
I tested the 10G - 64byte frames.
With ixgbe-modified driver, in single port, 92% of packet received in
driver level and in 2 port we received around 42% packets.
> It sounds like you are using a single card, would that be correct?
Yes, I tested a single card with 2 ports.
> If you are running close to line rate on both ports this could be
> causing you to saturate the PCIe x8 link. If you have a second card
> available you may want to try installing that in a secondary Gen2 PCIe
> slot and seeing if you can improve the performance by using 2 PCIe
> slots instead of one.
I tested it also, if it is tested with 2 card, it seems that the
performance of each port is almost same with a single port. (maximum
performance)
^ permalink raw reply
* Re: [PATCH] staging/octeon: Software should check the checksum of no tcp/udp packets
From: Greg KH @ 2011-09-27 0:51 UTC (permalink / raw)
To: David Daney; +Cc: rongqing.li, netdev, ralf, David Miller, linux-mips
In-Reply-To: <4E80D794.3040701@cavium.com>
On Mon, Sep 26, 2011 at 12:50:44PM -0700, David Daney wrote:
> On 09/25/2011 06:08 PM, rongqing.li@windriver.com wrote:
> >From: Roy.Li<rongqing.li@windriver.com>
> >
> >Icmp packets with wrong checksum are never dropped since
> >skb->ip_summed is set to CHECKSUM_UNNECESSARY.
> >
> >When icmp packets with wrong checksum pass through the octeon
> >net driver, the not_IP, IP_exc, L4_error hardware indicators
> >show no error. so the driver sets CHECKSUM_UNNECESSARY on
> >skb->ip_summed.
> >
> >L4_error only works for TCP/UDP, not for ICMP.
> >
> >Signed-off-by: Roy.Li<rongqing.li@windriver.com>
>
> We found the same problem, but have not yet sent the patch to fix it.
>
> This looks fine to me,
>
> Acked-by: David Daney <david.daney@cavium.com>
>
> I would let davem, Ralf and Greg KH fight over who gets to merge it.
I'll let Ralf take it, unless he wants me to.
Ralf?
^ permalink raw reply
* Re: [net-next 02/10] ixgbevf: Fix broken trunk vlan
From: Jesse Gross @ 2011-09-27 0:54 UTC (permalink / raw)
To: Rose, Gregory V
Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
gospo@redhat.com, Jiri Pirko
In-Reply-To: <43F901BD926A4E43B106BF17856F0755019C5E5B68@orsmsx508.amr.corp.intel.com>
On Mon, Sep 26, 2011 at 4:24 PM, Rose, Gregory V
<gregory.v.rose@intel.com> wrote:
>> -----Original Message-----
>> From: Jesse Gross [mailto:jesse@nicira.com]
>> Sent: Monday, September 26, 2011 4:09 PM
>> To: Rose, Gregory V
>> Cc: Kirsher, Jeffrey T; davem@davemloft.net; netdev@vger.kernel.org;
>> gospo@redhat.com; Jiri Pirko
>> Subject: Re: [net-next 02/10] ixgbevf: Fix broken trunk vlan
>>
>> On Mon, Sep 26, 2011 at 8:57 AM, Rose, Gregory V
>> <gregory.v.rose@intel.com> wrote:
>> >> -----Original Message-----
>> >> From: Jesse Gross [mailto:jesse@nicira.com]
>> >> Sent: Saturday, September 24, 2011 9:33 AM
>> >> To: Kirsher, Jeffrey T
>> >> Cc: davem@davemloft.net; Rose, Gregory V; netdev@vger.kernel.org;
>> >> gospo@redhat.com; Jiri Pirko
>> >> Subject: Re: [net-next 02/10] ixgbevf: Fix broken trunk vlan
>> >>
>> >> What happens if you run tcpdump without configuring vlan devices?
>> >> Shouldn't you see tagged packets for the vlans that are being trunked
>> >> to you? I think this will strip tags in that case. The apparent
>> >> behavior of vlan filters here is also surprising to me because on one
>> >> hand if they're truly filtering this test shouldn't be needed and on
>> >> the other hand they don't seem to be disabled in promiscuous mode.
>> >
>> > I think you're not quite understanding the action the HW is taking here.
>> In the physical function driver we have put the VF on a VLAN without it
>> knowing that it's on a VLAN. Once that is done by the PF, the VF is not
>> allowed to configure its own VLANs anymore. However, the descriptor still
>> includes a bit for the VLAN tag indicating it was a packet that arrived on
>> a VLAN. The HW is inserting and stripping the VLAN tag though without any
>> awareness of that by the VF driver.
>> >
>> > It's a security measure to allow an administrator to put a VF on a VLAN
>> to provide another level of isolation for the VF.
>> >
>> > Intel VFs don't support promiscuous mode. If you ran tcpdump you
>> wouldn't see the VLAN tags because they've been stripped by the HW. The
>> VF has no choice in this.
>>
>> I understand that VFs are limited in what they can do. What I'm
>> concerned about are differences in how the driver behaves when a vlan
>> device is configured on it vs. not. There are two cases that I think
>> you will see different behavior:
>>
>> * The VF driver has it's own vlan filters. I'm assuming (correct me
>> if I'm wrong) that this will further restrict the vlans that the guest
>> sees from the set that have been configured on the PF driver. If that
>> is correct, then putting the VF interface in promiscuous mode should
>> disable the filters and allow packets from all vlans that the PF
>> allows. This situation is the same as if an upstream physical switch
>> is trunking only a subset of vlans to an end host and the NIC is put
>> in promiscuous mode. However, it doesn't look like the VF driver does
>> anything in response to being in promiscuous mode. This shouldn't
>> require any hardware support because it is just clearing the filter
>> table.
>
> The VF does not have its own VLAN filters. It must request them through the PF via the mailbox messaging feature in the NIC. When the PF puts the VF in trunk VLAN mode via the 'ip link set <dev> vf <n> vlan <vlanid>' command then the PF also flags that VF and will not take VLAN filter requests from it. In effect, the VF is forced to a VLAN and has no choice. So it cannot further restrict what VLANs the guest sees. And the VF can only be on a single trunk VLAN, it does not support multiple trunk VLANs.
>
> And again, the VF doesn’t support promiscuous mode. The VF device HW doesn't have that feature.
>
>> * It looks like the VF driver is receiving not just an indication that
>> a tag was present but the actual tag and although it was already
>> stripped by the NIC it can be passed up to the network stack just as
>> if it was stripped in a non-SR-IOV NIC. After this patch, if you run
>> tcpdump on the VF interface you will see packets with tags if a vlan
>> device is configured but packets without tags if there is no vlan
>> device.
>>
>
> No. In trunk VLAN mode the receive descriptor provided by the HW has a VLAN tag present bit and that is set but unless someone has used vconfig to configure a VLAN and a bit is set for that VLAN in the active_vlans bit table then the packet is sent up the stack without the tag. Upper layers don't have access to the driver's internal descriptor rings and have no idea that the VLAN tag present bit was set in the descriptor. Upper layer SW in the network stack won't be looking for VLAN tags since it never requested any VLANs. And since the VLAN tag was stripped, and the upper layers haven't ever set a VLAN filter, then there is no expectation of a VLAN tag.
>
>> Does the VF know whether it is assigned to a particular vlan or is
>> allowed to send/receive tagged packets (essentially the difference
>> between an access port or a trunk port)?
>
> Yes, it knows if it has requested a VLAN filter via the active_vlans bit table. If no VLANs are set in the active_vlans table then it knows to ignore the VLAN tag present bit set in the descriptor since it must be in trunk VLAN mode.
>
>> Or is that why the presence
>> of vlan device is used as an indication?
>
> Correct, that is the case.
OK, maybe due to hardware limitations what I'm looking for just really
isn't possible. However, what I'm trying to emphasize is that vconfig
is not the only way that vlans can be consumed by the network stack
and active_vlans is just an indication of whether a vlan filter was
set, nothing more (perhaps I should have picked a better name when I
originally designed this stuff). In particular, it is not intended to
determine whether a tag should be stripped off or not because
non-vconfig users don't necessarily know which vlans they care about
(think tcpdump or trunking over a bridge). A major goal of the
existing vlan infrastructure is to avoid having drivers make
assumptions about the consumer of the tag and instead just hand all
information over to the network stack so it can behave in a consistent
manner. That's why I was looking for alternate ways to get this
information without depending on active_vlans as this driver behaves
quite a bit differently from others, include the ixgbe PF driver.
^ permalink raw reply
* Re: [PATCH v3 4/7] per-cgroup tcp buffers control
From: Glauber Costa @ 2011-09-27 1:53 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki
Cc: linux-kernel, paul, lizf, ebiederm, davem, gthelen, netdev,
linux-mm, kirill
In-Reply-To: <20110926195906.f1f5831c.kamezawa.hiroyu@jp.fujitsu.com>
On 09/26/2011 07:59 AM, KAMEZAWA Hiroyuki wrote:
> On Sun, 18 Sep 2011 21:56:42 -0300
> Glauber Costa<glommer@parallels.com> wrote:
>
>> With all the infrastructure in place, this patch implements
>> per-cgroup control for tcp memory pressure handling.
>>
>> Signed-off-by: Glauber Costa<glommer@parallels.com>
>> CC: David S. Miller<davem@davemloft.net>
>> CC: Hiroyouki Kamezawa<kamezawa.hiroyu@jp.fujitsu.com>
>> CC: Eric W. Biederman<ebiederm@xmission.com>
>
> a comment below.
>
>> +int tcp_init_cgroup(struct proto *prot, struct cgroup *cgrp,
>> + struct cgroup_subsys *ss)
>> +{
>> + struct mem_cgroup *cg = mem_cgroup_from_cont(cgrp);
>> + unsigned long limit;
>> +
>> + cg->tcp_memory_pressure = 0;
>> + atomic_long_set(&cg->tcp_memory_allocated, 0);
>> + percpu_counter_init(&cg->tcp_sockets_allocated, 0);
>> +
>> + limit = nr_free_buffer_pages() / 8;
>> + limit = max(limit, 128UL);
>> +
>> + cg->tcp_prot_mem[0] = sysctl_tcp_mem[0];
>> + cg->tcp_prot_mem[1] = sysctl_tcp_mem[1];
>> + cg->tcp_prot_mem[2] = sysctl_tcp_mem[2];
>> +
>
> Then, the parameter doesn't inherit parent's one ?
>
> I think sockets_populate should pass 'parent' and
>
>
> I think you should have a function
>
> mem_cgroup_should_inherit_parent_settings(parent)
>
> (This is because you made this feature as a part of memcg.
> please provide expected behavior.)
>
> Thanks,
> -Kame
Kame: Another look into this:
sysctl_tcp_mem is a global value, unless you have different namespaces.
So it is either global anyway, or should come from the namespace, not
the parent.
Now, the goal here is to set the maximum possible value for those
fields. That, indeed, should come from the parent.
That's my understanding...
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] staging/octeon: Software should check the checksum of no tcp/udp packets
From: Ralf Baechle @ 2011-09-27 2:01 UTC (permalink / raw)
To: Greg KH; +Cc: David Daney, rongqing.li, netdev, David Miller, linux-mips
In-Reply-To: <20110927005127.GB10447@kroah.com>
On Mon, Sep 26, 2011 at 05:51:27PM -0700, Greg KH wrote:
> > This looks fine to me,
> >
> > Acked-by: David Daney <david.daney@cavium.com>
> >
> > I would let davem, Ralf and Greg KH fight over who gets to merge it.
>
> I'll let Ralf take it, unless he wants me to.
>
> Ralf?
Ok. Yes, that seems the best solution.
Ralf
^ permalink raw reply
* linux-next: manual merge of the net tree with the wireless-current tree
From: Stephen Rothwell @ 2011-09-27 2:51 UTC (permalink / raw)
To: David Miller, netdev
Cc: linux-next, linux-kernel, Johannes Berg, John W. Linville,
Wey-Yi Guy, Emmanuel Grumbach
Hi all,
Today's linux-next merge of the net tree got a conflict in
drivers/net/wireless/iwlwifi/iwl-scan.c between commit 6c80c39d9a69
("iwlagn: fix dangling scan request") from the wireless-current tree and
commits 63013ae30159 ("iwlagn: priv->status moves to iwl_shared") and
6ac2f839b0b2 ("iwlagn: priv->mutex moves to iwl_shared") from the net
tree.
I fixed it up (see below) and can carry the fix as necessary.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --cc drivers/net/wireless/iwlwifi/iwl-scan.c
index 77e528f,fc5af34..0000000
--- a/drivers/net/wireless/iwlwifi/iwl-scan.c
+++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
@@@ -403,8 -413,19 +413,8 @@@ int iwl_mac_hw_scan(struct ieee80211_h
if (req->n_channels == 0)
return -EINVAL;
- mutex_lock(&priv->mutex);
+ mutex_lock(&priv->shrd->mutex);
- if (test_bit(STATUS_SCANNING, &priv->shrd->status) &&
- priv->scan_type != IWL_SCAN_NORMAL) {
- IWL_DEBUG_SCAN(priv, "Scan already in progress.\n");
- ret = -EAGAIN;
- goto out_unlock;
- }
-
- /* mac80211 will only ask for one band at a time */
- priv->scan_request = req;
- priv->scan_vif = vif;
-
/*
* If an internal scan is in progress, just set
* up the scan_request as per above.
@@@ -432,7 -439,8 +442,7 @@@
IWL_DEBUG_MAC80211(priv, "leave\n");
- mutex_unlock(&priv->mutex);
-out_unlock:
+ mutex_unlock(&priv->shrd->mutex);
return ret;
}
^ permalink raw reply
* Re: [net-next 4/7] bna: Brocade 1860 HW Enablement
From: David Miller @ 2011-09-27 4:41 UTC (permalink / raw)
To: rmody; +Cc: netdev, adapter_linux_open_src_team, gkaraje
In-Reply-To: <1317071541-9662-5-git-send-email-rmody@brocade.com>
From: Rasesh Mody <rmody@brocade.com>
Date: Mon, 26 Sep 2011 14:12:18 -0700
> include/linux/pci_ids.h | 1 +
Please do not add new PCI device ID entries to linux/pci_ids.h
for use by a single device driver.
That header is only used when the PCI device ID will be used in
multiple drivers (such as network + iSCSI as some of the broadcom
drivers are).
^ permalink raw reply
* Re: [PATCH] ehea: Remove sleep at .ndo_get_stats
From: David Miller @ 2011-09-27 4:47 UTC (permalink / raw)
To: eric.dumazet; +Cc: brenohl, netdev
In-Reply-To: <1317068935.2796.12.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 26 Sep 2011 22:28:55 +0200
> Le lundi 26 septembre 2011 à 17:11 -0300, brenohl@br.ibm.com a écrit :
>> Currently ehea ndo_get_stats can sleep in two places, in a hcall
>> and in a GFP_KERNEL alloc, which is not correct.
>> This patch creates a delayed workqueue that grabs the information each 1
>> sec from the hardware, and place it into the device structure, so that,
>> .ndo_get_stats quickly returns the device structure statistics block.
>>
>> Signed-off-by: Breno Leitao <brenohl@br.ibm.com>
>> ---
>
> Seems good, thanks
>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH] pch_gbe: remove unused variable
From: David Miller @ 2011-09-27 4:50 UTC (permalink / raw)
To: jdmason; +Cc: toshiharu-linux, netdev
In-Reply-To: <1317065858-3201-1-git-send-email-jdmason@kudzu.us>
From: Jon Mason <jdmason@kudzu.us>
Date: Mon, 26 Sep 2011 14:37:38 -0500
> netdev is unused in pch_gbe_setup_rctl. Remove this declaration to
> avoid a compiler warning.
>
> Signed-off-by: Jon Mason <jdmason@kudzu.us>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] sky2: manage irq better on single port card
From: David Miller @ 2011-09-27 4:51 UTC (permalink / raw)
To: shemminger; +Cc: netdev
In-Reply-To: <20110923141340.39a8de91@nehalam.linuxnetplumber.net>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Fri, 23 Sep 2011 14:13:40 -0700
> Most sky2 hardware only has a single port, although some variations of the
> chip support two interfaces. For the single port case, use the standard
> Ethernet driver convention of allocating IRQ when device is brought up
> rather than at probe time.
>
> Also, change the error handling of dual port cards so that if second
> port can not be brought up, then just fail. No point in continuing, since
> the failure is most certainly because of out of memory.
>
> The dual port sky2 device has a single irq and a single status ring,
> therefore it has a single NAPI object shared by both ports.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] tcp: ECN blackhole should not force quickack mode
From: David Miller @ 2011-09-27 4:59 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, hkchu, ilpo.jarvinen, jhs, jg, dave.taht
In-Reply-To: <1316757739.2560.12.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 23 Sep 2011 08:02:19 +0200
> While playing with a new ADSL box at home, I discovered that ECN
> blackhole can trigger suboptimal quickack mode on linux : We send one
> ACK for each incoming data frame, without any delay and eventual
> piggyback.
>
> This is because TCP_ECN_check_ce() considers that if no ECT is seen on a
> segment, this is because this segment was a retransmit.
>
> Refine this heuristic and apply it only if we seen ECT in a previous
> segment, to detect ECN blackhole at IP level.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied to net-next, thanks Eric.
^ permalink raw reply
* Re: [net-next-2.6 PATCH 0/3] enic: updates to version 2.1.1.28
From: David Miller @ 2011-09-27 5:13 UTC (permalink / raw)
To: roprabhu; +Cc: netdev
In-Reply-To: <20110922134410.2145.72210.stgit@savbu-pc100.cisco.com>
From: Roopa Prabhu <roprabhu@cisco.com>
Date: Thu, 22 Sep 2011 06:44:27 -0700
> This patch series implements the following enic driver updates:
>
> 01/3 - Add SRIOV support
> 02/3 - Helper code for SRIOV proxy commands
> 03/3 - Add support for port profile association on a enic SRIOV VF
>
> Signed-off-by: Roopa Prabhu <roprabhu@cisco.com>
> Signed-off-by: Sujith Sankar <ssujith@cisco.com>
> Signed-off-by: Christian Benvenuti <benve@cisco.com>
> Signed-off-by: David Wang <dwang2@cisco.com>
All applied, thank you.
^ permalink raw reply
* Re: Question about memory leak detector giving false positive report for net/core/flow.c
From: Huajun Li @ 2011-09-27 5:27 UTC (permalink / raw)
To: Catalin Marinas
Cc: Eric Dumazet, linux-mm@kvack.org, netdev, linux-kernel, Tejun Heo,
Christoph Lameter, Huajun Li
In-Reply-To: <20110926165024.GA21617@e102109-lin.cambridge.arm.com>
2011/9/27 Catalin Marinas <Catalin.Marinas@arm.com>:
> On Mon, Sep 26, 2011 at 05:32:54PM +0100, Eric Dumazet wrote:
>> Le lundi 26 septembre 2011 à 23:17 +0800, Huajun Li a écrit :
>> > Memory leak detector gives following memory leak report, it seems the
>> > report is triggered by net/core/flow.c, but actually, it should be a
>> > false positive report.
>> > So, is there any idea from kmemleak side to fix/disable this false
>> > positive report like this?
>> > Yes, kmemleak_not_leak(...) could disable it, but is it suitable for this case ?
> ...
>> CC lkml and percpu maintainers (Tejun Heo & Christoph Lameter ) as well
>>
>> AFAIK this false positive only occurs if percpu data is allocated
>> outside of embedded pcu space.
>>
>> (grep pcpu_get_vm_areas /proc/vmallocinfo)
>>
>> I suspect this is a percpu/kmemleak cooperation problem (a missing
>> kmemleak_alloc() ?)
>>
>> I am pretty sure kmemleak_not_leak() is not the right answer to this
>> problem.
>
> kmemleak_not_leak() definitely not the write answer. The alloc_percpu()
> call does not have any kmemleak_alloc() callback, so it doesn't scan
> them.
>
> Huajun, could you please try the patch below:
>
> 8<--------------------------------
> kmemleak: Handle percpu memory allocation
>
> From: Catalin Marinas <catalin.marinas@arm.com>
>
> This patch adds kmemleak callbacks from the percpu allocator, reducing a
> number of false positives caused by kmemleak not scanning such memory
> blocks.
>
> Reported-by: Huajun Li <huajun.li.lee@gmail.com>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> mm/percpu.c | 11 +++++++++--
> 1 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/mm/percpu.c b/mm/percpu.c
> index bf80e55..c47a90b 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -67,6 +67,7 @@
> #include <linux/spinlock.h>
> #include <linux/vmalloc.h>
> #include <linux/workqueue.h>
> +#include <linux/kmemleak.h>
>
> #include <asm/cacheflush.h>
> #include <asm/sections.h>
> @@ -833,7 +834,9 @@ fail_unlock_mutex:
> */
> void __percpu *__alloc_percpu(size_t size, size_t align)
> {
> - return pcpu_alloc(size, align, false);
> + void __percpu *ptr = pcpu_alloc(size, align, false);
> + kmemleak_alloc(ptr, size, 1, GFP_KERNEL);
> + return ptr;
> }
> EXPORT_SYMBOL_GPL(__alloc_percpu);
>
> @@ -855,7 +858,9 @@ EXPORT_SYMBOL_GPL(__alloc_percpu);
> */
> void __percpu *__alloc_reserved_percpu(size_t size, size_t align)
> {
> - return pcpu_alloc(size, align, true);
> + void __percpu *ptr = pcpu_alloc(size, align, true);
> + kmemleak_alloc(ptr, size, 1, GFP_KERNEL);
> + return ptr;
> }
>
> /**
> @@ -915,6 +920,8 @@ void free_percpu(void __percpu *ptr)
> if (!ptr)
> return;
>
> + kmemleak_free(ptr);
> +
> addr = __pcpu_ptr_to_addr(ptr);
>
> spin_lock_irqsave(&pcpu_lock, flags);
>
> --
> Catalin
>
Applied the patch and found following msg after system boot up :
--------------------------------------------------------------------------------------------------------
...
[ 48.040027] eth0: no IPv6 routers present
[ 69.240960] BUG: unable to handle kernel paging request at 00000000001d7270
[ 69.241010] IP: [<ffffffff81256c62>] scan_block+0x62/0x220
[ 69.241045] PGD 5f289067 PUD 5f292067 PMD 0
[ 69.241074] Oops: 0000 [#1] SMP
[ 69.241098] CPU 0
[ 69.241110] Modules linked in: binfmt_misc dm_crypt
snd_hda_codec_analog snd_hda_intel snd_hda_codec usbhid hid snd_hwdep
snd_pcm snd_seq_midi snd_rawmidi i915 snd_seq_midi_event hp_wmi ppdev
sparse_keymap snd_seq snd_timer snd_seq_device ehci_hcd uhci_hcd
drm_kms_helper usbcore tpm_infineon psmouse snd e1000e drm serio_raw
soundcore tpm_tis snd_page_alloc tpm floppy tpm_bios parport_pc
i2c_algo_bit video lp parport
[ 69.249303]
[ 69.250246] Pid: 47, comm: kmemleak Tainted: G I
3.1.0-rc7+ #24 Hewlett-Packard HP Compaq dc7800p Convertible
Minitower/0AACh
[ 69.250246] RIP: 0010:[<ffffffff81256c62>] [<ffffffff81256c62>]
scan_block+0x62/0x220
[ 69.250246] RSP: 0018:ffff880073f69d90 EFLAGS: 00010012
[ 69.250246] RAX: 0000000000000000 RBX: ffff880078750858 RCX: 0000000000000000
[ 69.250246] RDX: ffff880078750858 RSI: 0000000000000001 RDI: 00000000001d7270
[ 69.250246] RBP: ffff880073f69dd0 R08: 0000000000000000 R09: 0000000000000000
[ 69.250246] R10: 0000000000000001 R11: 0000000000000000 R12: ffff8800787508b8
[ 69.250246] R13: 00000000001d7270 R14: 00000000001d7431 R15: 0000000000000000
[ 69.250246] FS: 0000000000000000(0000) GS:ffff88007a200000(0000)
knlGS:0000000000000000
[ 69.250246] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 69.250246] CR2: 00000000001d7270 CR3: 000000006d735000 CR4: 00000000000006f0
[ 69.250246] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 69.250246] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 69.250246] Process kmemleak (pid: 47, threadinfo ffff880073f68000,
task ffff880073e4a460)
[ 69.250246] Stack:
[ 69.250246] ffff880073f69dd0 ffffffff8194b49a ffffffff81256ed1
ffff880078750858
[ 69.250246] ffff8800787508b8 00000000001d8270 00000000001d7438
0000000000000206
[ 69.250246] ffff880073f69e10 ffffffff8125702a 000000000007d2af
ffffffff81e0d120
[ 69.250246] Call Trace:
[ 69.250246] [<ffffffff8194b49a>] ? _raw_spin_lock_irqsave+0xfa/0x110
[ 69.250246] [<ffffffff81256ed1>] ? scan_gray_list+0xb1/0x280
[ 69.250246] [<ffffffff8125702a>] scan_gray_list+0x20a/0x280
[ 69.250246] [<ffffffff8125787a>] kmemleak_scan+0x44a/0xbd0
[ 69.250246] [<ffffffff81257430>] ? kmemleak_do_cleanup+0x210/0x210
[ 69.250246] [<ffffffff81258630>] ? kmemleak_write+0x630/0x630
[ 69.250246] [<ffffffff812586bf>] kmemleak_scan_thread+0x8f/0x150
[ 69.250246] [<ffffffff810dc4e6>] kthread+0xe6/0x100
[ 69.250246] [<ffffffff8195a4c4>] kernel_thread_helper+0x4/0x10
[ 69.250246] [<ffffffff8194ac50>] ? _raw_spin_unlock_irq+0x50/0x80
[ 69.250246] [<ffffffff8194b874>] ? retint_restore_args+0x13/0x13
[ 69.250246] [<ffffffff810dc400>] ? __init_kthread_worker+0x80/0x80
[ 69.250246] [<ffffffff8195a4c0>] ? gs_change+0x13/0x13
[ 69.250246] Code: 00 00 e9 d2 01 00 00 66 90 e8 ab f4 ff ff 48 83
05 33 ef 06 02 01 85 c0 0f 85 bb 01 00 00 48 83 05 2b ef 06 02 01 be
01 00 00 00
[ 69.250246] 8b 7d 00 e8 c5 fc ff ff 48 85 c0 49 89 c4 0f 84 69 01 00 00
[ 69.250246] RIP [<ffffffff81256c62>] scan_block+0x62/0x220
[ 69.250246] RSP <ffff880073f69d90>
[ 69.250246] CR2: 00000000001d7270
[ 69.360917] ---[ end trace a7919e7f17c0a727 ]---
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: Question about memory leak detector giving false positive report for net/core/flow.c
From: Huajun Li @ 2011-09-27 5:29 UTC (permalink / raw)
To: Eric Dumazet
Cc: Catalin Marinas, linux-mm@kvack.org, netdev, linux-kernel,
Tejun Heo, Christoph Lameter, Huajun Li
In-Reply-To: <1317066395.2796.11.camel@edumazet-laptop>
2011/9/27 Eric Dumazet <eric.dumazet@gmail.com>:
> Le lundi 26 septembre 2011 à 17:50 +0100, Catalin Marinas a écrit :
>> On Mon, Sep 26, 2011 at 05:32:54PM +0100, Eric Dumazet wrote:
>> > Le lundi 26 septembre 2011 à 23:17 +0800, Huajun Li a écrit :
>> > > Memory leak detector gives following memory leak report, it seems the
>> > > report is triggered by net/core/flow.c, but actually, it should be a
>> > > false positive report.
>> > > So, is there any idea from kmemleak side to fix/disable this false
>> > > positive report like this?
>> > > Yes, kmemleak_not_leak(...) could disable it, but is it suitable for this case ?
>> ...
>> > CC lkml and percpu maintainers (Tejun Heo & Christoph Lameter ) as well
>> >
>> > AFAIK this false positive only occurs if percpu data is allocated
>> > outside of embedded pcu space.
>> >
>> > (grep pcpu_get_vm_areas /proc/vmallocinfo)
>> >
>> > I suspect this is a percpu/kmemleak cooperation problem (a missing
>> > kmemleak_alloc() ?)
>> >
>> > I am pretty sure kmemleak_not_leak() is not the right answer to this
>> > problem.
>>
>> kmemleak_not_leak() definitely not the write answer. The alloc_percpu()
>> call does not have any kmemleak_alloc() callback, so it doesn't scan
>> them.
>>
>> Huajun, could you please try the patch below:
>>
>> 8<--------------------------------
>> kmemleak: Handle percpu memory allocation
>>
>> From: Catalin Marinas <catalin.marinas@arm.com>
>>
>> This patch adds kmemleak callbacks from the percpu allocator, reducing a
>> number of false positives caused by kmemleak not scanning such memory
>> blocks.
>>
>> Reported-by: Huajun Li <huajun.li.lee@gmail.com>
>> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
>> ---
>> mm/percpu.c | 11 +++++++++--
>> 1 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/percpu.c b/mm/percpu.c
>> index bf80e55..c47a90b 100644
>> --- a/mm/percpu.c
>> +++ b/mm/percpu.c
>> @@ -67,6 +67,7 @@
>> #include <linux/spinlock.h>
>> #include <linux/vmalloc.h>
>> #include <linux/workqueue.h>
>> +#include <linux/kmemleak.h>
>>
>> #include <asm/cacheflush.h>
>> #include <asm/sections.h>
>> @@ -833,7 +834,9 @@ fail_unlock_mutex:
>> */
>> void __percpu *__alloc_percpu(size_t size, size_t align)
>> {
>> - return pcpu_alloc(size, align, false);
>> + void __percpu *ptr = pcpu_alloc(size, align, false);
>> + kmemleak_alloc(ptr, size, 1, GFP_KERNEL);
>> + return ptr;
>> }
>> EXPORT_SYMBOL_GPL(__alloc_percpu);
>>
>> @@ -855,7 +858,9 @@ EXPORT_SYMBOL_GPL(__alloc_percpu);
>> */
>> void __percpu *__alloc_reserved_percpu(size_t size, size_t align)
>> {
>> - return pcpu_alloc(size, align, true);
>> + void __percpu *ptr = pcpu_alloc(size, align, true);
>> + kmemleak_alloc(ptr, size, 1, GFP_KERNEL);
>> + return ptr;
>> }
>>
>> /**
>> @@ -915,6 +920,8 @@ void free_percpu(void __percpu *ptr)
>> if (!ptr)
>> return;
>>
>> + kmemleak_free(ptr);
>> +
>> addr = __pcpu_ptr_to_addr(ptr);
>>
>> spin_lock_irqsave(&pcpu_lock, flags);
>>
>
> Hmm, you need to call kmemleak_alloc() for each chunk allocated per
> possible cpu.
>
> Here is the (untested) patch for the allocation phase, need the same at
> freeing time
>
> diff --git a/mm/percpu-km.c b/mm/percpu-km.c
> index 89633fe..5061ac5 100644
> --- a/mm/percpu-km.c
> +++ b/mm/percpu-km.c
> @@ -37,9 +37,12 @@ static int pcpu_populate_chunk(struct pcpu_chunk *chunk, int off, int size)
> {
> unsigned int cpu;
>
> - for_each_possible_cpu(cpu)
> - memset((void *)pcpu_chunk_addr(chunk, cpu, 0) + off, 0, size);
> + for_each_possible_cpu(cpu) {
> + void *chunk_addr = (void *)pcpu_chunk_addr(chunk, cpu, 0) + off;
>
> + kmemleak_alloc(chunk_addr, size, 1, GFP_KERNEL);
> + memset(chunk_addr, 0, size);
> + }
> return 0;
> }
>
> diff --git a/mm/percpu-vm.c b/mm/percpu-vm.c
> index ea53496..0d397cc 100644
> --- a/mm/percpu-vm.c
> +++ b/mm/percpu-vm.c
> @@ -342,8 +342,12 @@ static int pcpu_populate_chunk(struct pcpu_chunk *chunk, int off, int size)
> /* commit new bitmap */
> bitmap_copy(chunk->populated, populated, pcpu_unit_pages);
> clear:
> - for_each_possible_cpu(cpu)
> - memset((void *)pcpu_chunk_addr(chunk, cpu, 0) + off, 0, size);
> + for_each_possible_cpu(cpu) {
> + void *chunk_addr = (void *)pcpu_chunk_addr(chunk, cpu, 0) + off;
> +
> + kmemleak_alloc(chunk_addr, size, 1, GFP_KERNEL);
> + memset(chunk_addr, 0, size);
> + }
> return 0;
>
> err_unmap:
>
>
About this one, memory leak detector disabled(actually I enable it
while config the kernel) while booting system, and and found following
info in dmesg:
---------------------------------------------------------------------------------------------------------------
[ 0.370000] smpboot cpu 1: start_ip = 9a000
[ 0.010000] numa_add_cpu cpu 1 node 0: mask now 0-1
[ 0.530124] NMI watchdog enabled, takes one hw-pmu counter.
[ 0.530344] Brought up 2 CPUs
[ 0.530348] Total of 2 processors activated (10639.45 BogoMIPS).
[ 0.533083] kmemleak: Cannot insert 0xffff88007a3d7448 into the
object search tree (already existing)
[ 0.533083] Pid: 1, comm: swapper Tainted: G I 3.1.0-rc7+ #25
[ 0.533083] Call Trace:
[ 0.533083] [<ffffffff81256512>] create_object+0x2c2/0x360
[ 0.533083] [<ffffffff81920c5e>] kmemleak_alloc+0x7e/0x110
[ 0.533083] [<ffffffff811fd8cd>] pcpu_alloc+0xd2d/0x10f0
[ 0.533083] [<ffffffff81243129>] ? __kmalloc_track_caller+0x139/0x310
[ 0.533083] [<ffffffff811fdca8>] __alloc_percpu+0x18/0x30
[ 0.533083] [<ffffffff8128dded>] alloc_vfsmnt+0x11d/0x250
[ 0.533083] [<ffffffff8128dfd6>] vfs_kern_mount+0x46/0x130
[ 0.533083] [<ffffffff8194a283>] ? _raw_write_unlock+0x43/0x60
[ 0.533083] [<ffffffff82117cec>] shmem_init+0xb9/0x13b
[ 0.533083] [<ffffffff820ea2c6>] kernel_init+0x10e/0x23a
[ 0.533083] [<ffffffff8195a484>] kernel_thread_helper+0x4/0x10
[ 0.533083] [<ffffffff8194ac20>] ? _raw_spin_unlock_irq+0x50/0x80
[ 0.533083] [<ffffffff8194b834>] ? retint_restore_args+0x13/0x13
[ 0.533083] [<ffffffff820ea1b8>] ? start_kernel+0x6ce/0x6ce
[ 0.533083] [<ffffffff8195a480>] ? gs_change+0x13/0x13
[ 0.533083] kmemleak: Kernel memory leak detector disabled
[ 0.533083] kmemleak: Object 0xffff88007a200000 (size 1949696):
[ 0.533083] kmemleak: comm "swapper", pid 0, jiffies 4294937296
[ 0.533083] kmemleak: min_count = 0
[ 0.533083] kmemleak: count = 0
[ 0.533083] kmemleak: flags = 0x1
[ 0.533083] kmemleak: checksum = 0
[ 0.533083] kmemleak: backtrace:
[ 0.533083] [<ffffffff81256394>] create_object+0x144/0x360
[ 0.533083] [<ffffffff819214c8>] kmemleak_free_part+0x138/0x1d0
[ 0.533083] [<ffffffff8211e4e8>] kmemleak_init+0x30b/0x3fd
[ 0.533083] [<ffffffff820ea01c>] start_kernel+0x532/0x6ce
[ 0.533083] [<ffffffff820e938d>] x86_64_start_reservations+0x178/0x183
[ 0.533083] [<ffffffff820e94fb>] x86_64_start_kernel+0x163/0x179
[ 0.533083] [<ffffffffffffffff>] 0xffffffffffffffff
[ 0.533083] devtmpfs: initialized
[ 0.533083] gcov: version magic: 0x3430352a
[ 0.533083] PM: Registering ACPI NVS region at 7d2afe00 (8352 bytes)
[ 0.533083] print_constraints: dummy:
[ 0.533083] RTC time: 12:40:24, date: 09/27/11
[ 0.533083] NET: Registered protocol family 16
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] RDSRDMA: Fix cleanup of rds_iw_mr_pool
From: David Miller @ 2011-09-27 5:32 UTC (permalink / raw)
To: jonathan; +Cc: venkat.x.venkatsubra, netdev
In-Reply-To: <20110913194101.7497.65515.stgit@build.ogc.int>
From: Jonathan Lallinger <jonathan@ogc.us>
Date: Tue, 13 Sep 2011 14:41:01 -0500
> @@ -548,6 +550,7 @@ static int rds_iw_flush_mr_pool(struct rds_iw_mr_pool *pool, int free_all)
> spin_unlock_irqrestore(&pool->list_lock, flags);
> }
>
> + atomic_sub(unpinned, &poll->free_pinned);
> atomic_sub(ncleaned, &pool->dirty_count);
> atomic_sub(nfreed, &pool->item_count);
>
net/rds/iw_rdma.c: In function ‘rds_iw_flush_mr_pool’:
net/rds/iw_rdma.c:553:24: error: ‘poll’ undeclared (first use in this function)
net/rds/iw_rdma.c:553:24: note: each undeclared identifier is reported only once for each function it appears in
If you didn't even build test it, I know you didn't test it's
functionality either.
This is crazy.
Well if it's not important enough to even build test this change
before you post it, then it obviously doesn't matter if the RDMA
module crashes the kernel when it's unloaded.
^ permalink raw reply
* Re: Question about memory leak detector giving false positive report for net/core/flow.c
From: Eric Dumazet @ 2011-09-27 5:55 UTC (permalink / raw)
To: Huajun Li
Cc: Catalin Marinas, linux-mm@kvack.org, netdev, linux-kernel,
Tejun Heo, Christoph Lameter
In-Reply-To: <CA+v9cxYzWJScCa2mMoEovq3WULSZYQaq6EjoRV7SQUjr0L_RiQ@mail.gmail.com>
Le mardi 27 septembre 2011 à 13:29 +0800, Huajun Li a écrit :
> 2011/9/27 Eric Dumazet <eric.dumazet@gmail.com>:
> > Le lundi 26 septembre 2011 à 17:50 +0100, Catalin Marinas a écrit :
> >> On Mon, Sep 26, 2011 at 05:32:54PM +0100, Eric Dumazet wrote:
> >> > Le lundi 26 septembre 2011 à 23:17 +0800, Huajun Li a écrit :
> >> > > Memory leak detector gives following memory leak report, it seems the
> >> > > report is triggered by net/core/flow.c, but actually, it should be a
> >> > > false positive report.
> >> > > So, is there any idea from kmemleak side to fix/disable this false
> >> > > positive report like this?
> >> > > Yes, kmemleak_not_leak(...) could disable it, but is it suitable for this case ?
> >> ...
> >> > CC lkml and percpu maintainers (Tejun Heo & Christoph Lameter ) as well
> >> >
> >> > AFAIK this false positive only occurs if percpu data is allocated
> >> > outside of embedded pcu space.
> >> >
> >> > (grep pcpu_get_vm_areas /proc/vmallocinfo)
> >> >
> >> > I suspect this is a percpu/kmemleak cooperation problem (a missing
> >> > kmemleak_alloc() ?)
> >> >
> >> > I am pretty sure kmemleak_not_leak() is not the right answer to this
> >> > problem.
> >>
> >> kmemleak_not_leak() definitely not the write answer. The alloc_percpu()
> >> call does not have any kmemleak_alloc() callback, so it doesn't scan
> >> them.
> >>
> >> Huajun, could you please try the patch below:
> >>
> >> 8<--------------------------------
> >> kmemleak: Handle percpu memory allocation
> >>
> >> From: Catalin Marinas <catalin.marinas@arm.com>
> >>
> >> This patch adds kmemleak callbacks from the percpu allocator, reducing a
> >> number of false positives caused by kmemleak not scanning such memory
> >> blocks.
> >>
> >> Reported-by: Huajun Li <huajun.li.lee@gmail.com>
> >> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> >> ---
> >> mm/percpu.c | 11 +++++++++--
> >> 1 files changed, 9 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/mm/percpu.c b/mm/percpu.c
> >> index bf80e55..c47a90b 100644
> >> --- a/mm/percpu.c
> >> +++ b/mm/percpu.c
> >> @@ -67,6 +67,7 @@
> >> #include <linux/spinlock.h>
> >> #include <linux/vmalloc.h>
> >> #include <linux/workqueue.h>
> >> +#include <linux/kmemleak.h>
> >>
> >> #include <asm/cacheflush.h>
> >> #include <asm/sections.h>
> >> @@ -833,7 +834,9 @@ fail_unlock_mutex:
> >> */
> >> void __percpu *__alloc_percpu(size_t size, size_t align)
> >> {
> >> - return pcpu_alloc(size, align, false);
> >> + void __percpu *ptr = pcpu_alloc(size, align, false);
> >> + kmemleak_alloc(ptr, size, 1, GFP_KERNEL);
> >> + return ptr;
> >> }
> >> EXPORT_SYMBOL_GPL(__alloc_percpu);
> >>
> >> @@ -855,7 +858,9 @@ EXPORT_SYMBOL_GPL(__alloc_percpu);
> >> */
> >> void __percpu *__alloc_reserved_percpu(size_t size, size_t align)
> >> {
> >> - return pcpu_alloc(size, align, true);
> >> + void __percpu *ptr = pcpu_alloc(size, align, true);
> >> + kmemleak_alloc(ptr, size, 1, GFP_KERNEL);
> >> + return ptr;
> >> }
> >>
> >> /**
> >> @@ -915,6 +920,8 @@ void free_percpu(void __percpu *ptr)
> >> if (!ptr)
> >> return;
> >>
> >> + kmemleak_free(ptr);
> >> +
> >> addr = __pcpu_ptr_to_addr(ptr);
> >>
> >> spin_lock_irqsave(&pcpu_lock, flags);
> >>
> >
> > Hmm, you need to call kmemleak_alloc() for each chunk allocated per
> > possible cpu.
> >
> > Here is the (untested) patch for the allocation phase, need the same at
> > freeing time
> >
> > diff --git a/mm/percpu-km.c b/mm/percpu-km.c
> > index 89633fe..5061ac5 100644
> > --- a/mm/percpu-km.c
> > +++ b/mm/percpu-km.c
> > @@ -37,9 +37,12 @@ static int pcpu_populate_chunk(struct pcpu_chunk *chunk, int off, int size)
> > {
> > unsigned int cpu;
> >
> > - for_each_possible_cpu(cpu)
> > - memset((void *)pcpu_chunk_addr(chunk, cpu, 0) + off, 0, size);
> > + for_each_possible_cpu(cpu) {
> > + void *chunk_addr = (void *)pcpu_chunk_addr(chunk, cpu, 0) + off;
> >
> > + kmemleak_alloc(chunk_addr, size, 1, GFP_KERNEL);
> > + memset(chunk_addr, 0, size);
> > + }
> > return 0;
> > }
> >
> > diff --git a/mm/percpu-vm.c b/mm/percpu-vm.c
> > index ea53496..0d397cc 100644
> > --- a/mm/percpu-vm.c
> > +++ b/mm/percpu-vm.c
> > @@ -342,8 +342,12 @@ static int pcpu_populate_chunk(struct pcpu_chunk *chunk, int off, int size)
> > /* commit new bitmap */
> > bitmap_copy(chunk->populated, populated, pcpu_unit_pages);
> > clear:
> > - for_each_possible_cpu(cpu)
> > - memset((void *)pcpu_chunk_addr(chunk, cpu, 0) + off, 0, size);
> > + for_each_possible_cpu(cpu) {
> > + void *chunk_addr = (void *)pcpu_chunk_addr(chunk, cpu, 0) + off;
> > +
> > + kmemleak_alloc(chunk_addr, size, 1, GFP_KERNEL);
> > + memset(chunk_addr, 0, size);
> > + }
> > return 0;
> >
> > err_unmap:
> >
> >
>
> About this one, memory leak detector disabled(actually I enable it
> while config the kernel) while booting system, and and found following
> info in dmesg:
Yes, it was not a patch, but the general idea for Catalin ;)
You hit the fact that same zone (embedded percpu space) is now in a
mixed state.
In current kernels, the embedded percpu zone is already known by
kmemleak, but with a large granularity. kmemleak is not aware of
individual allocations/freeing in this large zone.
Once kmemleak and percpu allocator are cooperating, we might find more
kmemleaks. Right now, kmemleak can find pointers in percpu chunks that
are not anymore reachable (they were freed), and therefore doesnt warn
of possible memory leaks.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH net-next] tcp: unalias tcp_skb_cb flags and ip_dsfield
From: Eric Dumazet @ 2011-09-27 6:00 UTC (permalink / raw)
To: David Miller; +Cc: netdev, hkchu, ilpo.jarvinen, jhs
In-Reply-To: <20110927.005905.1166544891922183065.davem@davemloft.net>
struct tcp_skb_cb contains a "flags" field containing either tcp flags
or IP dsfield depending on context (input or output path)
Introduce ip_dsfield to make the difference clear and ease maintenance.
If later we want to save space, we can union flags/ip_dsfield
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
If there is no objection, I plan to rename "flags" to "tcp_flags" in a
following patch.
include/net/tcp.h | 3 ++-
net/ipv4/tcp_input.c | 2 +-
net/ipv4/tcp_ipv4.c | 2 +-
net/ipv6/tcp_ipv6.c | 2 +-
4 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 702aefc..28a9997 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -642,7 +642,8 @@ struct tcp_skb_cb {
#define TCPCB_SACKED_RETRANS 0x02 /* SKB retransmitted */
#define TCPCB_LOST 0x04 /* SKB is lost */
#define TCPCB_TAGBITS 0x07 /* All tag bits */
-
+ __u8 ip_dsfield; /* IPv4 tos or IPv6 dsfield */
+ /* 1 byte hole */
#define TCPCB_EVER_RETRANS 0x80 /* Ever retransmitted frame */
#define TCPCB_RETRANS (TCPCB_SACKED_RETRANS|TCPCB_EVER_RETRANS)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 5a4408c..7008fcc 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -222,7 +222,7 @@ static inline void TCP_ECN_check_ce(struct tcp_sock *tp, const struct sk_buff *s
if (!(tp->ecn_flags & TCP_ECN_OK))
return;
- switch (TCP_SKB_CB(skb)->flags & INET_ECN_MASK) {
+ switch (TCP_SKB_CB(skb)->ip_dsfield & INET_ECN_MASK) {
case INET_ECN_NOT_ECT:
/* Funny extension: if ECT is not set on a segment,
* and we already seen ECT on a previous segment,
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index c29912c..dd3fad9 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1677,7 +1677,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
skb->len - th->doff * 4);
TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
TCP_SKB_CB(skb)->when = 0;
- TCP_SKB_CB(skb)->flags = iph->tos;
+ TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
TCP_SKB_CB(skb)->sacked = 0;
sk = __inet_lookup_skb(&tcp_hashinfo, skb, th->source, th->dest);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 12bdb9a..00797d8 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1717,7 +1717,7 @@ static int tcp_v6_rcv(struct sk_buff *skb)
skb->len - th->doff*4);
TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
TCP_SKB_CB(skb)->when = 0;
- TCP_SKB_CB(skb)->flags = ipv6_get_dsfield(hdr);
+ TCP_SKB_CB(skb)->ip_dsfield = ipv6_get_dsfield(hdr);
TCP_SKB_CB(skb)->sacked = 0;
sk = __inet6_lookup_skb(&tcp_hashinfo, skb, th->source, th->dest);
^ permalink raw reply related
* Re: [PATCH net-next] tcp: unalias tcp_skb_cb flags and ip_dsfield
From: David Miller @ 2011-09-27 6:20 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, hkchu, ilpo.jarvinen, jhs
In-Reply-To: <1317103249.2796.25.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 27 Sep 2011 08:00:49 +0200
> struct tcp_skb_cb contains a "flags" field containing either tcp flags
> or IP dsfield depending on context (input or output path)
>
> Introduce ip_dsfield to make the difference clear and ease maintenance.
> If later we want to save space, we can union flags/ip_dsfield
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks Eric.
> If there is no objection, I plan to rename "flags" to "tcp_flags" in a
> following patch.
No objections.
^ permalink raw reply
* Re: [PATCH 1/2] virtio-net: Verify page list size before fitting into skb
From: Sasha Levin @ 2011-09-27 6:44 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Rusty Russell, virtualization, netdev, kvm
In-Reply-To: <20110926195541.GA23086@redhat.com>
On Mon, 2011-09-26 at 22:55 +0300, Michael S. Tsirkin wrote:
> On Mon, Sep 26, 2011 at 10:37:22PM +0300, Sasha Levin wrote:
> > On Mon, 2011-09-26 at 21:44 +0300, Michael S. Tsirkin wrote:
> > > On Mon, Sep 26, 2011 at 08:41:08PM +0300, Sasha Levin wrote:
> > > > This patch verifies that the length of a buffer stored in a linked list
> > > > of pages is small enough to fit into a skb.
> > > >
> > > > If the size is larger than a max size of a skb, it means that we shouldn't
> > > > go ahead building skbs anyway since we won't be able to send the buffer as
> > > > the user requested.
> > > >
> > > > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > > > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > > > Cc: virtualization@lists.linux-foundation.org
> > > > Cc: netdev@vger.kernel.org
> > > > Cc: kvm@vger.kernel.org
> > > > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> > >
> > > Interesting. This is a theoretical issue, correct?
> > > Not a crash you actually see.
> >
> > Actually it was an actual crash caused when our virtio-net driver in kvm
> > tools did funny things and passed '(u32)-1' length as a buffer length to
> > the guest kernel.
> >
> > > This crash would mean device is giving us packets
> > > that are way too large. Avoiding crashes even in the face of
> > > a misbehaved device is a good idea, but should
> > > we print a diagnostic to a system log?
> > > Maybe rate-limited or print once to avoid filling
> > > up the disk. Other places in driver print with pr_debug
> > > I'm not sure that's right but better than nothing.
> >
> > Yup, I'll add some debug info.
> >
> > > > ---
> > > > drivers/net/virtio_net.c | 3 +++
> > > > 1 files changed, 3 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > index 0c7321c..64e0717 100644
> > > > --- a/drivers/net/virtio_net.c
> > > > +++ b/drivers/net/virtio_net.c
> > > > @@ -165,6 +165,9 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> > > > unsigned int copy, hdr_len, offset;
> > > > char *p;
> > > >
> > > > + if (len > MAX_SKB_FRAGS * PAGE_SIZE)
> > >
> > > unlikely()?
> > >
> > > Also, this seems too aggressive: at this point len includes the header
> > > and the linear part. The right place for this
> > > test is probably where we fill in the frags, just before
> > > while (len)
> > >
> > > The whole can only happen when mergeable buffers
> > > are disabled, right?
> >
> > >From what I understand it can happen whenever you're going to build a
> > skb longer than PAGE_SIZE.
>
> Hmm how exactly? With mergeable buffers this only gets
> the length of the 1st chunk which is up to 4K unless the driver
> is buggy ...
What about the case where TSO or ECN features are set? The code flow
suggests that mergeable would get ignored in that case:
/* If we can receive ANY GSO packets, we must allocate large ones. */
if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO4) ||
virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO6) ||
virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_ECN))
vi->big_packets = true;
[...]
if (!vi->mergeable_rx_bufs && !vi->big_packets) {
skb = buf;
len -= sizeof(struct virtio_net_hdr);
skb_trim(skb, len);
} else {
page = buf;
skb = page_to_skb(vi, page, len);
...
I haven't actually tested it with mergeable buffers enabled, but could
do it later today.
--
Sasha.
^ permalink raw reply
* [patch] 9p: fix min_t() casting in p9pdu_vwritef()
From: Dan Carpenter @ 2011-09-27 6:44 UTC (permalink / raw)
To: Eric Van Hensbergen
Cc: David S. Miller, Eric Van Hensbergen, Venkateswararao Jujjuri,
Aneesh Kumar K.V, Harsh Prateek Bora, netdev, kernel-janitors
I don't think we're actually likely to hit this limit but if we do
then the comparison should be done as size_t. The original code
is equivalent to:
len = strlen(sptr) % USHRT_MAX;
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/net/9p/protocol.c b/net/9p/protocol.c
index 55e10a9..39a4c18 100644
--- a/net/9p/protocol.c
+++ b/net/9p/protocol.c
@@ -368,7 +368,7 @@ p9pdu_vwritef(struct p9_fcall *pdu, int proto_version, const char *fmt,
const char *sptr = va_arg(ap, const char *);
uint16_t len = 0;
if (sptr)
- len = min_t(uint16_t, strlen(sptr),
+ len = min_t(size_t, strlen(sptr),
USHRT_MAX);
errcode = p9pdu_writef(pdu, proto_version,
^ permalink raw reply related
* Re: [PATCH 1/2] virtio-net: Verify page list size before fitting into skb
From: Michael S. Tsirkin @ 2011-09-27 7:00 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-kernel, Rusty Russell, virtualization, netdev, kvm
In-Reply-To: <1317105842.3467.3.camel@lappy>
On Tue, Sep 27, 2011 at 09:44:02AM +0300, Sasha Levin wrote:
> On Mon, 2011-09-26 at 22:55 +0300, Michael S. Tsirkin wrote:
> > On Mon, Sep 26, 2011 at 10:37:22PM +0300, Sasha Levin wrote:
> > > On Mon, 2011-09-26 at 21:44 +0300, Michael S. Tsirkin wrote:
> > > > On Mon, Sep 26, 2011 at 08:41:08PM +0300, Sasha Levin wrote:
> > > > > This patch verifies that the length of a buffer stored in a linked list
> > > > > of pages is small enough to fit into a skb.
> > > > >
> > > > > If the size is larger than a max size of a skb, it means that we shouldn't
> > > > > go ahead building skbs anyway since we won't be able to send the buffer as
> > > > > the user requested.
> > > > >
> > > > > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > > > > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > > > > Cc: virtualization@lists.linux-foundation.org
> > > > > Cc: netdev@vger.kernel.org
> > > > > Cc: kvm@vger.kernel.org
> > > > > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> > > >
> > > > Interesting. This is a theoretical issue, correct?
> > > > Not a crash you actually see.
> > >
> > > Actually it was an actual crash caused when our virtio-net driver in kvm
> > > tools did funny things and passed '(u32)-1' length as a buffer length to
> > > the guest kernel.
> > >
> > > > This crash would mean device is giving us packets
> > > > that are way too large. Avoiding crashes even in the face of
> > > > a misbehaved device is a good idea, but should
> > > > we print a diagnostic to a system log?
> > > > Maybe rate-limited or print once to avoid filling
> > > > up the disk. Other places in driver print with pr_debug
> > > > I'm not sure that's right but better than nothing.
> > >
> > > Yup, I'll add some debug info.
> > >
> > > > > ---
> > > > > drivers/net/virtio_net.c | 3 +++
> > > > > 1 files changed, 3 insertions(+), 0 deletions(-)
> > > > >
> > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > > index 0c7321c..64e0717 100644
> > > > > --- a/drivers/net/virtio_net.c
> > > > > +++ b/drivers/net/virtio_net.c
> > > > > @@ -165,6 +165,9 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> > > > > unsigned int copy, hdr_len, offset;
> > > > > char *p;
> > > > >
> > > > > + if (len > MAX_SKB_FRAGS * PAGE_SIZE)
> > > >
> > > > unlikely()?
> > > >
> > > > Also, this seems too aggressive: at this point len includes the header
> > > > and the linear part. The right place for this
> > > > test is probably where we fill in the frags, just before
> > > > while (len)
> > > >
> > > > The whole can only happen when mergeable buffers
> > > > are disabled, right?
> > >
> > > >From what I understand it can happen whenever you're going to build a
> > > skb longer than PAGE_SIZE.
> >
> > Hmm how exactly? With mergeable buffers this only gets
> > the length of the 1st chunk which is up to 4K unless the driver
> > is buggy ...
>
> What about the case where TSO or ECN features are set? The code flow
> suggests that mergeable would get ignored in that case:
>
> /* If we can receive ANY GSO packets, we must allocate large ones. */
> if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO4) ||
> virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO6) ||
> virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_ECN))
> vi->big_packets = true;
>
> [...]
>
> if (!vi->mergeable_rx_bufs && !vi->big_packets) {
> skb = buf;
> len -= sizeof(struct virtio_net_hdr);
> skb_trim(skb, len);
> } else {
> page = buf;
> skb = page_to_skb(vi, page, len);
> ...
Sorry I don't get it yet. Where is mergeable ignored here?
> I haven't actually tested it with mergeable buffers enabled, but could
> do it later today.
Please do.
> --
>
> Sasha.
^ 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