Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] pktgen: fix crash at module unload
From: David Miller @ 2012-05-11  3:12 UTC (permalink / raw)
  To: eric.dumazet; +Cc: ebiederm, netdev
In-Reply-To: <1336606191.12504.97.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 10 May 2012 01:29:51 +0200

> [PATCH v2] pktgen: fix crash at module unload
> 
> commit 7d3d43dab4e9 (net: In unregister_netdevice_notifier unregister
> the netdevices.) makes pktgen crashing at module unload.
...
> Hold the pktgen_thread_lock while splicing pktgen_threads, and test
> pktgen_exiting in pktgen_device_event() to make unload faster.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [Lksctp-developers] Bug: sctp packets are dropped after IPSEC rekeying (route cache issue)
From: David Miller @ 2012-05-11  3:15 UTC (permalink / raw)
  To: vyasevich
  Cc: nicolas.dichtel, babu.srinivasan, lksctp-developers, linux-sctp,
	netdev, michael.kreuzer
In-Reply-To: <CAGCdqXFjyWst22DJgxNfakz3-JwRq0Z+WZVj-XT=qRQFY5w3rw@mail.gmail.com>

From: Vladislav Yasevich <vyasevich@gmail.com>
Date: Fri, 4 May 2012 14:15:05 -0400

> Acked-by: Vlad Yaseivch <vyasevich@gmail.com>

Applied.

^ permalink raw reply

* Re: [net-next 00/12][pull request] Intel Wired LAN Driver Updates
From: David Miller @ 2012-05-11  3:18 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, gospo, sassmann
In-Reply-To: <1336632413-19135-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Wed,  9 May 2012 23:46:41 -0700

> This series of patches contains updates for igb and ixgbe.
> Special note ixgbe patches contain the addition of PTP support from
> Jacob Keller.
> 
> The following are changes since commit 8feedbb4a710784d2858acba5c90e903e93e36eb:
>   dsa: Convert compare_ether_addr to ether_addr_equal
> and are available in the git repository at:
>   git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master

Pulled, thanks Jeff.

^ permalink raw reply

* Re: [patch 1/1] connector/userns: replace netlink uses of cap_raised() with capable()
From: David Miller @ 2012-05-11  3:21 UTC (permalink / raw)
  To: akpm
  Cc: netdev, ebiederm, dhowells, james.l.morris, kaber, morgan,
	philipp.reisner, segoon, serge.hallyn
In-Reply-To: <20120504213403.BD3A4A0367@akpm.mtv.corp.google.com>

From: akpm@linux-foundation.org
Date: Fri, 04 May 2012 14:34:03 -0700

> In 2009 Philip Reiser notied that a few users of netlink connector
> interface needed a capability check and added the idiom
> cap_raised(nsp->eff_cap, CAP_SYS_ADMIN) to a few of them, on the premise
> that netlink was asynchronous.
> 
> In 2011 Patrick McHardy noticed we were being silly because netlink is
> synchronous and removed eff_cap from the netlink_skb_params and changed
> the idiom to cap_raised(current_cap(), CAP_SYS_ADMIN).
> 
> Looking at those spots with a fresh eye we should be calling
> capable(CAP_SYS_ADMIN).  The only reason I can see for not calling capable
> is that it once appeared we were not in the same task as the caller which
> would have made calling capable() impossible.
> 
> In the initial user_namespace the only difference between between
> cap_raised(current_cap(), CAP_SYS_ADMIN) and capable(CAP_SYS_ADMIN) are a
> few sanity checks and the fact that capable(CAP_SYS_ADMIN) sets
> PF_SUPERPRIV if we use the capability.
> 
> Since we are going to be using root privilege setting PF_SUPERPRIV seems
> the right thing to do.
> 
> The motivation for this that patch is that in a child user namespace
> cap_raised(current_cap(),...) tests your capabilities with respect to that
> child user namespace not capabilities in the initial user namespace and
> thus will allow processes that should be unprivielged to use the kernel
> services that are only protected with cap_raised(current_cap(),..).
> 
> To fix possible user_namespace issues and to just clean up the code
> replace cap_raised(current_cap(), CAP_SYS_ADMIN) with
> capable(CAP_SYS_ADMIN).
> 
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: Philipp Reisner <philipp.reisner@linbit.com>
> Acked-by: Serge E. Hallyn <serge.hallyn@canonical.com>
> Acked-by: Andrew G. Morgan <morgan@kernel.org>
> Cc: Vasiliy Kulikov <segoon@openwall.com>
> Cc: David Howells <dhowells@redhat.com>
> Reviewed-by: James Morris <james.l.morris@oracle.com>
> Cc: David Miller <davem@davemloft.net>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] irda: irda-usb.c URL to sigmatel stir 4210/4220/4116 fw from archive.org
From: David Miller @ 2012-05-11  3:23 UTC (permalink / raw)
  To: xose.vazquez; +Cc: samuel, netdev, linux-kernel
In-Reply-To: <1336167477-28835-1-git-send-email-xose.vazquez@gmail.com>

From: Xose Vazquez Perez <xose.vazquez@gmail.com>
Date: Fri,  4 May 2012 23:37:57 +0200

> @@ -1081,6 +1081,7 @@ static int stir421x_patch_device(struct irda_usb_cb *self)
>          /*
>           * Known firmware patch file names for STIR421x dongles
>           * are "42101001.sb" or "42101002.sb"
> +         * http://web.archive.org/web/http://www.sigmatel.com/documents/stir4210_4220_4116_patch_files.tar.gz

This really doesn't scale in the long term.

Someone needs to host this firmware at a location which serves the
needs of the people who have an interest in using this driver.

^ permalink raw reply

* Re: [PATCH 1/3] tcp: Move rcvq sending to tcp_input.c
From: David Miller @ 2012-05-11  3:25 UTC (permalink / raw)
  To: eric.dumazet; +Cc: xemul, netdev
In-Reply-To: <1336651214.12504.149.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 10 May 2012 14:00:14 +0200

> On Thu, 2012-05-10 at 15:49 +0400, Pavel Emelyanov wrote:
>> It actually works on the input queue and will use its read mem
>> routines, thus it's better to have in in the tcp_input.c file.
>> 
>> Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
>> ---
>>  include/net/tcp.h    |    3 +--
>>  net/ipv4/tcp.c       |   33 ---------------------------------
>>  net/ipv4/tcp_input.c |   35 ++++++++++++++++++++++++++++++++++-
>>  3 files changed, 35 insertions(+), 36 deletions(-)
> 
> Acked-by: Eric Dumazet <edumazet@google.com>

Applied.

 

^ permalink raw reply

* Re: [PATCH 2/3] tcp: Schedule rmem for rcvq repair send
From: David Miller @ 2012-05-11  3:25 UTC (permalink / raw)
  To: eric.dumazet; +Cc: xemul, netdev
In-Reply-To: <1336651229.12504.150.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 10 May 2012 14:00:29 +0200

> On Thu, 2012-05-10 at 15:50 +0400, Pavel Emelyanov wrote:
>> As noted by Eric, no checks are performed on the data size we're
>> putting in the read queue during repair. Thus, validate the given
>> data size with the common rmem management routine.
>> 
>> Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
>> ---
>>  net/ipv4/tcp_input.c |    3 +++
>>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> Acked-by: Eric Dumazet <edumazet@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH 3/3] tcp: Out-line tcp_try_rmem_schedule
From: David Miller @ 2012-05-11  3:25 UTC (permalink / raw)
  To: eric.dumazet; +Cc: xemul, netdev
In-Reply-To: <1336651256.12504.151.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 10 May 2012 14:00:56 +0200

> On Thu, 2012-05-10 at 15:50 +0400, Pavel Emelyanov wrote:
>> As proposed by Eric, make the tcp_input.o thinner.
>> 
>> add/remove: 1/1 grow/shrink: 1/4 up/down: 868/-1329 (-461)
>> function                                     old     new   delta
>> tcp_try_rmem_schedule                          -     864    +864
>> tcp_ack                                     4811    4815      +4
>> tcp_validate_incoming                        817     815      -2
>> tcp_collapse                                 860     858      -2
>> tcp_send_rcvq                                555     353    -202
>> tcp_data_queue                              3435    3033    -402
>> tcp_prune_queue                              721       -    -721
>> 
>> Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
>> ---
>>  net/ipv4/tcp_input.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> Acked-by: Eric Dumazet <edumazet@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next 1/2] l2tp: fix reorder timeout recovery
From: David Miller @ 2012-05-11  3:27 UTC (permalink / raw)
  To: jchapman; +Cc: netdev
In-Reply-To: <1336642989-4785-1-git-send-email-jchapman@katalix.com>

From: James Chapman <jchapman@katalix.com>
Date: Thu, 10 May 2012 10:43:08 +0100

> When L2TP data packet reordering is enabled, packets are held in a
> queue while waiting for out-of-sequence packets. If a packet gets
> lost, packets will be held until the reorder timeout expires, when we
> are supposed to then advance to the sequence number of the next packet
> but we don't currently do so. As a result, the data channel is stuck
> because we are waiting for a packet that will never arrive - all
> packets age out and none are passed.
> 
> The fix is to add a flag to the session context, which is set when the
> reorder timeout expires and tells the receive code to reset the next
> expected sequence number to that of the next packet in the queue.
> 
> Tested in a production L2TP network with Starent and Nortel L2TP gear.
> 
> Signed-off-by: James Chapman <jchapman@katalix.com>

Applied, but reorder_skip should really be a 'bool'.

^ permalink raw reply

* Re: [PATCH net-next 2/2] l2tp: fix data packet sequence number handling
From: David Miller @ 2012-05-11  3:27 UTC (permalink / raw)
  To: jchapman; +Cc: netdev
In-Reply-To: <1336642989-4785-2-git-send-email-jchapman@katalix.com>

From: James Chapman <jchapman@katalix.com>
Date: Thu, 10 May 2012 10:43:09 +0100

> If enabled, L2TP data packets have sequence numbers which a receiver
> can use to drop out of sequence frames or try to reorder them. The
> first frame has sequence number 0, but the L2TP code currently expects
> it to be 1. This results in the first data frame being handled as out
> of sequence.
> 
> This one-line patch fixes the problem.
> 
> Signed-off-by: James Chapman <jchapman@katalix.com>

Applied.

^ permalink raw reply

* Re: [PATCH v3][resend] bonding: don't increase rx_dropped after processing LACPDUs
From: David Miller @ 2012-05-11  3:31 UTC (permalink / raw)
  To: jbohac; +Cc: andy, netdev, fubar
In-Reply-To: <20120509110140.GA9817@midget.suse.cz>

From: Jiri Bohac <jbohac@suse.cz>
Date: Wed, 9 May 2012 13:01:40 +0200

> Since commit 3aba891d, bonding processes LACP frames (802.3ad
> mode) with bond_handle_frame(). Currently a copy of the skb is
> made and the original is left to be processed by other
> rx_handlers and the rest of the network stack by returning
> RX_HANDLER_ANOTHER.  As there is no protocol handler for
> PKT_TYPE_LACPDU, the frame is dropped and dev->rx_dropped
> increased.
> 
> Fix this by making bond_handle_frame() return RX_HANDLER_CONSUMED
> if bonding has processed the LACP frame.
> 
> Signed-off-by: Jiri Bohac <jbohac@suse.cz>
> 

Don't send me garbage you didn't even check the compile of:

drivers/net/bonding/bond_main.c: In function ‘bond_handle_frame’:
drivers/net/bonding/bond_main.c:1463:13: warning: assignment from incompatible pointer type [enabled by default]
drivers/net/bonding/bond_main.c: In function ‘bond_open’:
drivers/net/bonding/bond_main.c:3441:21: warning: assignment from incompatible pointer type [enabled by default]
drivers/net/bonding/bond_main.c:3448:20: warning: assignment from incompatible pointer type [enabled by default]


^ permalink raw reply

* Re: [PATCH net-next] be2net: avoid disabling sriov while VFs are assigned
From: David Miller @ 2012-05-11  3:35 UTC (permalink / raw)
  To: sathya.perla; +Cc: netdev
In-Reply-To: <647fcd25-8a85-4d63-a1ef-8b867832c2c7@exht1.ad.emulex.com>

From: Sathya Perla <sathya.perla@emulex.com>
Date: Wed, 9 May 2012 11:11:24 +0530

> Calling pci_disable_sriov() while VFs are assigned to VMs causes
> kernel panic. This patch uses PCI_DEV_FLAGS_ASSIGNED bit state of the
> VF's pci_dev to avoid this. Also, the unconditional function reset cmd
> issued on a PF probe can delete the VF configuration for the
> previously enabled VFs. A scratchpad register is now used to issue a
> function reset only when needed (i.e., in a crash dump scenario.)
> 
> Signed-off-by: Sathya Perla <sathya.perla@emulex.com>

Applied.

^ permalink raw reply

* Re: [PATCH 1/3] drivers/net: Convert compare_ether_addr to ether_addr_equal
From: David Miller @ 2012-05-11  3:35 UTC (permalink / raw)
  To: joe; +Cc: linux-kernel, netdev, linux-wireless, ath5k-devel, ath9k-devel
In-Reply-To: <7c9881a67c52c2f218480b6742155b6d6928122d.1336618708.git.joe@perches.com>

From: Joe Perches <joe@perches.com>
Date: Wed,  9 May 2012 20:17:46 -0700

> Use the new bool function ether_addr_equal to add
> some clarity and reduce the likelihood for misuse
> of compare_ether_addr for sorting.
> 
> Done via cocci script:
 ...
> Signed-off-by: Joe Perches <joe@perches.com>

Applied.

^ permalink raw reply

* Re: [PATCH 2/3] etherdevice.h: Add ether_addr_equal_64bits
From: David Miller @ 2012-05-11  3:35 UTC (permalink / raw)
  To: joe; +Cc: linux-kernel, netdev
In-Reply-To: <7535e70ceb63f7da56f73cb37d4b269cd7798583.1336618708.git.joe@perches.com>

From: Joe Perches <joe@perches.com>
Date: Wed,  9 May 2012 20:04:03 -0700

> Add an optimized boolean function to check if
> 2 ethernet addresses are the same.
> 
> This is to avoid any confusion about compare_ether_addr_64bits
> returning an unsigned, and not being able to use the
> compare_ether_addr_64bits function for sorting ala memcmp.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Applied.

^ permalink raw reply

* Re: [PATCH 3/3] net,drivers/net: Convert compare_ether_addr_64bits to ether_addr_equal_64bits
From: David Miller @ 2012-05-11  3:35 UTC (permalink / raw)
  To: joe; +Cc: fubar, andy, kaber, netdev, linux-kernel
In-Reply-To: <acb363688c8ed729cc61e288605cdb4aa4089149.1336618708.git.joe@perches.com>

From: Joe Perches <joe@perches.com>
Date: Wed,  9 May 2012 20:04:04 -0700

> Use the new bool function ether_addr_equal_64bits to add
> some clarity and reduce the likelihood for misuse of
> compare_ether_addr_64bits for sorting.
> 
> Done via cocci script:
 ...
> Signed-off-by: Joe Perches <joe@perches.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] net_sched: update bstats in dequeue()
From: David Miller @ 2012-05-11  3:36 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1336664194.31653.8.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 10 May 2012 17:36:34 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> Class bytes/packets stats can be misleading because they are updated in
> enqueue() while packet might be dropped later.
> 
> We already fixed all qdiscs but sch_atm.
> 
> This patch makes the final cleanup.
> 
> class rate estimators can now match qdisc ones.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH v13 net-next] codel: Controlled Delay AQM
From: David Miller @ 2012-05-11  3:36 UTC (permalink / raw)
  To: eric.dumazet
  Cc: dave.taht, nichols, van, codel, bloat, netdev, therbert, ycheng,
	mattmathis, shemminger
In-Reply-To: <1336672285.31653.102.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 10 May 2012 19:51:25 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> An implementation of CoDel AQM, from Kathleen Nichols and Van Jacobson. 
 ...
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Dave Taht <dave.taht@bufferbloat.net>

Applied, but there was a lot of trailing whitespace and a few
comments mis-formatted which I had to fix up.

Thanks.

^ permalink raw reply

* Re: [PATCH net-next] 6lowpan: IPv6 link local address
From: David Miller @ 2012-05-11  3:38 UTC (permalink / raw)
  To: alex.bluesman.smirnov; +Cc: netdev
In-Reply-To: <1336656352-19518-1-git-send-email-alex.bluesman.smirnov@gmail.com>

From: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
Date: Thu, 10 May 2012 17:25:52 +0400

> According to the RFC4944 (Transmission of IPv6 Packets over
> IEEE 802.15.4 Networks), chapter 7:
> 
> The IPv6 link-local address [RFC4291] for an IEEE 802.15.4 interface
> is formed by appending the Interface Identifier, as defined above, to
> the prefix FE80::/64.
> 
>   10 bits            54 bits                  64 bits
> +----------+-----------------------+----------------------------+
> |1111111010|         (zeros)       |    Interface Identifier    |
> +----------+-----------------------+----------------------------+
> 
> This patch adds IPv6 address generation support for the 6lowpan
> interfaces.
> 
> Signed-off-by: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH 05/17] mm: allow PF_MEMALLOC from softirq context
From: David Miller @ 2012-05-11  4:39 UTC (permalink / raw)
  To: mgorman
  Cc: akpm, linux-mm, netdev, linux-kernel, neilb, a.p.zijlstra,
	michaelc, emunson
In-Reply-To: <1336657510-24378-6-git-send-email-mgorman@suse.de>

From: Mel Gorman <mgorman@suse.de>
Date: Thu, 10 May 2012 14:44:58 +0100

> This is needed to allow network softirq packet processing to make
> use of PF_MEMALLOC.
> 
> Currently softirq context cannot use PF_MEMALLOC due to it not being
> associated with a task, and therefore not having task flags to fiddle
> with - thus the gfp to alloc flag mapping ignores the task flags when
> in interrupts (hard or soft) context.
> 
> Allowing softirqs to make use of PF_MEMALLOC therefore requires some
> trickery.  We basically borrow the task flags from whatever process
> happens to be preempted by the softirq.
> 
> So we modify the gfp to alloc flags mapping to not exclude task flags
> in softirq context, and modify the softirq code to save, clear and
> restore the PF_MEMALLOC flag.
> 
> The save and clear, ensures the preempted task's PF_MEMALLOC flag
> doesn't leak into the softirq. The restore ensures a softirq's
> PF_MEMALLOC flag cannot leak back into the preempted process.
> 
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Signed-off-by: Mel Gorman <mgorman@suse.de>

We're now making changes to task->flags from both base and
softirq context, but with non-atomic operations and no other
kind of synchronization.

As far as I can tell, this has to be racy.

If this works via some magic combination of invariants, you
absolutely have to document this, verbosely.

--
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] wlcore: wlcore should depend on MAC80211
From: Sasha Levin @ 2012-05-11  4:39 UTC (permalink / raw)
  To: coelho, linville; +Cc: linux-wireless, netdev, linux-kernel, Sasha Levin

wlcore can't be built if MAC80211 isn't set.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 drivers/net/wireless/ti/wlcore/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/Kconfig b/drivers/net/wireless/ti/wlcore/Kconfig
index 9d04c38..54156b0 100644
--- a/drivers/net/wireless/ti/wlcore/Kconfig
+++ b/drivers/net/wireless/ti/wlcore/Kconfig
@@ -1,6 +1,6 @@
 config WLCORE
 	tristate "TI wlcore support"
-	depends on WL_TI && GENERIC_HARDIRQS
+	depends on WL_TI && GENERIC_HARDIRQS && MAC80211
 	depends on INET
 	select FW_LOADER
 	---help---
-- 
1.7.8.5

^ permalink raw reply related

* Re: [PATCH 08/17] net: Introduce sk_allocation() to allow addition of GFP flags depending on the individual socket
From: David Miller @ 2012-05-11  4:49 UTC (permalink / raw)
  To: mgorman
  Cc: akpm, linux-mm, netdev, linux-kernel, neilb, a.p.zijlstra,
	michaelc, emunson
In-Reply-To: <1336657510-24378-9-git-send-email-mgorman@suse.de>

From: Mel Gorman <mgorman@suse.de>
Date: Thu, 10 May 2012 14:45:01 +0100

> Introduce sk_allocation(), this function allows to inject sock specific
> flags to each sock related allocation. It is only used on allocation
> paths that may be required for writing pages back to network storage.
> 
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Signed-off-by: Mel Gorman <mgorman@suse.de>

This is still a little bit more than it needs to be.

You are trying to propagate a single bit from sk->sk_allocation into
all of the annotated socket memory allocation sites.

But many of them use sk->sk_allocation already.  In fact all of them
that use a variable rather than a constant GFP_* satisfy this
invariant.

All of those annotations are therefore spurious, and probably end up
generating unnecessary |'s in of that special bit in at least some
cases.

What you really, therefore, care about are the GFP_FOO cases.  And in
fact those are all GFP_ATOMIC.  So make something that says what it
is that you want, a GFP_ATOMIC with some socket specified bits |'d
in.

Something like this:

static inline gfp_t sk_gfp_atomic(struct sock *sk)
{
	return GFP_ATOMIC | (sk->sk_allocation & __GFP_MEMALLOC);
}

You'll also have to make your networking patches conform to the
networking subsystem coding style.

For example:

> -	skb = sock_wmalloc(sk, MAX_TCP_HEADER + 15 + s_data_desired, 1, GFP_ATOMIC);
> +	skb = sock_wmalloc(sk, MAX_TCP_HEADER + 15 + s_data_desired, 1,
> +					sk_allocation(sk, GFP_ATOMIC));

The sk_allocation() argument has to line up with the first column
after the openning parenthesis of the function call.  You can't just
use all TAB characters.  And this all TABs thing looks extremely ugly
to boot.

> -		newnp->pktoptions = skb_clone(treq->pktopts, GFP_ATOMIC);
> +		newnp->pktoptions = skb_clone(treq->pktopts,
> +						sk_allocation(sk, GFP_ATOMIC));

Same here.

What's really funny to me is that in several cases elsewhere in this
pach you get it right.

--
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 09/17] netvm: Allow the use of __GFP_MEMALLOC by specific sockets
From: David Miller @ 2012-05-11  4:50 UTC (permalink / raw)
  To: mgorman
  Cc: akpm, linux-mm, netdev, linux-kernel, neilb, a.p.zijlstra,
	michaelc, emunson
In-Reply-To: <1336657510-24378-10-git-send-email-mgorman@suse.de>

From: Mel Gorman <mgorman@suse.de>
Date: Thu, 10 May 2012 14:45:02 +0100

> Allow specific sockets to be tagged SOCK_MEMALLOC and use
> __GFP_MEMALLOC for their allocations. These sockets will be able to go
> below watermarks and allocate from the emergency reserve. Such sockets
> are to be used to service the VM (iow. to swap over). They must be
> handled kernel side, exposing such a socket to user-space is a bug.
> 
> There is a risk that the reserves be depleted so for now, the
> administrator is responsible for increasing min_free_kbytes as
> necessary to prevent deadlock for their workloads.
> 
> [a.p.zijlstra@chello.nl: Original patches]
> Signed-off-by: Mel Gorman <mgorman@suse.de>

After sk_allocation() is adjusted to be sk_gfp_atomic() as I suggested
in my feedback for patch #8, this is fine.

--
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 10/17] netvm: Allow skb allocation to use PFMEMALLOC reserves
From: David Miller @ 2012-05-11  4:57 UTC (permalink / raw)
  To: mgorman
  Cc: akpm, linux-mm, netdev, linux-kernel, neilb, a.p.zijlstra,
	michaelc, emunson
In-Reply-To: <1336657510-24378-11-git-send-email-mgorman@suse.de>

From: Mel Gorman <mgorman@suse.de>
Date: Thu, 10 May 2012 14:45:03 +0100

> +/* Returns true if the gfp_mask allows use of ALLOC_NO_WATERMARK */
> +bool gfp_pfmemalloc_allowed(gfp_t gfp_mask);

I know this gets added in an earlier patch, but it seems slightly
overkill to have a function call just for a simply bit test.

> +extern atomic_t memalloc_socks;
> +static inline int sk_memalloc_socks(void)
> +{
> +	return atomic_read(&memalloc_socks);
> +}

Please change this to be a static branch.

> +	skb = __alloc_skb(length + NET_SKB_PAD, gfp_mask,
> +						SKB_ALLOC_RX, NUMA_NO_NODE);

Please fix the argument indentation.

> +	data = kmalloc_reserve(size + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)),
> +		       gfp_mask, NUMA_NO_NODE, NULL);

Likewise.

> -	struct sk_buff *n = alloc_skb(newheadroom + skb->len + newtailroom,
> -				      gfp_mask);
> +	struct sk_buff *n = __alloc_skb(newheadroom + skb->len + newtailroom,
> +				      gfp_mask, skb_alloc_rx_flag(skb),
> +				      NUMA_NO_NODE);

Likewise.

> -			nskb = alloc_skb(hsize + doffset + headroom,
> -					 GFP_ATOMIC);
> +			nskb = __alloc_skb(hsize + doffset + headroom,
> +					 GFP_ATOMIC, skb_alloc_rx_flag(skb),
> +					 NUMA_NO_NODE);

Likewise.

--
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 12/17] netvm: Propagate page->pfmemalloc from netdev_alloc_page to skb
From: David Miller @ 2012-05-11  5:01 UTC (permalink / raw)
  To: mgorman
  Cc: akpm, linux-mm, netdev, linux-kernel, neilb, a.p.zijlstra,
	michaelc, emunson
In-Reply-To: <1336657510-24378-13-git-send-email-mgorman@suse.de>

From: Mel Gorman <mgorman@suse.de>
Date: Thu, 10 May 2012 14:45:05 +0100

> +/**
> + *	propagate_pfmemalloc_skb - Propagate pfmemalloc if skb is allocated after RX page
> + *	@page: The page that was allocated from netdev_alloc_page
> + *	@skb: The skb that may need pfmemalloc set
> + */
> +static inline void propagate_pfmemalloc_skb(struct page *page,
> +						struct sk_buff *skb)

Please use consistent prefixes in the names for new interfaces.

This one should probably be named "skb_propagate_pfmemalloc()" and
go into skbuff.h since it needs no knowledge of netdevices.

In fact all of these routines are about propagation of attributes
into SKBs, irregardless of any netdevice details.

Therefore they should probably all be named skb_*() and go into
skbuff.h

--
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 13/17] netvm: Set PF_MEMALLOC as appropriate during SKB processing
From: David Miller @ 2012-05-11  5:03 UTC (permalink / raw)
  To: mgorman
  Cc: akpm, linux-mm, netdev, linux-kernel, neilb, a.p.zijlstra,
	michaelc, emunson
In-Reply-To: <1336657510-24378-14-git-send-email-mgorman@suse.de>

From: Mel Gorman <mgorman@suse.de>
Date: Thu, 10 May 2012 14:45:06 +0100

> In order to make sure pfmemalloc packets receive all memory
> needed to proceed, ensure processing of pfmemalloc SKBs happens
> under PF_MEMALLOC. This is limited to a subset of protocols that
> are expected to be used for writing to swap. Taps are not allowed to
> use PF_MEMALLOC as these are expected to communicate with userspace
> processes which could be paged out.
> 
> [a.p.zijlstra@chello.nl: Ideas taken from various patches]
> [jslaby@suse.cz: Lock imbalance fix]
> Signed-off-by: Mel Gorman <mgorman@suse.de>

This adds more code where we're modifying task->flags from software
interrupt context.  I'm not convinced that's safe.

Also, this starts to add new tests in the fast paths.

Most of the time they are not going to trigger at all.

Please use the static branch I asked you to add in a previous
patch to mitigate this.

--
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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox