* Re: pull request: wireless 2012-08-03
From: David Miller @ 2012-08-03 23:41 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20120803145604.GA6372@tuxdriver.com>
From: "John W. Linville" <linville@tuxdriver.com>
Date: Fri, 3 Aug 2012 10:56:04 -0400
> This request covers a batch of fixes intended for the 3.6 stream.
Pulled, thanks John.
^ permalink raw reply
* Re: [PATCH V2 09/12] net/eipoib: Add main driver functionality
From: David Miller @ 2012-08-03 23:36 UTC (permalink / raw)
To: ali; +Cc: ebiederm, ogerlitz, roland, netdev, sean.hefty, erezsh
In-Reply-To: <501C5328.4060301@mellanox.com>
From: Ali Ayoub <ali@mellanox.com>
Date: Fri, 03 Aug 2012 15:39:36 -0700
> Users would like to use sockets API from the VM without re-writing their
> applications on top of IB verbs, this driver meant to allow such a user
> to do so.
That's what IPoIB was for, the application writers who don't want to have
to be knowledgable about IB verbs.
You're messing with the link layer here, and that's what is upsetting me.
It's a complete cop-out to changing the VM tools and emulators properly to
handle a new link layer.
The applications writers already have a way to use IB whilst using
something familiar, like IPv4, via IPoIB. You're doing something
completely different here, and it stinks.
^ permalink raw reply
* LONDON OLYMPIC NOTIFICATION
From: RINKU PATEL @ 2012-08-03 23:04 UTC (permalink / raw)
Congratulations!!! For more informations communicate to the below contact;
CONTACT PERSON Larry Williams Contact Number:+447035972724
E-mail Adress: larrywilliamslawchamber@yahoo.com.hk
^ permalink raw reply
* Re: [PATCH V2 09/12] net/eipoib: Add main driver functionality
From: Ali Ayoub @ 2012-08-03 22:39 UTC (permalink / raw)
To: David Miller; +Cc: ebiederm, ogerlitz, roland, netdev, sean.hefty, erezsh
In-Reply-To: <20120803.143315.151094375569109262.davem@davemloft.net>
On 8/3/2012 2:33 PM, David Miller wrote:
> From: Ali Ayoub <ali@mellanox.com>
> Date: Fri, 03 Aug 2012 13:31:35 -0700
>
>> With eIPoIB architecture, the VM sees standard Ethernet emulator,
>> allowing the administrator to enslave eIPoIB PIF to the vSwitch/vBridge
>> as if it was standard Ethernet. Other approaches that exposes IB QP to
>> the VM (with w/o bypassing the kernel) won't be possible with the
>> current emulators and management tools.
>
> So then fix the emulators and management tools to handle IB instead
> of adding this bogus new protocol?
>
> This new protocol seems to exist only because you don't want to have
> to enhance the emulators and tools, and I'm sorry that isn't a valid
> reason to do something like this.
This driver exists to allow the user to have an Ethernet interface on
top of a high-speed InfiniBand (IB) interconnect.
Users would like to use sockets API from the VM without re-writing their
applications on top of IB verbs, this driver meant to allow such a user
to do so.
Exposing IB emulators and having IB support in the management tools for
the VM/Hypervisor won't address the usecases that this driver meant for.
With this driver, existing VMs, and their existing IP applications, can
run as-is on InfiniBand network.
^ permalink raw reply
* Re: [RFC v2 1/7] hashtable: introduce a small and naive hashtable
From: Tejun Heo @ 2012-08-03 22:36 UTC (permalink / raw)
To: Linus Torvalds
Cc: Sasha Levin, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
rostedt, mingo, ebiederm, aarcange, ericvh, netdev
In-Reply-To: <CA+55aFyOst4c3WHbPVbYkSBdBmLJUui5OvoVOh5AuPMnigwnEA@mail.gmail.com>
On Fri, Aug 03, 2012 at 03:29:10PM -0700, Linus Torvalds wrote:
> On Fri, Aug 3, 2012 at 3:23 PM, Tejun Heo <tj@kernel.org> wrote:
> >
> > I actually meant an enclosing struct. When you're defining a struct
> > member, simply putting the storage after a struct with var array
> > should be good enough. If that doesn't work, quite a few things in
> > the kernel will break.
>
> The unsigned member of a struct has to be the last one, so your struct
> won't work.
I suppose you mean unsized. I remember this working. Maybe I'm
confusing it with zero-sized array. Hmm... gcc doesn't complain about
the following. --std=c99 seems happy too.
#include <stdio.h>
struct A {
int i;
long ar[];
};
struct B {
struct A a;
long ar_storage[32];
};
int main(void)
{
printf("sizeof(A)=%zd sizeof(B)=%zd\n", sizeof(struct A), sizeof(struct B));
return 0;
}
$ ./a.out
sizeof(A)=8 sizeof(B)=264
--
tejun
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC v2 1/7] hashtable: introduce a small and naive hashtable
From: Linus Torvalds @ 2012-08-03 22:29 UTC (permalink / raw)
To: Tejun Heo
Cc: Sasha Levin, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
rostedt, mingo, ebiederm, aarcange, ericvh, netdev
In-Reply-To: <20120803222339.GN15477@google.com>
On Fri, Aug 3, 2012 at 3:23 PM, Tejun Heo <tj@kernel.org> wrote:
>
> I actually meant an enclosing struct. When you're defining a struct
> member, simply putting the storage after a struct with var array
> should be good enough. If that doesn't work, quite a few things in
> the kernel will break.
The unsigned member of a struct has to be the last one, so your struct
won't work.
Linus
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC v2 1/7] hashtable: introduce a small and naive hashtable
From: Sasha Levin @ 2012-08-03 22:26 UTC (permalink / raw)
To: Tejun Heo
Cc: torvalds, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
rostedt, mingo, ebiederm, aarcange, ericvh, netdev
In-Reply-To: <20120803222339.GN15477@google.com>
On 08/04/2012 12:23 AM, Tejun Heo wrote:
> Hello,
>
> On Sat, Aug 04, 2012 at 12:20:02AM +0200, Sasha Levin wrote:
>> On 08/03/2012 11:48 PM, Tejun Heo wrote:
>>> On Fri, Aug 03, 2012 at 11:41:34PM +0200, Sasha Levin wrote:
>>>> I forgot to comment on that one, sorry.
>>>>
>>>> If we put hash entries after struct hash_table we don't take the
>>>> bits field size into account, or did I miss something?
>>>
>>> So, if you do the following,
>>>
>>> struct {
>>> struct {
>>> int i;
>>> long ar[];
>>> } B;
>>> long __ar_storage[32];
>>> } A;
>>
>> struct A should have been an union, right?
>
> I actually meant an enclosing struct. When you're defining a struct
> member, simply putting the storage after a struct with var array
> should be good enough. If that doesn't work, quite a few things in
> the kernel will break.
Ah, I see, I've missed that part.
Thanks!
> Thanks.
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC v2 1/7] hashtable: introduce a small and naive hashtable
From: Tejun Heo @ 2012-08-03 22:23 UTC (permalink / raw)
To: Sasha Levin
Cc: torvalds, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
rostedt, mingo, ebiederm, aarcange, ericvh, netdev
In-Reply-To: <501C4E92.1070801@gmail.com>
Hello,
On Sat, Aug 04, 2012 at 12:20:02AM +0200, Sasha Levin wrote:
> On 08/03/2012 11:48 PM, Tejun Heo wrote:
> > On Fri, Aug 03, 2012 at 11:41:34PM +0200, Sasha Levin wrote:
> >> I forgot to comment on that one, sorry.
> >>
> >> If we put hash entries after struct hash_table we don't take the
> >> bits field size into account, or did I miss something?
> >
> > So, if you do the following,
> >
> > struct {
> > struct {
> > int i;
> > long ar[];
> > } B;
> > long __ar_storage[32];
> > } A;
>
> struct A should have been an union, right?
I actually meant an enclosing struct. When you're defining a struct
member, simply putting the storage after a struct with var array
should be good enough. If that doesn't work, quite a few things in
the kernel will break.
Thanks.
--
tejun
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC v2 1/7] hashtable: introduce a small and naive hashtable
From: Sasha Levin @ 2012-08-03 22:20 UTC (permalink / raw)
To: Tejun Heo
Cc: torvalds, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
rostedt, mingo, ebiederm, aarcange, ericvh, netdev
In-Reply-To: <20120803214806.GM15477@google.com>
On 08/03/2012 11:48 PM, Tejun Heo wrote:
> Hello,
>
> On Fri, Aug 03, 2012 at 11:41:34PM +0200, Sasha Levin wrote:
>> I forgot to comment on that one, sorry.
>>
>> If we put hash entries after struct hash_table we don't take the
>> bits field size into account, or did I miss something?
>
> So, if you do the following,
>
> struct {
> struct {
> int i;
> long ar[];
> } B;
> long __ar_storage[32];
> } A;
struct A should have been an union, right?
> It should always be safe to dereference A.B.ar[31]. I'm not sure
> whether this is something guaranteed by C tho. Maybe compilers are
> allowed to put members in reverse order but I think we already depend
> on the above.
why is accessing A.B.ar[31] safe?
__ar_storage is only 32*sizeof(long) bytes long, while struct B would need to be 32*sizeof(long) + sizeof(int) bytes long so that A.B.ar[31] access would be safe.
> Thanks.
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC v2 1/7] hashtable: introduce a small and naive hashtable
From: Tejun Heo @ 2012-08-03 21:48 UTC (permalink / raw)
To: Sasha Levin
Cc: torvalds, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
rostedt, mingo, ebiederm, aarcange, ericvh, netdev
In-Reply-To: <501C458E.7050000@gmail.com>
Hello,
On Fri, Aug 03, 2012 at 11:41:34PM +0200, Sasha Levin wrote:
> I forgot to comment on that one, sorry.
>
> If we put hash entries after struct hash_table we don't take the
> bits field size into account, or did I miss something?
So, if you do the following,
struct {
struct {
int i;
long ar[];
} B;
long __ar_storage[32];
} A;
It should always be safe to dereference A.B.ar[31]. I'm not sure
whether this is something guaranteed by C tho. Maybe compilers are
allowed to put members in reverse order but I think we already depend
on the above.
Thanks.
--
tejun
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC v2 1/7] hashtable: introduce a small and naive hashtable
From: Tejun Heo @ 2012-08-03 21:44 UTC (permalink / raw)
To: Sasha Levin
Cc: torvalds, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
rostedt, mingo, ebiederm, aarcange, ericvh, netdev
In-Reply-To: <501C4471.4090706@gmail.com>
Hello, Sasha.
On Fri, Aug 03, 2012 at 11:36:49PM +0200, Sasha Levin wrote:
> On 08/03/2012 11:30 PM, Tejun Heo wrote:
> The function definition itself is just a macro, for example:
>
> #define MM_SLOTS_HASH_CMP(mm_slot, obj) ((mm_slot)->mm == (obj))
It seems like it would make things more difficult to follow and
error-prone. I'd definitely prefer just using functions.
> As an alternative, what do you think about simplifying that to be
> just a 'cond' instead of a function? Something like:
>
> hash_get(&mm_slots_hash, mm, struct mm_slot, hash, mm);
>
> In that case, the last param ("mm") will get unrolled to a condition like this:
>
> if ((obj)->mm == key)
>
> Which will be simple and easy for the user.
It seems a bit too magical(tm) to me. ;)
> The only reason I want to keep this interface is that most cases
> I've stumbled so far were easy short comparisons of a struct member
> with the key, and I don't want to make them more complex than they
> need to be. I probably will switch hash_get() to use
> hash_for_each_possible() as well, which will cut down on how
> hash_get() is a separate case.
I can understand that but I think the benefit we're talking about is a
bit too miniscule to matter and to have two different interfaces.
What do others think?
Thanks.
--
tejun
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC v2 1/7] hashtable: introduce a small and naive hashtable
From: Sasha Levin @ 2012-08-03 21:41 UTC (permalink / raw)
To: Tejun Heo
Cc: torvalds, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
rostedt, mingo, ebiederm, aarcange, ericvh, netdev
In-Reply-To: <20120803213017.GK15477@google.com>
On 08/03/2012 11:30 PM, Tejun Heo wrote:
> Hello,
>
> On Fri, Aug 03, 2012 at 11:19:57PM +0200, Sasha Levin wrote:
>>> Is this supposed to be embedded in struct definition? If so, the name
>>> is rather misleading as DEFINE_* is supposed to define and initialize
>>> stand-alone constructs. Also, for struct members, simply putting hash
>>> entries after struct hash_table should work.
>>
>> It would work, but I didn't want to just put them in the union since
>> I feel it's safer to keep them in a separate struct so they won't be
>> used by mistake,
>
> Just use ugly enough pre/postfixes. If the user still accesses that,
> it's the user's fault.
I forgot to comment on that one, sorry.
If we put hash entries after struct hash_table we don't take the bits field size into account, or did I miss something?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC v2 1/7] hashtable: introduce a small and naive hashtable
From: Sasha Levin @ 2012-08-03 21:36 UTC (permalink / raw)
To: Tejun Heo
Cc: torvalds, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
rostedt, mingo, ebiederm, aarcange, ericvh, netdev
In-Reply-To: <20120803213017.GK15477@google.com>
On 08/03/2012 11:30 PM, Tejun Heo wrote:
>> I think hash_for_for_each_possible() is useful if the comparison
>> > condition is more complex than a simple comparison of one of the
>> > object members with the key - there's no need to force it on all the
>> > users.
> I don't know. What's the difference? In terms of LOC, it might even
> not save any thanks to the extra function definition, right? I don't
> think it's saving enough complexity to justify a separate rather
> unusual interface.
The function definition itself is just a macro, for example:
#define MM_SLOTS_HASH_CMP(mm_slot, obj) ((mm_slot)->mm == (obj))
As an alternative, what do you think about simplifying that to be just a 'cond' instead of a function? Something like:
hash_get(&mm_slots_hash, mm, struct mm_slot, hash, mm);
In that case, the last param ("mm") will get unrolled to a condition like this:
if ((obj)->mm == key)
Which will be simple and easy for the user.
The only reason I want to keep this interface is that most cases I've stumbled so far were easy short comparisons of a struct member with the key, and I don't want to make them more complex than they need to be. I probably will switch hash_get() to use hash_for_each_possible() as well, which will cut down on how hash_get() is a separate case.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH V2 09/12] net/eipoib: Add main driver functionality
From: David Miller @ 2012-08-03 21:33 UTC (permalink / raw)
To: ali; +Cc: ebiederm, ogerlitz, roland, netdev, sean.hefty, erezsh
In-Reply-To: <501C3527.6060809@mellanox.com>
From: Ali Ayoub <ali@mellanox.com>
Date: Fri, 03 Aug 2012 13:31:35 -0700
> With eIPoIB architecture, the VM sees standard Ethernet emulator,
> allowing the administrator to enslave eIPoIB PIF to the vSwitch/vBridge
> as if it was standard Ethernet. Other approaches that exposes IB QP to
> the VM (with w/o bypassing the kernel) won't be possible with the
> current emulators and management tools.
So then fix the emulators and management tools to handle IB instead
of adding this bogus new protocol?
This new protocol seems to exist only because you don't want to have
to enhance the emulators and tools, and I'm sorry that isn't a valid
reason to do something like this.
^ permalink raw reply
* Re: [PATCH] ipv4: remove parentheses in return statement
From: David Miller @ 2012-08-03 21:31 UTC (permalink / raw)
To: sakiwit; +Cc: netdev, eric.dumazet
In-Reply-To: <20120803202350.GA7520@mail.gmail.com>
From: Jean Sacren <sakiwit@gmail.com>
Date: Fri, 3 Aug 2012 14:23:50 -0600
> To illustrate, the patch merely changes
>
> return (A);
>
> to
>
> return A;
Life is not black and white my friend, there are shades
of gray. I feel bad for you if you read coding style rules
in a %100 literal sense with no room for interpretation.
People add extra parenthesis to add clarity, so you are
entirely misrepresenting these cases.
This was not a simple case of:
return (A);
but more like something that looks as:
return ((A & B) | (C ^ D));
and that is perfectly fine.
^ permalink raw reply
* Re: [RFC v2 1/7] hashtable: introduce a small and naive hashtable
From: Tejun Heo @ 2012-08-03 21:30 UTC (permalink / raw)
To: Sasha Levin
Cc: torvalds, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
rostedt, mingo, ebiederm, aarcange, ericvh, netdev
In-Reply-To: <501C407D.9080900@gmail.com>
Hello,
On Fri, Aug 03, 2012 at 11:19:57PM +0200, Sasha Levin wrote:
> > Is this supposed to be embedded in struct definition? If so, the name
> > is rather misleading as DEFINE_* is supposed to define and initialize
> > stand-alone constructs. Also, for struct members, simply putting hash
> > entries after struct hash_table should work.
>
> It would work, but I didn't want to just put them in the union since
> I feel it's safer to keep them in a separate struct so they won't be
> used by mistake,
Just use ugly enough pre/postfixes. If the user still accesses that,
it's the user's fault.
> >> +static void hash_init(struct hash_table *ht, size_t bits)
> >> +{
> >> + size_t i;
> >
> > I would prefer int here but no biggie.
>
> Just wondering, is there a particular reason behind it?
It isn't a size and using unsigned when signed suffices seems to cause
more headache than helps anything usually due to lack of values to use
for exceptional conditions (usually -errno or -1).
> > As opposed to using hash_for_each_possible(), how much difference does
> > this make? Is it really worthwhile?
>
> Most of the places I've switched to using this hashtable so far (4
> out of 6) are using hash_get(). I think that the code looks cleaner
> when you an just provide a comparison function instead of
> implementing the iteration itself.
>
> I think hash_for_for_each_possible() is useful if the comparison
> condition is more complex than a simple comparison of one of the
> object members with the key - there's no need to force it on all the
> users.
I don't know. What's the difference? In terms of LOC, it might even
not save any thanks to the extra function definition, right? I don't
think it's saving enough complexity to justify a separate rather
unusual interface.
Thanks.
--
tejun
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC v2 1/7] hashtable: introduce a small and naive hashtable
From: Sasha Levin @ 2012-08-03 21:19 UTC (permalink / raw)
To: Tejun Heo
Cc: torvalds, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
rostedt, mingo, ebiederm, aarcange, ericvh, netdev
In-Reply-To: <20120803171515.GH15477@google.com>
On 08/03/2012 07:15 PM, Tejun Heo wrote:
> Hello, Sasha.
>
> On Fri, Aug 03, 2012 at 04:23:02PM +0200, Sasha Levin wrote:
>> +#define DEFINE_STATIC_HASHTABLE(n, b) \
>> + static struct hash_table n = { .bits = (b), \
>> + .buckets = { [0 ... ((1 << (b)) - 1)] = HLIST_HEAD_INIT } }
>
> What does this "static" mean?
>
>> +#define DEFINE_HASHTABLE(n, b) \
>> + union { \
>> + struct hash_table n; \
>> + struct { \
>> + size_t bits; \
>> + struct hlist_head buckets[1 << (b)]; \
>> + } __##n ; \
>> + };
>
> Is this supposed to be embedded in struct definition? If so, the name
> is rather misleading as DEFINE_* is supposed to define and initialize
> stand-alone constructs. Also, for struct members, simply putting hash
> entries after struct hash_table should work.
It would work, but I didn't want to just put them in the union since I feel it's safer to keep them in a separate struct so they won't be used by mistake,
> Wouldn't using DEFINE_HASHTABLE() for the first macro and
> DEFINE_HASHTABLE_MEMBER() for the latter be better?
Indeed that sounds better, will fix.
>> +#define HASH_BITS(name) ((name)->bits)
>> +#define HASH_SIZE(name) (1 << (HASH_BITS(name)))
>> +
>> +__attribute__ ((unused))
>
> Are we using __attribute__((unused)) for functions defined in headers
> instead of static inline now? If so, why?
>
>> +static void hash_init(struct hash_table *ht, size_t bits)
>> +{
>> + size_t i;
>
> I would prefer int here but no biggie.
Just wondering, is there a particular reason behind it?
>> + ht->bits = bits;
>> + for (i = 0; i < (1 << bits); i++)
>> + INIT_HLIST_HEAD(&ht->buckets[i]);
>> +}
>> +
>> +static void hash_add(struct hash_table *ht, struct hlist_node *node, long key)
>> +{
>> + hlist_add_head(node,
>> + &ht->buckets[hash_long((unsigned long)key, HASH_BITS(ht))]);
>> +}
>> +
>> +
>> +#define hash_get(name, key, type, member, cmp_fn) \
>> +({ \
>> + struct hlist_node *__node; \
>> + typeof(key) __key = key; \
>> + type *__obj = NULL; \
>> + hlist_for_each_entry(__obj, __node, &(name)->buckets[ \
>> + hash_long((unsigned long) __key, \
>> + HASH_BITS(name))], member) \
>> + if (cmp_fn(__obj, __key)) \
>> + break; \
>> + __obj; \
>> +})
>
> As opposed to using hash_for_each_possible(), how much difference does
> this make? Is it really worthwhile?
Most of the places I've switched to using this hashtable so far (4 out of 6) are using hash_get(). I think that the code looks cleaner when you an just provide a comparison function instead of implementing the iteration itself.
I think hash_for_for_each_possible() is useful if the comparison condition is more complex than a simple comparison of one of the object members with the key - there's no need to force it on all the users.
>
> Thanks.
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC v2 7/7] net,9p: use new hashtable implementation
From: Sasha Levin @ 2012-08-03 21:14 UTC (permalink / raw)
To: Eric Dumazet
Cc: torvalds, tj, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
rostedt, mingo, ebiederm, aarcange, ericvh, netdev
In-Reply-To: <1344016851.9299.1415.camel@edumazet-glaptop>
On 08/03/2012 08:00 PM, Eric Dumazet wrote:
> On Fri, 2012-08-03 at 16:23 +0200, Sasha Levin wrote:
>> /* initialize hash table */
>> - for (bucket = 0; bucket < ERRHASHSZ; bucket++)
>> - INIT_HLIST_HEAD(&hash_errmap[bucket]);
>> + hash_init(&hash_errmap, ERRHASHSZ);
>
> Why is hash_init() even needed ?
>
> If hash is "DEFINE_STATIC_HASHTABLE(...)", its already ready for use !
Indeed it is.
I've removed it, and then decided to put it back since the definition of the hashtable isn't fully cooked yet, and I didn't want to miss this initialization point if it turn out we need to initialize that hashtable afterall.
I will remove it once the hashtable definitions are clear.
The rest of the review comments will be addressed.
Thanks!
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH V2 09/12] net/eipoib: Add main driver functionality
From: Ali Ayoub @ 2012-08-03 20:31 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Or Gerlitz, davem, roland, netdev, sean.hefty, Erez Shitrit
In-Reply-To: <87boitz044.fsf@xmission.com>
On 8/2/2012 10:15 AM, Eric W. Biederman wrote:
> Or Gerlitz <ogerlitz@mellanox.com> writes:
>
>> From: Erez Shitrit <erezsh@mellanox.co.il>
>>
>> The eipoib driver provides a standard Ethernet netdevice over
>> the InfiniBand IPoIB interface .
>>
>> Some services can run only on top of Ethernet L2 interfaces, and cannot be
>> bound to an IPoIB interface. With this new driver, these services can run
>> seamlessly.
>
> Do I read this code correctly that what you are doing is not tunneling
> ethernet over IB but instead you are removing an ethernet header and
> replacing it with an IB header?
Correct.
eIPoIB runs standard IPoIB on the wire, thus it doesn't encapsulate the
Ethernet frame on top of IPoIB, but rather translates it to an IPoIB
packet, this allows us to expose an Ethernet L2 network device, and
still keep interoperability with existing IPoIB endpoints. Running full
encapsulation (i.e. EoIPoIB) will break interoperability.
> Do I also read this code correctly if you can't find your destination
> mac address in your ""neighbor table"" you do a normal IPoIB arp
> for the infiniband GUID?
Correct.
Wire protocol remains IPoIB.
> Do I read this right that if presented with a non-IPv4 or ARP packet
> this code will do something undefined and unpredictable?
The current code drops IPv6 packets (see IS_E_IPOIB_PROTO), IPv6 support
will be added later on.
> Maybe this makes some sense but just skimming it looks like you
> are trying to force a square peg into a round hole resulting in
> some weird code and some very weird maintainability issues.
>
> I am honestly surprised at this approach. I would think it would be
> faster and simpler to run an IB queue pair directly to the hypervisor or
> possibly even the guest operating system bypassing the kernel and doing
> all of this translation in userspace.
With eIPoIB architecture, the VM sees standard Ethernet emulator,
allowing the administrator to enslave eIPoIB PIF to the vSwitch/vBridge
as if it was standard Ethernet. Other approaches that exposes IB QP to
the VM (with w/o bypassing the kernel) won't be possible with the
current emulators and management tools.
--
Ali;
^ permalink raw reply
* Re: [PATCH] ipv4: remove parentheses in return statement
From: Jean Sacren @ 2012-08-03 20:23 UTC (permalink / raw)
To: David Miller; +Cc: netdev, eric.dumazet
In-Reply-To: <20120803.015243.1041602621154758483.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Fri, 03 Aug 2012 01:52:43 -0700
>
> From: Jean Sacren <sakiwit@gmail.com>
> Date: Fri, 3 Aug 2012 01:43:10 -0600
>
> > Signed-off-by: Jean Sacren <sakiwit@gmail.com>
>
> > - return ((val ^ (val >> 8) ^ (val >> 16) ^ (val >> 24)) &
> > - (IN4_ADDR_HSIZE - 1));
> > + return (val ^ (val >> 8) ^ (val >> 16) ^ (val >> 24)) &
> > + (IN4_ADDR_HSIZE - 1);
>
> Those parenthesis are there to make the evaluation order and
> grouping explicit.
>
> The other ones you changed are wrong for similar reasons.
Barring the speed issue raised by Eric Dumazet, this patch is correct.
To illustrate, the patch merely changes
return (A);
to
return A;
where A is nothing but an expression, regardless if it is a simple one
or a compound one.
Whatever A is, it evaluates to a value. The parentheses do _not_
contribute to the evaluation. They do not alter precedence. They do not
make the evaluation order explicit. They are there to serve no purpose,
yet they do make return statement look like a function call.
Moreover, A is one single entity. It does not group with anything else.
I really don't see the significance of making A a group by itself.
Thank you for reviewing the patch. With all due respect, your assessment
is hard to agree with this moment.
--
Jean Sacren
^ permalink raw reply
* ethtool 3.5 released
From: Ben Hutchings @ 2012-08-03 19:48 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 795 bytes --]
ethtool version 3.5 has been released.
Home page: https://ftp.kernel.org/pub/software/network/ethtool/
Download link:
https://ftp.kernel.org/pub/software/network/ethtool/ethtool-3.1.tar.gz
Release notes:
* Feature: Display support for 1000BASE-KX and 10GBASE-KX4 link modes
* Feature: Energy-Efficient Ethernet (EEE) configuration
(--show-eee and --set-eee options)
* Fix: Don't trust drivers to null-terminate strings
* Feature: Display support for 40G link modes
* Package: Update RPM summary, description and URL
* Package: Exclude redundant documentation from RPM
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* [PATCH net-next,1/1] hyperv: Move wait completion msg code into rndis_filter_halt_device()
From: Haiyang Zhang @ 2012-08-03 19:32 UTC (permalink / raw)
To: davem, netdev; +Cc: haiyangz, kys, olaf, jasowang, linux-kernel, devel
We need to wait for send_completion msg before put_rndis_request() at
the end of rndis_filter_halt_device(). Otherwise, netvsc_send_completion()
may reference freed memory which is overwritten, and cause panic.
Reported-by: Long Li <longli@microsoft.com>
Reported-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/net/hyperv/netvsc.c | 7 -------
drivers/net/hyperv/rndis_filter.c | 11 +++++++++++
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 6cee291..4a1a5f5 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -383,13 +383,6 @@ int netvsc_device_remove(struct hv_device *device)
unsigned long flags;
net_device = hv_get_drvdata(device);
- spin_lock_irqsave(&device->channel->inbound_lock, flags);
- net_device->destroy = true;
- spin_unlock_irqrestore(&device->channel->inbound_lock, flags);
-
- /* Wait for all send completions */
- wait_event(net_device->wait_drain,
- atomic_read(&net_device->num_outstanding_sends) == 0);
netvsc_disconnect_vsp(net_device);
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index e5d6146..1e88a10 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -718,6 +718,9 @@ static void rndis_filter_halt_device(struct rndis_device *dev)
{
struct rndis_request *request;
struct rndis_halt_request *halt;
+ struct netvsc_device *nvdev = dev->net_dev;
+ struct hv_device *hdev = nvdev->dev;
+ ulong flags;
/* Attempt to do a rndis device halt */
request = get_rndis_request(dev, RNDIS_MSG_HALT,
@@ -735,6 +738,14 @@ static void rndis_filter_halt_device(struct rndis_device *dev)
dev->state = RNDIS_DEV_UNINITIALIZED;
cleanup:
+ spin_lock_irqsave(&hdev->channel->inbound_lock, flags);
+ nvdev->destroy = true;
+ spin_unlock_irqrestore(&hdev->channel->inbound_lock, flags);
+
+ /* Wait for all send completions */
+ wait_event(nvdev->wait_drain,
+ atomic_read(&nvdev->num_outstanding_sends) == 0);
+
if (request)
put_rndis_request(dev, request);
return;
--
1.7.4.1
^ permalink raw reply related
* Re: [RFC v2 7/7] net,9p: use new hashtable implementation
From: Eric Dumazet @ 2012-08-03 18:00 UTC (permalink / raw)
To: Sasha Levin
Cc: torvalds, tj, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
rostedt, mingo, ebiederm, aarcange, ericvh, netdev
In-Reply-To: <1344003788-1417-8-git-send-email-levinsasha928@gmail.com>
On Fri, 2012-08-03 at 16:23 +0200, Sasha Levin wrote:
> Switch 9p error table to use the new hashtable implementation. This reduces the amount of
> generic unrelated code in 9p.
>
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
> net/9p/error.c | 17 ++++++++---------
> 1 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/net/9p/error.c b/net/9p/error.c
> index 2ab2de7..f1037db 100644
> --- a/net/9p/error.c
> +++ b/net/9p/error.c
> @@ -34,7 +34,7 @@
> #include <linux/jhash.h>
> #include <linux/errno.h>
> #include <net/9p/9p.h>
> -
> +#include <linux/hashtable.h>
> /**
> * struct errormap - map string errors from Plan 9 to Linux numeric ids
> * @name: string sent over 9P
> @@ -50,8 +50,8 @@ struct errormap {
> struct hlist_node list;
> };
>
> -#define ERRHASHSZ 32
> -static struct hlist_head hash_errmap[ERRHASHSZ];
> +#define ERRHASHSZ 5
This name is confusing, it should mention SHIFT or BITS maybe...
> +DEFINE_STATIC_HASHTABLE(hash_errmap, ERRHASHSZ);
>
> /* FixMe - reduce to a reasonable size */
> static struct errormap errmap[] = {
> @@ -196,15 +196,14 @@ int p9_error_init(void)
> int bucket;
remove "int bucket" and use :
u32 hash;
>
> /* initialize hash table */
> - for (bucket = 0; bucket < ERRHASHSZ; bucket++)
> - INIT_HLIST_HEAD(&hash_errmap[bucket]);
> + hash_init(&hash_errmap, ERRHASHSZ);
Why is hash_init() even needed ?
If hash is "DEFINE_STATIC_HASHTABLE(...)", its already ready for use !
>
> /* load initial error map into hash table */
> for (c = errmap; c->name != NULL; c++) {
> c->namelen = strlen(c->name);
> - bucket = jhash(c->name, c->namelen, 0) % ERRHASHSZ;
> + bucket = jhash(c->name, c->namelen, 0);
bucket is a wrong name here, its more like "key" or "hash"
> INIT_HLIST_NODE(&c->list);
> - hlist_add_head(&c->list, &hash_errmap[bucket]);
> + hash_add(&hash_errmap, &c->list, bucket);
> }
>
> return 1;
> @@ -228,8 +227,8 @@ int p9_errstr2errno(char *errstr, int len)
> errno = 0;
> p = NULL;
> c = NULL;
> - bucket = jhash(errstr, len, 0) % ERRHASHSZ;
> - hlist_for_each_entry(c, p, &hash_errmap[bucket], list) {
> + bucket = jhash(errstr, len, 0);
hash = jhash(errstr, len, 0);
> + hash_for_each_possible(&hash_errmap, p, c, list, bucket) {
> if (c->namelen == len && !memcmp(c->name, errstr, len)) {
> errno = c->val;
> break;
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [E1000-devel] discussion questions: SR-IOV, virtualization, and bonding
From: Ben Hutchings @ 2012-08-03 17:49 UTC (permalink / raw)
To: John Fastabend
Cc: Jay Vosburgh, Chris Friesen, e1000-devel@lists.sourceforge.net,
netdev
In-Reply-To: <501B587E.6040703@intel.com>
On Thu, 2012-08-02 at 21:50 -0700, John Fastabend wrote:
> On 8/2/2012 4:01 PM, Jay Vosburgh wrote:
[...]
> > Still, though, isn't "influence the guest's choice" pretty much
> > satisified by having the VF interface go carrier down in the guest when
> > the host wants it to? Or are you thinking about more fine grained than
> > that?
> >
>
> Perhaps one argument against this is if the hardware supports loopback
> modes or the edge relay in the hardware is acting like a VEB it may
> still be possible to support VF to VF traffic even if the external link
> is down. Not sure how useful this is though or if any existing hardware
> even supports it.
[...]
It seems to me that VF to VF traffic ought to still work. If it doesn't
then that's an unfortunate regression when moving from software bridging
and virtio to hardware-supported network virtualisation. (But hybrid
network virtualisation may help to solve that.)
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [RFC v2 1/7] hashtable: introduce a small and naive hashtable
From: Eric Dumazet @ 2012-08-03 17:39 UTC (permalink / raw)
To: Sasha Levin
Cc: torvalds, tj, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
rostedt, mingo, ebiederm, aarcange, ericvh, netdev
In-Reply-To: <1344003788-1417-2-git-send-email-levinsasha928@gmail.com>
On Fri, 2012-08-03 at 16:23 +0200, Sasha Levin wrote:
> This hashtable implementation is using hlist buckets to provide a simple
> hashtable to prevent it from getting reimplemented all over the kernel.
>
> +static void hash_add(struct hash_table *ht, struct hlist_node *node, long key)
> +{
> + hlist_add_head(node,
> + &ht->buckets[hash_long((unsigned long)key, HASH_BITS(ht))]);
> +}
> +
Why key is a long, casted later to "unsigned long" ?
hash_long() is expensive on 64bit arches, and not really needed
if key is an u32 from the beginning ( I am referring to your patches 6 &
7 using jhash() )
Maybe you could use a macro, so that we can automatically select
hash_32() if key is an u32, and hash_long() for other types.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
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