* [PATCH v6 1/8] Basic kernel memory functionality for the Memory Controller
From: Glauber Costa @ 2011-10-10 10:24 UTC (permalink / raw)
To: linux-kernel
Cc: paul, lizf, kamezawa.hiroyu, ebiederm, davem, gthelen, netdev,
linux-mm, kirill, avagin, devel, Glauber Costa
In-Reply-To: <1318242268-2234-1-git-send-email-glommer@parallels.com>
This patch lays down the foundation for the kernel memory component
of the Memory Controller.
As of today, I am only laying down the following files:
* memory.independent_kmem_limit
* memory.kmem.limit_in_bytes (currently ignored)
* memory.kmem.usage_in_bytes (always zero)
Signed-off-by: Glauber Costa <glommer@parallels.com>
Reviewed-by: Kirill A. Shutemov <kirill@shutemov.name>
CC: Paul Menage <paul@paulmenage.org>
CC: Greg Thelen <gthelen@google.com>
---
Documentation/cgroups/memory.txt | 36 ++++++++++++++-
init/Kconfig | 14 ++++++
mm/memcontrol.c | 91 +++++++++++++++++++++++++++++++++++--
3 files changed, 134 insertions(+), 7 deletions(-)
diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt
index 06eb6d9..bf00cd2 100644
--- a/Documentation/cgroups/memory.txt
+++ b/Documentation/cgroups/memory.txt
@@ -44,8 +44,9 @@ Features:
- oom-killer disable knob and oom-notifier
- Root cgroup has no limit controls.
- Kernel memory and Hugepages are not under control yet. We just manage
- pages on LRU. To add more controls, we have to take care of performance.
+ Hugepages is not under control yet. We just manage pages on LRU. To add more
+ controls, we have to take care of performance. Kernel memory support is work
+ in progress, and the current version provides basically functionality.
Brief summary of control files.
@@ -56,8 +57,11 @@ Brief summary of control files.
(See 5.5 for details)
memory.memsw.usage_in_bytes # show current res_counter usage for memory+Swap
(See 5.5 for details)
+ memory.kmem.usage_in_bytes # show current res_counter usage for kmem only.
+ (See 2.7 for details)
memory.limit_in_bytes # set/show limit of memory usage
memory.memsw.limit_in_bytes # set/show limit of memory+Swap usage
+ memory.kmem.limit_in_bytes # if allowed, set/show limit of kernel memory
memory.failcnt # show the number of memory usage hits limits
memory.memsw.failcnt # show the number of memory+Swap hits limits
memory.max_usage_in_bytes # show max memory usage recorded
@@ -72,6 +76,9 @@ Brief summary of control files.
memory.oom_control # set/show oom controls.
memory.numa_stat # show the number of memory usage per numa node
+ memory.independent_kmem_limit # select whether or not kernel memory limits are
+ independent of user limits
+
1. History
The memory controller has a long history. A request for comments for the memory
@@ -255,6 +262,31 @@ When oom event notifier is registered, event will be delivered.
per-zone-per-cgroup LRU (cgroup's private LRU) is just guarded by
zone->lru_lock, it has no lock of its own.
+2.7 Kernel Memory Extension (CONFIG_CGROUP_MEM_RES_CTLR_KMEM)
+
+ With the Kernel memory extension, the Memory Controller is able to limit
+the amount of kernel memory used by the system. Kernel memory is fundamentally
+different than user memory, since it can't be swapped out, which makes it
+possible to DoS the system by consuming too much of this precious resource.
+Kernel memory limits are not imposed for the root cgroup.
+
+Memory limits as specified by the standard Memory Controller may or may not
+take kernel memory into consideration. This is achieved through the file
+memory.independent_kmem_limit. A Value different than 0 will allow for kernel
+memory to be controlled separately.
+
+When kernel memory limits are not independent, the limit values set in
+memory.kmem files are ignored.
+
+Currently no soft limit is implemented for kernel memory. It is future work
+to trigger slab reclaim when those limits are reached.
+
+CAUTION: As of this writing, the kmem extention may prevent tasks from moving
+among cgroups. If a task has kmem accounting in a cgroup, the task cannot be
+moved until the kmem resource is released. Also, until the resource is fully
+released, the cgroup cannot be destroyed. So, please consider your use cases
+and set kmem extention config option carefully.
+
3. User Interface
0. Configuration
diff --git a/init/Kconfig b/init/Kconfig
index d627783..b62b9e0 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -689,6 +689,20 @@ config CGROUP_MEM_RES_CTLR_SWAP_ENABLED
For those who want to have the feature enabled by default should
select this option (if, for some reason, they need to disable it
then swapaccount=0 does the trick).
+config CGROUP_MEM_RES_CTLR_KMEM
+ bool "Memory Resource Controller Kernel Memory accounting (EXPERIMENTAL)"
+ depends on CGROUP_MEM_RES_CTLR && EXPERIMENTAL
+ default n
+ help
+ The Kernel Memory extension for Memory Resource Controller can limit
+ the amount of memory used by kernel objects in the system. Those are
+ fundamentally different from the entities handled by the standard
+ Memory Controller, which are page-based, and can be swapped. Users of
+ the kmem extension can use it to guarantee that no group of processes
+ will ever exhaust kernel resources alone.
+
+ WARNING: The current experimental implementation does not allow a
+ task to move among different cgroups with a kmem resource being held.
config CGROUP_PERF
bool "Enable perf_event per-cpu per-container group (cgroup) monitoring"
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 3508777..d25c5cb 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -226,6 +226,10 @@ struct mem_cgroup {
*/
struct res_counter memsw;
/*
+ * the counter to account for kmem usage.
+ */
+ struct res_counter kmem;
+ /*
* Per cgroup active and inactive list, similar to the
* per zone LRU lists.
*/
@@ -276,6 +280,11 @@ struct mem_cgroup {
*/
unsigned long move_charge_at_immigrate;
/*
+ * Should kernel memory limits be stabilished independently
+ * from user memory ?
+ */
+ int kmem_independent_accounting;
+ /*
* percpu counter.
*/
struct mem_cgroup_stat_cpu *stat;
@@ -343,9 +352,14 @@ enum charge_type {
};
/* for encoding cft->private value on file */
-#define _MEM (0)
-#define _MEMSWAP (1)
-#define _OOM_TYPE (2)
+
+enum mem_type {
+ _MEM = 0,
+ _MEMSWAP,
+ _OOM_TYPE,
+ _KMEM,
+};
+
#define MEMFILE_PRIVATE(x, val) (((x) << 16) | (val))
#define MEMFILE_TYPE(val) (((val) >> 16) & 0xffff)
#define MEMFILE_ATTR(val) ((val) & 0xffff)
@@ -3837,10 +3851,15 @@ static inline u64 mem_cgroup_usage(struct mem_cgroup *mem, bool swap)
u64 val;
if (!mem_cgroup_is_root(mem)) {
+ val = 0;
+ if (!mem->kmem_independent_accounting)
+ val = res_counter_read_u64(&mem->kmem, RES_USAGE);
if (!swap)
- return res_counter_read_u64(&mem->res, RES_USAGE);
+ val += res_counter_read_u64(&mem->res, RES_USAGE);
else
- return res_counter_read_u64(&mem->memsw, RES_USAGE);
+ val += res_counter_read_u64(&mem->memsw, RES_USAGE);
+
+ return val;
}
val = mem_cgroup_recursive_stat(mem, MEM_CGROUP_STAT_CACHE);
@@ -3873,6 +3892,10 @@ static u64 mem_cgroup_read(struct cgroup *cont, struct cftype *cft)
else
val = res_counter_read_u64(&mem->memsw, name);
break;
+ case _KMEM:
+ val = res_counter_read_u64(&mem->kmem, name);
+ break;
+
default:
BUG();
break;
@@ -4603,6 +4626,22 @@ static int mem_control_numa_stat_open(struct inode *unused, struct file *file)
}
#endif /* CONFIG_NUMA */
+#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM
+static u64 kmem_limit_independent_read(struct cgroup *cont, struct cftype *cft)
+{
+ return mem_cgroup_from_cont(cont)->kmem_independent_accounting;
+}
+
+static int kmem_limit_independent_write(struct cgroup *cont, struct cftype *cft,
+ u64 val)
+{
+ cgroup_lock();
+ mem_cgroup_from_cont(cont)->kmem_independent_accounting = !!val;
+ cgroup_unlock();
+ return 0;
+}
+#endif
+
static struct cftype mem_cgroup_files[] = {
{
.name = "usage_in_bytes",
@@ -4718,6 +4757,42 @@ static int register_memsw_files(struct cgroup *cont, struct cgroup_subsys *ss)
}
#endif
+
+#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM
+static struct cftype kmem_cgroup_files[] = {
+ {
+ .name = "independent_kmem_limit",
+ .read_u64 = kmem_limit_independent_read,
+ .write_u64 = kmem_limit_independent_write,
+ },
+ {
+ .name = "kmem.usage_in_bytes",
+ .private = MEMFILE_PRIVATE(_KMEM, RES_USAGE),
+ .read_u64 = mem_cgroup_read,
+ },
+ {
+ .name = "kmem.limit_in_bytes",
+ .private = MEMFILE_PRIVATE(_KMEM, RES_LIMIT),
+ .read_u64 = mem_cgroup_read,
+ },
+};
+
+static int register_kmem_files(struct cgroup *cont, struct cgroup_subsys *ss)
+{
+ int ret = 0;
+
+ ret = cgroup_add_files(cont, ss, kmem_cgroup_files,
+ ARRAY_SIZE(kmem_cgroup_files));
+ return ret;
+};
+
+#else
+static int register_kmem_files(struct cgroup *cont, struct cgroup_subsys *ss)
+{
+ return 0;
+}
+#endif
+
static int alloc_mem_cgroup_per_zone_info(struct mem_cgroup *mem, int node)
{
struct mem_cgroup_per_node *pn;
@@ -4916,6 +4991,7 @@ mem_cgroup_create(struct cgroup_subsys *ss, struct cgroup *cont)
if (parent && parent->use_hierarchy) {
res_counter_init(&mem->res, &parent->res);
res_counter_init(&mem->memsw, &parent->memsw);
+ res_counter_init(&mem->kmem, &parent->kmem);
/*
* We increment refcnt of the parent to ensure that we can
* safely access it on res_counter_charge/uncharge.
@@ -4926,6 +5002,7 @@ mem_cgroup_create(struct cgroup_subsys *ss, struct cgroup *cont)
} else {
res_counter_init(&mem->res, NULL);
res_counter_init(&mem->memsw, NULL);
+ res_counter_init(&mem->kmem, NULL);
}
mem->last_scanned_child = 0;
mem->last_scanned_node = MAX_NUMNODES;
@@ -4969,6 +5046,10 @@ static int mem_cgroup_populate(struct cgroup_subsys *ss,
if (!ret)
ret = register_memsw_files(cont, ss);
+
+ if (!ret)
+ ret = register_kmem_files(cont, ss);
+
return ret;
}
--
1.7.6.4
--
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 related
* [PATCH v6 0/8] per-cgroup tcp memory pressure handling
From: Glauber Costa @ 2011-10-10 10:24 UTC (permalink / raw)
To: linux-kernel
Cc: paul, lizf, kamezawa.hiroyu, ebiederm, davem, gthelen, netdev,
linux-mm, kirill, avagin, devel
Hi Guys,
I hope we have it all sorted this time. Main differences from
v5:
* using res_counters for tcp_memory_allocated
* tried to get maximum consistency with the rest of memcg files
Now files appear even in root memcg, but we can't set any limits
on it. (comes more or less for free with re-use of the files)
* softlimit and failcnt are not shown, but I intend to introduce them
in a followup series (failcnt is easy, but softlimit may need some more
network-side work, at least testing)
I will be happy to address any comments you may have.
Glauber Costa (8):
Basic kernel memory functionality for the Memory Controller
socket: initial cgroup code.
foundations of per-cgroup memory pressure controlling.
per-cgroup tcp buffers control
per-netns ipv4 sysctl_tcp_mem
tcp buffer limitation: per-cgroup limit
Display current tcp memory allocation in kmem cgroup
Disable task moving when using kernel memory accounting
Documentation/cgroups/memory.txt | 38 ++++-
crypto/af_alg.c | 8 +-
include/linux/memcontrol.h | 49 +++++
include/net/netns/ipv4.h | 1 +
include/net/sock.h | 130 +++++++++++++-
include/net/tcp.h | 30 +++-
include/net/udp.h | 4 +-
include/trace/events/sock.h | 10 +-
init/Kconfig | 14 ++
mm/memcontrol.c | 375 ++++++++++++++++++++++++++++++++++++--
net/core/sock.c | 104 ++++++++---
net/decnet/af_decnet.c | 22 ++-
net/ipv4/proc.c | 7 +-
net/ipv4/sysctl_net_ipv4.c | 71 +++++++-
net/ipv4/tcp.c | 60 ++++---
net/ipv4/tcp_input.c | 12 +-
net/ipv4/tcp_ipv4.c | 23 ++-
net/ipv4/tcp_output.c | 2 +-
net/ipv4/tcp_timer.c | 2 +-
net/ipv4/udp.c | 21 ++-
net/ipv6/tcp_ipv6.c | 20 ++-
net/ipv6/udp.c | 4 +-
net/sctp/socket.c | 37 +++-
23 files changed, 912 insertions(+), 132 deletions(-)
--
1.7.6.4
--
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: e100 + VLANs?
From: David Lamparter @ 2011-10-10 10:19 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: Eric Dumazet, Michael Tokarev, netdev
In-Reply-To: <4E9097C0.2030307@gmail.com>
On Sat, Oct 08, 2011 at 11:34:40AM -0700, Jeff Kirsher wrote:
> On 10/08/2011 09:24 AM, Eric Dumazet wrote:
> > Le samedi 08 octobre 2011 à 14:08 +0400, Michael Tokarev a écrit :
> >> > Yesterday I tried to use 802.1Q VLAN tagging with an (oldish)
> >> > e100-driven network card, identified by lspci like this:
> >> >
> >> > 00:12.0 Ethernet controller: Intel Corporation 82557/8/9/0/1 Ethernet Pro 100 (rev 02)
> >> >
> >> > Just to discover that it does not quite work: packets of
> >> > size 1497+ bytes gets lost.
[...]
> > e100 driver seems VLAN enabled at a first glance.
> Eric is correct, that e100 does support VLANs.
>
> In addition to Eric's suggestion, can you also provide all the output of
> lspci -vvv for the network card?
I'm opening the lore box here, but early e100 cards AFAIK have a
hardware limit at 1500 (+18 src/dst/proto) bytes. At least, Juniper's
JUNOS does not support full-sized .1Q on their e100 control plane
interfaces...
-equi
^ permalink raw reply
* Re: [PATCH] mlx4_en: fix transmit of packages when blue frame is enabled
From: Benjamin Herrenschmidt @ 2011-10-10 10:18 UTC (permalink / raw)
To: Eli Cohen
Cc: David Laight, netdev, Eli Cohen, linuxppc-dev,
Thadeu Lima de Souza Cascardo, Yevgeny Petrilin
In-Reply-To: <20111010092926.GO2681@mtldesk30>
On Mon, 2011-10-10 at 11:29 +0200, Eli Cohen wrote:
> On Mon, Oct 10, 2011 at 11:24:05AM +0200, Benjamin Herrenschmidt wrote:
> > On Mon, 2011-10-10 at 11:16 +0200, Eli Cohen wrote:
> >
> > > Until then I think we need to have the logic working right on ppc and
> > > measure if blue flame buys us any improvement in ppc. If that's not
> > > the case (e.g because write combining is not working), then maybe we
> > > should avoid using blueflame in ppc.
> > > Could any of the guys from IBM check this and give us feedback?
> >
> > I don't have the necessary hardware myself to test that but maybe Thadeu
> > can.
> >
> > Note that for WC to work, things must be mapped non-guarded. You can do
> > that by using ioremap_prot() with pgprot_noncached_wc(PAGE_KERNEL) or
> > ioremap_wc() (dunno how "generic" the later is).
>
> I use the io mapping API:
>
> at driver statrt:
> priv->bf_mapping = io_mapping_create_wc(bf_start, bf_len);
> if (!priv->bf_mapping)
> err = -ENOMEM;
>
> and then:
> uar->bf_map = io_mapping_map_wc(priv->bf_mapping, uar->index << PAGE_SHIFT);
>
>
> Will this work on ppc?
That API has never been tested on ppc I suspect. We don't have
CONFIG_HAVE_ATOMIC_IOMAP (mostly because we never needed it, it
was designed and only ever used for Intel graphics before), so
it will fallback to:
static inline struct io_mapping *
io_mapping_create_wc(resource_size_t base, unsigned long size)
{
return (struct io_mapping __force *) ioremap_wc(base, size);
}
Which should work (hopefully :-)
Cheers,
Ben.
^ permalink raw reply
* [PATCH net] bnx2x: fix cl_id allocation for non-eth clients for NPAR mode
From: Dmitry Kravkov @ 2011-10-10 9:57 UTC (permalink / raw)
To: davem, netdev; +Cc: Dmitry Kravkov, Eilon Greenstein
There are some consolidations of NPAR configuration
when FCoE and iSCSI L2 clients will get the same id,
in this case FCoE ring will be non-functional.
Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/bnx2x/bnx2x.h | 20 +++++++++++++-------
drivers/net/bnx2x/bnx2x_cmn.h | 2 +-
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/net/bnx2x/bnx2x.h b/drivers/net/bnx2x/bnx2x.h
index e46df53..9a7eb3b 100644
--- a/drivers/net/bnx2x/bnx2x.h
+++ b/drivers/net/bnx2x/bnx2x.h
@@ -239,13 +239,19 @@ void bnx2x_int_disable(struct bnx2x *bp);
* FUNC_N_CLID_X = N * NUM_SPECIAL_CLIENTS + FUNC_0_CLID_X
*
*/
-/* iSCSI L2 */
-#define BNX2X_ISCSI_ETH_CL_ID_IDX 1
-#define BNX2X_ISCSI_ETH_CID 49
+enum {
+ BNX2X_ISCSI_ETH_CL_ID_IDX,
+ BNX2X_FCOE_ETH_CL_ID_IDX,
+ BNX2X_MAX_CNIC_ETH_CL_ID_IDX,
+};
-/* FCoE L2 */
-#define BNX2X_FCOE_ETH_CL_ID_IDX 2
-#define BNX2X_FCOE_ETH_CID 50
+#define BNX2X_CNIC_START_ETH_CID 48
+enum {
+ /* iSCSI L2 */
+ BNX2X_ISCSI_ETH_CID = BNX2X_CNIC_START_ETH_CID,
+ /* FCoE L2 */
+ BNX2X_FCOE_ETH_CID,
+};
/** Additional rings budgeting */
#ifdef BCM_CNIC
diff --git a/drivers/net/bnx2x/bnx2x_cmn.h b/drivers/net/bnx2x/bnx2x_cmn.h
index 223bfee..2dc1199 100644
--- a/drivers/net/bnx2x/bnx2x_cmn.h
+++ b/drivers/net/bnx2x/bnx2x_cmn.h
@@ -1297,7 +1297,7 @@ static inline void bnx2x_init_txdata(struct bnx2x *bp,
static inline u8 bnx2x_cnic_eth_cl_id(struct bnx2x *bp, u8 cl_idx)
{
return bp->cnic_base_cl_id + cl_idx +
- (bp->pf_num >> 1) * NON_ETH_CONTEXT_USE;
+ (bp->pf_num >> 1) * BNX2X_MAX_CNIC_ETH_CL_ID_IDX;
}
static inline u8 bnx2x_cnic_fw_sb_id(struct bnx2x *bp)
--
1.7.2.2
^ permalink raw reply related
* Re: [PATCH] mlx4_en: fix transmit of packages when blue frame is enabled
From: Eli Cohen @ 2011-10-10 9:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: David Laight, netdev, Eli Cohen, linuxppc-dev,
Thadeu Lima de Souza Cascardo, Yevgeny Petrilin
In-Reply-To: <1318238645.29415.426.camel@pasglop>
On Mon, Oct 10, 2011 at 11:24:05AM +0200, Benjamin Herrenschmidt wrote:
> On Mon, 2011-10-10 at 11:16 +0200, Eli Cohen wrote:
>
> > Until then I think we need to have the logic working right on ppc and
> > measure if blue flame buys us any improvement in ppc. If that's not
> > the case (e.g because write combining is not working), then maybe we
> > should avoid using blueflame in ppc.
> > Could any of the guys from IBM check this and give us feedback?
>
> I don't have the necessary hardware myself to test that but maybe Thadeu
> can.
>
> Note that for WC to work, things must be mapped non-guarded. You can do
> that by using ioremap_prot() with pgprot_noncached_wc(PAGE_KERNEL) or
> ioremap_wc() (dunno how "generic" the later is).
I use the io mapping API:
at driver statrt:
priv->bf_mapping = io_mapping_create_wc(bf_start, bf_len);
if (!priv->bf_mapping)
err = -ENOMEM;
and then:
uar->bf_map = io_mapping_map_wc(priv->bf_mapping, uar->index << PAGE_SHIFT);
Will this work on ppc?
>
> >From there, you should get write combining provided that you don't have
> barriers between every access (ie those copy operations in their current
> form should do the trick).
>
> Cheers,
> Ben.
>
> > > Maybe it's time for us to revive those discussions about providing a
> > > good set of relaxed MMIO accessors with explicit barriers :-)
> > >
> > > Cheers,
> > > Ben.
> > >
>
^ permalink raw reply
* Re: [PATCH] mlx4_en: fix transmit of packages when blue frame is enabled
From: Benjamin Herrenschmidt @ 2011-10-10 9:24 UTC (permalink / raw)
To: Eli Cohen
Cc: David Laight, netdev, Eli Cohen, linuxppc-dev,
Thadeu Lima de Souza Cascardo, Yevgeny Petrilin
In-Reply-To: <20111010091611.GN2681@mtldesk30>
On Mon, 2011-10-10 at 11:16 +0200, Eli Cohen wrote:
> Until then I think we need to have the logic working right on ppc and
> measure if blue flame buys us any improvement in ppc. If that's not
> the case (e.g because write combining is not working), then maybe we
> should avoid using blueflame in ppc.
> Could any of the guys from IBM check this and give us feedback?
I don't have the necessary hardware myself to test that but maybe Thadeu
can.
Note that for WC to work, things must be mapped non-guarded. You can do
that by using ioremap_prot() with pgprot_noncached_wc(PAGE_KERNEL) or
ioremap_wc() (dunno how "generic" the later is).
>From there, you should get write combining provided that you don't have
barriers between every access (ie those copy operations in their current
form should do the trick).
Cheers,
Ben.
> > Maybe it's time for us to revive those discussions about providing a
> > good set of relaxed MMIO accessors with explicit barriers :-)
> >
> > Cheers,
> > Ben.
> >
^ permalink raw reply
* Re: [PATCH] mlx4_en: fix transmit of packages when blue frame is enabled
From: Eli Cohen @ 2011-10-10 9:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: David Laight, netdev, Eli Cohen, linuxppc-dev,
Thadeu Lima de Souza Cascardo, Yevgeny Petrilin
In-Reply-To: <1318237284.29415.422.camel@pasglop>
On Mon, Oct 10, 2011 at 11:01:24AM +0200, Benjamin Herrenschmidt wrote:
>
> The case where things get a bit more nasty is when you try to use MMIO
> for low latency small-data type transfers instead of DMA, in which case
> you do want the ability for the chipset to write-combine and control the
> barriers more precisely.
>
> However, this is hard and Linux doesn't provide very good accessors to
> do so, thus you need to be extra careful (see my example about wmb()
>
> In the case of the iomap "copy" operations, my problem is that they
> don't properly advertise their lack of ordering since normal iomap does
> have full ordering.
>
> I believe they should provide ordering with a barrier before & a barrier
> after, eventually with _relaxed variants or _raw variants for those who
> "know what they are doing".
Until then I think we need to have the logic working right on ppc and
measure if blue flame buys us any improvement in ppc. If that's not
the case (e.g because write combining is not working), then maybe we
should avoid using blueflame in ppc.
Could any of the guys from IBM check this and give us feedback?
>
> Maybe it's time for us to revive those discussions about providing a
> good set of relaxed MMIO accessors with explicit barriers :-)
>
> Cheers,
> Ben.
>
^ permalink raw reply
* Re: [PATCH] mlx4_en: fix transmit of packages when blue frame is enabled
From: Benjamin Herrenschmidt @ 2011-10-10 9:01 UTC (permalink / raw)
To: Eli Cohen
Cc: David Laight, netdev, Eli Cohen, linuxppc-dev,
Thadeu Lima de Souza Cascardo, Yevgeny Petrilin
In-Reply-To: <20111010084726.GM2681@mtldesk30>
On Mon, 2011-10-10 at 10:47 +0200, Eli Cohen wrote:
> On Mon, Oct 10, 2011 at 09:40:17AM +0100, David Laight wrote:
> >
> > Actually memory barriers shouldn't really be added to
> > any of these 'accessor' functions.
> > (Or, at least, ones without barriers should be provided.)
> >
> > The driver may want to to a series of writes, then a
> > single barrier, before a final write of a command (etc).
> >
> > in_le32() from io.h is specially horrid!
> >
> > David
> >
> The driver would like to control if and when we want to put a memory
> barrier. We really don't want it to be done under the hood. In this
> respect we prefer raw functions which are still available to all
> platforms.
... but not necessarily the corresponding barriers.
That's why on powerpc we had to make all rmb,wmb and mb the same, aka a
full sync, because our weaker barriers don't order cachable vs.
non-cachable.
In any case, the raw functions are a bit nasty to use because they both
don't have barriers -and- don't handle endianness. So you have to be
extra careful.
In 90% of the cases, the barriers are what you want anyway. For example
in the else case of the driver, the doorbell MMIO typically wants it, so
using writel() is fine (or iowrite32be) and will have the necessary
barriers.
The case where things get a bit more nasty is when you try to use MMIO
for low latency small-data type transfers instead of DMA, in which case
you do want the ability for the chipset to write-combine and control the
barriers more precisely.
However, this is hard and Linux doesn't provide very good accessors to
do so, thus you need to be extra careful (see my example about wmb()
In the case of the iomap "copy" operations, my problem is that they
don't properly advertise their lack of ordering since normal iomap does
have full ordering.
I believe they should provide ordering with a barrier before & a barrier
after, eventually with _relaxed variants or _raw variants for those who
"know what they are doing".
Maybe it's time for us to revive those discussions about providing a
good set of relaxed MMIO accessors with explicit barriers :-)
Cheers,
Ben.
^ permalink raw reply
* RE: [PATCH] mlx4_en: fix transmit of packages when blue frame is enabled
From: Benjamin Herrenschmidt @ 2011-10-10 8:53 UTC (permalink / raw)
To: David Laight
Cc: Eli Cohen, netdev, Eli Cohen, linuxppc-dev,
Thadeu Lima de Souza Cascardo, Yevgeny Petrilin
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6D8AE78@saturn3.aculab.com>
On Mon, 2011-10-10 at 09:40 +0100, David Laight wrote:
> > What is this __iowrite64_copy... oh I see
> >
> > Nice, somebody _AGAIN_ added a bunch of "generic" IO
> > accessors that are utterly wrong on all archs except
> > x86 (ok, -almost-).
> > There isn't a single bloody memory barrier in there !
>
> Actually memory barriers shouldn't really be added to
> any of these 'accessor' functions.
> (Or, at least, ones without barriers should be provided.)
As long as they are documented to provide no guarantee of ordering
between the stores... And x86 driver writers have any clue that they
will not be ordered vs. surrounding accesses.
> The driver may want to to a series of writes, then a
> single barrier, before a final write of a command (etc).
>
> in_le32() from io.h is specially horrid!
The reason for that is that drivers expect fully ordered writel() vs
everything (including DMA).
Unfortunately, this is how Linux defines those semantics. I would much
prefer to require barriers explicitely but the decision was made back
then simply because the vast majority of driver writers do not
understand weakly ordered memory models and "everything should be made
to look like x86".
It would be great to come up with a set of more relaxed accessors along
with the appropriate barrier to use for drivers who "know better" but so
far all attempts at doing so have failed due to the inability to agree
on their precise semantics. Tho that was a while ago, we should probably
give it a new shot.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] mlx4_en: fix transmit of packages when blue frame is enabled
From: Eli Cohen @ 2011-10-10 8:47 UTC (permalink / raw)
To: David Laight
Cc: Yevgeny Petrilin, Eli Cohen, Thadeu Lima de Souza Cascardo,
netdev, linuxppc-dev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6D8AE78@saturn3.aculab.com>
On Mon, Oct 10, 2011 at 09:40:17AM +0100, David Laight wrote:
>
> Actually memory barriers shouldn't really be added to
> any of these 'accessor' functions.
> (Or, at least, ones without barriers should be provided.)
>
> The driver may want to to a series of writes, then a
> single barrier, before a final write of a command (etc).
>
> in_le32() from io.h is specially horrid!
>
> David
>
The driver would like to control if and when we want to put a memory
barrier. We really don't want it to be done under the hood. In this
respect we prefer raw functions which are still available to all
platforms.
^ permalink raw reply
* Re: [PATCH] dev: use name hash for dev_seq_ops.
From: Mihai Maruseac @ 2011-10-10 8:43 UTC (permalink / raw)
To: Stephen Hemminger
Cc: davem, eric.dumazet, mirq-linux, therbert, jpirko, netdev,
linux-kernel, dbaluta, Mihai Maruseac
In-Reply-To: <20111007092445.4f097ed9@nehalam.linuxnetplumber.net>
On Fri, Oct 7, 2011 at 7:24 PM, Stephen Hemminger <shemminger@vyatta.com> wrote:
> On Fri, 7 Oct 2011 18:20:49 +0300
> Mihai Maruseac <mihai.maruseac@gmail.com> wrote:
>
>> Instead of using the dev->next chain and trying to resync at each call to
>> dev_seq_start, use this hash and store bucket number and bucket offset in
>> seq->private field.
>>
>> As one can notice the improvement is of 1 order of magnitude.
>
> Good idea,
> This will change the ordering of entries in /proc which may upset
> some program, not a critical flaw but worth noting.
>
> Rather than recording the bucket and offset of last entry, another
> alternative would be to just record the ifindex.
>
I tried to record the ifindex but I think that using it and
dev_get_by_index can result in an infinite loop or a NULL
dereferrence. If a device is removed and ifindex points to it we'll
get a NULL from dev_get_by_index. Checking for NULL and calling again
dev_get_by_index will end in an infinite loop at the end of the hlist.
Augmenting the structure to also contain the number of indexes when
the seq_file is opened returns to the current situation with two ints.
Also, it is more prone to bugs caused by device removal while
printing.
--
Mihai
^ permalink raw reply
* RE: [PATCH] mlx4_en: fix transmit of packages when blue frame is enabled
From: David Laight @ 2011-10-10 8:40 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Eli Cohen
Cc: netdev, Eli Cohen, linuxppc-dev, Thadeu Lima de Souza Cascardo,
Yevgeny Petrilin
In-Reply-To: <1318145118.29415.371.camel@pasglop>
> What is this __iowrite64_copy... oh I see
>
> Nice, somebody _AGAIN_ added a bunch of "generic" IO
> accessors that are utterly wrong on all archs except
> x86 (ok, -almost-).
> There isn't a single bloody memory barrier in there !
Actually memory barriers shouldn't really be added to
any of these 'accessor' functions.
(Or, at least, ones without barriers should be provided.)
The driver may want to to a series of writes, then a
single barrier, before a final write of a command (etc).
in_le32() from io.h is specially horrid!
David
^ permalink raw reply
* RE: [PATCH] mlx4_en: fix transmit of packages when blue frame is enabled
From: Benjamin Herrenschmidt @ 2011-10-10 8:29 UTC (permalink / raw)
To: David Laight
Cc: Eli Cohen, netdev, Eli Cohen, linuxppc-dev,
Thadeu Lima de Souza Cascardo, Yevgeny Petrilin
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6D8AE77@saturn3.aculab.com>
On Mon, 2011-10-10 at 09:20 +0100, David Laight wrote:
>
> For the above I'd actually suggest making 'doorbell_qpn' have the
> correct endianness in order to avoid the (potential) swap every
> time it is set.
Well, the problem is that either you'll end up swapping on x86 or you'll
end up swapping on ppc, there is no "native" MMIO accessor that allow
you to do a no-swap access whatever the arch you are on. Or rather,
there is the __raw_ one but you shouldn't use it for most things :-)
(Because it also doesn't have the right memory barriers).
So I'd rather they do it right using the simpler method, the cost of
swap is going to be negligible, probably not even measurable, and if and
only if they think they can improve on that in a second step, then
consider doing otherwise with appropriate measurements showing a
significant difference.
> You also need to treble-check the required endianness for the
> 'vlan_tag' in the tx descriptor. What would be needed is the
> MAC PCI slave were on an x86 (LE) system.
Cheers,
Ben.
^ permalink raw reply
* RE: [PATCH] mlx4_en: fix transmit of packages when blue frame is enabled
From: David Laight @ 2011-10-10 8:20 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Eli Cohen
Cc: netdev, Eli Cohen, linuxppc-dev, Thadeu Lima de Souza Cascardo,
Yevgeny Petrilin
In-Reply-To: <1318153939.29415.401.camel@pasglop>
> Then, this statement:
>
> *(u32 *) (&tx_desc->ctrl.vlan_tag) |= ring->doorbell_qpn;
...
> instead do ... :
> *(u32 *) (&tx_desc->ctrl.vlan_tag) |=
cpu_to_be32(ring->doorbell_qpn);
>
> (Also get rid of that cast and define vlan_tag as a __be32 to start
> with).
Agreed, casts that change the type of memory - *(foo *)&xxx - are
generally bad news unless you are casting a generic 'buffer' to
a specific structure.
I've seen far to much code that ends up being depending on the
endianness and system word size.
For the above I'd actually suggest making 'doorbell_qpn' have the
correct endianness in order to avoid the (potential) swap every
time it is set.
You also need to treble-check the required endianness for the
'vlan_tag' in the tx descriptor. What would be needed is the
MAC PCI slave were on an x86 (LE) system.
David
^ permalink raw reply
* Re: [PATCH] af_packet: tpacket_destruct_skb, deref skb after BUG_ON assertion
From: danborkmann @ 2011-10-10 8:02 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, netdev
In-Reply-To: <1318193866.21116.3.camel@edumazet-laptop>
Hi Eric,
Quoting Eric Dumazet <eric.dumazet@gmail.com>:
> Le dimanche 09 octobre 2011 à 17:19 +0200, danborkmann@iogearbox.net a
> écrit :
>> This tiny patch derefs the skb only after BUG_ON(skb==NULL) was evaluated
>> and not before. Patched against latest Linus tree.
>>
>> Thanks,
>> Daniel
>>
>> Signed-off-by: Daniel Borkmann <danborkmann@iogearbox.net>
>>
>> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
>> index fabb4fa..d9d833b 100644
>> --- a/net/packet/af_packet.c
>> +++ b/net/packet/af_packet.c
>> @@ -1167,11 +1167,12 @@ ring_is_full:
>>
>> static void tpacket_destruct_skb(struct sk_buff *skb)
>> {
>> - struct packet_sock *po = pkt_sk(skb->sk);
>> + struct packet_sock *po;
>> void *ph;
>>
>> BUG_ON(skb == NULL);
>>
>> + po = pkt_sk(skb->sk);
>> if (likely(po->tx_ring.pg_vec)) {
>> ph = skb_shinfo(skb)->destructor_arg;
>> BUG_ON(__packet_get_status(po, ph) != TP_STATUS_SENDING);
>>
>>
>
> Well, to be honest, this BUG_ON(!skb) is absolutely useless for two
> reasons.
>
> 1) If skb happens to be NULL, the NULL dereference is trapped and stack
> trace dumped as well.
>
> 2) Of course, tpacket_destruct_skb() being an skb destructor, skb cannot
> be NULL at this point by design.
>
> Please remove the BUG_ON() instead of trying to move it ;)
Thanks, you're absolutely right! Here's the trivial patch:
af_packet: removed unnecessary BUG_ON assertion in tpacket_destruct_skb
If skb is NULL, then stack trace is thrown on anyway on dereference.
Therefore,
the stack trace triggered by BUG_ON is duplicate.
Signed-off-by: Daniel Borkmann <danborkmann@googlemail.com>
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index fabb4fa..886ae50 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1170,8 +1170,6 @@ static void tpacket_destruct_skb(struct sk_buff *skb)
struct packet_sock *po = pkt_sk(skb->sk);
void *ph;
- BUG_ON(skb == NULL);
-
if (likely(po->tx_ring.pg_vec)) {
ph = skb_shinfo(skb)->destructor_arg;
BUG_ON(__packet_get_status(po, ph) != TP_STATUS_SENDING);
^ permalink raw reply related
* Re: [PATCH] mlx4_en: fix transmit of packages when blue frame is enabled
From: Benjamin Herrenschmidt @ 2011-10-10 7:32 UTC (permalink / raw)
To: Eli Cohen
Cc: Thadeu Lima de Souza Cascardo, Yevgeny Petrilin,
netdev@vger.kernel.org, Eli Cohen, linuxppc-dev
In-Reply-To: <20111009103020.GL2681@mtldesk30>
On Sun, 2011-10-09 at 12:30 +0200, Eli Cohen wrote:
> > Ideally you want to avoid that swapping altogether and use the right
> > accessor that indicates that your register is BE to start with. IE.
> > remove the swab32 completely and then use something like
> > iowrite32be() instead of writel().
> I agree, this looks better but does it work on memory mapped io or
> only on io pci space? All our registers are memory mapped...
The iomap functions work on both.
> > Basically, the problem you have is that writel() has an implicit "write
> > to LE register" semantic. Your register is BE. the "iomap" variants
> > provide you with more fine grained "be" variants to use in that case.
> > There's also writel_be() but that one doesn't exist on every
> > architecture afaik.
> So writel_be is the function I should use for memory mapped io? If it
> does not exist for all platforms it's a pitty :-(
Just use the iomap variant. Usually you also use pci_iomap() instead of
ioremap() but afaik, for straight MMIO, it works with normal ioremap as
well.
> > Now, once the mmio problem is out of the way, let's look back at how you
> > then use that qpn.
> >
> > With the current code, you've generated something in memory which is
> > byte reversed, so essentially "LE" on ppc and "BE" on x86.
> >
> > Then, this statement:
> >
> > *(u32 *) (&tx_desc->ctrl.vlan_tag) |= ring->doorbell_qpn;
> >
> > Will essentially write it out as-is in memory for use by the chip. The chip,
> > from what you say, expects BE, so this will be broken on PPC.
> I see. So this field is layed in le for ppc and the rest of the
> descriptor is be. so I assum that __iowrite64_copy() does not swap
> anything but we still have tx_desc->ctrl.vlan_tag in the wrong
> endianess.
Yes because you had swapped it initially. IE your original swab32 is
what busts it for you on ppc.
> > Here too, the right solution is to instead not do that swab32 to begin
> > with (ring->doorbell_qpn remains a native endian value) and instead do,
> > in addition to the above mentioned change to the writel:
> >
> > *(u32 *) (&tx_desc->ctrl.vlan_tag) |= cpu_to_be32(ring->doorbell_qpn);
> >
> > (Also get rid of that cast and define vlan_tag as a __be32 to start
> > with).
> >
> > Cheers,
> > Ben.
>
> Thanks for your review. I will send another patch which should fix the
> deficiencies.
Cheers,
Ben.
^ permalink raw reply
* Re: [RFC] truesize lies
From: Eric Dumazet @ 2011-10-10 6:54 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20111009.181023.328192272226333109.davem@davemloft.net>
Le dimanche 09 octobre 2011 à 18:10 -0400, David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Sun, 09 Oct 2011 23:55:20 +0200
>
> > Some drivers splits a page in two (or more) pieces, so we cant know what
> > was really reserved by a driver for paged skbs.
> >
> > Only thing we could enforce at the moment is the proper accounting for
> > skb head :
> >
> > WARN_ON(skb->truesize < (skb_end_pointer(skb) - skb->head) + sizeof(sk_buff) + skb->data_len);
>
> This is partly true.
>
> Drivers that use page pools divide pages up into different pools, each
> with some specific block size. At least this is how NIU works.
>
> So NIU knows exactly how much of the block is logically part of that
> SKB yet unused.
>
> And if we really wanted to we could add a frag[].reserved field that
> keeps track of this for debugging.
Hmm, this would still be possible for a driver to lie and consume lot of
ram on 64bit arches.
How about using a "struct page" field then, since for a given page, we
can assume/enforce it was divided in equal units (kind of a negative
order) by a driver.
Instead of using alloc_page(order) a driver could use
alloc_page_truesize(gfp_t mask, int page_order, int subunit_order)
For example, an x86 driver using 4KB page splitted in two pieces would
use alloc_page_truesize(gfp, 0, 1)
To get frag[i].reserved (aka truesize), we then would do
static inline int frag_truesize(const skb_frag_t *frag)
{
return compound_head(frag->page)->truesize;
}
(Not even sure compound_head() is even necessary here, but I see NIU
uses it in niu_rbr_add_page() ?)
^ permalink raw reply
* Re: [PATCH net] vlan:make mtu of vlan equal to physical dev
From: David Miller @ 2011-10-10 4:00 UTC (permalink / raw)
To: panweiping3; +Cc: netdev, herbert
In-Reply-To: <bc33a6142ce48b71b9c232a9154ed76f6048f0cb.1318068629.git.panweiping3@gmail.com>
From: Weiping Pan <panweiping3@gmail.com>
Date: Sat, 8 Oct 2011 18:12:15 +0800
[ Herbert, Weiping is suggesting to remove the "vlandev->mtu <= dev->mtu"
test in your patch below. ]
> Default mtu of vlan device is the same with mtu of physical device,
> for example 1500, but when change physics mtu to 1600,
> VLAN device's mtu is still 1500.
> Certainly, you can change vlan device's mtu to 1600 manually,
> but I think when you change physics device's mtu, VLAN's mtu should be changed
> automatically instead of by manually.
>
> Steps to Reproduce:
> 1.vconfig add eth4 3
> 2.ifconfig eth4 mtu 1600
> 3.check mtu on eth4.3
>
> And what's worse is that if you decrease mtu of pyhsical device,
> and when you want to increase it, the mtu of vlan device won't be changed.
>
> Steps to Reproduce:
> 1.vconfig add eth4 3
> 2.ifconfig eth4 mtu 100
> 3.ifconfig eth4 mtu 1500
> 4.the mtu of eth4.3 is still 100
>
> This bug is reported by Liang Zheng(lzheng@redhat.com).
>
> Signed-off-by: Weiping Pan <panweiping3@gmail.com>
But now this setting is unconditional, and thus value user made
settings will be ignored.
The code that is there now is just a safety measure, and the
test appears to very much be intentional:
--------------------
commit 2e477c9bd2bb6a1606e498adb53ba913378ecdf2
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date: Mon Jul 20 07:35:37 2009 -0700
vlan: Propagate physical MTU changes
When the physical MTU changes we want to ensure that all existing
VLAN device MTUs do not exceed the new underlying MTU. This patch
adds that propagation.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index fe64908..6d37b7e 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -468,6 +468,19 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
}
break;
+ case NETDEV_CHANGEMTU:
+ for (i = 0; i < VLAN_GROUP_ARRAY_LEN; i++) {
+ vlandev = vlan_group_get_device(grp, i);
+ if (!vlandev)
+ continue;
+
+ if (vlandev->mtu <= dev->mtu)
+ continue;
+
+ dev_set_mtu(vlandev, dev->mtu);
+ }
+ break;
+
case NETDEV_FEAT_CHANGE:
/* Propagate device features to underlying device */
for (i = 0; i < VLAN_GROUP_ARRAY_LEN; i++) {
^ permalink raw reply related
* Re: [PATCH 2/2 net-next] r6040: bump version to 0.28 and date to 07Oct2011.
From: David Miller @ 2011-10-10 3:51 UTC (permalink / raw)
To: florian; +Cc: netdev
In-Reply-To: <201110071136.28266.florian@openwrt.org>
From: Florian Fainelli <florian@openwrt.org>
Date: Fri, 7 Oct 2011 11:36:28 +0200
> Signed-off-by: Florian Fainelli <florian@openwrt.org>
Applied.
^ permalink raw reply
* Re: [PATCH 1/2 net-next] r6040: invoke phy_{start,stop} when appropriate
From: David Miller @ 2011-10-10 3:51 UTC (permalink / raw)
To: florian; +Cc: netdev
In-Reply-To: <201110071136.22107.florian@openwrt.org>
From: Florian Fainelli <florian@openwrt.org>
Date: Fri, 7 Oct 2011 11:36:22 +0200
> Joe reported to me that right after a bring up of a r6040 interface
> the ethtool output had no consistent output with respect to link duplex
> and speed. Fix this by adding a missing phy_start call in r6040_up and
> conversely a phy_stop call in r6040_down to properly initialize phy states.
>
> Reported-by: Joe Chou <Joe.Chou@rdc.com.tw>
> Signed-off-by: Florian Fainelli <florian@openwrt.org>
Applied.
^ permalink raw reply
* Re: [PATCH 0/7] mlx4_en: bug fixes and performance enhancement
From: David Miller @ 2011-10-10 3:44 UTC (permalink / raw)
To: alexg; +Cc: netdev, yevgenyp
In-Reply-To: <4E91BD1B.2040902@mellanox.com>
From: Alexander Guller <alexg@mellanox.com>
Date: Sun, 9 Oct 2011 17:26:19 +0200
> This is a series of patches for the mlx4_en driver generated against
> net-next tree. The patches include several bug fixes and a fix for
> a better initialization of TX interrupts.
Applied.
For some reason another copy of patch #3 was appended after the
end of patch #4. Make sure that doesn't happen next time.
^ permalink raw reply
* Re: [PATCH] bluetooth: Properly clone LSM attributes to newly created child connections
From: James Morris @ 2011-10-10 0:31 UTC (permalink / raw)
To: Paul Moore; +Cc: netdev, linux-security-module, selinux
In-Reply-To: <20111007194059.12345.13398.stgit@sifl>
On Fri, 7 Oct 2011, Paul Moore wrote:
> Reported-by: James M. Cape <jcape@ignore-your.tv>
> Signed-off-by: Paul Moore <pmoore@redhat.com>
Acked-by: James Morris <jmorris@namei.org>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [RFC] truesize lies
From: David Miller @ 2011-10-09 22:10 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1318197320.21116.11.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 09 Oct 2011 23:55:20 +0200
> Some drivers splits a page in two (or more) pieces, so we cant know what
> was really reserved by a driver for paged skbs.
>
> Only thing we could enforce at the moment is the proper accounting for
> skb head :
>
> WARN_ON(skb->truesize < (skb_end_pointer(skb) - skb->head) + sizeof(sk_buff) + skb->data_len);
This is partly true.
Drivers that use page pools divide pages up into different pools, each
with some specific block size. At least this is how NIU works.
So NIU knows exactly how much of the block is logically part of that
SKB yet unused.
And if we really wanted to we could add a frag[].reserved field that
keeps track of this for debugging.
^ permalink raw reply
* Re: [RFC] truesize lies
From: Eric Dumazet @ 2011-10-09 21:55 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20111009.172739.2151675769111763947.davem@davemloft.net>
Le dimanche 09 octobre 2011 à 17:27 -0400, David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Sun, 09 Oct 2011 21:02:28 +0200
>
> > I guess most driver writers adjust truesize to please TCP stack and get
> > nice performance numbers.
> >
> > skb->truesize = frame_len + sizeof(sk_buff);
> >
> > What should we do exactly ?
>
> We should pick a specification that accounts for all the metadata as well
> as unused data areas properly, and enforce this in netif_receive_skb().
>
> That will get the inaccurate cases fixed real fast.
>
> We've had similar problems in the past and dealt with it in the same
> exact way.
Some drivers splits a page in two (or more) pieces, so we cant know what
was really reserved by a driver for paged skbs.
Only thing we could enforce at the moment is the proper accounting for
skb head :
WARN_ON(skb->truesize < (skb_end_pointer(skb) - skb->head) + sizeof(sk_buff) + skb->data_len);
^ 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