* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Steven Rostedt @ 2012-09-06 17:15 UTC (permalink / raw)
To: Sasha Levin
Cc: Josh Triplett, Mathieu Desnoyers, Pedro Alves, Tejun Heo,
torvalds, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
mingo, ebiederm, aarcange, ericvh, netdev, eric.dumazet, axboe,
agk, dm-devel, neilb, ccaulfie, teigland, Trond.Myklebust,
bfields, fweisbec, jesse, venkat.x.venkatsubra, ejt, snitzer,
edumazet, linux-nfs, dev, rds-devel, lw
In-Reply-To: <5048CDA2.10300@gmail.com>
On Thu, 2012-09-06 at 18:21 +0200, Sasha Levin wrote:
> On 09/06/2012 06:00 PM, Steven Rostedt wrote:
> >> > I think that that code doesn't make sense. The users of hlist_for_each_* aren't
> >> > supposed to be changing the loop cursor.
> > I totally agree. Modifying the 'node' pointer is just asking for issues.
> > Yes that is error prone, but not due to the double loop. It's due to the
> > modifying of the node pointer that is used internally by the loop
> > counter. Don't do that :-)
>
> While we're on this subject, I haven't actually seen hlist_for_each_entry() code
> that even *touches* 'pos'.
>
> Will people yell at me loudly if I change the prototype of those macros to be:
>
> hlist_for_each_entry(tpos, head, member)
>
> (Dropping the 'pos' parameter), and updating anything that calls those macros to
> drop it as well?
If 'pos' is no longer used in the macro, I don't see any reason for
keeping it around.
-- Steve
--
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] sctp: check dst validity after IPsec operations
From: Vlad Yasevich @ 2012-09-06 17:03 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: sri, linux-sctp, netdev
In-Reply-To: <5048D219.4020001@6wind.com>
On 09/06/2012 12:40 PM, Nicolas Dichtel wrote:
> Le 06/09/2012 18:04, Vlad Yasevich a écrit :
>> On 09/06/2012 01:40 PM, Nicolas Dichtel wrote:
>>> dst stored in struct sctp_transport needs to be recalculated when
>>> ipsec policy
>>> are updated. We use flow_cache_genid for that.
>>>
>>> For example, if a SCTP connection is established and then an IPsec
>>> policy is
>>> set, the old SCTP flow will not be updated and thus will not use the new
>>> IPsec policy.
>>>
>>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>>
>> why doesn't this need to be done for TCP? What makes SCTP special in
>> this case?
> Tests prove that the pb does not exist with TCP. I made the patch some
> times ago, I will look again deeply to find the difference.
>
TCP appears to cache the flowi and uses that to re-route the packet.
However, re-route still seems predicated on dst_check()...
>>
>> ip_queue_xmit does an __sk_dst_check() which is essentially what
>> sctp_transport_dst_check() does. That should determine if the
>> currently cached
>> route is valid or not.
> The problem is that route will not be invalidated, because dst->check()
> has no xfrm path so xfrm_dst_check() will never be called.
>
Shouldn't the cache be invalidated in this case? If the cache is
invalidated, that should cause a new lookup. If the cache isn't
invalidated, then any established connections that may now be impacted
by the policy will not pick it up.
-vlad
>
> Regards,
> Nicolas
>
>>
>> Looks like sctp may need to change to using ip_route_output_ports() call
>> as ip_route_output_key may not do all that is necessary
>>
>> -vlad
>>
>>> ---
>>> include/net/sctp/sctp.h | 3 ++-
>>> include/net/sctp/structs.h | 3 +++
>>> net/sctp/ipv6.c | 1 +
>>> net/sctp/protocol.c | 1 +
>>> 4 files changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
>>> index 9c6414f..3267246 100644
>>> --- a/include/net/sctp/sctp.h
>>> +++ b/include/net/sctp/sctp.h
>>> @@ -712,7 +712,8 @@ static inline void sctp_v4_map_v6(union sctp_addr
>>> *addr)
>>> */
>>> static inline struct dst_entry *sctp_transport_dst_check(struct
>>> sctp_transport *t)
>>> {
>>> - if (t->dst && !dst_check(t->dst, 0)) {
>>> + if ((t->dst && !dst_check(t->dst, 0)) ||
>>> + (t->flow_cache_genid != atomic_read(&flow_cache_genid))) {
>>> dst_release(t->dst);
>>> t->dst = NULL;
>>> }
>>> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
>>> index 0fef00f..9dab882 100644
>>> --- a/include/net/sctp/structs.h
>>> +++ b/include/net/sctp/structs.h
>>> @@ -948,6 +948,9 @@ struct sctp_transport {
>>>
>>> /* 64-bit random number sent with heartbeat. */
>>> __u64 hb_nonce;
>>> +
>>> + /* used to check validity of dst */
>>> + __u32 flow_cache_genid;
>>> };
>>>
>>> struct sctp_transport *sctp_transport_new(struct net *, const union
>>> sctp_addr *,
>>> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
>>> index ea14cb4..2ed7410 100644
>>> --- a/net/sctp/ipv6.c
>>> +++ b/net/sctp/ipv6.c
>>> @@ -349,6 +349,7 @@ out:
>>> struct rt6_info *rt;
>>> rt = (struct rt6_info *)dst;
>>> t->dst = dst;
>>> + t->flow_cache_genid = atomic_read(&flow_cache_genid);
>>> SCTP_DEBUG_PRINTK("rt6_dst:%pI6 rt6_src:%pI6\n",
>>> &rt->rt6i_dst.addr, &fl6->saddr);
>>> } else {
>>> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
>>> index 2d51842..4795a1a 100644
>>> --- a/net/sctp/protocol.c
>>> +++ b/net/sctp/protocol.c
>>> @@ -512,6 +512,7 @@ out_unlock:
>>> rcu_read_unlock();
>>> out:
>>> t->dst = dst;
>>> + t->flow_cache_genid = atomic_read(&flow_cache_genid);
>>> if (dst)
>>> SCTP_DEBUG_PRINTK("rt_dst:%pI4, rt_src:%pI4\n",
>>> &fl4->daddr, &fl4->saddr);
>>>
>>
^ permalink raw reply
* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Mathieu Desnoyers @ 2012-09-06 17:02 UTC (permalink / raw)
To: Sasha Levin
Cc: Steven Rostedt, Josh Triplett, Pedro Alves, Tejun Heo,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, mingo-X9Un+BFzKDI,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, aarcange-H+wXaHxf7aLQT0dZR+AlfA,
ericvh-Re5JQEeQqe8AvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w, axboe-tSWWG44O7X1aa/9Udqfwiw,
agk-H+wXaHxf7aLQT0dZR+AlfA, dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
neilb-l3A5Bk7waGM, ccaulfie-H+wXaHxf7aLQT0dZR+AlfA,
teigland-H+wXaHxf7aLQT0dZR+AlfA,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw, fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
jesse-l0M0P4e3n4LQT0dZR+AlfA,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ejt-H+wXaHxf7aLQT0dZR+AlfA, snitzer-H+wXaHxf7aLQT0dZR+AlfA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
dev-yBygre7rU0TnMu66kgdUjQ, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
lw-BthXqXjhjHXQFUHtdCDX3A
In-Reply-To: <5048D6DE.8090805-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
* Sasha Levin (levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote:
> On 09/06/2012 06:50 PM, Mathieu Desnoyers wrote:
> > * Sasha Levin (levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote:
> >> On 09/06/2012 06:00 PM, Steven Rostedt wrote:
> >>>>> I think that that code doesn't make sense. The users of hlist_for_each_* aren't
> >>>>> supposed to be changing the loop cursor.
> >>> I totally agree. Modifying the 'node' pointer is just asking for issues.
> >>> Yes that is error prone, but not due to the double loop. It's due to the
> >>> modifying of the node pointer that is used internally by the loop
> >>> counter. Don't do that :-)
> >>
> >> While we're on this subject, I haven't actually seen hlist_for_each_entry() code
> >> that even *touches* 'pos'.
> >>
> >> Will people yell at me loudly if I change the prototype of those macros to be:
> >>
> >> hlist_for_each_entry(tpos, head, member)
> >>
> >> (Dropping the 'pos' parameter), and updating anything that calls those macros to
> >> drop it as well?
> >
> > I think the intent there is to keep hlist macros and list macros
> > slightly in sync. Given those are vastly used, I'm not sure you want to
> > touch them. But hey, that's just my 2 cents.
>
> Actually, the corresponding list macro looks like this:
>
> list_for_each_entry(pos, head, member)
>
> With 'pos' being the equivalent of 'tpos' in the hlist macros (the type *).
> Changing hlist macro will make them both look as follows:
>
> hlist_for_each_entry(pos, head, member)
> list_for_each_entry(pos, head, member)
>
> So following this suggesting will actually bring them back to sync...
>
> The only issue I can see is that as you've said, they're used almost everywhere,
> so doing something to change that will require some coordination.
if this brings hlist and list in sync, then it looks like an
improvement. It might be good to propose this change as a separate
patchset.
Thanks,
Mathieu
>
>
> Thanks,
> Sasha
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Sasha Levin @ 2012-09-06 17:01 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: snitzer-H+wXaHxf7aLQT0dZR+AlfA, neilb-l3A5Bk7waGM,
fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA, agk-H+wXaHxf7aLQT0dZR+AlfA,
aarcange-H+wXaHxf7aLQT0dZR+AlfA, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ccaulfie-H+wXaHxf7aLQT0dZR+AlfA, mingo-X9Un+BFzKDI,
dev-yBygre7rU0TnMu66kgdUjQ, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
Josh Triplett, Steven Rostedt, lw-BthXqXjhjHXQFUHtdCDX3A,
teigland-H+wXaHxf7aLQT0dZR+AlfA, axboe-tSWWG44O7X1aa/9Udqfwiw,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Pedro Alves,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, ejt-H+wXaHxf7aLQT0dZR+AlfA,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
Tejun Heo, akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20120906165055.GC7385@Krystal>
On 09/06/2012 06:50 PM, Mathieu Desnoyers wrote:
> * Sasha Levin (levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote:
>> On 09/06/2012 06:00 PM, Steven Rostedt wrote:
>>>>> I think that that code doesn't make sense. The users of hlist_for_each_* aren't
>>>>> supposed to be changing the loop cursor.
>>> I totally agree. Modifying the 'node' pointer is just asking for issues.
>>> Yes that is error prone, but not due to the double loop. It's due to the
>>> modifying of the node pointer that is used internally by the loop
>>> counter. Don't do that :-)
>>
>> While we're on this subject, I haven't actually seen hlist_for_each_entry() code
>> that even *touches* 'pos'.
>>
>> Will people yell at me loudly if I change the prototype of those macros to be:
>>
>> hlist_for_each_entry(tpos, head, member)
>>
>> (Dropping the 'pos' parameter), and updating anything that calls those macros to
>> drop it as well?
>
> I think the intent there is to keep hlist macros and list macros
> slightly in sync. Given those are vastly used, I'm not sure you want to
> touch them. But hey, that's just my 2 cents.
Actually, the corresponding list macro looks like this:
list_for_each_entry(pos, head, member)
With 'pos' being the equivalent of 'tpos' in the hlist macros (the type *).
Changing hlist macro will make them both look as follows:
hlist_for_each_entry(pos, head, member)
list_for_each_entry(pos, head, member)
So following this suggesting will actually bring them back to sync...
The only issue I can see is that as you've said, they're used almost everywhere,
so doing something to change that will require some coordination.
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Mathieu Desnoyers @ 2012-09-06 16:50 UTC (permalink / raw)
To: Sasha Levin
Cc: Steven Rostedt, Josh Triplett, Pedro Alves, Tejun Heo,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, mingo-X9Un+BFzKDI,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, aarcange-H+wXaHxf7aLQT0dZR+AlfA,
ericvh-Re5JQEeQqe8AvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w, axboe-tSWWG44O7X1aa/9Udqfwiw,
agk-H+wXaHxf7aLQT0dZR+AlfA, dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
neilb-l3A5Bk7waGM, ccaulfie-H+wXaHxf7aLQT0dZR+AlfA,
teigland-H+wXaHxf7aLQT0dZR+AlfA,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw, fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
jesse-l0M0P4e3n4LQT0dZR+AlfA,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ejt-H+wXaHxf7aLQT0dZR+AlfA, snitzer-H+wXaHxf7aLQT0dZR+AlfA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
dev-yBygre7rU0TnMu66kgdUjQ, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
lw-BthXqXjhjHXQFUHtdCDX3A
In-Reply-To: <5048CDA2.10300-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
* Sasha Levin (levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote:
> On 09/06/2012 06:00 PM, Steven Rostedt wrote:
> >> > I think that that code doesn't make sense. The users of hlist_for_each_* aren't
> >> > supposed to be changing the loop cursor.
> > I totally agree. Modifying the 'node' pointer is just asking for issues.
> > Yes that is error prone, but not due to the double loop. It's due to the
> > modifying of the node pointer that is used internally by the loop
> > counter. Don't do that :-)
>
> While we're on this subject, I haven't actually seen hlist_for_each_entry() code
> that even *touches* 'pos'.
>
> Will people yell at me loudly if I change the prototype of those macros to be:
>
> hlist_for_each_entry(tpos, head, member)
>
> (Dropping the 'pos' parameter), and updating anything that calls those macros to
> drop it as well?
I think the intent there is to keep hlist macros and list macros
slightly in sync. Given those are vastly used, I'm not sure you want to
touch them. But hey, that's just my 2 cents.
Thanks,
Mathieu
>
>
> Thanks,
> Sasha
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 9/10] drivers/net/wireless/rtlwifi/rtl8192de/phy.c: removes unnecessary semicolon
From: Larry Finger @ 2012-09-06 16:49 UTC (permalink / raw)
To: Peter Senna Tschudin
Cc: kernel-janitors, Chaoming Li, John W. Linville, linux-wireless,
netdev, linux-kernel
In-Reply-To: <1346947757-10481-10-git-send-email-peter.senna@gmail.com>
On 09/06/2012 11:09 AM, Peter Senna Tschudin wrote:
> From: Peter Senna Tschudin <peter.senna@gmail.com>
>
> removes unnecessary semicolon
>
> Found by Coccinelle: http://coccinelle.lip6.fr/
>
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
>
> ---
> drivers/net/wireless/rtlwifi/rtl8192de/phy.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff -u -p a/drivers/net/wireless/rtlwifi/rtl8192de/phy.c b/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
> --- a/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
> +++ b/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
> @@ -1314,7 +1314,7 @@ static void _rtl92d_phy_restore_rf_env(s
> struct bb_reg_def *pphyreg = &rtlphy->phyreg_def[rfpath];
>
> RT_TRACE(rtlpriv, COMP_RF, DBG_LOUD, "=====>\n");
> - /*----Restore RFENV control type----*/ ;
> + /*----Restore RFENV control type----*/
> switch (rfpath) {
> case RF90_PATH_A:
> case RF90_PATH_C:
>
>
Acked-by: Larry Finger <Larry.Finger@lwfinger.net>
Thanks,
Larry
^ permalink raw reply
* Re: [PATCHv3] virtio-spec: virtio network device multiqueue support
From: Paolo Bonzini @ 2012-09-06 16:43 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, kvm, virtualization
In-Reply-To: <20120906120828.GA1534@redhat.com>
Il 06/09/2012 14:08, Michael S. Tsirkin ha scritto:
> +#define VIRTIO_NET_CTRL_STEERING_HOST 1
You didn't change this occurrence.
Paolo
^ permalink raw reply
* Re: [PATCH] sctp: check dst validity after IPsec operations
From: Nicolas Dichtel @ 2012-09-06 16:40 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: sri, linux-sctp, netdev
In-Reply-To: <5048C984.3030306@gmail.com>
Le 06/09/2012 18:04, Vlad Yasevich a écrit :
> On 09/06/2012 01:40 PM, Nicolas Dichtel wrote:
>> dst stored in struct sctp_transport needs to be recalculated when ipsec policy
>> are updated. We use flow_cache_genid for that.
>>
>> For example, if a SCTP connection is established and then an IPsec policy is
>> set, the old SCTP flow will not be updated and thus will not use the new
>> IPsec policy.
>>
>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>
> why doesn't this need to be done for TCP? What makes SCTP special in this case?
Tests prove that the pb does not exist with TCP. I made the patch some times
ago, I will look again deeply to find the difference.
>
> ip_queue_xmit does an __sk_dst_check() which is essentially what
> sctp_transport_dst_check() does. That should determine if the currently cached
> route is valid or not.
The problem is that route will not be invalidated, because dst->check() has no
xfrm path so xfrm_dst_check() will never be called.
Regards,
Nicolas
>
> Looks like sctp may need to change to using ip_route_output_ports() call
> as ip_route_output_key may not do all that is necessary
>
> -vlad
>
>> ---
>> include/net/sctp/sctp.h | 3 ++-
>> include/net/sctp/structs.h | 3 +++
>> net/sctp/ipv6.c | 1 +
>> net/sctp/protocol.c | 1 +
>> 4 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
>> index 9c6414f..3267246 100644
>> --- a/include/net/sctp/sctp.h
>> +++ b/include/net/sctp/sctp.h
>> @@ -712,7 +712,8 @@ static inline void sctp_v4_map_v6(union sctp_addr *addr)
>> */
>> static inline struct dst_entry *sctp_transport_dst_check(struct
>> sctp_transport *t)
>> {
>> - if (t->dst && !dst_check(t->dst, 0)) {
>> + if ((t->dst && !dst_check(t->dst, 0)) ||
>> + (t->flow_cache_genid != atomic_read(&flow_cache_genid))) {
>> dst_release(t->dst);
>> t->dst = NULL;
>> }
>> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
>> index 0fef00f..9dab882 100644
>> --- a/include/net/sctp/structs.h
>> +++ b/include/net/sctp/structs.h
>> @@ -948,6 +948,9 @@ struct sctp_transport {
>>
>> /* 64-bit random number sent with heartbeat. */
>> __u64 hb_nonce;
>> +
>> + /* used to check validity of dst */
>> + __u32 flow_cache_genid;
>> };
>>
>> struct sctp_transport *sctp_transport_new(struct net *, const union
>> sctp_addr *,
>> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
>> index ea14cb4..2ed7410 100644
>> --- a/net/sctp/ipv6.c
>> +++ b/net/sctp/ipv6.c
>> @@ -349,6 +349,7 @@ out:
>> struct rt6_info *rt;
>> rt = (struct rt6_info *)dst;
>> t->dst = dst;
>> + t->flow_cache_genid = atomic_read(&flow_cache_genid);
>> SCTP_DEBUG_PRINTK("rt6_dst:%pI6 rt6_src:%pI6\n",
>> &rt->rt6i_dst.addr, &fl6->saddr);
>> } else {
>> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
>> index 2d51842..4795a1a 100644
>> --- a/net/sctp/protocol.c
>> +++ b/net/sctp/protocol.c
>> @@ -512,6 +512,7 @@ out_unlock:
>> rcu_read_unlock();
>> out:
>> t->dst = dst;
>> + t->flow_cache_genid = atomic_read(&flow_cache_genid);
>> if (dst)
>> SCTP_DEBUG_PRINTK("rt_dst:%pI4, rt_src:%pI4\n",
>> &fl4->daddr, &fl4->saddr);
>>
>
^ permalink raw reply
* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Sasha Levin @ 2012-09-06 16:21 UTC (permalink / raw)
To: Steven Rostedt
Cc: Josh Triplett, Mathieu Desnoyers, Pedro Alves, Tejun Heo,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, mingo-X9Un+BFzKDI,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, aarcange-H+wXaHxf7aLQT0dZR+AlfA,
ericvh-Re5JQEeQqe8AvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w, axboe-tSWWG44O7X1aa/9Udqfwiw,
agk-H+wXaHxf7aLQT0dZR+AlfA, dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
neilb-l3A5Bk7waGM, ccaulfie-H+wXaHxf7aLQT0dZR+AlfA,
teigland-H+wXaHxf7aLQT0dZR+AlfA,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw, fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
jesse-l0M0P4e3n4LQT0dZR+AlfA,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ejt-H+wXaHxf7aLQT0dZR+AlfA, snitzer-H+wXaHxf7aLQT0dZR+AlfA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
dev-yBygre7rU0TnMu66kgdUjQ, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
lw-BthXqXjhjHXQFUHtdCDX3A
In-Reply-To: <1346947206.1680.36.camel-f9ZlEuEWxVcJvu8Pb33WZ0EMvNT87kid@public.gmane.org>
On 09/06/2012 06:00 PM, Steven Rostedt wrote:
>> > I think that that code doesn't make sense. The users of hlist_for_each_* aren't
>> > supposed to be changing the loop cursor.
> I totally agree. Modifying the 'node' pointer is just asking for issues.
> Yes that is error prone, but not due to the double loop. It's due to the
> modifying of the node pointer that is used internally by the loop
> counter. Don't do that :-)
While we're on this subject, I haven't actually seen hlist_for_each_entry() code
that even *touches* 'pos'.
Will people yell at me loudly if I change the prototype of those macros to be:
hlist_for_each_entry(tpos, head, member)
(Dropping the 'pos' parameter), and updating anything that calls those macros to
drop it as well?
Thanks,
Sasha
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 0/5] dev_<level> and dynamic_debug cleanups
From: Greg Kroah-Hartman @ 2012-09-06 16:13 UTC (permalink / raw)
To: Jim Cromie, Jason Baron
Cc: Kay Sievers, Joe Perches, Andrew Morton, netdev, David S. Miller,
linux-kernel
In-Reply-To: <CAJfuBxyToA9-=c3H88GGba3GFX64kdbP4OCEC0f4trnBnjpU-w@mail.gmail.com>
On Thu, Aug 30, 2012 at 09:48:12PM -0600, Jim Cromie wrote:
> On Thu, Aug 30, 2012 at 11:43 AM, Jim Cromie <jim.cromie@gmail.com> wrote:
> > On Sun, Aug 26, 2012 at 5:25 AM, Joe Perches <joe@perches.com> wrote:
> >> The recent commit to fix dynamic_debug was a bit unclean.
> >> Neaten the style for dynamic_debug.
> >> Reduce the stack use of message logging that uses netdev_printk
> >> Add utility functions dev_printk_emit and dev_vprintk_emit for /dev/kmsg.
> >>
> >> Joe Perches (5):
> >> dev_dbg/dynamic_debug: Update to use printk_emit, optimize stack
> >> netdev_printk/dynamic_netdev_dbg: Directly call printk_emit
> >> netdev_printk/netif_printk: Remove a superfluous logging colon
> >> dev: Add dev_vprintk_emit and dev_printk_emit
> >> device and dynamic_debug: Use dev_vprintk_emit and dev_printk_emit
> >>
> >
> > Ive tested this on 2 builds differing only by DYNAMIC_DEBUG
> > It works for me on x86-64
> >
> > However, I just booted a non-dyndbg build on x86-32, and got this.
> >
>
> Ok, transient error, went away with a clean build.
>
> tested-by: Jim Cromie <jim.cromie@gmail.com>
Jason, any ACK on these, or any of the other random dynamic debug
patches floating around? What am I supposed to be applying here?
thanks,
greg k-h
^ permalink raw reply
* [PATCH 9/10] drivers/net/wireless/rtlwifi/rtl8192de/phy.c: removes unnecessary semicolon
From: Peter Senna Tschudin @ 2012-09-06 16:09 UTC (permalink / raw)
To: Larry Finger
Cc: kernel-janitors, Peter Senna Tschudin, Chaoming Li,
John W. Linville, linux-wireless, netdev, linux-kernel
In-Reply-To: <1346947757-10481-1-git-send-email-peter.senna@gmail.com>
From: Peter Senna Tschudin <peter.senna@gmail.com>
removes unnecessary semicolon
Found by Coccinelle: http://coccinelle.lip6.fr/
Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
---
drivers/net/wireless/rtlwifi/rtl8192de/phy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff -u -p a/drivers/net/wireless/rtlwifi/rtl8192de/phy.c b/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
--- a/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
@@ -1314,7 +1314,7 @@ static void _rtl92d_phy_restore_rf_env(s
struct bb_reg_def *pphyreg = &rtlphy->phyreg_def[rfpath];
RT_TRACE(rtlpriv, COMP_RF, DBG_LOUD, "=====>\n");
- /*----Restore RFENV control type----*/ ;
+ /*----Restore RFENV control type----*/
switch (rfpath) {
case RF90_PATH_A:
case RF90_PATH_C:
^ permalink raw reply
* [PATCH 8/10] net/mac80211/scan.c: removes unnecessary semicolon
From: Peter Senna Tschudin @ 2012-09-06 16:09 UTC (permalink / raw)
To: John W. Linville
Cc: kernel-janitors, Peter Senna Tschudin, Johannes Berg,
David S. Miller, linux-wireless, netdev, linux-kernel
In-Reply-To: <1346947757-10481-1-git-send-email-peter.senna@gmail.com>
From: Peter Senna Tschudin <peter.senna@gmail.com>
removes unnecessary semicolon
Found by Coccinelle: http://coccinelle.lip6.fr/
Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
---
net/mac80211/scan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff -u -p a/net/mac80211/scan.c b/net/mac80211/scan.c
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -407,7 +407,7 @@ static void ieee80211_scan_state_send_pr
enum ieee80211_band band = local->hw.conf.channel->band;
sdata = rcu_dereference_protected(local->scan_sdata,
- lockdep_is_held(&local->mtx));;
+ lockdep_is_held(&local->mtx));
for (i = 0; i < local->scan_req->n_ssids; i++)
ieee80211_send_probe_req(
^ permalink raw reply
* Re: changing usbnet's API to better deal with cdc-ncm
From: Ming Lei @ 2012-09-06 16:09 UTC (permalink / raw)
To: Bjørn Mork
Cc: Oliver Neukum, alexey.orishko-0IS4wlFg1OjSUeElwK9/Pw,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <87a9x33656.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
On Thu, Sep 6, 2012 at 4:30 PM, Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org> wrote:
> Ming Lei <tom.leiming-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>> On Thu, Sep 6, 2012 at 4:12 AM, Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org> wrote:
>>> Hi,
>>>
>>> looking at cdc-ncm it seeems to me that cdc-ncm is forced to play
>>> very dirty games because usbnet doesn't have a notion about aggregating
>>> packets for a single transfer.
>>
>> The Ethernet API we are using does not support transmitting multiple Ethernet
>> frames in a single call, so the aggregation things should be done inside low
>> level driver, in fact it is just what some wlan(802.11n) drivers have
>> been doing.
>>
>> IMO, the current .tx_fixup is intelligent enough to handle aggregation:
>>
>> - return a skb_out for sending if low level drivers think it is
>> ready to send
>> the aggregated frames
>> - return NULL if the low level drivers think they need to wait
>> for more frames
>>
>> Of course, the low level drivers need to start a timer to trigger sending
>> remainder frames in case of timeout and no further frames come from
>> upper protocol stack.
>>
>> Looks the introduced .tx_bundle is not necessary since .tx_fixup is OK.
>
> The minidriver does not have any information about tx in progress. The
Inside .tx_fixup, the low level driver will get the tx progress information.
> strategy above, which is what cdc_ncm uses today, is fine as long as you
> always want to wait as long as you always want to fill as many frames as
> possible in each packet. But what if the queue is empty and the device
For cdc_ncm, the wait time is controlled by cdc_ncm driver, see
cdc_ncm_tx_timeout_start().
> is just sitting there doing nothing while you are waiting for more
> frames? Then you are just adding unnecessary latency.
As said above, cdc_ncm can control the latency by itself.
>
> There are also several usbnet minidrivers for protocols with frame
> aggregation support. Most of them do not currently implement tx
> aggregation, possibly due to the complexity. This makes a common
> aggregation framework interesting in any case. Reimplementing similar
> loginc in multiple minidrivers is meaningless.
If we can abstract some common things about aggregation, it should be
meaningful. As far as I know, most of aggregation protocol is very different,
so almost all aggregation work is only done by low level driver, such as
cdc_ncm.
If we want to implement some aggregation framework, maybe below is
one solution, correct me if it is wrong.
usbnet_start_xmit():
to_send = info->tx_bundle(in_skb, &out_skb)
if (!to_send) {
usbnet_tx_timeout_start(usbnet);
goto not_drop;
}
skb = info->tx_fixup(dev, out_skb, GFP_ATOMIC);
...
Looks the above is no much advantage than doing bundling in .tx_fixup
just like cdc_ncm.
>
> I believe Oliver is adding very useful functionality to usbnet here.
> Functionality which at least cdc_ncm and possibly other existing
> minidrivers can take advantage of immediately. There also seems to be a
Sorry, I don't know other minidrivers, and is there some common
things in building aggregation with cdc_ncm?
Considered that the patch doesn't implement wait_for_more, suggest
Oliver to post a new one with basic wait_for_more support for further
discussion.
Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 1/10] drivers/net/usb/sierra_net.c: removes unnecessary semicolon
From: Peter Senna Tschudin @ 2012-09-06 16:09 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: kernel-janitors, Peter Senna Tschudin, linux-usb, netdev,
linux-kernel
From: Peter Senna Tschudin <peter.senna@gmail.com>
removes unnecessary semicolon
Found by Coccinelle: http://coccinelle.lip6.fr/
Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
---
drivers/net/usb/sierra_net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff -u -p a/drivers/net/usb/sierra_net.c b/drivers/net/usb/sierra_net.c
--- a/drivers/net/usb/sierra_net.c
+++ b/drivers/net/usb/sierra_net.c
@@ -842,7 +842,7 @@ static int sierra_net_rx_fixup(struct us
netdev_err(dev->net, "HIP/ETH: Invalid pkt\n");
dev->net->stats.rx_frame_errors++;
- /* dev->net->stats.rx_errors incremented by caller */;
+ /* dev->net->stats.rx_errors incremented by caller */
return 0;
}
^ permalink raw reply
* Re: [PATCH] sctp: check dst validity after IPsec operations
From: Vlad Yasevich @ 2012-09-06 16:04 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: sri, linux-sctp, netdev
In-Reply-To: <1346953229-3825-1-git-send-email-nicolas.dichtel@6wind.com>
On 09/06/2012 01:40 PM, Nicolas Dichtel wrote:
> dst stored in struct sctp_transport needs to be recalculated when ipsec policy
> are updated. We use flow_cache_genid for that.
>
> For example, if a SCTP connection is established and then an IPsec policy is
> set, the old SCTP flow will not be updated and thus will not use the new
> IPsec policy.
>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
why doesn't this need to be done for TCP? What makes SCTP special in
this case?
ip_queue_xmit does an __sk_dst_check() which is essentially what
sctp_transport_dst_check() does. That should determine if the currently
cached route is valid or not.
Looks like sctp may need to change to using ip_route_output_ports() call
as ip_route_output_key may not do all that is necessary
-vlad
> ---
> include/net/sctp/sctp.h | 3 ++-
> include/net/sctp/structs.h | 3 +++
> net/sctp/ipv6.c | 1 +
> net/sctp/protocol.c | 1 +
> 4 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
> index 9c6414f..3267246 100644
> --- a/include/net/sctp/sctp.h
> +++ b/include/net/sctp/sctp.h
> @@ -712,7 +712,8 @@ static inline void sctp_v4_map_v6(union sctp_addr *addr)
> */
> static inline struct dst_entry *sctp_transport_dst_check(struct sctp_transport *t)
> {
> - if (t->dst && !dst_check(t->dst, 0)) {
> + if ((t->dst && !dst_check(t->dst, 0)) ||
> + (t->flow_cache_genid != atomic_read(&flow_cache_genid))) {
> dst_release(t->dst);
> t->dst = NULL;
> }
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index 0fef00f..9dab882 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -948,6 +948,9 @@ struct sctp_transport {
>
> /* 64-bit random number sent with heartbeat. */
> __u64 hb_nonce;
> +
> + /* used to check validity of dst */
> + __u32 flow_cache_genid;
> };
>
> struct sctp_transport *sctp_transport_new(struct net *, const union sctp_addr *,
> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> index ea14cb4..2ed7410 100644
> --- a/net/sctp/ipv6.c
> +++ b/net/sctp/ipv6.c
> @@ -349,6 +349,7 @@ out:
> struct rt6_info *rt;
> rt = (struct rt6_info *)dst;
> t->dst = dst;
> + t->flow_cache_genid = atomic_read(&flow_cache_genid);
> SCTP_DEBUG_PRINTK("rt6_dst:%pI6 rt6_src:%pI6\n",
> &rt->rt6i_dst.addr, &fl6->saddr);
> } else {
> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> index 2d51842..4795a1a 100644
> --- a/net/sctp/protocol.c
> +++ b/net/sctp/protocol.c
> @@ -512,6 +512,7 @@ out_unlock:
> rcu_read_unlock();
> out:
> t->dst = dst;
> + t->flow_cache_genid = atomic_read(&flow_cache_genid);
> if (dst)
> SCTP_DEBUG_PRINTK("rt_dst:%pI4, rt_src:%pI4\n",
> &fl4->daddr, &fl4->saddr);
>
^ permalink raw reply
* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Steven Rostedt @ 2012-09-06 16:00 UTC (permalink / raw)
To: Sasha Levin
Cc: Josh Triplett, Mathieu Desnoyers, Pedro Alves, Tejun Heo,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, mingo-X9Un+BFzKDI,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, aarcange-H+wXaHxf7aLQT0dZR+AlfA,
ericvh-Re5JQEeQqe8AvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w, axboe-tSWWG44O7X1aa/9Udqfwiw,
agk-H+wXaHxf7aLQT0dZR+AlfA, dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
neilb-l3A5Bk7waGM, ccaulfie-H+wXaHxf7aLQT0dZR+AlfA,
teigland-H+wXaHxf7aLQT0dZR+AlfA,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw, fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
jesse-l0M0P4e3n4LQT0dZR+AlfA,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ejt-H+wXaHxf7aLQT0dZR+AlfA, snitzer-H+wXaHxf7aLQT0dZR+AlfA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
dev-yBygre7rU0TnMu66kgdUjQ, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
lw-BthXqXjhjHXQFUHtdCDX3A
In-Reply-To: <5048C615.4070204-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Thu, 2012-09-06 at 17:49 +0200, Sasha Levin wrote:
>
> > Looks reasonable. However, it would break (or rather, not break) on
> > code like this:
> >
> > hash_for_each_entry(...) {
> > if (...) {
> > foo(node);
> > node = NULL;
ug, I didn't even notice this. Ignore my last email :-p
/me needs to wake-up a bit more.
> > break;
> > }
> > }
> >
> > Hiding the double loop still seems error-prone.
>
> I think that that code doesn't make sense. The users of hlist_for_each_* aren't
> supposed to be changing the loop cursor.
I totally agree. Modifying the 'node' pointer is just asking for issues.
Yes that is error prone, but not due to the double loop. It's due to the
modifying of the node pointer that is used internally by the loop
counter. Don't do that :-)
>
> We have three options here:
>
> 1. Stuff everything into a single for(). While not too difficult, it will make
> the readability of the code difficult as it will force us to abandon using
> hlist_for_each_* macros.
>
> 2. Over-complicate everything, and check for 'node == NULL && obj &&
> obj->member.next == NULL' instead. That one will fail only if the user has
> specifically set the object as the last object in the list and the node as NULL.
>
> 3. Use 2 loops which might not work properly if the user does something odd,
> with a big fat warning above them.
>
>
> To sum it up, I'd rather go with 3 and let anyone who does things he shouldn't
> be doing break.
I agree, the double loop itself is not error prone. If you are modifying
'node' you had better know what the hell you are doing.
Actually, it may be something that is legitimate. That is, if you want
to skip to the next bucket, just set node to NULL and do the break (as
Josh had done). This would break if the macro loop changed later on, but
hey, like I said, it's error prone ;-) If you really want to do that,
then hand coding the double loop would be a better bet. IOW, don't use
the macro loop.
-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Sasha Levin @ 2012-09-06 15:49 UTC (permalink / raw)
To: Josh Triplett
Cc: snitzer-H+wXaHxf7aLQT0dZR+AlfA, neilb-l3A5Bk7waGM,
fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA, agk-H+wXaHxf7aLQT0dZR+AlfA,
aarcange-H+wXaHxf7aLQT0dZR+AlfA, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ccaulfie-H+wXaHxf7aLQT0dZR+AlfA, mingo-X9Un+BFzKDI,
dev-yBygre7rU0TnMu66kgdUjQ, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
Steven Rostedt, lw-BthXqXjhjHXQFUHtdCDX3A, Mathieu Desnoyers,
axboe-tSWWG44O7X1aa/9Udqfwiw, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
Pedro Alves, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
ejt-H+wXaHxf7aLQT0dZR+AlfA, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
netdev-u79uwXL29TY76Z2rM5mHXA, Tejun Heo,
teigland-H+wXaHxf7aLQT0dZR+AlfA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20120906145545.GA17332@leaf>
On 09/06/2012 04:55 PM, Josh Triplett wrote:
> On Thu, Sep 06, 2012 at 03:53:58PM +0200, Sasha Levin wrote:
>> On 09/04/2012 07:01 PM, Mathieu Desnoyers wrote:
>>>> #define do_for_each_ftrace_rec(pg, rec) \
>>>>> for (pg = ftrace_pages_start, rec = &pg->records[pg->index]; \
>>>>> pg && rec == &pg->records[pg->index]; \
>>>>> pg = pg->next) \
>>>>> for (rec = pg->records; rec < &pg->records[pg->index]; rec++)
>>> Maybe in some cases there might be ways to combine the two loops into
>>> one ? I'm not seeing exactly how to do it for this one, but it should
>>> not be impossible. If the inner loop condition can be moved to the outer
>>> loop, and if we use (blah ? loop1_conf : loop2_cond) to test for
>>> different conditions depending on the context, and do the same for the
>>> 3rd argument of the for() loop. The details elude me for now though, so
>>> maybe it's complete non-sense ;)
>>>
>>> It might not be that useful for do_for_each_ftrace_rec, but if we can do
>>> it for the hash table iterator, it might be worth it.
>>
>> So I think that for the hash iterator it might actually be simpler.
>>
>> My solution to making 'break' work in the iterator is:A code like that doesn
>>
>> for (bkt = 0, node = NULL; bkt < HASH_SIZE(name) && node == NULL; bkt++)
>> hlist_for_each_entry(obj, node, &name[bkt], member)
>>
>> We initialize our node loop cursor with NULL in the external loop, and the
>> external loop will have a new condition to loop while that cursor is NULL.
>>
>> My logic is that we can only 'break' when we are iterating over an object in the
>> internal loop. If we're iterating over an object in that loop then 'node != NULL'.
>>
>> This way, if we broke from within the internal loop, the external loop will see
>> node as not NULL, and so it will stop looping itself. On the other hand, if the
>> internal loop has actually ended, then node will be NULL, and the outer loop
>> will keep running.
>>
>> Is there anything I've missed?
>
> Looks reasonable. However, it would break (or rather, not break) on
> code like this:
>
> hash_for_each_entry(...) {
> if (...) {
> foo(node);
> node = NULL;
> break;
> }
> }
>
> Hiding the double loop still seems error-prone.
I think that that code doesn't make sense. The users of hlist_for_each_* aren't
supposed to be changing the loop cursor.
We have three options here:
1. Stuff everything into a single for(). While not too difficult, it will make
the readability of the code difficult as it will force us to abandon using
hlist_for_each_* macros.
2. Over-complicate everything, and check for 'node == NULL && obj &&
obj->member.next == NULL' instead. That one will fail only if the user has
specifically set the object as the last object in the list and the node as NULL.
3. Use 2 loops which might not work properly if the user does something odd,
with a big fat warning above them.
To sum it up, I'd rather go with 3 and let anyone who does things he shouldn't
be doing break.
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH 04/10] net/macb: Fix a race in macb_start_xmit()
From: Havard Skinnemoen @ 2012-09-06 15:49 UTC (permalink / raw)
To: David Miller
Cc: nicolas.ferre, netdev, linux-arm-kernel, plagnioj, jamie,
linux-kernel, patrice.vilchez
In-Reply-To: <20120905.173023.2235590074897156746.davem@davemloft.net>
On Wed, Sep 5, 2012 at 11:30 PM, David Miller <davem@davemloft.net> wrote:
> From: Nicolas Ferre <nicolas.ferre@atmel.com>
> Date: Wed, 5 Sep 2012 10:19:11 +0200
>
>> From: Havard Skinnemoen <havard@skinnemoen.net>
>>
>> Fix a race in macb_start_xmit() where we unconditionally set the TSTART bit.
>> If an underrun just happened (we do this with interrupts disabled, so it might
>> not have been handled yet), the controller starts transmitting from the first
>> entry in the ring, which is usually wrong.
>> Restart the controller after error handling.
>>
>> Signed-off-by: Havard Skinnemoen <havard@skinnemoen.net>
>> [nicolas.ferre@atmel.com: split patch in topics]
>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>
> Accumulating special case code and checks into the hot path of TX packet
> processing is extremely unwise.
>
> Instead, when you handle the TX error conditions and reset the chip you
> should first ensure that there are no flows of control in the transmit
> function of your driver by using the appropriate locking et al. facilities.
IIRC, the hardware resets the ring pointers when an error happens, and
if we set TSTART right after that happens, the hardware will happily
transmit whatever is sitting in the beginning of the ring. This is
what I was trying to avoid.
The details are a bit hazy as it's been a while since I looked at
this, so it could be that simply letting it happen and using a bigger
hammer during reset processing might work just as well. Just want to
make sure y'all understand that we're talking about a race against
hardware, not against interrupt handlers, threads or anything that can
be solved by locking :)
Havard
^ permalink raw reply
* [PATCH] sctp: check dst validity after IPsec operations
From: Nicolas Dichtel @ 2012-09-06 17:40 UTC (permalink / raw)
To: vyasevich, sri, linux-sctp; +Cc: netdev, Nicolas Dichtel
dst stored in struct sctp_transport needs to be recalculated when ipsec policy
are updated. We use flow_cache_genid for that.
For example, if a SCTP connection is established and then an IPsec policy is
set, the old SCTP flow will not be updated and thus will not use the new
IPsec policy.
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
include/net/sctp/sctp.h | 3 ++-
include/net/sctp/structs.h | 3 +++
net/sctp/ipv6.c | 1 +
net/sctp/protocol.c | 1 +
4 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 9c6414f..3267246 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -712,7 +712,8 @@ static inline void sctp_v4_map_v6(union sctp_addr *addr)
*/
static inline struct dst_entry *sctp_transport_dst_check(struct sctp_transport *t)
{
- if (t->dst && !dst_check(t->dst, 0)) {
+ if ((t->dst && !dst_check(t->dst, 0)) ||
+ (t->flow_cache_genid != atomic_read(&flow_cache_genid))) {
dst_release(t->dst);
t->dst = NULL;
}
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 0fef00f..9dab882 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -948,6 +948,9 @@ struct sctp_transport {
/* 64-bit random number sent with heartbeat. */
__u64 hb_nonce;
+
+ /* used to check validity of dst */
+ __u32 flow_cache_genid;
};
struct sctp_transport *sctp_transport_new(struct net *, const union sctp_addr *,
diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index ea14cb4..2ed7410 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -349,6 +349,7 @@ out:
struct rt6_info *rt;
rt = (struct rt6_info *)dst;
t->dst = dst;
+ t->flow_cache_genid = atomic_read(&flow_cache_genid);
SCTP_DEBUG_PRINTK("rt6_dst:%pI6 rt6_src:%pI6\n",
&rt->rt6i_dst.addr, &fl6->saddr);
} else {
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index 2d51842..4795a1a 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -512,6 +512,7 @@ out_unlock:
rcu_read_unlock();
out:
t->dst = dst;
+ t->flow_cache_genid = atomic_read(&flow_cache_genid);
if (dst)
SCTP_DEBUG_PRINTK("rt_dst:%pI4, rt_src:%pI4\n",
&fl4->daddr, &fl4->saddr);
--
1.7.12
^ permalink raw reply related
* WARNING: at net/ipv4/tcp.c:1303 tcp_cleanup_rbuf+0x54/0x120()
From: Dave Jones @ 2012-09-06 15:32 UTC (permalink / raw)
To: netdev; +Cc: Fedora Kernel Team
In-Reply-To: <bug-854367-176318@bugzilla.redhat.com>
We've had about 20 reports of this bug in the last 2 days.
No idea why it only just started showing up, but it may coincide with
users updating from 3.4->3.5 when we pushed an update.
Seems to be affecting multiple different network drivers.
(The original trace is a 'crap' driver from staging, but there are
reports from e1000e and iwlwifi for eg)
> https://bugzilla.redhat.com/show_bug.cgi?id=854367
>
> backtrace:
> :WARNING: at net/ipv4/tcp.c:1303 tcp_cleanup_rbuf+0x54/0x120()
> :Hardware name: GA-MA785GM-US2H
> :cleanup rbuf bug: copied A48FD80 seq A48FD80 rcvnxt A48FD80
> :Modules linked in: fuse ebtable_nat ebtables ipt_MASQUERADE
> nf_conntrack_netbios_ns nf_conntrack_broadcast ip6table_mangle ip6t_REJECT
> nf_conntrack_ipv6 nf_defrag_ipv6 lockd ip6table_filter sunrpc ip6_tables
> iptable_nat nf_nat rfcomm iptable_mangle nf_conntrack_ipv4 nf_defrag_ipv4 bnep
> xt_conntrack nf_conntrack binfmt_misc btusb bluetooth snd_hda_codec_hdmi
> snd_hda_codec_realtek snd_hda_intel joydev r8712u(C) snd_hda_codec rfkill
> snd_hwdep shpchp sp5100_tco r8169 mii serio_raw snd_pcm snd_page_alloc
> snd_timer snd soundcore i2c_piix4 edac_core edac_mce_amd ppdev parport_pc
> parport microcode k10temp vhost_net tun macvtap macvlan kvm_amd kvm uinput
> ata_generic pata_acpi firewire_ohci firewire_core crc_itu_t pata_atiixp wmi
> usb_storage radeon i2c_algo_bit drm_kms_helper ttm drm i2c_core [last unloaded:
> scsi_wait_scan]
> :Pid: 2241, comm: Chrome_IOThread Tainted: G C 3.5.3-1.fc17.x86_64 #1
> :Call Trace:
> : [<ffffffff810584bf>] warn_slowpath_common+0x7f/0xc0
> : [<ffffffff810585b6>] warn_slowpath_fmt+0x46/0x50
> : [<ffffffff815443c4>] tcp_cleanup_rbuf+0x54/0x120
> : [<ffffffff8154564c>] tcp_recvmsg+0x76c/0xd30
> : [<ffffffff81569c1b>] inet_recvmsg+0x6b/0x80
> : [<ffffffff814e7612>] sock_aio_read.part.9+0x142/0x170
> : [<ffffffff814e7665>] sock_aio_read+0x25/0x40
> : [<ffffffff81187a06>] do_sync_read+0xe6/0x120
> : [<ffffffff8127948a>] ? inode_has_perm.isra.31.constprop.61+0x2a/0x30
> : [<ffffffff812764d2>] ? security_file_permission+0x92/0xb0
> : [<ffffffff81187ea1>] ? rw_verify_area+0x61/0xf0
> : [<ffffffff811883ed>] vfs_read+0x15d/0x180
> : [<ffffffff8118845a>] sys_read+0x4a/0x90
> : [<ffffffff81614ae9>] system_call_fastpath+0x16/0x1b
Could this be related to the "recvmsg bug" WARN's we've also been seeing recently ?
Dave
^ permalink raw reply
* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Steven Rostedt @ 2012-09-06 15:11 UTC (permalink / raw)
To: Josh Triplett
Cc: snitzer-H+wXaHxf7aLQT0dZR+AlfA, neilb-l3A5Bk7waGM,
fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA, agk-H+wXaHxf7aLQT0dZR+AlfA,
aarcange-H+wXaHxf7aLQT0dZR+AlfA, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
Tejun-/PVsmBQoxgPKo9QCiBeYKEEOCMrvLtNR,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ccaulfie-H+wXaHxf7aLQT0dZR+AlfA, mingo-X9Un+BFzKDI,
dev-yBygre7rU0TnMu66kgdUjQ, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
lw-BthXqXjhjHXQFUHtdCDX3A, Mathieu Desnoyers, Sasha Levin,
axboe-tSWWG44O7X1aa/9Udqfwiw, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
Pedro Alves, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
ejt-H+wXaHxf7aLQT0dZR+AlfA, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
netdev-u79uwXL29TY76Z2rM5mHXA, Heo,
teigland-H+wXaHxf7aLQT0dZR+AlfA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20120906145545.GA17332@leaf>
On Thu, 2012-09-06 at 07:55 -0700, Josh Triplett wrote:
> > My solution to making 'break' work in the iterator is:
> >
> > for (bkt = 0, node = NULL; bkt < HASH_SIZE(name) && node == NULL; bkt++)
> > hlist_for_each_entry(obj, node, &name[bkt], member)
> >
>
> Looks reasonable. However, it would break (or rather, not break) on
> code like this:
>
> hash_for_each_entry(...) {
> if (...) {
> foo(node);
> node = NULL;
> break;
> }
> }
>
> Hiding the double loop still seems error-prone.
We've already had this conversation ;-) A guess a big comment is in
order:
/*
* NOTE! Although this is a double loop, 'break' still works because of
* the 'node == NULL' condition in the outer loop. On break of
* the inner loop, node will be !NULL, and the outer loop will
* exit as well.
*/
-- Steve
^ permalink raw reply
* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Josh Triplett @ 2012-09-06 14:55 UTC (permalink / raw)
To: Sasha Levin
Cc: Mathieu Desnoyers, Pedro Alves, Steven Rostedt, Tejun Heo,
torvalds, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
mingo, ebiederm, aarcange, ericvh, netdev, eric.dumazet, axboe,
agk, dm-devel, neilb, ccaulfie, teigland, Trond.Myklebust,
bfields, fweisbec, jesse, venkat.x.venkatsubra, ejt, snitzer,
edumazet, linux-nfs, dev, rds-devel, lw
In-Reply-To: <5048AAF6.5090101@gmail.com>
On Thu, Sep 06, 2012 at 03:53:58PM +0200, Sasha Levin wrote:
> On 09/04/2012 07:01 PM, Mathieu Desnoyers wrote:
> >> #define do_for_each_ftrace_rec(pg, rec) \
> >> > for (pg = ftrace_pages_start, rec = &pg->records[pg->index]; \
> >> > pg && rec == &pg->records[pg->index]; \
> >> > pg = pg->next) \
> >> > for (rec = pg->records; rec < &pg->records[pg->index]; rec++)
> > Maybe in some cases there might be ways to combine the two loops into
> > one ? I'm not seeing exactly how to do it for this one, but it should
> > not be impossible. If the inner loop condition can be moved to the outer
> > loop, and if we use (blah ? loop1_conf : loop2_cond) to test for
> > different conditions depending on the context, and do the same for the
> > 3rd argument of the for() loop. The details elude me for now though, so
> > maybe it's complete non-sense ;)
> >
> > It might not be that useful for do_for_each_ftrace_rec, but if we can do
> > it for the hash table iterator, it might be worth it.
>
> So I think that for the hash iterator it might actually be simpler.
>
> My solution to making 'break' work in the iterator is:
>
> for (bkt = 0, node = NULL; bkt < HASH_SIZE(name) && node == NULL; bkt++)
> hlist_for_each_entry(obj, node, &name[bkt], member)
>
> We initialize our node loop cursor with NULL in the external loop, and the
> external loop will have a new condition to loop while that cursor is NULL.
>
> My logic is that we can only 'break' when we are iterating over an object in the
> internal loop. If we're iterating over an object in that loop then 'node != NULL'.
>
> This way, if we broke from within the internal loop, the external loop will see
> node as not NULL, and so it will stop looping itself. On the other hand, if the
> internal loop has actually ended, then node will be NULL, and the outer loop
> will keep running.
>
> Is there anything I've missed?
Looks reasonable. However, it would break (or rather, not break) on
code like this:
hash_for_each_entry(...) {
if (...) {
foo(node);
node = NULL;
break;
}
}
Hiding the double loop still seems error-prone.
- Josh Triplett
--
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 04/10] net/macb: Fix a race in macb_start_xmit()
From: Nicolas Ferre @ 2012-09-06 14:52 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-arm-kernel, havard, plagnioj, jamie, linux-kernel,
patrice.vilchez
In-Reply-To: <20120905.173023.2235590074897156746.davem@davemloft.net>
On 09/05/2012 11:30 PM, David Miller :
> From: Nicolas Ferre <nicolas.ferre@atmel.com>
> Date: Wed, 5 Sep 2012 10:19:11 +0200
>
>> From: Havard Skinnemoen <havard@skinnemoen.net>
>>
>> Fix a race in macb_start_xmit() where we unconditionally set the TSTART bit.
>> If an underrun just happened (we do this with interrupts disabled, so it might
>> not have been handled yet), the controller starts transmitting from the first
>> entry in the ring, which is usually wrong.
>> Restart the controller after error handling.
>>
>> Signed-off-by: Havard Skinnemoen <havard@skinnemoen.net>
>> [nicolas.ferre@atmel.com: split patch in topics]
>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>
> Accumulating special case code and checks into the hot path of TX packet
> processing is extremely unwise.
>
> Instead, when you handle the TX error conditions and reset the chip you
> should first ensure that there are no flows of control in the transmit
> function of your driver by using the appropriate locking et al. facilities.
>
> For example, you can quiesce the transmit path by handling the chip error
> interrupt as follows:
>
> 1) Disable chip interrupt generation.
>
> 2) Schedule a workqueue so you can process the reset outside of hard
> interrupt context.
>
> 3) In the workqueue function, disable NAPI and perform a
> netif_tx_disable() to guarentee there are no threads of
> execution trying to queue up packets for TX into the driver.
>
> 4) Perform your chip reset and whatever else is necessary.
>
> 5) Re-enable NAPI and TX.
>
> Then you don't need any special checks in your xmit method at all.
I see... I will rework the series and try to implement this as part of
the "[PATCH 06/10] net/macb: better manage tx errors"
So this patch will disappear in future v2 series and patch 06 will be
seriously modified. In fact I will also try to stack "cosmetic" patches
at the beginning of the series.
Thanks, best regards,
--
Nicolas Ferre
^ permalink raw reply
* Re: NULL pointer dereference in xt_register_target()
From: Pablo Neira Ayuso @ 2012-09-06 14:44 UTC (permalink / raw)
To: Cong Wang; +Cc: Eric Dumazet, netfilter-devel, Linux Kernel Network Developers
In-Reply-To: <CAM_iQpVAZwKhZxLdk-BLMjnK3eX4DD_o9KvzuAoTttyxvmCsJA@mail.gmail.com>
On Thu, Sep 06, 2012 at 10:27:22PM +0800, Cong Wang wrote:
> On Thu, Sep 6, 2012 at 12:48 AM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Wed, Sep 05, 2012 at 05:55:06PM +0200, Eric Dumazet wrote:
> >> On Wed, 2012-09-05 at 23:43 +0800, Cong Wang wrote:
> >> > Hi, folks,
> >> >
> >> > The latest net-next tree can't boot due to a NULL ptr def
> >> > bug in the kernel, the full backtrack is:
> >> >
> >> > http://img1.douban.com/view/photo/photo/public/p1697139550.jpg
> >> >
> >> > the kernel .config file is:
> >> >
> >> > http://pastebin.com/9YTnkqKN
> >> >
> >> > I don't have time to look into the issue. If you need other info,
> >> > please let me know.
> >>
> >> It seems xt_nat_init() is called before xt_init(), so xt array is not
> >> yet setup.
> >
> > I have enqueued the following patch to fix this:
> >
> > http://1984.lsi.us.es/git/nf-next/commit/?id=00545bec9412d130c77f72a08d6c8b6ad21d4a1
> > e
> > commit 00545bec9412d130c77f72a08d6c8b6ad21d4a1e
> > Author: Pablo Neira Ayuso <pablo@netfilter.org>
> > Date: Wed Sep 5 18:24:55 2012 +0200
> >
> > netfilter: fix crash during boot if NAT has been compiled built-in
> >
>
> Yeah, this indeed fixes the bug.
>
> Please push it to net-next as soon as possible?
Will do, thanks for testing.
^ permalink raw reply
* RE: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: David Laight @ 2012-09-06 14:36 UTC (permalink / raw)
To: Sasha Levin, Mathieu Desnoyers
Cc: Pedro Alves, Steven Rostedt, Tejun Heo, torvalds, akpm,
linux-kernel, linux-mm, paul.gortmaker, davem, mingo, ebiederm,
aarcange, ericvh, netdev, josh, eric.dumazet, axboe, agk,
dm-devel, neilb, ccaulfie, teigland, Trond.Myklebust, bfields,
fweisbec, jesse, venkat.x.venkatsubra, ejt, snitzer, edumazet,
linux-nfs, dev, rds-devel, lw
In-Reply-To: <5048AAF6.5090101@gmail.com>
> My solution to making 'break' work in the iterator is:
>
> for (bkt = 0, node = NULL; bkt < HASH_SIZE(name) && node ==
NULL; bkt++)
> hlist_for_each_entry(obj, node, &name[bkt], member)
I'd take a look at the generated code.
Might come out a bit better if the condition is changed to:
node == NULL && bkt < HASH_SIZE(name)
you might find the compiler always optimises out the
node == NULL comparison.
(It might anyway, but switching the order gives it a better
chance.)
David
--
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