* Re: [PATCH 1/1] ipv6: fix the noflags test in addrconf_get_prefix_route
From: David Miller @ 2013-01-10 22:39 UTC (permalink / raw)
To: yoshfuji; +Cc: r.kuntz, netdev, linux-kernel, andi
In-Reply-To: <50EE9141.6020805@linux-ipv6.org>
From: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Date: Thu, 10 Jan 2013 19:00:33 +0900
> Romain KUNTZ wrote:
>> From e7ece201c35615c44a3cfdc10ee28ad5a5878f41 Mon Sep 17 00:00:00 2001
>> From: Romain Kuntz <r.kuntz@ipflavors.com>
>> Date: Wed, 9 Jan 2013 15:02:26 +0100
>> Subject: [PATCH 1/1] ipv6: fix the noflags test in addrconf_get_prefix_route
>>
>> The tests on the flags in addrconf_get_prefix_route() does no make
>> much sense: the 'noflags' parameter contains the set of flags that
>> must not match with the route flags, so the test must be done
>> against 'noflags', and not against 'flags'.
>>
>> Signed-off-by: Romain Kuntz <r.kuntz@ipflavors.com>
...
> Acked-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Applied.
^ permalink raw reply
* Re: [PATCH 1/1] ipv6: use addrconf_get_prefix_route for prefix route lookup [v2]
From: David Miller @ 2013-01-10 22:39 UTC (permalink / raw)
To: yoshfuji; +Cc: r.kuntz, nicolas.dichtel, netdev, eric.dumazet
In-Reply-To: <50EE8D79.1080807@linux-ipv6.org>
From: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Date: Thu, 10 Jan 2013 18:44:25 +0900
> Romain KUNTZ wrote:
>
>> From 203474c87f45da40b5c9d9e629164561307b4199 Mon Sep 17 00:00:00 2001
>> From: Romain Kuntz <r.kuntz@ipflavors.com>
>> Date: Thu, 10 Jan 2013 07:41:36 +0100
>> Subject: [PATCH 1/1] ipv6: use addrconf_get_prefix_route for prefix route lookup [v2]
>>
>> Replace ip6_route_lookup() with addrconf_get_prefix_route() when
>> looking up for a prefix route. This ensures that the connected prefix
>> is looked up in the main table, and avoids the selection of other
>> matching routes located in different tables as well as blackhole
>> or prohibited entries.
>>
>> In addition, this fixes an Opps introduced by commit 64c6d08e (ipv6:
>> del unreachable route when an addr is deleted on lo), that would occur
>> when a blackhole or prohibited entry is selected by ip6_route_lookup().
>> Such entries have a NULL rt6i_table argument, which is accessed by
>> __ip6_del_rt() when trying to lock rt6i_table->tb6_lock.
>>
>> The function addrconf_is_prefix_route() is not used anymore and is
>> removed.
>>
>> [v2] Minor indentation cleanup and log updates.
>>
>> Signed-off-by: Romain Kuntz <r.kuntz@ipflavors.com>
>> Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
...
> Acked-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
>
Applied.
^ permalink raw reply
* Re: [patch] bnx2x: move debugging code before the return
From: David Miller @ 2013-01-10 22:39 UTC (permalink / raw)
To: dan.carpenter; +Cc: eilong, netdev, kernel-janitors
In-Reply-To: <20130110085807.GB23063@elgon.mountain>
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Thu, 10 Jan 2013 11:58:07 +0300
> I move the return down a line after the debugging printk.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied.
^ permalink raw reply
* Re: [PATCH 3.8-rc] tuntap: refuse to re-attach to different tun_struct
From: David Miller @ 2013-01-10 22:39 UTC (permalink / raw)
To: stefanha; +Cc: jasowang, mst, netdev, linux-kernel
In-Reply-To: <1357804788-19976-1-git-send-email-stefanha@redhat.com>
From: Stefan Hajnoczi <stefanha@redhat.com>
Date: Thu, 10 Jan 2013 08:59:48 +0100
> Multiqueue tun devices support detaching a tun_file from its tun_struct
> and re-attaching at a later point in time. This allows users to disable
> a specific queue temporarily.
>
> ioctl(TUNSETIFF) allows the user to specify the network interface to
> attach by name. This means the user can attempt to attach to interface
> "B" after detaching from interface "A".
>
> The driver is not designed to support this so check we are re-attaching
> to the right tun_struct. Failure to do so may lead to oops.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH] tun: avoid owner checks on IFF_ATTACH_QUEUE
From: David Miller @ 2013-01-10 22:38 UTC (permalink / raw)
To: stefanha; +Cc: mst, jasowang, netdev, linux-kernel
In-Reply-To: <20130110115503.GD30885@stefanha-thinkpad.redhat.com>
From: Stefan Hajnoczi <stefanha@redhat.com>
Date: Thu, 10 Jan 2013 12:55:03 +0100
> On Thu, Jan 10, 2013 at 01:31:08PM +0200, Michael S. Tsirkin wrote:
>> At the moment, we check owner when we enable queue in tun.
>> This seems redundant and will break some valid uses
>> where fd is passed around: I think TUNSETOWNER is there
>> to prevent others from attaching to a persistent device not
>> owned by them. Here the fd is already attached,
>> enabling/disabling queue is more like read/write.
>>
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>
>> ---
>>
>> Note: this is unrelated to Stefan's bugfix.
>
> This should go into 3.8-rc.
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH net 0/2] bnx2x: bug fix patch series
From: David Miller @ 2013-01-10 22:38 UTC (permalink / raw)
To: yuvalmin; +Cc: netdev, eilong, ariele
In-Reply-To: <1357829620-17890-1-git-send-email-yuvalmin@broadcom.com>
From: "Yuval Mintz" <yuvalmin@broadcom.com>
Date: Thu, 10 Jan 2013 16:53:38 +0200
> Hi Dave,
>
> This patch series contains 2 bug fixes:
>
> 1. Failed memory allocation during Tx queue allocation can
> later cause the driver to dereference incorrect pointers,
> causing a panic.
>
> 2. During probe, management traffic is stopped and resumes
> only after some of its functions gets loaded.
>
> Please consider applying this patch-series to 'net'.
Applied.
^ permalink raw reply
* Re: [PATCH 2/2] net: ethernet: xilinx: Do not use NO_IRQ in axienet
From: David Miller @ 2013-01-10 22:38 UTC (permalink / raw)
To: michal.simek; +Cc: netdev
In-Reply-To: <1357837123-18996-2-git-send-email-michal.simek@xilinx.com>
From: Michal Simek <michal.simek@xilinx.com>
Date: Thu, 10 Jan 2013 17:58:43 +0100
> This driver is used on Microblaze and will be used
> on Arm Zynq.
> Microblaze doesn't define NO_IRQ and no IRQ is 0.
> Arm still uses NO_IRQ as -1 and there is no option
> to connect IRQ to irq 0.
>
> That's why <= 0 is only one option how to find out
> undefined IRQ.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Applied.
^ permalink raw reply
* Re: [PATCH 1/2] net: ethernet: xilinx: Do not use axienet on PPC
From: David Miller @ 2013-01-10 22:38 UTC (permalink / raw)
To: michal.simek; +Cc: netdev
In-Reply-To: <1357837123-18996-1-git-send-email-michal.simek@xilinx.com>
From: Michal Simek <michal.simek@xilinx.com>
Date: Thu, 10 Jan 2013 17:58:42 +0100
> Axi ethernet can't be used on PPC because it is
> little endian IP and PPC is big endian.
> This system can't be designed.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Applied.
^ permalink raw reply
* Re: [PATCH] bfin_mac: Restore hardware time-stamping dependency on BF518
From: David Miller @ 2013-01-10 22:37 UTC (permalink / raw)
To: richardcochran; +Cc: lars, uclinux-dist-devel, netdev
In-Reply-To: <20130110164016.GA2295@netboy.at.omicron.at>
From: Richard Cochran <richardcochran@gmail.com>
Date: Thu, 10 Jan 2013 17:40:16 +0100
> On Thu, Jan 10, 2013 at 03:42:28PM +0100, Lars-Peter Clausen wrote:
>>
>> This patch adds back the dependency on BF518, and since it does not make sense
>> to expose this config option when the blackfin MAC driver is not enabled also
>> restore the dependency on BFIN_MAC.
>
> Thanks a lot for fixing this.
>
> Acked-by: Richard Cochran <richardcochran@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH] tcp: splice: fix an infinite loop in tcp_read_sock()
From: David Miller @ 2013-01-10 22:37 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, w
In-Reply-To: <1357837570.27446.2285.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 10 Jan 2013 09:06:10 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> commit 02275a2ee7c0 (tcp: don't abort splice() after small transfers)
> added a regression.
...
> if recv_actor() returns 0, we should stop immediately,
> because looping wont give a chance to drain the pipe.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied.
^ permalink raw reply
* Re: BUG: NULL pointer dereference in netif_carrier_off
From: David Miller @ 2013-01-10 22:37 UTC (permalink / raw)
To: eric.dumazet; +Cc: tom.parkin, netdev
In-Reply-To: <1357842765.27446.2396.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 10 Jan 2013 10:32:45 -0800
> [PATCH net-next] veth: fix a NULL deref in netif_carrier_off
>
> In commit d0e2c55e7c94 (veth: avoid a NULL deref in veth_stats_one)
> we now clear the peer pointers in veth_dellink()
>
> veth_close() must therefore make sure the peer pointer is set.
>
> Reported-by: Tom Parkin <tom.parkin@gmail.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied.
^ permalink raw reply
* [PATCH v2 net-next] net_sched: more precise pkt_len computation
From: Eric Dumazet @ 2013-01-10 22:36 UTC (permalink / raw)
To: David Miller
Cc: netdev, Stephen Hemminger, Patrick McHardy, Paolo Valente,
Jamal Hadi Salim, Herbert Xu
In-Reply-To: <1351243970.6537.321.camel@edumazet-glaptop>
From: Eric Dumazet <edumazet@google.com>
One long standing problem with TSO/GSO/GRO packets is that skb->len
doesn't represent a precise amount of bytes on wire.
Headers are only accounted for the first segment.
For TCP, thats typically 66 bytes per 1448 bytes segment missing,
an error of 4.5 % for normal MSS value.
As consequences :
1) TBF/CBQ/HTB/NETEM/... can send more bytes than the assigned limits.
2) Device stats are slightly under estimated as well.
Fix this by taking account of headers in qdisc_skb_cb(skb)->pkt_len
computation.
Packet schedulers should use qdisc pkt_len instead of skb->len for their
bandwidth limitations, and TSO enabled devices drivers could use pkt_len
if their statistics are not hardware assisted, and if they don't scratch
skb->cb[] first word.
Both egress and ingress paths work, thanks to commit fda55eca5a
(net: introduce skb_transport_header_was_set()) : If GRO built
a GSO packet, it also set the transport header for us.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Stephen Hemminger <shemminger@vyatta.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Patrick McHardy <kaber@trash.net>
---
net/core/dev.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 594830e..3625f97 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2532,6 +2532,26 @@ struct netdev_queue *netdev_pick_tx(struct net_device *dev,
return netdev_get_tx_queue(dev, queue_index);
}
+static void qdisc_pkt_len_init(struct sk_buff *skb)
+{
+ const struct skb_shared_info *shinfo = skb_shinfo(skb);
+
+ qdisc_skb_cb(skb)->pkt_len = skb->len;
+
+ /* To get more precise estimation of bytes sent on wire,
+ * we add to pkt_len the headers size of all segments
+ */
+ if (shinfo->gso_size) {
+ unsigned int hdr_len = skb_transport_offset(skb);
+
+ if (likely(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6)))
+ hdr_len += tcp_hdrlen(skb);
+ else
+ hdr_len += sizeof(struct udphdr);
+ qdisc_skb_cb(skb)->pkt_len += (shinfo->gso_segs - 1) * hdr_len;
+ }
+}
+
static inline int __dev_xmit_skb(struct sk_buff *skb, struct Qdisc *q,
struct net_device *dev,
struct netdev_queue *txq)
@@ -2540,7 +2560,7 @@ static inline int __dev_xmit_skb(struct sk_buff *skb, struct Qdisc *q,
bool contended;
int rc;
- qdisc_skb_cb(skb)->pkt_len = skb->len;
+ qdisc_pkt_len_init(skb);
qdisc_calculate_pkt_len(skb, q);
/*
* Heuristic to force contended enqueues to serialize on a
^ permalink raw reply related
* Re: [PATCH/RFC] ipv6: fib: Drop cached routes with dead neighbours on fib GC
From: David Miller @ 2013-01-10 22:18 UTC (permalink / raw)
To: roland; +Cc: netdev, roland
In-Reply-To: <1357753459-12872-1-git-send-email-roland@kernel.org>
From: Roland Dreier <roland@kernel.org>
Date: Wed, 9 Jan 2013 09:44:19 -0800
> When a bonding interface changes slaves (or goes from no active slaves
> to having a slave with link), the bonding driver generates a
> NETDEV_CHANGEADDR notification. In this case, the ipv6 neighbour
> discovery code calls neigh_changeaddr(), which basically just calls
> neigh_flush_dev().
>
> Now, neigh_flush_dev() just goes through the neighbour hash table and
> tries to free every neighbour from the device being flushed. However,
> if someone else has an additional reference on the neighbour, we hit
...
> which leaves the final freeing of the "stray" neighbour until the last
> reference is dropped; in the meantime the output function is set to
> neigh_blackhole, which does nothing but free the skb and return ENETDOWN.
Another reason we must make ipv6 like ipv4, which looks up neighbours
on demand at packet output time rather than caching them in the route
entries.
^ permalink raw reply
* Re: [PATCH net-next v5 02/14] bridge: Add vlan filtering infrastructure
From: Stephen Hemminger @ 2013-01-10 22:10 UTC (permalink / raw)
To: vyasevic; +Cc: netdev, davem, stephen, bridge, shmulik.ladkani, mst
In-Reply-To: <50EF1537.7030209@redhat.com>
The problem with your approach is that it
adds a hash lookup in the packet process critical path.
Also the concept of different filters for egress vs ingress is feature
madness. It doesn't make sense to have half-duplex connectivity.
^ permalink raw reply
* Re: [PATCH net-next v5 01/14] vlan: wrap hw-acceleration calls in separate functions.
From: Stephen Hemminger @ 2013-01-10 22:07 UTC (permalink / raw)
To: vyasevic; +Cc: netdev, davem, stephen, bridge, shmulik.ladkani, mst
In-Reply-To: <50EF0B76.2040503@redhat.com>
On Thu, 10 Jan 2013 13:41:58 -0500
Vlad Yasevich <vyasevic@redhat.com> wrote:
> On 01/10/2013 01:25 PM, Stephen Hemminger wrote:
> > On Wed, 9 Jan 2013 12:17:48 -0500
> > Vlad Yasevich <vyasevic@redhat.com> wrote:
> >
> >>
> >> /**
> >> + * vlan_hw_buggy - Check to see if VLAN hw acceleration is supported.
> >> + * @dev: netdevice of the lowerdev/hw nic
> >> + *
> >> + * Checks to see if HW and driver report VLAN acceleration correctly.
> >> + */
> >> +static inline bool vlan_hw_buggy(const struct net_device *dev)
> >> +{
> >> + const struct net_device_ops *ops = dev->netdev_ops;
> >> +
> >> + if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
> >> + (!ops->ndo_vlan_rx_add_vid || !ops->ndo_vlan_rx_kill_vid))
> >> + return true;
> >> +
> >> + return false;
> >> +}
> >> +
> >> +/**
> >> + * vlan_vid_add_hw - Add the VLAN vid to the HW filter
> >> + * @dev: netdevice of the lowerdev/hw nic
> >> + * @vid: vlan id.
> >> + *
> >> + * Inserts the vid into the HW vlan filter table if hw supports it.
> >> + */
> >> +static inline int vlan_vid_add_hw(struct net_device *dev,
> >> + unsigned short vid)
> >> +{
> >> + const struct net_device_ops *ops = dev->netdev_ops;
> >> + int err = 0;
> >> +
> >> + if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
> >> + ops->ndo_vlan_rx_add_vid)
> >> + err = ops->ndo_vlan_rx_add_vid(dev, vid);
> >> +
> >> + return err;
> >> +}
> >> +
> >> +/**
> >> + * vlan_vid_del_hw - Delete the VLAN vid from the HW filter
> >> + * @dev: netdevice of the lowerdev/hw nic
> >> + * @vid: vlan id.
> >> + *
> >> + * Delete the vid from the HW vlan filter table if hw supports it.
> >> + */
> >> +static inline int vlan_vid_del_hw(struct net_device *dev,
> >> + unsigned short vid)
> >> +{
> >> + const struct net_device_ops *ops = dev->netdev_ops;
> >> + int err = 0;
> >> +
> >> + if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
> >> + ops->ndo_vlan_rx_kill_vid)
> >> + err = ops->ndo_vlan_rx_add_vid(dev, vid);
> >> +
> >> + return err;
> >> +}
> >> +
> >
> > I would rather not have all these inline's. This isn't performance critical.
>
> I kind of need to keep them as inlines because of the VLAN support is
> built. Right now, none of the VLAN files are build if VLAN support is
> turned off. So all we get access to are inlines from if_vlan.h.
>
> I suppose I can change how VLANs get built, but not if that's the right
> thing. It looks like it is set up the way it is on purpose.
>
> > Also, the check for buggy devices should be done inside the vlan code,
> > not repeated in the functions using the add/remove API. When device is
> > registered the flag and add/kill should be checked, and if the device driver
> > is buggy it should fail the register_netdevice.
> >
>
> Not sure what you mean here. I don't check if it's buggy again. I
> check that the device supports filter and the pointer is set. I does
> exactly what the code used to do. I suppose that the checks for valid
> function pointers may be a little redundant since otherwise
> vlan_hw_buggy() would have triggered, but it's safer to have them since
> we can't guarantee that other users have checked for buggy implementations.
>
> -vlad
The best way to handle this is to add stubs for the unconfigure case, and
include real code if configured.
I.e something like
#if IS_ENABLED(CONFIG_VLAN_8012Q)
extern int vlan_vid_add_hw(struct net_device *, unsigned short);
extern int vlan_vid_del_hw(struct net_device *, unsigned short);
#else
#define vlan_vid_add_hw(dev,vid) (-ENOTSUPP)
#define vlan_vid_del_hw(dev,vid) (-ENOTSUPP)
#endif
^ permalink raw reply
* Re: [PATCH v2] net: set default_ethtool_ops in register_netdevice
From: David Miller @ 2013-01-10 21:00 UTC (permalink / raw)
To: johannes
Cc: sgruszka, netdev, edumazet, greearb, bjorn, linux-wireless,
bhutchings, mirqus
In-Reply-To: <1357818103.17902.3.camel@jlt4.sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Thu, 10 Jan 2013 12:41:43 +0100
> On Thu, 2013-01-10 at 02:12 -0800, David Miller wrote:
>
>> >> On Wed, Jan 09, 2013 at 11:57:38PM -0800, David Miller wrote:
>> >>> From: Stanislaw Gruszka <sgruszka@redhat.com>
>> >>> Date: Tue, 8 Jan 2013 16:38:51 +0100
>> >>>
>> >>> > Since:
>> >>> >
>> >>> > commit 2c60db037034d27f8c636403355d52872da92f81
>> >>> > Author: Eric Dumazet <edumazet@google.com>
>> >>> > Date: Sun Sep 16 09:17:26 2012 +0000
>> >>> >
>> >>> > net: provide a default dev->ethtool_ops
>> >>> >
>> >>> > wireless core does not correctly assign ethtool_ops. In order to fix
>> >>> > the problem, move assignement of default_ethtool_ops to
>> >>> > register_netdevice(). This is safe because both register_netdevice()
>> >>> > and dev_ethtool() are protected by RTNL lock.
>> >>> >
>> >>> > Patch is besed on hint of Michał Mirosław.
>> >>> >
>> >>> > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
>> >>> > Cc: stable@vger.kernel.org # 3.7+
>> >>> > ---
>> >>> > v1 -> v2: change order of default_ethtool_ops initialization to avoid
>> >>> > the problem. Change the subject accordingly.
>> >>>
>> >>> I don't understand this. Why is the assignment of default_ethtool_ops
>> >>> at netdev allocation time not working? Is wireless really not using
>> >>> alloc_netdev*()?
>> >>
>> >> It does. This is done on individual cfg80211 drivers , i.e. on mac80211
>> >> or full mac drivers. After alloc_netdev*() call, some cfg80211 drivers
>> >> provide they own ethtool_ops, but some do not. For them, wireless core
>> >> provide generic cfg80211_ethtool_ops, which is assigned in
>> >> NETDEV_REGISTER notify call:
>> >>
>> >> if (!dev->ethtool_ops)
>> >> dev->ethtool_ops = &cfg80211_ethtool_ops;
>> >>
>> >> But after Eric's commit, dev->ethtool_ops is no longer NULL (on cfg80211
>> >> drivers without custom ethtool_ops), but points to &default_ethtool_ops.
>> >
>> > The whole idea is to remove these kinds of NULL tests against
>> > dev->ethtool_ops, thus creating the invariant that given any netdev
>> > one must never be able to see a non-NULL value there.
>>
>> Of course I meant that we should never see a NULL value in
>> dev->ethtool_ops.
>>
>> I would suggest fixing this by making the wireless core express it's
>> intentions, that it wants to use a different default ethtool ops, to
>> the netdev allocation layer.
>>
>> You have two way to do this:
>>
>> 1) Add default_ethtool_ops argument to alloc_netdev*().
>>
>> 2) Add a "netdev_set_default_ethtool_ops(netdev, ops)"
>
> Neither of these work as (I think) you think they would :-)
2) works because all netdev_set_ethtool_ops() is going to do is
compare netdev->ops against &default_ethtool_ops as the test,
and therefore you can replace the above cfg80211 test and
assignment with a call to it.
^ permalink raw reply
* Re: ppoll() stuck on POLLIN while TCP peer is sending
From: Eric Dumazet @ 2013-01-10 20:58 UTC (permalink / raw)
To: Mel Gorman
Cc: Eric Wong, linux-mm, netdev, linux-kernel, Rik van Riel,
Minchan Kim, Andrew Morton, Linus Torvalds
In-Reply-To: <20130110194212.GJ13304@suse.de>
On Thu, 2013-01-10 at 19:42 +0000, Mel Gorman wrote:
> Thanks Eric, it's much appreciated. However, I'm still very much in favour
> of a partial revert as in retrospect the implementation of capture took the
> wrong approach. Could you confirm the following patch works for you?
> It's should functionally have the same effect as the first revert and
> there are only minor changes from the last revert prototype I sent you
> but there is no harm in being sure.
>
> ---8<---
> mm: compaction: Partially revert capture of suitable high-order page
>
> Eric Wong reported on 3.7 and 3.8-rc2 that ppoll() got stuck when waiting
> for POLLIN on a local TCP socket. It was easier to trigger if there was disk
> IO and dirty pages at the same time and he bisected it to commit 1fb3f8ca
> "mm: compaction: capture a suitable high-order page immediately when it
> is made available".
>
> The intention of that patch was to improve high-order allocations under
> memory pressure after changes made to reclaim in 3.6 drastically hurt
> THP allocations but the approach was flawed. For Eric, the problem was
> that page->pfmemalloc was not being cleared for captured pages leading to
> a poor interaction with swap-over-NFS support causing the packets to be
> dropped. However, I identified a few more problems with the patch including
> the fact that it can increase contention on zone->lock in some cases which
> could result in async direct compaction being aborted early.
>
> In retrospect the capture patch took the wrong approach. What it should
> have done is mark the pageblock being migrated as MIGRATE_ISOLATE if it
> was allocating for THP and avoided races that way. While the patch was
> showing to improve allocation success rates at the time, the benefit is
> marginal given the relative complexity and it should be revisited from
> scratch in the context of the other reclaim-related changes that have taken
> place since the patch was first written and tested. This patch partially
> reverts commit 1fb3f8ca "mm: compaction: capture a suitable high-order
> page immediately when it is made available".
>
> Reported-by: Eric Wong <normalperson@yhbt.net>
> Cc: stable@vger.kernel.org
> Signed-off-by: Mel Gorman <mgorman@suse.de>
> ---
It seems to solve the problem on my kvm testbed
(512 MB of ram, 2 vcpus)
Tested-by: Eric Dumazet <edumazet@google.com>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: PROBLEM: Software injected vlan tagged packets are unable to be identified using recent BPF modifications
From: Bill Fenner @ 2013-01-10 20:37 UTC (permalink / raw)
To: Paul Pearce
Cc: jpirko, netdev, Francesco Ruggeri, edumazet, tcpdump-workers,
David Miller
In-Reply-To: <CAOUgPvQhyC5LTVAZeeRd3N-Jdpe+cHK4LQWZUB-qTKT_og7dSw@mail.gmail.com>
On Mon, Jan 7, 2013 at 10:04 PM, Paul Pearce <pearce@cs.berkeley.edu> wrote:
> However, raw vlan tagged packets that are *injected* into the
> interface using libpcap's pcap_inject() (which is just a fancy wrapper
> for the send() syscall) are not identified by filters using the recent
> BPF modifications.
>
> The bug manifests itself if you attempt to use the new BPF
> modifications to filter vlan tagged packets on a live interface. All
> packets from the medium show up, but all injected packets are dropped.
Given that the vlan tag metadata is supplied to userland with
PACKET_AUXDATA, does the symmetrical sendmsg() with PACKET_AUXDATA
work to put the vlan info in the metadata? I.e., this would require
modifying pcap_inject() to parse the packet and extract the VLAN tag
info into a struct tpacket_auxdata, but obviously you could write a
little test program to test the underlying PF_PACKET socket behavior.
Even if it doesn't currently work, I think this may be a more
acceptable change ("provide a way to set PACKET_AUXDATA using
sendmsg") than having the packet send code munge/parse the vlan tag on
output.
Bill
_______________________________________________
tcpdump-workers mailing list
tcpdump-workers@lists.tcpdump.org
https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
^ permalink raw reply
* Re: ppoll() stuck on POLLIN while TCP peer is sending
From: Eric Wong @ 2013-01-10 20:03 UTC (permalink / raw)
To: Mel Gorman
Cc: linux-mm, netdev, linux-kernel, Rik van Riel, Minchan Kim,
Eric Dumazet, Andrew Morton, Linus Torvalds
In-Reply-To: <20130110194212.GJ13304@suse.de>
Mel Gorman <mgorman@suse.de> wrote:
> Thanks Eric, it's much appreciated. However, I'm still very much in favour
> of a partial revert as in retrospect the implementation of capture took the
> wrong approach. Could you confirm the following patch works for you?
> It's should functionally have the same effect as the first revert and
> there are only minor changes from the last revert prototype I sent you
> but there is no harm in being sure.
Thanks, I was just about to report back on the last partial revert
being successful :) Will start testing this one, now.
^ permalink raw reply
* Re: [PATCH 1/5] GRE: Add segmentation offload for GRE TAP device.
From: Eric Dumazet @ 2013-01-10 19:47 UTC (permalink / raw)
To: Pravin B Shelar; +Cc: dev, netdev, jesse
In-Reply-To: <1357612296-1715-1-git-send-email-pshelar@nicira.com>
On Mon, 2013-01-07 at 18:31 -0800, Pravin B Shelar wrote:
> From: Pravin Shelar <pshelar@nicira.com>
Do you have some perf numbers to share ?
> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
> ---
> include/linux/skbuff.h | 12 ++++++
> include/net/gre.h | 6 +++
> net/ipv4/af_inet.c | 1 +
> net/ipv4/gre.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++
> net/ipv4/tcp.c | 1 +
> net/ipv4/udp.c | 3 +-
> net/ipv6/ip6_offload.c | 1 +
> net/ipv6/udp_offload.c | 3 +-
> 8 files changed, 126 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 320e976..43033f0 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -307,6 +307,8 @@ enum {
> SKB_GSO_TCPV6 = 1 << 4,
>
> SKB_GSO_FCOE = 1 << 5,
> +
> + SKB_GSO_GRE = 1 << 6,
> };
>
> #if BITS_PER_LONG > 32
> @@ -797,6 +799,16 @@ static inline int skb_cloned(const struct sk_buff *skb)
> (atomic_read(&skb_shinfo(skb)->dataref) & SKB_DATAREF_MASK) != 1;
> }
>
> +static inline int skb_unclone(struct sk_buff *skb, gfp_t pri)
> +{
> + might_sleep_if(pri & __GFP_WAIT);
> +
> + if (skb_cloned(skb))
> + return pskb_expand_head(skb, 0, 0, pri);
> +
> + return 0;
> +}
Should be a commit on its own, and use a "bool"
We have some places in network stack that could use this helper.
(net/ipv6/ah6.c, net/ipv6/reassembly.c,
net/ipv6/xfrm6_mode_tunnel.c, ...)
> +
> /**
> * skb_header_cloned - is the header a clone
> * @skb: buffer to check
> diff --git a/include/net/gre.h b/include/net/gre.h
> index 8266547..f7d5bfb 100644
> --- a/include/net/gre.h
> +++ b/include/net/gre.h
> @@ -15,4 +15,10 @@ struct gre_protocol {
> int gre_add_protocol(const struct gre_protocol *proto, u8 version);
> int gre_del_protocol(const struct gre_protocol *proto, u8 version);
>
> +struct gre_base_hdr {
> + __be16 flags;
> + __be16 protocol;
> +};
> +#define GRE_HEADER_SECTION 4
> +
> #endif
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 24b384b..9a3d5d2 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -1306,6 +1306,7 @@ static struct sk_buff *inet_gso_segment(struct sk_buff *skb,
> SKB_GSO_UDP |
> SKB_GSO_DODGY |
> SKB_GSO_TCP_ECN |
> + SKB_GSO_GRE |
> 0)))
> goto out;
>
> diff --git a/net/ipv4/gre.c b/net/ipv4/gre.c
> index 42a4910..0eaaf44 100644
> --- a/net/ipv4/gre.c
> +++ b/net/ipv4/gre.c
> @@ -19,6 +19,7 @@
> #include <linux/in.h>
> #include <linux/ip.h>
> #include <linux/netdevice.h>
> +#include <linux/if_tunnel.h>
> #include <linux/spinlock.h>
> #include <net/protocol.h>
> #include <net/gre.h>
> @@ -112,12 +113,105 @@ static void gre_err(struct sk_buff *skb, u32 info)
> rcu_read_unlock();
> }
>
> +static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
> + netdev_features_t features)
> +{
> + struct sk_buff *segs = ERR_PTR(-EINVAL);
> + struct gre_base_hdr *greh;
> + unsigned char *mac = skb_mac_header(skb);
You should not init mac here, but after the pskb_may_pull()
> + int net_hlen = skb_network_header_len(skb);
> + int ghl = GRE_HEADER_SECTION;
> + int mac_len;
> + int doffset;
> +
> + if (unlikely(skb_shinfo(skb)->gso_type &
> + ~(SKB_GSO_TCPV4 |
> + SKB_GSO_UDP |
> + SKB_GSO_DODGY |
> + SKB_GSO_TCP_ECN |
> + SKB_GSO_GRE |
> + 0)))
> + goto out;
> +
> + if (unlikely(!pskb_may_pull(skb, sizeof(*greh))))
> + goto out;
> +
> + greh = (struct gre_base_hdr *)skb_transport_header(skb);
> +
> + if (greh->flags & GRE_KEY)
> + ghl += GRE_HEADER_SECTION;
> + if (greh->flags & GRE_CSUM)
> + ghl += GRE_HEADER_SECTION;
> +
> + if (unlikely(!pskb_may_pull(skb, ghl)))
> + goto out;
> +
mac can now point to freed memory. You should move its init here :
mac = skb_mac_header(skb);
same for greh
greh = (struct gre_base_hdr *)skb_transport_header(skb);
> + __skb_pull(skb, ghl);
> + skb_reset_mac_header(skb);
> + skb_set_network_header(skb, skb->mac_len);
> + doffset = skb_mac_header(skb) - mac;
> + /* segment inner packet. */
> + segs = skb_gso_segment(skb, 0);
> + if (!segs || IS_ERR(segs))
> + goto out;
> +
> + mac_len = skb_inner_network_header(skb) - ((unsigned char *)greh + ghl);
> + skb = segs;
> + do {
> + unsigned char *smac;
> +
> + skb_push(skb, doffset);
> +
> + skb_reset_mac_header(skb);
> + skb_set_network_header(skb, mac_len);
> + skb_set_transport_header(skb,
> + skb_network_offset(skb) + net_hlen);
> + smac = skb_mac_header(skb);
> + skb->mac_len = mac_len;
> + /* Copy entire outer header from original skb. */
> + memcpy(smac, mac, doffset);
> +
> + greh = (struct gre_base_hdr *)skb_transport_header(skb);
> + if (greh->flags & GRE_CSUM) {
> + __be32 *gre_csum = (__be32 *)(greh + 1);
> + *gre_csum = 0;
> + *(__sum16 *)gre_csum = csum_fold(skb_checksum(skb,
> + skb_transport_offset(skb),
> + skb->len - skb_transport_offset(skb),
> + 0));
> + }
> + } while ((skb = skb->next));
> +
> +out:
> + return segs;
> +}
> +
^ permalink raw reply
* Re: tainted warnings with tcp splicing in 3.7.1
From: Willy Tarreau @ 2013-01-10 19:43 UTC (permalink / raw)
To: Rick Jones
Cc: Eric Dumazet, Christian Becker, David Miller,
netdev@vger.kernel.org
In-Reply-To: <50EF0D2D.3040700@hp.com>
On Thu, Jan 10, 2013 at 10:49:17AM -0800, Rick Jones wrote:
> On 01/10/2013 10:42 AM, Eric Dumazet wrote:
> >Can we use zero copy for the sender as well (sendfile() or vmsplice()) ?
>
> Not at present. The TCP_SENDFILE test has not been "migrated" and there
> is no corresponding sendfile() test in the omni code, so the attempt to
> enable copy-avoidance won't "take."
Note that in my httpterm server, I'm using tee+splice(). I had a problem
with vmsplice() in the past, I don't remember which one.
For the "inject" http client, I'm using recv(MSG_TRUNC) which is always
faster than splice() and does the job well, considering that the goal is
to count and destroy data very fast.
By combining them both on the same machine over the loopback, I see
70 Gbps on a core-2 duo 2.66 GHz. These are obviously not real gigs,
but at least it shows what the network stack can do when we remove
memory copies !
Regards,
Willy
^ permalink raw reply
* Re: ppoll() stuck on POLLIN while TCP peer is sending
From: Mel Gorman @ 2013-01-10 19:42 UTC (permalink / raw)
To: Eric Wong
Cc: linux-mm, netdev, linux-kernel, Rik van Riel, Minchan Kim,
Eric Dumazet, Andrew Morton, Linus Torvalds
In-Reply-To: <20130110092511.GA32333@dcvr.yhbt.net>
On Thu, Jan 10, 2013 at 09:25:11AM +0000, Eric Wong wrote:
> Mel Gorman <mgorman@suse.de> wrote:
> > page->pfmemalloc can be left set for captured pages so try this but as
> > capture is rarely used I'm strongly favouring a partial revert even if
> > this works for you. I haven't reproduced this using your workload yet
> > but I have found that high-order allocation stress tests for 3.8-rc2 are
> > completely screwed. 71% success rates at rest in 3.7 and 6% in 3.8-rc2 so
> > I have to chase that down too.
> >
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 9d20c13..c242d21 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -2180,8 +2180,10 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
> > current->flags &= ~PF_MEMALLOC;
> >
> > /* If compaction captured a page, prep and use it */
> > - if (page && !prep_new_page(page, order, gfp_mask))
> > + if (page && !prep_new_page(page, order, gfp_mask)) {
> > + page->pfmemalloc = false;
> > goto got_page;
> > + }
> >
> > if (*did_some_progress != COMPACT_SKIPPED) {
> > /* Page migration frees to the PCP lists but we want merging */
>
> This (on top of your previous patch) seems to work great after several
> hours of testing on both my VM and real machine. I haven't tried your
> partial revert, yet. Will try that in a bit on the VM.
Thanks Eric, it's much appreciated. However, I'm still very much in favour
of a partial revert as in retrospect the implementation of capture took the
wrong approach. Could you confirm the following patch works for you?
It's should functionally have the same effect as the first revert and
there are only minor changes from the last revert prototype I sent you
but there is no harm in being sure.
---8<---
mm: compaction: Partially revert capture of suitable high-order page
Eric Wong reported on 3.7 and 3.8-rc2 that ppoll() got stuck when waiting
for POLLIN on a local TCP socket. It was easier to trigger if there was disk
IO and dirty pages at the same time and he bisected it to commit 1fb3f8ca
"mm: compaction: capture a suitable high-order page immediately when it
is made available".
The intention of that patch was to improve high-order allocations under
memory pressure after changes made to reclaim in 3.6 drastically hurt
THP allocations but the approach was flawed. For Eric, the problem was
that page->pfmemalloc was not being cleared for captured pages leading to
a poor interaction with swap-over-NFS support causing the packets to be
dropped. However, I identified a few more problems with the patch including
the fact that it can increase contention on zone->lock in some cases which
could result in async direct compaction being aborted early.
In retrospect the capture patch took the wrong approach. What it should
have done is mark the pageblock being migrated as MIGRATE_ISOLATE if it
was allocating for THP and avoided races that way. While the patch was
showing to improve allocation success rates at the time, the benefit is
marginal given the relative complexity and it should be revisited from
scratch in the context of the other reclaim-related changes that have taken
place since the patch was first written and tested. This patch partially
reverts commit 1fb3f8ca "mm: compaction: capture a suitable high-order
page immediately when it is made available".
Reported-by: Eric Wong <normalperson@yhbt.net>
Cc: stable@vger.kernel.org
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
include/linux/compaction.h | 4 +-
include/linux/mm.h | 1 -
mm/compaction.c | 92 +++++++-------------------------------------
mm/internal.h | 1 -
mm/page_alloc.c | 35 ++++-------------
5 files changed, 23 insertions(+), 110 deletions(-)
diff --git a/include/linux/compaction.h b/include/linux/compaction.h
index 6ecb6dc..cc7bdde 100644
--- a/include/linux/compaction.h
+++ b/include/linux/compaction.h
@@ -22,7 +22,7 @@ extern int sysctl_extfrag_handler(struct ctl_table *table, int write,
extern int fragmentation_index(struct zone *zone, unsigned int order);
extern unsigned long try_to_compact_pages(struct zonelist *zonelist,
int order, gfp_t gfp_mask, nodemask_t *mask,
- bool sync, bool *contended, struct page **page);
+ bool sync, bool *contended);
extern int compact_pgdat(pg_data_t *pgdat, int order);
extern void reset_isolation_suitable(pg_data_t *pgdat);
extern unsigned long compaction_suitable(struct zone *zone, int order);
@@ -75,7 +75,7 @@ static inline bool compaction_restarting(struct zone *zone, int order)
#else
static inline unsigned long try_to_compact_pages(struct zonelist *zonelist,
int order, gfp_t gfp_mask, nodemask_t *nodemask,
- bool sync, bool *contended, struct page **page)
+ bool sync, bool *contended)
{
return COMPACT_CONTINUE;
}
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 6320407..66e2f7c 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -455,7 +455,6 @@ void put_pages_list(struct list_head *pages);
void split_page(struct page *page, unsigned int order);
int split_free_page(struct page *page);
-int capture_free_page(struct page *page, int alloc_order, int migratetype);
/*
* Compound pages have a destructor function. Provide a
diff --git a/mm/compaction.c b/mm/compaction.c
index 6b807e4..2c57043 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -816,6 +816,7 @@ static isolate_migrate_t isolate_migratepages(struct zone *zone,
static int compact_finished(struct zone *zone,
struct compact_control *cc)
{
+ unsigned int order;
unsigned long watermark;
if (fatal_signal_pending(current))
@@ -850,22 +851,16 @@ static int compact_finished(struct zone *zone,
return COMPACT_CONTINUE;
/* Direct compactor: Is a suitable page free? */
- if (cc->page) {
- /* Was a suitable page captured? */
- if (*cc->page)
+ for (order = cc->order; order < MAX_ORDER; order++) {
+ struct free_area *area = &zone->free_area[order];
+
+ /* Job done if page is free of the right migratetype */
+ if (!list_empty(&area->free_list[cc->migratetype]))
+ return COMPACT_PARTIAL;
+
+ /* Job done if allocation would set block type */
+ if (cc->order >= pageblock_order && area->nr_free)
return COMPACT_PARTIAL;
- } else {
- unsigned int order;
- for (order = cc->order; order < MAX_ORDER; order++) {
- struct free_area *area = &zone->free_area[cc->order];
- /* Job done if page is free of the right migratetype */
- if (!list_empty(&area->free_list[cc->migratetype]))
- return COMPACT_PARTIAL;
-
- /* Job done if allocation would set block type */
- if (cc->order >= pageblock_order && area->nr_free)
- return COMPACT_PARTIAL;
- }
}
return COMPACT_CONTINUE;
@@ -921,60 +916,6 @@ unsigned long compaction_suitable(struct zone *zone, int order)
return COMPACT_CONTINUE;
}
-static void compact_capture_page(struct compact_control *cc)
-{
- unsigned long flags;
- int mtype, mtype_low, mtype_high;
-
- if (!cc->page || *cc->page)
- return;
-
- /*
- * For MIGRATE_MOVABLE allocations we capture a suitable page ASAP
- * regardless of the migratetype of the freelist is is captured from.
- * This is fine because the order for a high-order MIGRATE_MOVABLE
- * allocation is typically at least a pageblock size and overall
- * fragmentation is not impaired. Other allocation types must
- * capture pages from their own migratelist because otherwise they
- * could pollute other pageblocks like MIGRATE_MOVABLE with
- * difficult to move pages and making fragmentation worse overall.
- */
- if (cc->migratetype == MIGRATE_MOVABLE) {
- mtype_low = 0;
- mtype_high = MIGRATE_PCPTYPES;
- } else {
- mtype_low = cc->migratetype;
- mtype_high = cc->migratetype + 1;
- }
-
- /* Speculatively examine the free lists without zone lock */
- for (mtype = mtype_low; mtype < mtype_high; mtype++) {
- int order;
- for (order = cc->order; order < MAX_ORDER; order++) {
- struct page *page;
- struct free_area *area;
- area = &(cc->zone->free_area[order]);
- if (list_empty(&area->free_list[mtype]))
- continue;
-
- /* Take the lock and attempt capture of the page */
- if (!compact_trylock_irqsave(&cc->zone->lock, &flags, cc))
- return;
- if (!list_empty(&area->free_list[mtype])) {
- page = list_entry(area->free_list[mtype].next,
- struct page, lru);
- if (capture_free_page(page, cc->order, mtype)) {
- spin_unlock_irqrestore(&cc->zone->lock,
- flags);
- *cc->page = page;
- return;
- }
- }
- spin_unlock_irqrestore(&cc->zone->lock, flags);
- }
- }
-}
-
static int compact_zone(struct zone *zone, struct compact_control *cc)
{
int ret;
@@ -1054,9 +995,6 @@ static int compact_zone(struct zone *zone, struct compact_control *cc)
goto out;
}
}
-
- /* Capture a page now if it is a suitable size */
- compact_capture_page(cc);
}
out:
@@ -1069,8 +1007,7 @@ out:
static unsigned long compact_zone_order(struct zone *zone,
int order, gfp_t gfp_mask,
- bool sync, bool *contended,
- struct page **page)
+ bool sync, bool *contended)
{
unsigned long ret;
struct compact_control cc = {
@@ -1080,7 +1017,6 @@ static unsigned long compact_zone_order(struct zone *zone,
.migratetype = allocflags_to_migratetype(gfp_mask),
.zone = zone,
.sync = sync,
- .page = page,
};
INIT_LIST_HEAD(&cc.freepages);
INIT_LIST_HEAD(&cc.migratepages);
@@ -1110,7 +1046,7 @@ int sysctl_extfrag_threshold = 500;
*/
unsigned long try_to_compact_pages(struct zonelist *zonelist,
int order, gfp_t gfp_mask, nodemask_t *nodemask,
- bool sync, bool *contended, struct page **page)
+ bool sync, bool *contended)
{
enum zone_type high_zoneidx = gfp_zone(gfp_mask);
int may_enter_fs = gfp_mask & __GFP_FS;
@@ -1136,7 +1072,7 @@ unsigned long try_to_compact_pages(struct zonelist *zonelist,
int status;
status = compact_zone_order(zone, order, gfp_mask, sync,
- contended, page);
+ contended);
rc = max(status, rc);
/* If a normal allocation would succeed, stop compacting */
@@ -1192,7 +1128,6 @@ int compact_pgdat(pg_data_t *pgdat, int order)
struct compact_control cc = {
.order = order,
.sync = false,
- .page = NULL,
};
return __compact_pgdat(pgdat, &cc);
@@ -1203,7 +1138,6 @@ static int compact_node(int nid)
struct compact_control cc = {
.order = -1,
.sync = true,
- .page = NULL,
};
return __compact_pgdat(NODE_DATA(nid), &cc);
diff --git a/mm/internal.h b/mm/internal.h
index d597f94..9ba2110 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -135,7 +135,6 @@ struct compact_control {
int migratetype; /* MOVABLE, RECLAIMABLE etc */
struct zone *zone;
bool contended; /* True if a lock was contended */
- struct page **page; /* Page captured of requested size */
};
unsigned long
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 4ba5e37..7e4ae85 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1389,14 +1389,8 @@ void split_page(struct page *page, unsigned int order)
set_page_refcounted(page + i);
}
-/*
- * Similar to the split_page family of functions except that the page
- * required at the given order and being isolated now to prevent races
- * with parallel allocators
- */
-int capture_free_page(struct page *page, int alloc_order, int migratetype)
+static int __isolate_free_page(struct page *page, unsigned int order)
{
- unsigned int order;
unsigned long watermark;
struct zone *zone;
int mt;
@@ -1404,7 +1398,6 @@ int capture_free_page(struct page *page, int alloc_order, int migratetype)
BUG_ON(!PageBuddy(page));
zone = page_zone(page);
- order = page_order(page);
mt = get_pageblock_migratetype(page);
if (mt != MIGRATE_ISOLATE) {
@@ -1413,7 +1406,7 @@ int capture_free_page(struct page *page, int alloc_order, int migratetype)
if (!zone_watermark_ok(zone, 0, watermark, 0, 0))
return 0;
- __mod_zone_freepage_state(zone, -(1UL << alloc_order), mt);
+ __mod_zone_freepage_state(zone, -(1UL << order), mt);
}
/* Remove page from free list */
@@ -1421,11 +1414,7 @@ int capture_free_page(struct page *page, int alloc_order, int migratetype)
zone->free_area[order].nr_free--;
rmv_page_order(page);
- if (alloc_order != order)
- expand(zone, page, alloc_order, order,
- &zone->free_area[order], migratetype);
-
- /* Set the pageblock if the captured page is at least a pageblock */
+ /* Set the pageblock if the isolated page is at least a pageblock */
if (order >= pageblock_order - 1) {
struct page *endpage = page + (1 << order) - 1;
for (; page < endpage; page += pageblock_nr_pages) {
@@ -1436,7 +1425,7 @@ int capture_free_page(struct page *page, int alloc_order, int migratetype)
}
}
- return 1UL << alloc_order;
+ return 1UL << order;
}
/*
@@ -1454,10 +1443,9 @@ int split_free_page(struct page *page)
unsigned int order;
int nr_pages;
- BUG_ON(!PageBuddy(page));
order = page_order(page);
- nr_pages = capture_free_page(page, order, 0);
+ nr_pages = __isolate_free_page(page, order);
if (!nr_pages)
return 0;
@@ -2163,8 +2151,6 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
bool *contended_compaction, bool *deferred_compaction,
unsigned long *did_some_progress)
{
- struct page *page = NULL;
-
if (!order)
return NULL;
@@ -2176,16 +2162,12 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
current->flags |= PF_MEMALLOC;
*did_some_progress = try_to_compact_pages(zonelist, order, gfp_mask,
nodemask, sync_migration,
- contended_compaction, &page);
+ contended_compaction);
current->flags &= ~PF_MEMALLOC;
- /* If compaction captured a page, prep and use it */
- if (page) {
- prep_new_page(page, order, gfp_mask);
- goto got_page;
- }
-
if (*did_some_progress != COMPACT_SKIPPED) {
+ struct page *page;
+
/* Page migration frees to the PCP lists but we want merging */
drain_pages(get_cpu());
put_cpu();
@@ -2195,7 +2177,6 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
alloc_flags & ~ALLOC_NO_WATERMARKS,
preferred_zone, migratetype);
if (page) {
-got_page:
preferred_zone->compact_blockskip_flush = false;
preferred_zone->compact_considered = 0;
preferred_zone->compact_defer_shift = 0;
^ permalink raw reply related
* Re: [PATCH 0/5] ipv4_tunnels: Modularize ipv4 tunneling.
From: Pravin Shelar @ 2013-01-10 19:28 UTC (permalink / raw)
To: David Miller; +Cc: dev@openvswitch.org, netdev, Jesse Gross
In-Reply-To: <20130109.235239.1789560948007134858.davem@davemloft.net>
On Wed, Jan 9, 2013 at 11:52 PM, David Miller <davem@davemloft.net> wrote:
> From: Pravin B Shelar <pshelar@nicira.com>
> Date: Mon, 7 Jan 2013 18:31:32 -0800
>
>> Following patch series restructure GRE and IPIP tunneling code
>> to make it modular. It adds ip_tunnel module which acts as
>> generic tunneling layer which has common code. I have patch
>> to do same for VXLAN too.
>>
>> In Addidtion to restructuring it adds demultiplexer for ipgre
>> protocol, so that linux kernel IPGRE module and OVS gre module
>> can co-exist.
>> Last patch adds linux state updates to tunnel device from
>> lower device.
>
> Please resubmit this when you can also submit the patches which
> actually use this new modularity.
>
> Otherwise the value proposition is impossible to analyze.
>
Ok. These patches are not just modularize ipv4 tunnels but also does
code refactoring, so that there is less code duplication in different
tunneling implementation.
There are couple of independent patches from this patch series. I will
submit them separately. Later I will resubmit modular ip-tunnel
patches as you have suggested.
Thanks,
Pravin.
^ permalink raw reply
* Re: [PATCH net-next v5 02/14] bridge: Add vlan filtering infrastructure
From: Vlad Yasevich @ 2013-01-10 19:23 UTC (permalink / raw)
To: vyasevic
Cc: Stephen Hemminger, netdev, davem, stephen, bridge,
shmulik.ladkani, mst
In-Reply-To: <50EF0FF7.3080407@redhat.com>
On 01/10/2013 02:01 PM, Vlad Yasevich wrote:
> On 01/10/2013 01:36 PM, Stephen Hemminger wrote:
>> This patch has some minor whitespace and spelling errors:
>>
>> WARNING: line over 80 characters
>> #429: FILE: net/bridge/br_private.h:205:
>> +static inline struct net_bridge_port *vlans_to_port(struct
>> net_port_vlans *vlans)
>>
>> ERROR: trailing whitespace
>> #432: FILE: net/bridge/br_private.h:208:
>> + $
>>
>> WARNING: please, no spaces at the start of a line
>> #432: FILE: net/bridge/br_private.h:208:
>> + $
>>
>> +/* Must be protected by RTNL */
>> +static void br_vlan_del(struct net_bridge_vlan *vlan)
>>
>> + /* Drop the self-ref to trigger descrution. */
>> ^^^^^^^^^^
>>
>
> sorry, will fix. I thought I ran everything through checkpatch. I'll
> re-run them.
>
>> Also, the data structure vlan's seems inverted. Why do you keep a hash
>> list
>> of vlan's and then a bitmap of ports. Seems more natural to just put a
>> bitmap
>> on each port that has vlan filtering rather than introducing yet
>> another list
>> to manage.
>>
>
> I originally had it this way. I found that it wasted space and made
> other things more difficult to do. For instance, to do egress policy, I
> would have needed another bitmap of vlans... With 4096 vlans, that's
> 512 bytes per port (1024 with policy).
>
> I could probably remove the per-port list. It would make 2 things a
> little harder:
> 1) detecting if the port has any vlan info at all.
> 2) dumping the vlan information
One other thing would become a little more difficult. fdb management
for manually added neighbors.
-vlad
>
> I could pursue this if you think it'll be better.
>
> -vlad
^ permalink raw reply
* Re: [PATCH V3 1/2] virtio-net: fix the set affinity bug when CPU IDs are not consecutive
From: Ben Hutchings @ 2013-01-10 19:12 UTC (permalink / raw)
To: Rusty Russell
Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization,
Eric Dumazet
In-Reply-To: <87a9sh3lru.fsf@rustcorp.com.au>
On Thu, 2013-01-10 at 11:19 +1030, Rusty Russell wrote:
> Wanlong Gao <gaowanlong@cn.fujitsu.com> writes:
> > On 01/09/2013 07:31 AM, Rusty Russell wrote:
> >> Wanlong Gao <gaowanlong@cn.fujitsu.com> writes:
> >>> */
> >>> static u16 virtnet_select_queue(struct net_device *dev, struct sk_buff *skb)
> >>> {
> >>> - int txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
> >>> - smp_processor_id();
> >>> + int txq = 0;
> >>> +
> >>> + if (skb_rx_queue_recorded(skb))
> >>> + txq = skb_get_rx_queue(skb);
> >>> + else if ((txq = per_cpu(vq_index, smp_processor_id())) == -1)
> >>> + txq = 0;
> >>
> >> You should use __get_cpu_var() instead of smp_processor_id() here, ie:
> >>
> >> else if ((txq = __get_cpu_var(vq_index)) == -1)
> >>
> >> And AFAICT, no reason to initialize txq to 0 to start with.
> >>
> >> So:
> >>
> >> int txq;
> >>
> >> if (skb_rx_queue_recorded(skb))
> >> txq = skb_get_rx_queue(skb);
> >> else {
> >> txq = __get_cpu_var(vq_index);
> >> if (txq == -1)
> >> txq = 0;
> >> }
> >
> > Got it, thank you.
> >
> >>
> >> Now, just to confirm, I assume this can happen even if we use vq_index,
> >> right, because of races with virtnet_set_channels?
> >
> > I still can't understand this race, could you explain more? thank you.
>
> I assume that someone can call virtnet_set_channels() while we are
> inside virtnet_select_queue(), so they reduce dev->real_num_tx_queues,
> causing virtnet_set_channels to do:
>
> while (unlikely(txq >= dev->real_num_tx_queues))
> txq -= dev->real_num_tx_queues;
>
> Otherwise, when is this loop called?
In fact, this race can result in the TX scheduler using a queue that has
been disabled, or other weirdness (consider what happens if
real_num_tx_queues increases between those two uses).
virtnet_set_channels() really must disable TX temporarily:
netif_tx_lock(dev);
netif_device_detach(dev);
netif_tx_unlock(dev);
...
netif_device_attach(dev);
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
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