* Re: [RESEND PATCH 0/2] Make functions of dev_<level> macros, recursive vsnprintf
From: Linus Torvalds @ 2010-03-06 23:48 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Nick Andrew, linux-kernel, Greg Kroah-Hartman,
netdev
In-Reply-To: <alpine.LFD.2.00.1003061540470.23804@localhost.localdomain>
On Sat, 6 Mar 2010, Linus Torvalds wrote:
>
> width: 5 (make it 6 bits and signed)
> precision: 5 (make it 6 bits, and signed)
Oh, actually, no, we do that whole left-justification with the flags, we
don't need that signed stuff. We do need one special value for the "no
width, no precision", though.
Linus
^ permalink raw reply
* Re: Help On function to get IPV6 address of an interface in kernel
From: Mark Smith @ 2010-03-06 23:23 UTC (permalink / raw)
To: Venkata Mohan Reddy; +Cc: Brian Haley, netdev
In-Reply-To: <20100305154932.GA5139@mohan-laptop>
On Fri, 5 Mar 2010 21:19:34 +0530
Venkata Mohan Reddy <mohanreddykv@gmail.com> wrote:
> Thanks a lot Brain. ipv6_get_lladdr() helped me. Actually I want to get site-local or univeral(global) address. Though there is no direct function to get theseaddresses, ipv6_get_lladdr() shows how we can get the other addresses too.
>
Site-locals are deprecated (http://www.rfc-editor.org/rfc/rfc3879.txt),
so I wouldn't think you'll ever find a site-local specific address
function. If you do find one, I'd be fairly sure it'd go away.
> Thanks,
> Mohan Reddy
>
> On Thu, Mar 04, 2010 at 11:17:39AM -0500, Brian Haley wrote:
> > Mohan Reddy wrote:
> > > Hi,
> > >
> > > Is there a function or a way in kernel to get an interface ipv6 address if interface name or net_device object is known? I searched in the kernel i got a function ipv6_get_ifaddr(). But it is expecting an ipv6 address as a parameter.
> >
> > What type of IPv6 address? For link-locals you can use ipv6_get_lladdr(), but
> > for others ipv6_dev_get_saddr() is more comprehensive, but you'll need a
> > destination address. There's lots of reference code in addrconf.c to do
> > other things.
> >
> > -Brian
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RESEND PATCH 0/2] Make functions of dev_<level> macros, recursive vsnprintf
From: Joe Perches @ 2010-03-06 23:57 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andrew Morton, Nick Andrew, linux-kernel, Greg Kroah-Hartman,
netdev
In-Reply-To: <alpine.LFD.2.00.1003061547490.23804@localhost.localdomain>
On Sat, 2010-03-06 at 15:48 -0800, Linus Torvalds wrote:
> On Sat, 6 Mar 2010, Linus Torvalds wrote:
> > width: 5 (make it 6 bits and signed)
> > precision: 5 (make it 6 bits, and signed)
> Oh, actually, no, we do that whole left-justification with the flags, we
> don't need that signed stuff. We do need one special value for the "no
> width, no precision", though.
What I posted is pretty simple.
I think the added complexity isn't worth it.
If you or anyone else does, all yours.
^ permalink raw reply
* Re: [RESEND PATCH 0/2] Make functions of dev_<level> macros, recursive vsnprintf
From: Linus Torvalds @ 2010-03-06 23:58 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Nick Andrew, linux-kernel, Greg Kroah-Hartman,
netdev
In-Reply-To: <alpine.LFD.2.00.1003061547490.23804@localhost.localdomain>
On Sat, 6 Mar 2010, Linus Torvalds wrote:
>
>
> On Sat, 6 Mar 2010, Linus Torvalds wrote:
> >
> > width: 5 (make it 6 bits and signed)
> > precision: 5 (make it 6 bits, and signed)
>
> Oh, actually, no, we do that whole left-justification with the flags, we
> don't need that signed stuff. We do need one special value for the "no
> width, no precision", though.
Btw, yeah, we do have a couple of "%.100s" etc, but we could fix those if
we really wanted to do this.
And no, I guess we don't really need to make it 32-bit. Fitting in 64 bits
would already be a big improvement over what we have now.
Linus
^ permalink raw reply
* [PATCH] vsprintf.c: Reduce sizeof struct printf_spec from 24 to 8 bytes
From: Joe Perches @ 2010-03-07 1:10 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andrew Morton, Nick Andrew, linux-kernel, Greg Kroah-Hartman,
netdev
In-Reply-To: <alpine.LFD.2.00.1003061556420.23804@localhost.localdomain>
On Sat, 2010-03-06 at 15:58 -0800, Linus Torvalds wrote:
> And no, I guess we don't really need to make it 32-bit. Fitting in 64 bits
> would already be a big improvement over what we have now.
Reducing the size of struct printf_spec is a good thing
because multiple instances are commonly passed on stack.
It's possible for type to be u8 and field_width to be s8,
but this is likely small enough for now.
Signed-off-by: Joe Perches <joe@perches.com>
---
lib/vsprintf.c | 22 ++++++++++++----------
1 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index af4aaa6..fdee7f7 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -408,12 +408,12 @@ enum format_type {
};
struct printf_spec {
- enum format_type type;
- int flags; /* flags to number() */
- int field_width; /* width of output field */
- int base;
- int precision; /* # of digits/chars */
- int qualifier;
+ u16 type;
+ s16 field_width; /* width of output field */
+ u8 flags; /* flags to number() */
+ u8 base;
+ s8 precision; /* # of digits/chars */
+ u8 qualifier;
};
static char *number(char *buf, char *end, unsigned long long num,
@@ -1333,7 +1333,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
break;
case FORMAT_TYPE_NRCHARS: {
- int qualifier = spec.qualifier;
+ u8 qualifier = spec.qualifier;
if (qualifier == 'l') {
long *ip = va_arg(args, long *);
@@ -1619,7 +1619,7 @@ do { \
case FORMAT_TYPE_NRCHARS: {
/* skip %n 's argument */
- int qualifier = spec.qualifier;
+ u8 qualifier = spec.qualifier;
void *skip_arg;
if (qualifier == 'l')
skip_arg = va_arg(args, long *);
@@ -1885,7 +1885,9 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
char *next;
char digit;
int num = 0;
- int qualifier, base, field_width;
+ u8 qualifier;
+ u8 base;
+ s16 field_width;
bool is_sign;
while (*fmt && *str) {
@@ -1963,7 +1965,7 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
{
char *s = (char *)va_arg(args, char *);
if (field_width == -1)
- field_width = INT_MAX;
+ field_width = SHORT_MAX;
/* first, skip leading white space in buffer */
str = skip_spaces(str);
^ permalink raw reply related
* Re: [net-next-2.6 PATCH v2] net: consolidate netif_needs_gso() checks
From: Herbert Xu @ 2010-03-07 1:43 UTC (permalink / raw)
To: John Fastabend
Cc: David Miller, Kirsher, Jeffrey T, netdev@vger.kernel.org,
gospo@redhat.com
In-Reply-To: <4B92ACB6.1090909@intel.com>
On Sat, Mar 06, 2010 at 11:27:50AM -0800, John Fastabend wrote:
>
> It looks like dev_gso_segment() could be used to "Verify header
> integrity only" according to the comment? If this is true I think the
> logic should probably be
>
> if (netif_needs_gso(dev, skb)) {
> if (unlikely(dev_gso_segment(skb)))
> goto out_kfree_skb;
> if (skb->next)
> goto gso;
> } do your thing
>
>
>
> That way we linearize the skb if necessary in the case were
> dev_gso_segment() only verifies the header and does not return a list of
> segments.
If we needed to linearise the skb then dev_gso_segment should
perform the segmentation. Is there a case where it doesn't?
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] vsprintf.c: Reduce sizeof struct printf_spec from 24 to 8 bytes
From: Linus Torvalds @ 2010-03-07 2:03 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Nick Andrew, Linux Kernel Mailing List,
Greg Kroah-Hartman, netdev, Bjorn Helgaas
In-Reply-To: <1267924214.1937.12.camel@Joe-Laptop.home>
On Sat, 6 Mar 2010, Joe Perches wrote:
>
> Reducing the size of struct printf_spec is a good thing
> because multiple instances are commonly passed on stack.
Sadly, this is not enough.
the 'pointer()' function has a 200+ byte stack footprint on x86-64. And
vsnprintf itself is about 100+ bytes. So that stack depth is way bigger
than I would have expected.
I'm not sure _why_ that stack footprint for 'pointer()' is so big, but I
bet it's due to some simple inlining, and gcc (once more) sucking at it
and not being able to combine stack frames. It's a damn shame.
I suspect it's mac_address_string() having 20-odd bytes of temporary data,
ip6_compressed_string() having some more, ip6_addr_string() having 50
bytes, uuid_string() adding thirty-odd bytes etc. Inline them all, suck up
merging stack slots, and 200 bytes is easy.
So no, I don't think we can do the recursion as things stand. I've applied
your cleanup patch, along with the two from Bjorn Helgaas (which he did
for his pnp set), but they don't help this fundamental problem.
A few noinlines might be appropriate. As would a good gcc cluestick about
inlining and stack usage. The latter is unlikely to materialize, I guess.
Linus
^ permalink raw reply
* Re: [PATCH] vsprintf.c: Reduce sizeof struct printf_spec from 24 to 8 bytes
From: Linus Torvalds @ 2010-03-07 2:24 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Nick Andrew, Linux Kernel Mailing List,
Greg Kroah-Hartman, netdev, Bjorn Helgaas
In-Reply-To: <alpine.LFD.2.00.1003061755400.23804@localhost.localdomain>
On Sat, 6 Mar 2010, Linus Torvalds wrote:
> the 'pointer()' function has a 200+ byte stack footprint on x86-64. And
> vsnprintf itself is about 100+ bytes. So that stack depth is way bigger
> than I would have expected.
>
> I'm not sure _why_ that stack footprint for 'pointer()' is so big, but I
> bet it's due to some simple inlining, and gcc (once more) sucking at it
> and not being able to combine stack frames. It's a damn shame.
Yeah, a few noinline's gets 'pointer()' to just save registers on the
stack, no need for any extra buffers (which then is ok for your recursion
case - the other subfunctions it can call have their own buffers, of
course, but they won't be in the recursive call-path except at the leaf.
vsnprintf() itself seems less obviously fixable. I'm not sure wht gcc
decides it needs 88 bytes of temp-space there.
Linus
^ permalink raw reply
* [PATCH] vsprintf.c: Use noinline_for_stack
From: Joe Perches @ 2010-03-07 2:33 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andrew Morton, Nick Andrew, Linux Kernel Mailing List,
Greg Kroah-Hartman, netdev, Bjorn Helgaas
In-Reply-To: <alpine.LFD.2.00.1003061755400.23804@localhost.localdomain>
On Sat, 2010-03-06 at 18:03 -0800, Linus Torvalds wrote:
> A few noinlines might be appropriate.
Mark static functions with noinline_for_stack
Signed-off-by: Joe Perches <joe@perches.com>
---
lib/vsprintf.c | 67 +++++++++++++++++++++++++++++++++++---------------------
1 files changed, 42 insertions(+), 25 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 0d461c7..e9335a8 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -266,7 +266,8 @@ int strict_strtoll(const char *cp, unsigned int base, long long *res)
}
EXPORT_SYMBOL(strict_strtoll);
-static int skip_atoi(const char **s)
+static noinline_for_stack
+int skip_atoi(const char **s)
{
int i = 0;
@@ -286,7 +287,8 @@ static int skip_atoi(const char **s)
/* Formats correctly any integer in [0,99999].
* Outputs from one to five digits depending on input.
* On i386 gcc 4.1.2 -O2: ~250 bytes of code. */
-static char *put_dec_trunc(char *buf, unsigned q)
+static noinline_for_stack
+char *put_dec_trunc(char *buf, unsigned q)
{
unsigned d3, d2, d1, d0;
d1 = (q>>4) & 0xf;
@@ -323,7 +325,8 @@ static char *put_dec_trunc(char *buf, unsigned q)
return buf;
}
/* Same with if's removed. Always emits five digits */
-static char *put_dec_full(char *buf, unsigned q)
+static noinline_for_stack
+char *put_dec_full(char *buf, unsigned q)
{
/* BTW, if q is in [0,9999], 8-bit ints will be enough, */
/* but anyway, gcc produces better code with full-sized ints */
@@ -365,7 +368,8 @@ static char *put_dec_full(char *buf, unsigned q)
return buf;
}
/* No inlining helps gcc to use registers better */
-static noinline char *put_dec(char *buf, unsigned long long num)
+static noinline_for_stack
+char *put_dec(char *buf, unsigned long long num)
{
while (1) {
unsigned rem;
@@ -416,8 +420,9 @@ struct printf_spec {
u8 qualifier;
};
-static char *number(char *buf, char *end, unsigned long long num,
- struct printf_spec spec)
+static noinline_for_stack
+char *number(char *buf, char *end, unsigned long long num,
+ struct printf_spec spec)
{
/* we are called with base 8, 10 or 16, only, thus don't need "G..." */
static const char digits[16] = "0123456789ABCDEF"; /* "GHIJKLMNOPQRSTUVWXYZ"; */
@@ -536,7 +541,8 @@ static char *number(char *buf, char *end, unsigned long long num,
return buf;
}
-static char *string(char *buf, char *end, const char *s, struct printf_spec spec)
+static noinline_for_stack
+char *string(char *buf, char *end, const char *s, struct printf_spec spec)
{
int len, i;
@@ -566,8 +572,9 @@ static char *string(char *buf, char *end, const char *s, struct printf_spec spec
return buf;
}
-static char *symbol_string(char *buf, char *end, void *ptr,
- struct printf_spec spec, char ext)
+static noinline_for_stack
+char *symbol_string(char *buf, char *end, void *ptr,
+ struct printf_spec spec, char ext)
{
unsigned long value = (unsigned long) ptr;
#ifdef CONFIG_KALLSYMS
@@ -587,8 +594,9 @@ static char *symbol_string(char *buf, char *end, void *ptr,
#endif
}
-static char *resource_string(char *buf, char *end, struct resource *res,
- struct printf_spec spec, const char *fmt)
+static noinline_for_stack
+char *resource_string(char *buf, char *end, struct resource *res,
+ struct printf_spec spec, const char *fmt)
{
#ifndef IO_RSRC_PRINTK_SIZE
#define IO_RSRC_PRINTK_SIZE 6
@@ -678,8 +686,9 @@ static char *resource_string(char *buf, char *end, struct resource *res,
return string(buf, end, sym, spec);
}
-static char *mac_address_string(char *buf, char *end, u8 *addr,
- struct printf_spec spec, const char *fmt)
+static noinline_for_stack
+char *mac_address_string(char *buf, char *end, u8 *addr,
+ struct printf_spec spec, const char *fmt)
{
char mac_addr[sizeof("xx:xx:xx:xx:xx:xx")];
char *p = mac_addr;
@@ -702,7 +711,8 @@ static char *mac_address_string(char *buf, char *end, u8 *addr,
return string(buf, end, mac_addr, spec);
}
-static char *ip4_string(char *p, const u8 *addr, const char *fmt)
+static noinline_for_stack
+char *ip4_string(char *p, const u8 *addr, const char *fmt)
{
int i;
bool leading_zeros = (fmt[0] == 'i');
@@ -751,7 +761,8 @@ static char *ip4_string(char *p, const u8 *addr, const char *fmt)
return p;
}
-static char *ip6_compressed_string(char *p, const char *addr)
+static noinline_for_stack
+char *ip6_compressed_string(char *p, const char *addr)
{
int i, j, range;
unsigned char zerolength[8];
@@ -831,7 +842,8 @@ static char *ip6_compressed_string(char *p, const char *addr)
return p;
}
-static char *ip6_string(char *p, const char *addr, const char *fmt)
+static noinline_for_stack
+char *ip6_string(char *p, const char *addr, const char *fmt)
{
int i;
@@ -846,8 +858,9 @@ static char *ip6_string(char *p, const char *addr, const char *fmt)
return p;
}
-static char *ip6_addr_string(char *buf, char *end, const u8 *addr,
- struct printf_spec spec, const char *fmt)
+static noinline_for_stack
+char *ip6_addr_string(char *buf, char *end, const u8 *addr,
+ struct printf_spec spec, const char *fmt)
{
char ip6_addr[sizeof("xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:255.255.255.255")];
@@ -859,8 +872,9 @@ static char *ip6_addr_string(char *buf, char *end, const u8 *addr,
return string(buf, end, ip6_addr, spec);
}
-static char *ip4_addr_string(char *buf, char *end, const u8 *addr,
- struct printf_spec spec, const char *fmt)
+static noinline_for_stack
+char *ip4_addr_string(char *buf, char *end, const u8 *addr,
+ struct printf_spec spec, const char *fmt)
{
char ip4_addr[sizeof("255.255.255.255")];
@@ -869,8 +883,9 @@ static char *ip4_addr_string(char *buf, char *end, const u8 *addr,
return string(buf, end, ip4_addr, spec);
}
-static char *uuid_string(char *buf, char *end, const u8 *addr,
- struct printf_spec spec, const char *fmt)
+static noinline_for_stack
+char *uuid_string(char *buf, char *end, const u8 *addr,
+ struct printf_spec spec, const char *fmt)
{
char uuid[sizeof("xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx")];
char *p = uuid;
@@ -958,8 +973,9 @@ static char *uuid_string(char *buf, char *end, const u8 *addr,
* function pointers are really function descriptors, which contain a
* pointer to the real address.
*/
-static char *pointer(const char *fmt, char *buf, char *end, void *ptr,
- struct printf_spec spec)
+static noinline_for_stack
+char *pointer(const char *fmt, char *buf, char *end, void *ptr,
+ struct printf_spec spec)
{
if (!ptr)
return string(buf, end, "(null)", spec);
@@ -1028,7 +1044,8 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr,
* @precision: precision of a number
* @qualifier: qualifier of a number (long, size_t, ...)
*/
-static int format_decode(const char *fmt, struct printf_spec *spec)
+static noinline_for_stack
+int format_decode(const char *fmt, struct printf_spec *spec)
{
const char *start = fmt;
^ permalink raw reply related
* Re: [PATCH 6/13] bridge: Add core IGMP snooping support
From: Herbert Xu @ 2010-03-07 2:45 UTC (permalink / raw)
To: Paul E. McKenney; +Cc: David S. Miller, netdev, Stephen Hemminger
In-Reply-To: <20100306190000.GA24445@linux.vnet.ibm.com>
On Sat, Mar 06, 2010 at 11:00:00AM -0800, Paul E. McKenney wrote:
>
> > Hmmm... rcu_barrier() definitely does -not- imply rcu_barrier_bh(),
> > because there are separate sets of callbacks whose execution can
> > be throttled separately. So, while you would expect RCU-bh grace
> > periods to complete more quickly, if there was a large number of
> > RCU-bh callbacks on a given CPU but very few RCU callbacks, it might
> > well take longer for the RCU-bh callbacks to be invoked.
> >
> > With TREE_PREEMPT_RCU, if there were no RCU readers but one long-running
> > RCU-bh reader, then synchronize_rcu_bh() could return before
> > synchronize_rcu() does.
OK, then we definitely do have some issues under net/ with respect
to the two types of RCU usage. As you can see, we use the RCU-BH
variant on the read-side in various places, and call_rcu_bh on the
write-side too, but we only ever use the non-BH version of the
functions rcu_barrier and synchronize_rcu.
Now there is a possibility that the places where we use synchronize
and rcu_barrier don't really care about the BH variant, but an
audit wouldn't hurt.
> You really are talking about code like the following, correct?
>
> rcu_read_lock();
> p = rcu_dereference(global_p);
> do_something_with(p);
> rcu_read_unlock();
>
> . . .
>
> rcu_read_lock_bh();
> p = rcu_dereference(global_p);
> do_something_else_with(p);
> rcu_read_unlock_bh();
>
> . . .
>
> spin_lock(&my_lock);
> p = global_p;
> rcu_assign_pointer(global_p, NULL);
> synchronize_rcu(); /* BUG -- also need synchronize_rcu_bh(). */
> kfree(p);
> spin_unlock(&my_lock);
>
> In other words, different readers traversing the same data structure
> under different flavors of RCU protection, but then using only one
> flavor of RCU grace period during the update?
We usually don't use synchronize_rcu/rcu_barrier on the update side,
but rather they are used in the tear-down process.
But otherwise yes this is exactly my concern.
Note that we may have a problem on the update side too if we used
the wrong call_rcu variant, but it would require a thorough audit
to reveal those.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 6/13] bridge: Add core IGMP snooping support
From: Paul E. McKenney @ 2010-03-07 3:11 UTC (permalink / raw)
To: Herbert Xu; +Cc: David S. Miller, netdev, Stephen Hemminger, arnd
In-Reply-To: <20100307024500.GA20126@gondor.apana.org.au>
On Sun, Mar 07, 2010 at 10:45:00AM +0800, Herbert Xu wrote:
> On Sat, Mar 06, 2010 at 11:00:00AM -0800, Paul E. McKenney wrote:
> >
> > > Hmmm... rcu_barrier() definitely does -not- imply rcu_barrier_bh(),
> > > because there are separate sets of callbacks whose execution can
> > > be throttled separately. So, while you would expect RCU-bh grace
> > > periods to complete more quickly, if there was a large number of
> > > RCU-bh callbacks on a given CPU but very few RCU callbacks, it might
> > > well take longer for the RCU-bh callbacks to be invoked.
> > >
> > > With TREE_PREEMPT_RCU, if there were no RCU readers but one long-running
> > > RCU-bh reader, then synchronize_rcu_bh() could return before
> > > synchronize_rcu() does.
>
> OK, then we definitely do have some issues under net/ with respect
> to the two types of RCU usage. As you can see, we use the RCU-BH
> variant on the read-side in various places, and call_rcu_bh on the
> write-side too, but we only ever use the non-BH version of the
> functions rcu_barrier and synchronize_rcu.
>
> Now there is a possibility that the places where we use synchronize
> and rcu_barrier don't really care about the BH variant, but an
> audit wouldn't hurt.
>
> > You really are talking about code like the following, correct?
> >
> > rcu_read_lock();
> > p = rcu_dereference(global_p);
> > do_something_with(p);
> > rcu_read_unlock();
> >
> > . . .
> >
> > rcu_read_lock_bh();
> > p = rcu_dereference(global_p);
> > do_something_else_with(p);
> > rcu_read_unlock_bh();
> >
> > . . .
> >
> > spin_lock(&my_lock);
> > p = global_p;
> > rcu_assign_pointer(global_p, NULL);
> > synchronize_rcu(); /* BUG -- also need synchronize_rcu_bh(). */
> > kfree(p);
> > spin_unlock(&my_lock);
> >
> > In other words, different readers traversing the same data structure
> > under different flavors of RCU protection, but then using only one
> > flavor of RCU grace period during the update?
>
> We usually don't use synchronize_rcu/rcu_barrier on the update side,
> but rather they are used in the tear-down process.
>
> But otherwise yes this is exactly my concern.
>
> Note that we may have a problem on the update side too if we used
> the wrong call_rcu variant, but it would require a thorough audit
> to reveal those.
OK, just re-checked your patch, and it looks OK.
Also adding Arnd to CC.
Arnd, would it be reasonable to extend your RCU-sparse changes to have
four different pointer namespaces, one for each flavor of RCU? (RCU,
RCU-bh, RCU-sched, and SRCU)? Always a fan of making the computer do
the auditing where reasonable. ;-)
This could potentially catch the mismatched call_rcu()s, at least if the
rcu_head could be labeled.
Other thoughts?
Thanx, Paul
^ permalink raw reply
* Re: [RFC v2 00/10] snet: Security for NETwork syscalls
From: Tetsuo Handa @ 2010-03-07 5:45 UTC (permalink / raw)
To: sam
Cc: linux-kernel, netdev, netfilter-devel, hadi, kaber, zbr, nhorman,
root, linux-security-module
In-Reply-To: <m2vdd9p9r7.fsf@ssh.synack.fr>
Hello.
Samir Bellabes wrote:
> Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> writes:
>
> > Regarding [RFC v2 09/10] snet: introduce snet_ticket
> > +enum snet_verdict snet_ticket_check(struct snet_info *info)
> > +{
> > + struct snet_ticket *st = NULL;
> > + unsigned int h = 0, verdict = SNET_VERDICT_NONE;
> > + struct list_head *l = NULL;
> > + struct snet_task_security *tsec = NULL;
> > +
> > + if (snet_ticket_mode == SNET_TICKET_OFF)
> > + goto out;
> > +
> > + tsec = (struct snet_task_security*) current_security();
> > +
> > + h = jhash_2words(info->syscall, info->protocol, 0) % HSIZE;
> > + l = &tsec->hash[h];
> > +
> > + read_lock_bh(&tsec->lock);
> >
> > Credentials are allocated for copy-on-write basis.
> > Sharing "tsec" among multiple "struct task_struct" is what you intended?
>
> No, there is no shared "tsec".
> snet_ticket_check() is called from the process context. So "tsec" is
> a pointer to the "void *security" pointer from its own "struct
> task_struct".
>
Until 2.6.28:
"void *security" is directory attached to "struct task_struct".
copy_process() calls security_task_alloc().
Therefore, task1->security != task2->security is guaranteed as long as you do
task->security = kmalloc();
at security_task_alloc().
Since 2.6.29:
"void *security" is attached to "struct cred", and "struct cred *" is
attached to "struct task_struct". copy_process() calls copy_creds() and
prepare_creds() calls security_prepare_creds(). But copy_creds() does not
call prepare_creds() for clone(CLONE_THREAD) case.
Therefore, task1->cred->security != task2->cred->security is not guaranteed
even if you do
cred->security = kmalloc();
at security_prepare_creds().
> every task_struct have a "tsec" allocated to its "void *security"
> pointer.
You meant to have assigned "void *security" dedicated to "struct task_struct".
But "void *security" is no longer directly attached to "struct task_struct".
I couldn't find code that checks whether "current->cred" is used by only
current thread or not. "current->cred" being used by only current thread is
a requirement for having a "tsec" allocated to every "struct task_struct".
Your code will share "tsec" among multiple threads if a process created
threads using clone(CLONE_THREAD | CLONE_SIGHAND | CLONE_VM). Each thread has
its own "struct task_struct" but they share "cred->security".
Sharing "tsec" among multiple threads is what you intended?
Regards.
^ permalink raw reply
* Re: [RFC v2 00/10] snet: Security for NETwork syscalls
From: Tetsuo Handa @ 2010-03-07 5:47 UTC (permalink / raw)
To: sam
Cc: linux-kernel, netdev, netfilter-devel, hadi, kaber, zbr, nhorman,
root, linux-security-module, serue
In-Reply-To: <m2zl2lpa4d.fsf@ssh.synack.fr>
Samir Bellabes wrote:
> Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> writes:
>
> > Regarding [RFC v2 02/10] Revert "lsm: Remove the socket_post_accept() hook"
> > @@ -1538,6 +1538,8 @@ SYSCALL_DEFINE4(accept4, int, fd, struct sockaddr __user *, upeer_sockaddr,
> > fd_install(newfd, newfile);
> > err = newfd;
> >
> > + security_socket_post_accept(sock, newsock);
> > +
> > out_put:
> > fput_light(sock->file, fput_needed);
> > out:
> >
> > Please move security_socket_post_accept() to before fd_install().
> > Otherwise, other threads which share fd tables can use
> > security-informations-not-yet-updated accept()ed sockets.
>
> Tetsuo, what about this patch ?
>
Looks OK to me. Thanks.
^ permalink raw reply
* [PATCH net-2.6] netfilter: ebt_ip6: Use ipv6_masked_addr_cmp().
From: YOSHIFUJI Hideaki @ 2010-03-07 8:39 UTC (permalink / raw)
To: bart.de.schuymer, kaber, davem
Cc: ebtables-devel, netfilter-devel, netdev, yoshfuji
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
net/bridge/netfilter/ebt_ip6.c | 18 ++++--------------
1 files changed, 4 insertions(+), 14 deletions(-)
diff --git a/net/bridge/netfilter/ebt_ip6.c b/net/bridge/netfilter/ebt_ip6.c
index bbf2534..4644cc9 100644
--- a/net/bridge/netfilter/ebt_ip6.c
+++ b/net/bridge/netfilter/ebt_ip6.c
@@ -35,8 +35,6 @@ ebt_ip6_mt(const struct sk_buff *skb, const struct xt_match_param *par)
struct ipv6hdr _ip6h;
const struct tcpudphdr *pptr;
struct tcpudphdr _ports;
- struct in6_addr tmp_addr;
- int i;
ih6 = skb_header_pointer(skb, 0, sizeof(_ip6h), &_ip6h);
if (ih6 == NULL)
@@ -44,18 +42,10 @@ ebt_ip6_mt(const struct sk_buff *skb, const struct xt_match_param *par)
if (info->bitmask & EBT_IP6_TCLASS &&
FWINV(info->tclass != ipv6_get_dsfield(ih6), EBT_IP6_TCLASS))
return false;
- for (i = 0; i < 4; i++)
- tmp_addr.in6_u.u6_addr32[i] = ih6->saddr.in6_u.u6_addr32[i] &
- info->smsk.in6_u.u6_addr32[i];
- if (info->bitmask & EBT_IP6_SOURCE &&
- FWINV((ipv6_addr_cmp(&tmp_addr, &info->saddr) != 0),
- EBT_IP6_SOURCE))
- return false;
- for (i = 0; i < 4; i++)
- tmp_addr.in6_u.u6_addr32[i] = ih6->daddr.in6_u.u6_addr32[i] &
- info->dmsk.in6_u.u6_addr32[i];
- if (info->bitmask & EBT_IP6_DEST &&
- FWINV((ipv6_addr_cmp(&tmp_addr, &info->daddr) != 0), EBT_IP6_DEST))
+ if (FWINV(ipv6_masked_addr_cmp(&ih6->saddr, &info->smsk,
+ &info->saddr), EBT_IP6_SOURCE) ||
+ FWINV(ipv6_masked_addr_cmp(&ih6->daddr, &info->dmsk,
+ &info->daddr), EBT_IP6_DEST))
return false;
if (info->bitmask & EBT_IP6_PROTO) {
uint8_t nexthdr = ih6->nexthdr;
--
1.5.6.5
^ permalink raw reply related
* [PATCH net-2.6] netfilter: Remove stale declaration for ip6_masked_addrcmp().
From: YOSHIFUJI Hideaki @ 2010-03-07 8:39 UTC (permalink / raw)
To: kaber, davem; +Cc: netfilter-devel, netdev, yoshfuji
Commit f2ffd9ee... ("[NETFILTER]: Move ip6_masked_addrcmp to
include/net/ipv6.h") replaced ip6_masked_addrcmp() with
ipv6_masked_addr_cmp(). Function definition went away.
Let's remove its declaration as well in header file.
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
include/linux/netfilter_ipv6/ip6_tables.h | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/include/linux/netfilter_ipv6/ip6_tables.h b/include/linux/netfilter_ipv6/ip6_tables.h
index e5ba03d..18442ff 100644
--- a/include/linux/netfilter_ipv6/ip6_tables.h
+++ b/include/linux/netfilter_ipv6/ip6_tables.h
@@ -316,10 +316,6 @@ extern int ip6t_ext_hdr(u8 nexthdr);
extern int ipv6_find_hdr(const struct sk_buff *skb, unsigned int *offset,
int target, unsigned short *fragoff);
-extern int ip6_masked_addrcmp(const struct in6_addr *addr1,
- const struct in6_addr *mask,
- const struct in6_addr *addr2);
-
#define IP6T_ALIGN(s) XT_ALIGN(s)
#ifdef CONFIG_COMPAT
--
1.5.6.5
^ permalink raw reply related
* Re: drivers/net/mac8390.c: Remove useless memcpy casting
From: Geert Uytterhoeven @ 2010-03-07 9:19 UTC (permalink / raw)
To: Joe Perches
Cc: David S. Miller, netdev, Linux Kernel Mailing List, Linux/m68k
In-Reply-To: <201003022001.o22K16vJ011337@hera.kernel.org>
On Tue, Mar 2, 2010 at 21:01, Linux Kernel Mailing List
<linux-kernel@vger.kernel.org> wrote:
> Gitweb: http://git.kernel.org/linus/5c7fffd0e3b57cb63f50bbd710868f012d67654f
> Commit: 5c7fffd0e3b57cb63f50bbd710868f012d67654f
> Parent: 35076402a9936fa8a73b57a1f97fecbeceeec34a
> Author: Joe Perches <joe@perches.com>
> AuthorDate: Mon Jan 4 11:53:00 2010 +0000
> Committer: David S. Miller <davem@davemloft.net>
> CommitDate: Wed Jan 6 20:44:02 2010 -0800
>
> drivers/net/mac8390.c: Remove useless memcpy casting
>
> Signed-off-by: Joe Perches <joe@perches.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
> drivers/net/mac8390.c | 19 ++++++++++---------
> 1 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/mac8390.c b/drivers/net/mac8390.c
> index 6b6f375..517cee4 100644
> --- a/drivers/net/mac8390.c
> +++ b/drivers/net/mac8390.c
> @@ -237,14 +237,14 @@ static enum mac8390_access __init mac8390_testio(volatile unsigned long membase)
As indicated by the prototype above, membase is not a pointer...
> unsigned long outdata = 0xA5A0B5B0;
> unsigned long indata = 0x00000000;
> /* Try writing 32 bits */
> - memcpy((char *)membase, (char *)&outdata, 4);
> + memcpy(membase, &outdata, 4);
> /* Now compare them */
> if (memcmp((char *)&outdata, (char *)membase, 4) == 0)
> return ACCESS_32;
> /* Write 16 bit output */
> - word_memcpy_tocard((char *)membase, (char *)&outdata, 4);
> + word_memcpy_tocard(membase, &outdata, 4);
> /* Now read it back */
> - word_memcpy_fromcard((char *)&indata, (char *)membase, 4);
> + word_memcpy_fromcard(&indata, membase, 4);
> if (outdata == indata)
> return ACCESS_16;
> return ACCESS_UNKNOWN;
... hence you introduced 3 compiler warnings:
drivers/net/mac8390.c:249: warning: passing argument 1 of
'__builtin_memcpy' makes pointer from integer without a cast
drivers/net/mac8390.c:254: warning: passing argument 1 of
'word_memcpy_tocard' makes pointer from integer without a cast
drivers/net/mac8390.c:256: warning: passing argument 2 of
'word_memcpy_fromcard' makes pointer from integer without a cast
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH net-next-2.6] ipv6: Optmize translation between IPV6_PREFER_SRC_xxx and RT6_LOOKUP_F_xxx.
From: YOSHIFUJI Hideaki @ 2010-03-07 10:14 UTC (permalink / raw)
To: davem; +Cc: yoshfuji, netdev
IPV6_PREFER_SRC_xxx definitions:
| #define IPV6_PREFER_SRC_TMP 0x0001
| #define IPV6_PREFER_SRC_PUBLIC 0x0002
| #define IPV6_PREFER_SRC_COA 0x0004
RT6_LOOKUP_F_xxx definitions:
| #define RT6_LOOKUP_F_SRCPREF_TMP 0x00000008
| #define RT6_LOOKUP_F_SRCPREF_PUBLIC 0x00000010
| #define RT6_LOOKUP_F_SRCPREF_COA 0x00000020
So, we can translate between these two groups by shift operation
instead of multiple 'if's.
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
include/net/ip6_route.h | 18 ++++++++++++++++++
net/ipv6/fib6_rules.c | 11 ++---------
net/ipv6/route.c | 11 ++---------
3 files changed, 22 insertions(+), 18 deletions(-)
diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 4a808de..68f6783 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -37,6 +37,24 @@ struct route_info {
#define RT6_LOOKUP_F_SRCPREF_PUBLIC 0x00000010
#define RT6_LOOKUP_F_SRCPREF_COA 0x00000020
+/*
+ * rt6_srcprefs2flags() and rt6_flags2srcprefs() translate
+ * between IPV6_ADDR_PREFERENCES socket option values
+ * IPV6_PREFER_SRC_TMP = 0x1
+ * IPV6_PREFER_SRC_PUBLIC = 0x2
+ * IPV6_PREFER_SRC_COA = 0x4
+ * and above RT6_LOOKUP_F_SRCPREF_xxx flags.
+ */
+static inline int rt6_srcprefs2flags(unsigned int srcprefs)
+{
+ /* No need to bitmask because srcprefs have only 3 bits. */
+ return srcprefs << 3;
+}
+
+static inline unsigned int rt6_flags2srcprefs(int flags)
+{
+ return (flags >> 3) & 7;
+}
extern void ip6_route_input(struct sk_buff *skb);
diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
index 551882b..5e463c4 100644
--- a/net/ipv6/fib6_rules.c
+++ b/net/ipv6/fib6_rules.c
@@ -84,18 +84,11 @@ static int fib6_rule_action(struct fib_rule *rule, struct flowi *flp,
if ((rule->flags & FIB_RULE_FIND_SADDR) &&
r->src.plen && !(flags & RT6_LOOKUP_F_HAS_SADDR)) {
struct in6_addr saddr;
- unsigned int srcprefs = 0;
-
- if (flags & RT6_LOOKUP_F_SRCPREF_TMP)
- srcprefs |= IPV6_PREFER_SRC_TMP;
- if (flags & RT6_LOOKUP_F_SRCPREF_PUBLIC)
- srcprefs |= IPV6_PREFER_SRC_PUBLIC;
- if (flags & RT6_LOOKUP_F_SRCPREF_COA)
- srcprefs |= IPV6_PREFER_SRC_COA;
if (ipv6_dev_get_saddr(net,
ip6_dst_idev(&rt->u.dst)->dev,
- &flp->fl6_dst, srcprefs,
+ &flp->fl6_dst,
+ rt6_flags2srcprefs(flags),
&saddr))
goto again;
if (!ipv6_prefix_equal(&saddr, &r->src.addr,
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index b08879e..52cd3ef 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -819,15 +819,8 @@ struct dst_entry * ip6_route_output(struct net *net, struct sock *sk,
if (!ipv6_addr_any(&fl->fl6_src))
flags |= RT6_LOOKUP_F_HAS_SADDR;
- else if (sk) {
- unsigned int prefs = inet6_sk(sk)->srcprefs;
- if (prefs & IPV6_PREFER_SRC_TMP)
- flags |= RT6_LOOKUP_F_SRCPREF_TMP;
- if (prefs & IPV6_PREFER_SRC_PUBLIC)
- flags |= RT6_LOOKUP_F_SRCPREF_PUBLIC;
- if (prefs & IPV6_PREFER_SRC_COA)
- flags |= RT6_LOOKUP_F_SRCPREF_COA;
- }
+ else if (sk)
+ flags |= rt6_srcprefs2flags(inet6_sk(sk)->srcprefs);
return fib6_rule_lookup(net, fl, flags, ip6_pol_route_output);
}
--
1.5.6.5
^ permalink raw reply related
* inconsistent lock state
From: Sergey Senozhatsky @ 2010-03-07 10:21 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, Ingo Molnar
[-- Attachment #1: Type: text/plain, Size: 5293 bytes --]
Hello,
Hardly reproducible.
/*
* 2.6.33. x86. ASUS f3jc
*/
[329645.010697] =================================
[329645.010699] [ INFO: inconsistent lock state ]
[329645.010703] 2.6.33-33-0-dbg #31
[329645.010705] ---------------------------------
[329645.010708] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
[329645.010712] events/0/9 [HC0[0]:SC0[0]:HE1:SE1] takes:
[329645.010715] (&(&table->hash[i].lock)->rlock){+.?.-.}, at: [<c12c1892>] spin_lock+0x8/0xa
[329645.010729] {IN-SOFTIRQ-W} state was registered at:
[329645.010732] [<c105878f>] __lock_acquire+0x27e/0xb86
[329645.010739] [<c1059988>] lock_acquire+0xa1/0xb8
[329645.010744] [<c12f39b2>] _raw_spin_lock+0x28/0x58
[329645.010750] [<c12c1892>] spin_lock+0x8/0xa
[329645.010755] [<c12c2c8f>] T.958+0x3c/0x141
[329645.010759] [<c12c2f7a>] __udp4_lib_rcv+0x1e6/0x3cd
[329645.010764] [<c12c3173>] udp_rcv+0x12/0x14
[329645.010769] [<c12a559b>] ip_local_deliver_finish+0xc9/0x130
[329645.010774] [<c12a5663>] ip_local_deliver+0x61/0x66
[329645.010778] [<c12a51a5>] ip_rcv_finish+0x275/0x29d
[329645.010783] [<c12a539a>] ip_rcv+0x1cd/0x1ed
[329645.010786] [<c128858d>] netif_receive_skb+0x340/0x360
[329645.010791] [<fd1ab6f4>] rtl8169_rx_interrupt+0x2bf/0x37e [r8169]
[329645.010801] [<fd1adab8>] rtl8169_poll+0x29/0x15a [r8169]
[329645.010808] [<c1288b8c>] net_rx_action+0x95/0x1af
[329645.010812] [<c1037243>] __do_softirq+0xc6/0x187
[329645.010819] [<c103732f>] do_softirq+0x2b/0x43
[329645.010823] [<c10374bf>] irq_exit+0x38/0x75
[329645.010828] [<c1004116>] do_IRQ+0x88/0x9c
[329645.010833] [<c1002f35>] common_interrupt+0x35/0x3c
[329645.010837] [<c1275d0b>] cpuidle_idle_call+0x72/0xd3
[329645.010844] [<c1001c1b>] cpu_idle+0x92/0xbf
[329645.010848] [<c12e3f16>] rest_init+0x76/0x78
[329645.010853] [<c14c2868>] start_kernel+0x33c/0x341
[329645.010859] [<c14c2092>] i386_start_kernel+0x92/0x99
[329645.010864] irq event stamp: 157782307
[329645.010866] hardirqs last enabled at (157782307): [<c10b74b8>] kmem_cache_free+0x97/0xd6
[329645.010873] hardirqs last disabled at (157782306): [<c10b745a>] kmem_cache_free+0x39/0xd6
[329645.010878] softirqs last enabled at (157782304): [<c12928ef>] rcu_read_unlock_bh+0x1c/0x1e
[329645.010885] softirqs last disabled at (157782302): [<c129289b>] rcu_read_lock_bh+0x8/0x26
[329645.010892]
[329645.010893] other info that might help us debug this:
[329645.010896] 5 locks held by events/0/9:
[329645.010898] #0: (events){+.+.+.}, at: [<c1044f2f>] worker_thread+0x16a/0x27c
[329645.010908] #1: ((&(&tp->task)->work)){+.+...}, at: [<c1044f2f>] worker_thread+0x16a/0x27c
[329645.010915] #2: (rtnl_mutex){+.+.+.}, at: [<c12911dd>] rtnl_lock+0xf/0x11
[329645.010922] #3: (rcu_read_lock){.+.+..}, at: [<c1286a01>] rcu_read_lock+0x0/0x2b
[329645.010931] #4: (rcu_read_lock){.+.+..}, at: [<c12a4ed0>] rcu_read_lock+0x0/0x2b
[329645.010938]
[329645.010939] stack backtrace:
[329645.010942] Pid: 9, comm: events/0 Not tainted 2.6.33-33-0-dbg #31
[329645.010945] Call Trace:
[329645.010950] [<c12f18ec>] ? printk+0xf/0x11
[329645.010955] [<c1057800>] valid_state+0x12a/0x13d
[329645.010960] [<c1057904>] mark_lock+0xf1/0x1e2
[329645.010965] [<c1057ffa>] ? check_usage_backwards+0x0/0x6f
[329645.010970] [<c10587fd>] __lock_acquire+0x2ec/0xb86
[329645.010976] [<c100864f>] ? native_sched_clock+0x48/0x8d
[329645.010982] [<c104d24b>] ? sched_clock_local+0x17/0x11e
[329645.010987] [<c12c1892>] ? spin_lock+0x8/0xa
[329645.010992] [<c1059988>] lock_acquire+0xa1/0xb8
[329645.010997] [<c12c1892>] ? spin_lock+0x8/0xa
[329645.011002] [<c12f39b2>] _raw_spin_lock+0x28/0x58
[329645.011006] [<c12c1892>] ? spin_lock+0x8/0xa
[329645.011010] [<c12c1892>] spin_lock+0x8/0xa
[329645.011015] [<c12c2c8f>] T.958+0x3c/0x141
[329645.011020] [<c104d472>] ? sched_clock_cpu+0x120/0x128
[329645.011025] [<c100864f>] ? native_sched_clock+0x48/0x8d
[329645.011031] [<c1059088>] ? __lock_acquire+0xb77/0xb86
[329645.011037] [<c12a1345>] ? rcu_read_unlock+0x0/0x35
[329645.011042] [<c12a38c0>] ? ip_route_input+0x102/0xacb
[329645.011046] [<c1057831>] ? mark_lock+0x1e/0x1e2
[329645.011051] [<c12a4ed0>] ? rcu_read_lock+0x0/0x2b
[329645.011056] [<c12c2f7a>] __udp4_lib_rcv+0x1e6/0x3cd
[329645.011061] [<c12c3173>] udp_rcv+0x12/0x14
[329645.011065] [<c12a559b>] ip_local_deliver_finish+0xc9/0x130
[329645.011070] [<c12a5663>] ip_local_deliver+0x61/0x66
[329645.011074] [<c12a51a5>] ip_rcv_finish+0x275/0x29d
[329645.011078] [<c12a539a>] ip_rcv+0x1cd/0x1ed
[329645.011083] [<c128858d>] netif_receive_skb+0x340/0x360
[329645.011093] [<fd1ab6f4>] rtl8169_rx_interrupt+0x2bf/0x37e [r8169]
[329645.011100] [<fd1aba02>] rtl8169_reset_task+0x38/0xcd [r8169]
[329645.011105] [<c1044f71>] worker_thread+0x1ac/0x27c
[329645.011110] [<c1044f2f>] ? worker_thread+0x16a/0x27c
[329645.011116] [<fd1ab9ca>] ? rtl8169_reset_task+0x0/0xcd [r8169]
[329645.011123] [<c1048725>] ? autoremove_wake_function+0x0/0x2f
[329645.011128] [<c1044dc5>] ? worker_thread+0x0/0x27c
[329645.011132] [<c104838a>] kthread+0x6a/0x6f
[329645.011137] [<c1048320>] ? kthread+0x0/0x6f
[329645.011142] [<c1002f42>] kernel_thread_helper+0x6/0x1a
Sergey
[-- Attachment #2: Type: application/pgp-signature, Size: 316 bytes --]
^ permalink raw reply
* Re: [PATCH v1 0/3] Provide a zero-copy method on KVM virtio-net.
From: Michael S. Tsirkin @ 2010-03-07 10:50 UTC (permalink / raw)
To: xiaohui.xin; +Cc: netdev, kvm, linux-kernel, mingo, jdike
In-Reply-To: <1267868318-19268-1-git-send-email-xiaohui.xin@intel.com>
On Sat, Mar 06, 2010 at 05:38:35PM +0800, xiaohui.xin@intel.com wrote:
> The idea is simple, just to pin the guest VM user space and then
> let host NIC driver has the chance to directly DMA to it.
> The patches are based on vhost-net backend driver. We add a device
> which provides proto_ops as sendmsg/recvmsg to vhost-net to
> send/recv directly to/from the NIC driver. KVM guest who use the
> vhost-net backend may bind any ethX interface in the host side to
> get copyless data transfer thru guest virtio-net frontend.
>
> We provide multiple submits and asynchronous notifiicaton to
> vhost-net too.
>
> Our goal is to improve the bandwidth and reduce the CPU usage.
> Exact performance data will be provided later. But for simple
> test with netperf, we found bindwidth up and CPU % up too,
> but the bindwidth up ratio is much more than CPU % up ratio.
>
> What we have not done yet:
> packet split support
> To support GRO
> Performance tuning
Am I right to say that nic driver needs changes for these patches
to work? If so, please publish nic driver patches as well.
> what we have done in v1:
> polish the RCU usage
> deal with write logging in asynchroush mode in vhost
> add notifier block for mp device
> rename page_ctor to mp_port in netdevice.h to make it looks generic
> add mp_dev_change_flags() for mp device to change NIC state
> add CONIFG_VHOST_MPASSTHRU to limit the usage when module is not load
> a small fix for missing dev_put when fail
> using dynamic minor instead of static minor number
> a __KERNEL__ protect to mp_get_sock()
>
> performance:
> using netperf with GSO/TSO disabled, 10G NIC,
> disabled packet split mode, with raw socket case compared to vhost.
>
> bindwidth will be from 1.1Gbps to 1.7Gbps
> CPU % from 120%-140% to 140%-160%
That's pretty low for a 10Gb nic. Are you hitting some other bottleneck,
like high interrupt rate? Also, GSO support and performance tuning
for raw are incomplete. Try comparing with e.g. tap with GSO.
--
MST
^ permalink raw reply
* [PATCH 0/3] cpmac fixes
From: Florian Fainelli @ 2010-03-07 10:55 UTC (permalink / raw)
To: netdev, David S. Miller
Hi David,
Here are 2 small cpmac fixes and a driver version bumping. Thanks!
Florian Fainelli (3):
cpmac: fix the receiving of 802.1q frames
cpmac: fallback to switch mode if no PHY chip found
cpmac: bump version to 0.5.2
^ permalink raw reply
* [PATCH 1/3] cpmac: fix the receiving of 802.1q frames
From: Florian Fainelli @ 2010-03-07 10:55 UTC (permalink / raw)
To: netdev, David S. Miller
Despite what the comment above CPMAC_SKB_SIZE says, the hardware also
needs to account for the FCS length in a received frame. This patch fix
the receiving of 802.1q frames which have 4 more bytes. While at it
unhardcode the definition and use the one from if_vlan.h.
Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
diff --git a/drivers/net/cpmac.c b/drivers/net/cpmac.c
index b85c81f..722a196 100644
--- a/drivers/net/cpmac.c
+++ b/drivers/net/cpmac.c
@@ -28,6 +28,7 @@
#include <linux/delay.h>
#include <linux/netdevice.h>
+#include <linux/if_vlan.h>
#include <linux/etherdevice.h>
#include <linux/ethtool.h>
#include <linux/skbuff.h>
@@ -56,8 +57,8 @@ MODULE_PARM_DESC(debug_level, "Number of NETIF_MSG bits to enable");
MODULE_PARM_DESC(dumb_switch, "Assume switch is not connected to MDIO bus");
#define CPMAC_VERSION "0.5.1"
-/* frame size + 802.1q tag */
-#define CPMAC_SKB_SIZE (ETH_FRAME_LEN + 4)
+/* frame size + 802.1q tag + FCS size */
+#define CPMAC_SKB_SIZE (ETH_FRAME_LEN + ETH_FCS_LEN + VLAN_HLEN)
#define CPMAC_QUEUES 8
/* Ethernet registers */
^ permalink raw reply related
* [PATCH 2/3] cpmac: fallback to switch mode if no PHY chip found
From: Florian Fainelli @ 2010-03-07 10:55 UTC (permalink / raw)
To: netdev, David S. Miller
If we were unable to detect a PHY on any of the MDIO bus id we tried instead of
bailing out with -ENODEV, assume the MAC is connected to a switch and use MDIO
bus 0. This unbreaks quite a lot of devices out there whose switch cannot be
detected.
Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
diff --git a/drivers/net/cpmac.c b/drivers/net/cpmac.c
index 722a196..4051e81 100644
--- a/drivers/net/cpmac.c
+++ b/drivers/net/cpmac.c
@@ -1137,8 +1137,9 @@ static int __devinit cpmac_probe(struct platform_device *pdev)
}
if (phy_id == PHY_MAX_ADDR) {
- dev_err(&pdev->dev, "no PHY present\n");
- return -ENODEV;
+ dev_err(&pdev->dev, "no PHY present, falling back to switch on MDIO bus 0\n");
+ strncpy(mdio_bus_id, "0", MII_BUS_ID_SIZE); /* fixed phys bus */
+ phy_id = pdev->id;
}
dev = alloc_etherdev_mq(sizeof(*priv), CPMAC_QUEUES);
^ permalink raw reply related
* [PATCH 3/3] cpmac: bump version to 0.5.2
From: Florian Fainelli @ 2010-03-07 10:55 UTC (permalink / raw)
To: netdev, David S. Miller
Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
diff --git a/drivers/net/cpmac.c b/drivers/net/cpmac.c
index 4051e81..97b8890 100644
--- a/drivers/net/cpmac.c
+++ b/drivers/net/cpmac.c
@@ -56,7 +56,7 @@ module_param(dumb_switch, int, 0444);
MODULE_PARM_DESC(debug_level, "Number of NETIF_MSG bits to enable");
MODULE_PARM_DESC(dumb_switch, "Assume switch is not connected to MDIO bus");
-#define CPMAC_VERSION "0.5.1"
+#define CPMAC_VERSION "0.5.2"
/* frame size + 802.1q tag + FCS size */
#define CPMAC_SKB_SIZE (ETH_FRAME_LEN + ETH_FCS_LEN + VLAN_HLEN)
#define CPMAC_QUEUES 8
^ permalink raw reply related
* Re: [PATCH v1 2/3] Provides multiple submits and asynchronous notifications.
From: Michael S. Tsirkin @ 2010-03-07 11:18 UTC (permalink / raw)
To: xiaohui.xin; +Cc: netdev, kvm, linux-kernel, mingo, jdike
In-Reply-To: <1267868318-19268-3-git-send-email-xiaohui.xin@intel.com>
> +/* The structure to notify the virtqueue for async socket */
> +struct vhost_notifier {
> + struct list_head list;
> + struct vhost_virtqueue *vq;
> + int head;
> + int size;
> + int log;
> + void *ctrl;
> + void (*dtor)(struct vhost_notifier *);
> +};
> +
So IMO, this is not the best interface between vhost
and your driver, exposing them to each other unnecessarily.
If you think about it, your driver should not care about this structure.
It could get e.g. a kiocb (sendmsg already gets one), and call ki_dtor
on completion. vhost could save it's state in ki_user_data. If your
driver needs to add more data to do more tracking, I think it can put
skb pointer in the private pointer.
--
MST
^ permalink raw reply
* [patch] irda-usb: add error handling and fix leak
From: Dan Carpenter @ 2010-03-07 12:35 UTC (permalink / raw)
To: Samuel Ortiz
Cc: David S. Miller, Stephen Hemminger, Patrick McHardy, Tadashi Abe,
Julia Lawall, netdev, kernel-janitors
If the call to kcalloc() fails then we should return -ENOMEM.
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/drivers/net/irda/irda-usb.c b/drivers/net/irda/irda-usb.c
index e8e33bb..2c9b3af 100644
--- a/drivers/net/irda/irda-usb.c
+++ b/drivers/net/irda/irda-usb.c
@@ -1651,6 +1651,8 @@ static int irda_usb_probe(struct usb_interface *intf,
self->rx_urb = kcalloc(self->max_rx_urb, sizeof(struct urb *),
GFP_KERNEL);
+ if (!self->rx_urb)
+ goto err_free_net;
for (i = 0; i < self->max_rx_urb; i++) {
self->rx_urb[i] = usb_alloc_urb(0, GFP_KERNEL);
@@ -1783,6 +1785,8 @@ err_out_2:
err_out_1:
for (i = 0; i < self->max_rx_urb; i++)
usb_free_urb(self->rx_urb[i]);
+ kfree(self->rx_urb);
+err_free_net:
free_netdev(net);
err_out:
return ret;
^ permalink raw reply related
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