RE: BUG report about ipt_do_table( )
hi Will yes, of course . by the way, could you send me your delivered commit link after you submit ? Thanks -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Thursday, October 17, 2013 6:41 PM To: Wang, Yalin Cc: 'linux-arm-msm-ow...@vger.kernel.org'; 'linux-kernel@vger.kernel.org'; Peng, Arthur; Zhang, Bojie; Gu, Youcai 1 (EXT); Alevoor, Raghavendra 2 Subject: Re: BUG report about ipt_do_table( ) On Thu, Oct 17, 2013 at 02:51:34AM +0100, Wang, Yalin wrote: > Hi Will, Hello, > I am happy to notify that our stability test has passed, And this > Crash don't happen again, So seems this patch work now . Great! > We has merged it into our release SW . > Could I know if this patch will be delivered into kernel Mainline by > you ? Sure, can I add your tested-by please? Will -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: BUG report about ipt_do_table( )
On Thu, Oct 17, 2013 at 02:51:34AM +0100, Wang, Yalin wrote: > Hi Will, Hello, > I am happy to notify that our stability test has passed, > And this Crash don't happen again, > So seems this patch work now . Great! > We has merged it into our release SW . > Could I know if this patch will be delivered into kernel > Mainline by you ? Sure, can I add your tested-by please? Will -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: BUG report about ipt_do_table( )
On Thu, Oct 17, 2013 at 02:51:34AM +0100, Wang, Yalin wrote: Hi Will, Hello, I am happy to notify that our stability test has passed, And this Crash don't happen again, So seems this patch work now . Great! We has merged it into our release SW . Could I know if this patch will be delivered into kernel Mainline by you ? Sure, can I add your tested-by please? Will -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: BUG report about ipt_do_table( )
hi Will yes, of course . by the way, could you send me your delivered commit link after you submit ? Thanks -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Thursday, October 17, 2013 6:41 PM To: Wang, Yalin Cc: 'linux-arm-msm-ow...@vger.kernel.org'; 'linux-kernel@vger.kernel.org'; Peng, Arthur; Zhang, Bojie; Gu, Youcai 1 (EXT); Alevoor, Raghavendra 2 Subject: Re: BUG report about ipt_do_table( ) On Thu, Oct 17, 2013 at 02:51:34AM +0100, Wang, Yalin wrote: Hi Will, Hello, I am happy to notify that our stability test has passed, And this Crash don't happen again, So seems this patch work now . Great! We has merged it into our release SW . Could I know if this patch will be delivered into kernel Mainline by you ? Sure, can I add your tested-by please? Will -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: BUG report about ipt_do_table( )
Hi Will, I am happy to notify that our stability test has passed, And this Crash don't happen again, So seems this patch work now . We has merged it into our release SW . Could I know if this patch will be delivered into kernel Mainline by you ? Thanks again ! -Original Message- From: Wang, Yalin Sent: Friday, October 11, 2013 7:15 PM To: 'Will Deacon' Cc: 'linux-arm-msm-ow...@vger.kernel.org'; linux-kernel@vger.kernel.org; Peng, Arthur; Zhang, Bojie; Gu, Youcai 1 (EXT); Alevoor, Raghavendra 2 Subject: RE: BUG report about ipt_do_table( ) Hi Will, Thanks for your clarification . I have merged the patch, And need wait some time to get The test result , You know that this BUG is not easy to reproduce, It's very infrequent. Maybe we need run several times stability test to Make sure the patch works . I will update to you the result as soon as possible ! Thanks -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Friday, October 11, 2013 7:03 PM To: Wang, Yalin Cc: 'linux-arm-msm-ow...@vger.kernel.org'; linux-kernel@vger.kernel.org; Peng, Arthur; Zhang, Bojie; Gu, Youcai 1 (EXT); Alevoor, Raghavendra 2 Subject: Re: BUG report about ipt_do_table( ) On Fri, Oct 11, 2013 at 02:50:24AM +0100, Wang, Yalin wrote: > Hi Will, Hello again, > Maybe I know your meaning , > If it use spinlock to protected the shared data, The bug will not > happen, because spinlock will Use DSB( ) to sync . Actually, the dsb is for something else (the sev). It is the smp_mb() call which guarantees the ordering of critical sections with respect to spinlock operations. > Unluckily, here, it use a special seqcount_t( ) (see get_counters( ) > function) Well, there is a comment about a write_lock being held, so you should be ok if that's true. The issue I saw was with the newinfo population, as I described in my earlier mail. > To make sure there is no others using the old data, Before release the > old data, this is much like RCU Work, but RCU use rcu_assign_pointer( > ) --> Which use smp_wmb( ) , so it's safe, am I right ? RCU is safe. There are *many* weakly ordered architectures on which Linux runs, so I don't think you have to worry too much about the core data structures and locking/synchronisation/atomic primitives. The major scope for errors is in lockless code, where the barrier usage is explicit. > In my patch, I use mb( ), because this macro Is DSB( ) , while > smp_wmb( ) is DMS( ), I just think DSB is much strict than DMS, mmm.. > so , DSM( ) or DMS ( ) are both ok ? I think you're getting confused with your barriers. We have two memory barriers on ARM: dmb and dsb. dmb is sufficient to enforce ordering of observability. dsb is used to enforce completion. > The whitepaper I use is here: > https://www.google.com/#q=cortex+a15+microarchitecture > > the first: [PDF] Exploring the Design of the Cortex-A15 Processor - > ARM > > I just search in Google, and you know that qcom don't release Much > document about its krait cpu's micro architecture details, I just use > cortex-a15 for a reference, I am not sure if their pipeline ( > load/store unit) are the same, I think the lawyers would have a field day if the pipelines were the same! You really can't use an A15 slide-deck to infer micro-architectural details about Krait. Please can you test the patch I sent you yesterday? Will -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: BUG report about ipt_do_table( )
Hi Will, I am happy to notify that our stability test has passed, And this Crash don't happen again, So seems this patch work now . We has merged it into our release SW . Could I know if this patch will be delivered into kernel Mainline by you ? Thanks again ! -Original Message- From: Wang, Yalin Sent: Friday, October 11, 2013 7:15 PM To: 'Will Deacon' Cc: 'linux-arm-msm-ow...@vger.kernel.org'; linux-kernel@vger.kernel.org; Peng, Arthur; Zhang, Bojie; Gu, Youcai 1 (EXT); Alevoor, Raghavendra 2 Subject: RE: BUG report about ipt_do_table( ) Hi Will, Thanks for your clarification . I have merged the patch, And need wait some time to get The test result , You know that this BUG is not easy to reproduce, It's very infrequent. Maybe we need run several times stability test to Make sure the patch works . I will update to you the result as soon as possible ! Thanks -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Friday, October 11, 2013 7:03 PM To: Wang, Yalin Cc: 'linux-arm-msm-ow...@vger.kernel.org'; linux-kernel@vger.kernel.org; Peng, Arthur; Zhang, Bojie; Gu, Youcai 1 (EXT); Alevoor, Raghavendra 2 Subject: Re: BUG report about ipt_do_table( ) On Fri, Oct 11, 2013 at 02:50:24AM +0100, Wang, Yalin wrote: Hi Will, Hello again, Maybe I know your meaning , If it use spinlock to protected the shared data, The bug will not happen, because spinlock will Use DSB( ) to sync . Actually, the dsb is for something else (the sev). It is the smp_mb() call which guarantees the ordering of critical sections with respect to spinlock operations. Unluckily, here, it use a special seqcount_t( ) (see get_counters( ) function) Well, there is a comment about a write_lock being held, so you should be ok if that's true. The issue I saw was with the newinfo population, as I described in my earlier mail. To make sure there is no others using the old data, Before release the old data, this is much like RCU Work, but RCU use rcu_assign_pointer( ) -- Which use smp_wmb( ) , so it's safe, am I right ? RCU is safe. There are *many* weakly ordered architectures on which Linux runs, so I don't think you have to worry too much about the core data structures and locking/synchronisation/atomic primitives. The major scope for errors is in lockless code, where the barrier usage is explicit. In my patch, I use mb( ), because this macro Is DSB( ) , while smp_wmb( ) is DMS( ), I just think DSB is much strict than DMS, mmm.. so , DSM( ) or DMS ( ) are both ok ? I think you're getting confused with your barriers. We have two memory barriers on ARM: dmb and dsb. dmb is sufficient to enforce ordering of observability. dsb is used to enforce completion. The whitepaper I use is here: https://www.google.com/#q=cortex+a15+microarchitecture the first: [PDF] Exploring the Design of the Cortex-A15 Processor - ARM I just search in Google, and you know that qcom don't release Much document about its krait cpu's micro architecture details, I just use cortex-a15 for a reference, I am not sure if their pipeline ( load/store unit) are the same, I think the lawyers would have a field day if the pipelines were the same! You really can't use an A15 slide-deck to infer micro-architectural details about Krait. Please can you test the patch I sent you yesterday? Will -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: BUG report about ipt_do_table( )
Hi Will, Thanks for your clarification . I have merged the patch, And need wait some time to get The test result , You know that this BUG is not easy to reproduce, It's very infrequent. Maybe we need run several times stability test to Make sure the patch works . I will update to you the result as soon as possible ! Thanks -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Friday, October 11, 2013 7:03 PM To: Wang, Yalin Cc: 'linux-arm-msm-ow...@vger.kernel.org'; linux-kernel@vger.kernel.org; Peng, Arthur; Zhang, Bojie; Gu, Youcai 1 (EXT); Alevoor, Raghavendra 2 Subject: Re: BUG report about ipt_do_table( ) On Fri, Oct 11, 2013 at 02:50:24AM +0100, Wang, Yalin wrote: > Hi Will, Hello again, > Maybe I know your meaning , > If it use spinlock to protected the shared data, The bug will not > happen, because spinlock will Use DSB( ) to sync . Actually, the dsb is for something else (the sev). It is the smp_mb() call which guarantees the ordering of critical sections with respect to spinlock operations. > Unluckily, here, it use a special seqcount_t( ) (see get_counters( ) > function) Well, there is a comment about a write_lock being held, so you should be ok if that's true. The issue I saw was with the newinfo population, as I described in my earlier mail. > To make sure there is no others using the old data, Before release the > old data, this is much like RCU Work, but RCU use rcu_assign_pointer( > ) --> Which use smp_wmb( ) , so it's safe, am I right ? RCU is safe. There are *many* weakly ordered architectures on which Linux runs, so I don't think you have to worry too much about the core data structures and locking/synchronisation/atomic primitives. The major scope for errors is in lockless code, where the barrier usage is explicit. > In my patch, I use mb( ), because this macro Is DSB( ) , while > smp_wmb( ) is DMS( ), I just think DSB is much strict than DMS, mmm.. > so , DSM( ) or DMS ( ) are both ok ? I think you're getting confused with your barriers. We have two memory barriers on ARM: dmb and dsb. dmb is sufficient to enforce ordering of observability. dsb is used to enforce completion. > The whitepaper I use is here: > https://www.google.com/#q=cortex+a15+microarchitecture > > the first: [PDF] Exploring the Design of the Cortex-A15 Processor - > ARM > > I just search in Google, and you know that qcom don't release Much > document about its krait cpu's micro architecture details, I just use > cortex-a15 for a reference, I am not sure if their pipeline ( > load/store unit) are the same, I think the lawyers would have a field day if the pipelines were the same! You really can't use an A15 slide-deck to infer micro-architectural details about Krait. Please can you test the patch I sent you yesterday? Will -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: BUG report about ipt_do_table( )
On Fri, Oct 11, 2013 at 02:50:24AM +0100, Wang, Yalin wrote: > Hi Will, Hello again, > Maybe I know your meaning , > If it use spinlock to protected the shared data, > The bug will not happen, because spinlock will > Use DSB( ) to sync . Actually, the dsb is for something else (the sev). It is the smp_mb() call which guarantees the ordering of critical sections with respect to spinlock operations. > Unluckily, here, it use a special seqcount_t( ) (see get_counters( ) function) Well, there is a comment about a write_lock being held, so you should be ok if that's true. The issue I saw was with the newinfo population, as I described in my earlier mail. > To make sure there is no others using the old data, > Before release the old data, this is much like RCU > Work, but RCU use rcu_assign_pointer( ) --> > Which use smp_wmb( ) , so it's safe, am I right ? RCU is safe. There are *many* weakly ordered architectures on which Linux runs, so I don't think you have to worry too much about the core data structures and locking/synchronisation/atomic primitives. The major scope for errors is in lockless code, where the barrier usage is explicit. > In my patch, I use mb( ), because this macro > Is DSB( ) , while smp_wmb( ) is DMS( ), > I just think DSB is much strict than DMS, > mmm.. so , DSM( ) or DMS ( ) are both ok ? I think you're getting confused with your barriers. We have two memory barriers on ARM: dmb and dsb. dmb is sufficient to enforce ordering of observability. dsb is used to enforce completion. > The whitepaper I use is here: > https://www.google.com/#q=cortex+a15+microarchitecture > > the first: [PDF] Exploring the Design of the Cortex-A15 Processor - ARM > > I just search in Google, and you know that qcom don't release > Much document about its krait cpu's micro architecture details, > I just use cortex-a15 for a reference, I am not sure if their > pipeline ( load/store unit) are the same, I think the lawyers would have a field day if the pipelines were the same! You really can't use an A15 slide-deck to infer micro-architectural details about Krait. Please can you test the patch I sent you yesterday? Will -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: BUG report about ipt_do_table( )
On Fri, Oct 11, 2013 at 02:50:24AM +0100, Wang, Yalin wrote: Hi Will, Hello again, Maybe I know your meaning , If it use spinlock to protected the shared data, The bug will not happen, because spinlock will Use DSB( ) to sync . Actually, the dsb is for something else (the sev). It is the smp_mb() call which guarantees the ordering of critical sections with respect to spinlock operations. Unluckily, here, it use a special seqcount_t( ) (see get_counters( ) function) Well, there is a comment about a write_lock being held, so you should be ok if that's true. The issue I saw was with the newinfo population, as I described in my earlier mail. To make sure there is no others using the old data, Before release the old data, this is much like RCU Work, but RCU use rcu_assign_pointer( ) -- Which use smp_wmb( ) , so it's safe, am I right ? RCU is safe. There are *many* weakly ordered architectures on which Linux runs, so I don't think you have to worry too much about the core data structures and locking/synchronisation/atomic primitives. The major scope for errors is in lockless code, where the barrier usage is explicit. In my patch, I use mb( ), because this macro Is DSB( ) , while smp_wmb( ) is DMS( ), I just think DSB is much strict than DMS, mmm.. so , DSM( ) or DMS ( ) are both ok ? I think you're getting confused with your barriers. We have two memory barriers on ARM: dmb and dsb. dmb is sufficient to enforce ordering of observability. dsb is used to enforce completion. The whitepaper I use is here: https://www.google.com/#q=cortex+a15+microarchitecture the first: [PDF] Exploring the Design of the Cortex-A15 Processor - ARM I just search in Google, and you know that qcom don't release Much document about its krait cpu's micro architecture details, I just use cortex-a15 for a reference, I am not sure if their pipeline ( load/store unit) are the same, I think the lawyers would have a field day if the pipelines were the same! You really can't use an A15 slide-deck to infer micro-architectural details about Krait. Please can you test the patch I sent you yesterday? Will -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: BUG report about ipt_do_table( )
Hi Will, Thanks for your clarification . I have merged the patch, And need wait some time to get The test result , You know that this BUG is not easy to reproduce, It's very infrequent. Maybe we need run several times stability test to Make sure the patch works . I will update to you the result as soon as possible ! Thanks -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Friday, October 11, 2013 7:03 PM To: Wang, Yalin Cc: 'linux-arm-msm-ow...@vger.kernel.org'; linux-kernel@vger.kernel.org; Peng, Arthur; Zhang, Bojie; Gu, Youcai 1 (EXT); Alevoor, Raghavendra 2 Subject: Re: BUG report about ipt_do_table( ) On Fri, Oct 11, 2013 at 02:50:24AM +0100, Wang, Yalin wrote: Hi Will, Hello again, Maybe I know your meaning , If it use spinlock to protected the shared data, The bug will not happen, because spinlock will Use DSB( ) to sync . Actually, the dsb is for something else (the sev). It is the smp_mb() call which guarantees the ordering of critical sections with respect to spinlock operations. Unluckily, here, it use a special seqcount_t( ) (see get_counters( ) function) Well, there is a comment about a write_lock being held, so you should be ok if that's true. The issue I saw was with the newinfo population, as I described in my earlier mail. To make sure there is no others using the old data, Before release the old data, this is much like RCU Work, but RCU use rcu_assign_pointer( ) -- Which use smp_wmb( ) , so it's safe, am I right ? RCU is safe. There are *many* weakly ordered architectures on which Linux runs, so I don't think you have to worry too much about the core data structures and locking/synchronisation/atomic primitives. The major scope for errors is in lockless code, where the barrier usage is explicit. In my patch, I use mb( ), because this macro Is DSB( ) , while smp_wmb( ) is DMS( ), I just think DSB is much strict than DMS, mmm.. so , DSM( ) or DMS ( ) are both ok ? I think you're getting confused with your barriers. We have two memory barriers on ARM: dmb and dsb. dmb is sufficient to enforce ordering of observability. dsb is used to enforce completion. The whitepaper I use is here: https://www.google.com/#q=cortex+a15+microarchitecture the first: [PDF] Exploring the Design of the Cortex-A15 Processor - ARM I just search in Google, and you know that qcom don't release Much document about its krait cpu's micro architecture details, I just use cortex-a15 for a reference, I am not sure if their pipeline ( load/store unit) are the same, I think the lawyers would have a field day if the pipelines were the same! You really can't use an A15 slide-deck to infer micro-architectural details about Krait. Please can you test the patch I sent you yesterday? Will -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: BUG report about ipt_do_table( )
Hi Will, Maybe I know your meaning , If it use spinlock to protected the shared data, The bug will not happen, because spinlock will Use DSB( ) to sync . Unluckily, here, it use a special seqcount_t( ) (see get_counters( ) function) To make sure there is no others using the old data, Before release the old data, this is much like RCU Work, but RCU use rcu_assign_pointer( ) --> Which use smp_wmb( ) , so it's safe, am I right ? In my patch, I use mb( ), because this macro Is DSB( ) , while smp_wmb( ) is DMS( ), I just think DSB is much strict than DMS, mmm.. so , DSM( ) or DMS ( ) are both ok ? The whitepaper I use is here: https://www.google.com/#q=cortex+a15+microarchitecture the first: [PDF] Exploring the Design of the Cortex-A15 Processor - ARM I just search in Google, and you know that qcom don't release Much document about its krait cpu's micro architecture details, I just use cortex-a15 for a reference, I am not sure if their pipeline ( load/store unit) are the same, if you have cortex-a15 micro architecture details document, I am very appreciated that you can share to me . :) Thanks -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Thursday, October 10, 2013 10:19 PM To: Wang, Yalin Cc: 'linux-arm-msm-ow...@vger.kernel.org'; linux-kernel@vger.kernel.org; Peng, Arthur; Zhang, Bojie; Gu, Youcai 1 (EXT); Alevoor, Raghavendra 2 Subject: Re: BUG report about ipt_do_table( ) On Thu, Oct 10, 2013 at 12:26:38PM +0100, Wang, Yalin wrote: > Hi Will , Hello, > Seems your patch is better than mine, > It make sure newinfo->initial_entries update is Seen by others . > > I will test by your patches , and update to you ASAP . Thanks. > I have another questions about this BUG, Since all shared data will > have this problem especially Shared between different CPUs, > > This means all shared data need a smp_wmb() , after One CPU update it > ? Only if the ordering of observability matters and isn't enforced by other mechanisms (e.g. locks). > But I see not all shared data are updated by this way, And kernel > works well . Why ?? a little curious . Again, the barriers tend to be hidden inside things like locking primitives, atomics, I/O accessors etc. > Another is that , I read cortex-a15 whitepaper , It says : > > Load/Store cluster > § All Load/Store, data transfers and cache maintenance operations § > Partially out-of-order, 1 Load and 1 Store executed per cycle § Load > cannot bypass a Store, Store cannot bypass a Store It's hard to interpret this out of context. Please can you point me at the whitepaper? Also, I was talking from the perspective of the architecture and you're not using an A15 afaik. > !! Store can't bypass store !! , but in this BUG , it seems later store can > Be wrote into cache even before the earlier store completed ! > Am I right ? The architecture requires stores to be observed in program order from the perspective of the CPU issuing them, but there aren't ordering guarantees for stores to different locations when observed by a different CPU. Will
Re: BUG report about ipt_do_table( )
On Thu, Oct 10, 2013 at 12:26:38PM +0100, Wang, Yalin wrote: > Hi Will , Hello, > Seems your patch is better than mine, > It make sure newinfo->initial_entries update is > Seen by others . > > I will test by your patches , and update to you ASAP . Thanks. > I have another questions about this BUG, > Since all shared data will have this problem especially > Shared between different CPUs, > > This means all shared data need a smp_wmb() , after > One CPU update it ? Only if the ordering of observability matters and isn't enforced by other mechanisms (e.g. locks). > But I see not all shared data are updated by this way, > And kernel works well . Why ?? a little curious . Again, the barriers tend to be hidden inside things like locking primitives, atomics, I/O accessors etc. > Another is that , I read cortex-a15 whitepaper , > It says : > > Load/Store cluster > All Load/Store, data transfers and cache maintenance operations > Partially out-of-order, 1 Load and 1 Store executed per cycle > Load cannot bypass a Store, Store cannot bypass a Store It's hard to interpret this out of context. Please can you point me at the whitepaper? Also, I was talking from the perspective of the architecture and you're not using an A15 afaik. > !! Store can't bypass store !! , but in this BUG , it seems later store can > Be wrote into cache even before the earlier store completed ! > Am I right ? The architecture requires stores to be observed in program order from the perspective of the CPU issuing them, but there aren't ordering guarantees for stores to different locations when observed by a different CPU. Will -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: BUG report about ipt_do_table( )
Hi Will , Seems your patch is better than mine, It make sure newinfo->initial_entries update is Seen by others . I will test by your patches , and update to you ASAP . I have another questions about this BUG, Since all shared data will have this problem especially Shared between different CPUs, This means all shared data need a smp_wmb() , after One CPU update it ? But I see not all shared data are updated by this way, And kernel works well . Why ?? a little curious . Another is that , I read cortex-a15 whitepaper , It says : Load/Store cluster All Load/Store, data transfers and cache maintenance operations Partially out-of-order, 1 Load and 1 Store executed per cycle Load cannot bypass a Store, Store cannot bypass a Store !! Store can't bypass store !! , but in this BUG , it seems later store can Be wrote into cache even before the earlier store completed ! Am I right ? Thank you ! -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Thursday, October 10, 2013 7:03 PM To: Wang, Yalin Cc: 'linux-arm-msm-ow...@vger.kernel.org'; linux-kernel@vger.kernel.org; Peng, Arthur; Zhang, Bojie Subject: Re: BUG report about ipt_do_table( ) On Thu, Oct 10, 2013 at 11:22:21AM +0100, Wang, Yalin wrote: > Thanks for your reply . No problem. > I have compare our kernel with 3.12 , Ip_tables.c x_tables.c is the > same , So the BUG should can also be reproduce on 3.12 (just my > guess). [...] > /- > --/ diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c > index 8d987c3..2353bcc 100644 > --- a/net/netfilter/x_tables.c > +++ b/net/netfilter/x_tables.c > @@ -819,6 +819,12 @@ xt_replace_table(struct xt_table *table, > return NULL; > } > > + /* > + * make sure the change is write to the memory > + * so that the other CPU can see the changes > + */ > + mb(); > + > /* Do the substitution. */ > local_bh_disable(); > private = table->private; > > /- > --/ > > > I add a memory barrier before update table->private . > Make sure the other CPU can see the update memory correctly. > When the BUG happened, the other CPU can get the new private (struct > xt_table_info *), But sometimes it see private->jumpstack == NULL , > or sometimes it see private->jumpstack[cpu] == NULL , On one CPU, xt_replace_table is basically doing: newinfo->jumpstack = kzalloc(...); table->private = newinfo; so this can be thought of as the `writer' thread. Then, on another CPU (the `reader'), we run ipt_do_table: private = table->private; jumpstack = (struct ipt_entry **)private->jumpstack[cpu]; The reader has an address dependency, so the loads are guaranteed to be observed in order (on sane CPUs... this is probably broken for Alpha). However, the two stores from the writer can be observed in any order by other CPUs. To make this clearer, I think you actually want an smb_wmb() immediately before the assignment to table->private (and that assignment to newinfo->initial_entries probably needs moving above it). Then you want newinfo->an smb_read_barrier_depends on the read path immediately after reading table->private. > This is caused by CPU write buffer ? > It has written table->private , but has not update private-> members > (still in write buffer) , This is really out of order write, will this > happened on modern armv7 CPU? > Especially like cortex-a15 , it can execute code out of order . Well, stores aren't speculated and stores to the same location are always observed in order (on ARM). This is more a consequence of the weakly ordered memory model, which largely comes about due to speculative loads and write buffering (including read forwarding). Things like cache coherence protocols and buffers in the interconnect can also cause stores to be observed in different orders, since there conceptually end up being multi copies of the data being written. Anyway, can you see if the patch below fixes your problem please? Will --->8 diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index d23118d..cadda40 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c @@ -326,6 +326,7 @@ ipt_do_table(struct sk_buff *skb, local_bh_disable(); addend = xt_write_recseq_begin(); private = table->private; + smp_read_barrier_depends(); cpu= smp_processor_id(); table_base = private->entries[cpu]; jumpstack = (struct ipt_entry **)private->jumpstack[cpu]; diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index 8b03028..ee5e184 100644 --- a/net
Re: BUG report about ipt_do_table( )
On Thu, Oct 10, 2013 at 11:22:21AM +0100, Wang, Yalin wrote: > Thanks for your reply . No problem. > I have compare our kernel with 3.12 , > Ip_tables.c x_tables.c is the same , > So the BUG should can also be reproduce on 3.12 (just my guess). [...] > /---/ > diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c > index 8d987c3..2353bcc 100644 > --- a/net/netfilter/x_tables.c > +++ b/net/netfilter/x_tables.c > @@ -819,6 +819,12 @@ xt_replace_table(struct xt_table *table, > return NULL; > } > > + /* > + * make sure the change is write to the memory > + * so that the other CPU can see the changes > + */ > + mb(); > + > /* Do the substitution. */ > local_bh_disable(); > private = table->private; > > /---/ > > > I add a memory barrier before update table->private . > Make sure the other CPU can see the update memory correctly. > When the BUG happened, the other CPU can get the new private (struct > xt_table_info *), > But sometimes it see private->jumpstack == NULL , or sometimes it see > private->jumpstack[cpu] == NULL , On one CPU, xt_replace_table is basically doing: newinfo->jumpstack = kzalloc(...); table->private = newinfo; so this can be thought of as the `writer' thread. Then, on another CPU (the `reader'), we run ipt_do_table: private = table->private; jumpstack = (struct ipt_entry **)private->jumpstack[cpu]; The reader has an address dependency, so the loads are guaranteed to be observed in order (on sane CPUs... this is probably broken for Alpha). However, the two stores from the writer can be observed in any order by other CPUs. To make this clearer, I think you actually want an smb_wmb() immediately before the assignment to table->private (and that assignment to newinfo->initial_entries probably needs moving above it). Then you want an smb_read_barrier_depends on the read path immediately after reading table->private. > This is caused by CPU write buffer ? > It has written table->private , but has not update private-> members (still > in write buffer) , > This is really out of order write, will this happened on modern armv7 CPU? > Especially like cortex-a15 , it can execute code out of order . Well, stores aren't speculated and stores to the same location are always observed in order (on ARM). This is more a consequence of the weakly ordered memory model, which largely comes about due to speculative loads and write buffering (including read forwarding). Things like cache coherence protocols and buffers in the interconnect can also cause stores to be observed in different orders, since there conceptually end up being multi copies of the data being written. Anyway, can you see if the patch below fixes your problem please? Will --->8 diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index d23118d..cadda40 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c @@ -326,6 +326,7 @@ ipt_do_table(struct sk_buff *skb, local_bh_disable(); addend = xt_write_recseq_begin(); private = table->private; + smp_read_barrier_depends(); cpu= smp_processor_id(); table_base = private->entries[cpu]; jumpstack = (struct ipt_entry **)private->jumpstack[cpu]; diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index 8b03028..ee5e184 100644 --- a/net/netfilter/x_tables.c +++ b/net/netfilter/x_tables.c @@ -845,8 +845,9 @@ xt_replace_table(struct xt_table *table, return NULL; } - table->private = newinfo; newinfo->initial_entries = private->initial_entries; + smb_wmb(); + table->private = newinfo; /* * Even though table entries have now been swapped, other CPU's -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: BUG report about ipt_do_table( )
Hi Will, Thanks for your reply . This is the kernel that we use: https://www.codeaurora.org/cgit/quic/la/kernel/msm/tree/net/ipv4/netfilter/ip_tables.c?id=M8960ANLYA26144005 https://www.codeaurora.org/cgit/quic/la/kernel/msm/tree/net/netfilter/x_tables.c?id=M8960ANLYA26144005 I am sorry that I can't use the latest kernel to reproduce This BUG, make the new kernel running on our Platform is not easy, and need do a lot of porting work . I have compare our kernel with 3.12 , Ip_tables.c x_tables.c is the same , So the BUG should can also be reproduce on 3.12 (just my guess). I make a patch for this BUG, but have not test it: /---/ diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index 8d987c3..2353bcc 100644 --- a/net/netfilter/x_tables.c +++ b/net/netfilter/x_tables.c @@ -819,6 +819,12 @@ xt_replace_table(struct xt_table *table, return NULL; } + /* +* make sure the change is write to the memory +* so that the other CPU can see the changes +*/ + mb(); + /* Do the substitution. */ local_bh_disable(); private = table->private; /---/ I add a memory barrier before update table->private . Make sure the other CPU can see the update memory correctly. When the BUG happened, the other CPU can get the new private (struct xt_table_info *), But sometimes it see private->jumpstack == NULL , or sometimes it see private->jumpstack[cpu] == NULL , Our several crash dumps show different crash point . This is caused by CPU write buffer ? It has written table->private , but has not update private-> members (still in write buffer) , This is really out of order write, will this happened on modern armv7 CPU? Especially like cortex-a15 , it can execute code out of order . Thanks you . -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Thursday, October 10, 2013 5:48 PM To: Wang, Yalin Cc: 'linux-arm-msm-ow...@vger.kernel.org'; linux-kernel@vger.kernel.org Subject: Re: BUG report about ipt_do_table( ) On Thu, Oct 10, 2013 at 06:16:05AM +0100, Wang, Yalin wrote: > Dear all, Hello, > We encounter a crash in ipt_do_table( ) function During our stability > test . > > The CPU is qcom msm8960 / dual core , linux kernel version is 3.4 I appreciate that this is a mammoth task, but can you reproduce this failure with a mainline kernel (3.12-rc4)? If you suspect a synchronisation issue in core code, I'm afraid you'll have to show the failure with the current sources. Will -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: BUG report about ipt_do_table( )
On Thu, Oct 10, 2013 at 06:16:05AM +0100, Wang, Yalin wrote: > Dear all, Hello, > We encounter a crash in ipt_do_table( ) function > During our stability test . > > The CPU is qcom msm8960 / dual core , linux kernel version is 3.4 I appreciate that this is a mammoth task, but can you reproduce this failure with a mainline kernel (3.12-rc4)? If you suspect a synchronisation issue in core code, I'm afraid you'll have to show the failure with the current sources. Will -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: BUG report about ipt_do_table( )
On Thu, Oct 10, 2013 at 06:16:05AM +0100, Wang, Yalin wrote: Dear all, Hello, We encounter a crash in ipt_do_table( ) function During our stability test . The CPU is qcom msm8960 / dual core , linux kernel version is 3.4 I appreciate that this is a mammoth task, but can you reproduce this failure with a mainline kernel (3.12-rc4)? If you suspect a synchronisation issue in core code, I'm afraid you'll have to show the failure with the current sources. Will -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: BUG report about ipt_do_table( )
Hi Will, Thanks for your reply . This is the kernel that we use: https://www.codeaurora.org/cgit/quic/la/kernel/msm/tree/net/ipv4/netfilter/ip_tables.c?id=M8960ANLYA26144005 https://www.codeaurora.org/cgit/quic/la/kernel/msm/tree/net/netfilter/x_tables.c?id=M8960ANLYA26144005 I am sorry that I can't use the latest kernel to reproduce This BUG, make the new kernel running on our Platform is not easy, and need do a lot of porting work . I have compare our kernel with 3.12 , Ip_tables.c x_tables.c is the same , So the BUG should can also be reproduce on 3.12 (just my guess). I make a patch for this BUG, but have not test it: /---/ diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index 8d987c3..2353bcc 100644 --- a/net/netfilter/x_tables.c +++ b/net/netfilter/x_tables.c @@ -819,6 +819,12 @@ xt_replace_table(struct xt_table *table, return NULL; } + /* +* make sure the change is write to the memory +* so that the other CPU can see the changes +*/ + mb(); + /* Do the substitution. */ local_bh_disable(); private = table-private; /---/ I add a memory barrier before update table-private . Make sure the other CPU can see the update memory correctly. When the BUG happened, the other CPU can get the new private (struct xt_table_info *), But sometimes it see private-jumpstack == NULL , or sometimes it see private-jumpstack[cpu] == NULL , Our several crash dumps show different crash point . This is caused by CPU write buffer ? It has written table-private , but has not update private- members (still in write buffer) , This is really out of order write, will this happened on modern armv7 CPU? Especially like cortex-a15 , it can execute code out of order . Thanks you . -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Thursday, October 10, 2013 5:48 PM To: Wang, Yalin Cc: 'linux-arm-msm-ow...@vger.kernel.org'; linux-kernel@vger.kernel.org Subject: Re: BUG report about ipt_do_table( ) On Thu, Oct 10, 2013 at 06:16:05AM +0100, Wang, Yalin wrote: Dear all, Hello, We encounter a crash in ipt_do_table( ) function During our stability test . The CPU is qcom msm8960 / dual core , linux kernel version is 3.4 I appreciate that this is a mammoth task, but can you reproduce this failure with a mainline kernel (3.12-rc4)? If you suspect a synchronisation issue in core code, I'm afraid you'll have to show the failure with the current sources. Will -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: BUG report about ipt_do_table( )
On Thu, Oct 10, 2013 at 11:22:21AM +0100, Wang, Yalin wrote: Thanks for your reply . No problem. I have compare our kernel with 3.12 , Ip_tables.c x_tables.c is the same , So the BUG should can also be reproduce on 3.12 (just my guess). [...] /---/ diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index 8d987c3..2353bcc 100644 --- a/net/netfilter/x_tables.c +++ b/net/netfilter/x_tables.c @@ -819,6 +819,12 @@ xt_replace_table(struct xt_table *table, return NULL; } + /* + * make sure the change is write to the memory + * so that the other CPU can see the changes + */ + mb(); + /* Do the substitution. */ local_bh_disable(); private = table-private; /---/ I add a memory barrier before update table-private . Make sure the other CPU can see the update memory correctly. When the BUG happened, the other CPU can get the new private (struct xt_table_info *), But sometimes it see private-jumpstack == NULL , or sometimes it see private-jumpstack[cpu] == NULL , On one CPU, xt_replace_table is basically doing: newinfo-jumpstack = kzalloc(...); table-private = newinfo; so this can be thought of as the `writer' thread. Then, on another CPU (the `reader'), we run ipt_do_table: private = table-private; jumpstack = (struct ipt_entry **)private-jumpstack[cpu]; The reader has an address dependency, so the loads are guaranteed to be observed in order (on sane CPUs... this is probably broken for Alpha). However, the two stores from the writer can be observed in any order by other CPUs. To make this clearer, I think you actually want an smb_wmb() immediately before the assignment to table-private (and that assignment to newinfo-initial_entries probably needs moving above it). Then you want an smb_read_barrier_depends on the read path immediately after reading table-private. This is caused by CPU write buffer ? It has written table-private , but has not update private- members (still in write buffer) , This is really out of order write, will this happened on modern armv7 CPU? Especially like cortex-a15 , it can execute code out of order . Well, stores aren't speculated and stores to the same location are always observed in order (on ARM). This is more a consequence of the weakly ordered memory model, which largely comes about due to speculative loads and write buffering (including read forwarding). Things like cache coherence protocols and buffers in the interconnect can also cause stores to be observed in different orders, since there conceptually end up being multi copies of the data being written. Anyway, can you see if the patch below fixes your problem please? Will ---8 diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index d23118d..cadda40 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c @@ -326,6 +326,7 @@ ipt_do_table(struct sk_buff *skb, local_bh_disable(); addend = xt_write_recseq_begin(); private = table-private; + smp_read_barrier_depends(); cpu= smp_processor_id(); table_base = private-entries[cpu]; jumpstack = (struct ipt_entry **)private-jumpstack[cpu]; diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index 8b03028..ee5e184 100644 --- a/net/netfilter/x_tables.c +++ b/net/netfilter/x_tables.c @@ -845,8 +845,9 @@ xt_replace_table(struct xt_table *table, return NULL; } - table-private = newinfo; newinfo-initial_entries = private-initial_entries; + smb_wmb(); + table-private = newinfo; /* * Even though table entries have now been swapped, other CPU's -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: BUG report about ipt_do_table( )
Hi Will , Seems your patch is better than mine, It make sure newinfo-initial_entries update is Seen by others . I will test by your patches , and update to you ASAP . I have another questions about this BUG, Since all shared data will have this problem especially Shared between different CPUs, This means all shared data need a smp_wmb() , after One CPU update it ? But I see not all shared data are updated by this way, And kernel works well . Why ?? a little curious . Another is that , I read cortex-a15 whitepaper , It says : Load/Store cluster All Load/Store, data transfers and cache maintenance operations Partially out-of-order, 1 Load and 1 Store executed per cycle Load cannot bypass a Store, Store cannot bypass a Store !! Store can't bypass store !! , but in this BUG , it seems later store can Be wrote into cache even before the earlier store completed ! Am I right ? Thank you ! -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Thursday, October 10, 2013 7:03 PM To: Wang, Yalin Cc: 'linux-arm-msm-ow...@vger.kernel.org'; linux-kernel@vger.kernel.org; Peng, Arthur; Zhang, Bojie Subject: Re: BUG report about ipt_do_table( ) On Thu, Oct 10, 2013 at 11:22:21AM +0100, Wang, Yalin wrote: Thanks for your reply . No problem. I have compare our kernel with 3.12 , Ip_tables.c x_tables.c is the same , So the BUG should can also be reproduce on 3.12 (just my guess). [...] /- --/ diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index 8d987c3..2353bcc 100644 --- a/net/netfilter/x_tables.c +++ b/net/netfilter/x_tables.c @@ -819,6 +819,12 @@ xt_replace_table(struct xt_table *table, return NULL; } + /* + * make sure the change is write to the memory + * so that the other CPU can see the changes + */ + mb(); + /* Do the substitution. */ local_bh_disable(); private = table-private; /- --/ I add a memory barrier before update table-private . Make sure the other CPU can see the update memory correctly. When the BUG happened, the other CPU can get the new private (struct xt_table_info *), But sometimes it see private-jumpstack == NULL , or sometimes it see private-jumpstack[cpu] == NULL , On one CPU, xt_replace_table is basically doing: newinfo-jumpstack = kzalloc(...); table-private = newinfo; so this can be thought of as the `writer' thread. Then, on another CPU (the `reader'), we run ipt_do_table: private = table-private; jumpstack = (struct ipt_entry **)private-jumpstack[cpu]; The reader has an address dependency, so the loads are guaranteed to be observed in order (on sane CPUs... this is probably broken for Alpha). However, the two stores from the writer can be observed in any order by other CPUs. To make this clearer, I think you actually want an smb_wmb() immediately before the assignment to table-private (and that assignment to newinfo-initial_entries probably needs moving above it). Then you want newinfo-an smb_read_barrier_depends on the read path immediately after reading table-private. This is caused by CPU write buffer ? It has written table-private , but has not update private- members (still in write buffer) , This is really out of order write, will this happened on modern armv7 CPU? Especially like cortex-a15 , it can execute code out of order . Well, stores aren't speculated and stores to the same location are always observed in order (on ARM). This is more a consequence of the weakly ordered memory model, which largely comes about due to speculative loads and write buffering (including read forwarding). Things like cache coherence protocols and buffers in the interconnect can also cause stores to be observed in different orders, since there conceptually end up being multi copies of the data being written. Anyway, can you see if the patch below fixes your problem please? Will ---8 diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index d23118d..cadda40 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c @@ -326,6 +326,7 @@ ipt_do_table(struct sk_buff *skb, local_bh_disable(); addend = xt_write_recseq_begin(); private = table-private; + smp_read_barrier_depends(); cpu= smp_processor_id(); table_base = private-entries[cpu]; jumpstack = (struct ipt_entry **)private-jumpstack[cpu]; diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index 8b03028..ee5e184 100644 --- a/net/netfilter/x_tables.c +++ b/net/netfilter/x_tables.c @@ -845,8 +845,9 @@ xt_replace_table(struct xt_table *table, return NULL; } - table-private = newinfo; newinfo-initial_entries
Re: BUG report about ipt_do_table( )
On Thu, Oct 10, 2013 at 12:26:38PM +0100, Wang, Yalin wrote: Hi Will , Hello, Seems your patch is better than mine, It make sure newinfo-initial_entries update is Seen by others . I will test by your patches , and update to you ASAP . Thanks. I have another questions about this BUG, Since all shared data will have this problem especially Shared between different CPUs, This means all shared data need a smp_wmb() , after One CPU update it ? Only if the ordering of observability matters and isn't enforced by other mechanisms (e.g. locks). But I see not all shared data are updated by this way, And kernel works well . Why ?? a little curious . Again, the barriers tend to be hidden inside things like locking primitives, atomics, I/O accessors etc. Another is that , I read cortex-a15 whitepaper , It says : Load/Store cluster All Load/Store, data transfers and cache maintenance operations Partially out-of-order, 1 Load and 1 Store executed per cycle Load cannot bypass a Store, Store cannot bypass a Store It's hard to interpret this out of context. Please can you point me at the whitepaper? Also, I was talking from the perspective of the architecture and you're not using an A15 afaik. !! Store can't bypass store !! , but in this BUG , it seems later store can Be wrote into cache even before the earlier store completed ! Am I right ? The architecture requires stores to be observed in program order from the perspective of the CPU issuing them, but there aren't ordering guarantees for stores to different locations when observed by a different CPU. Will -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: BUG report about ipt_do_table( )
Hi Will, Maybe I know your meaning , If it use spinlock to protected the shared data, The bug will not happen, because spinlock will Use DSB( ) to sync . Unluckily, here, it use a special seqcount_t( ) (see get_counters( ) function) To make sure there is no others using the old data, Before release the old data, this is much like RCU Work, but RCU use rcu_assign_pointer( ) -- Which use smp_wmb( ) , so it's safe, am I right ? In my patch, I use mb( ), because this macro Is DSB( ) , while smp_wmb( ) is DMS( ), I just think DSB is much strict than DMS, mmm.. so , DSM( ) or DMS ( ) are both ok ? The whitepaper I use is here: https://www.google.com/#q=cortex+a15+microarchitecture the first: [PDF] Exploring the Design of the Cortex-A15 Processor - ARM I just search in Google, and you know that qcom don't release Much document about its krait cpu's micro architecture details, I just use cortex-a15 for a reference, I am not sure if their pipeline ( load/store unit) are the same, if you have cortex-a15 micro architecture details document, I am very appreciated that you can share to me . :) Thanks -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Thursday, October 10, 2013 10:19 PM To: Wang, Yalin Cc: 'linux-arm-msm-ow...@vger.kernel.org'; linux-kernel@vger.kernel.org; Peng, Arthur; Zhang, Bojie; Gu, Youcai 1 (EXT); Alevoor, Raghavendra 2 Subject: Re: BUG report about ipt_do_table( ) On Thu, Oct 10, 2013 at 12:26:38PM +0100, Wang, Yalin wrote: Hi Will , Hello, Seems your patch is better than mine, It make sure newinfo-initial_entries update is Seen by others . I will test by your patches , and update to you ASAP . Thanks. I have another questions about this BUG, Since all shared data will have this problem especially Shared between different CPUs, This means all shared data need a smp_wmb() , after One CPU update it ? Only if the ordering of observability matters and isn't enforced by other mechanisms (e.g. locks). But I see not all shared data are updated by this way, And kernel works well . Why ?? a little curious . Again, the barriers tend to be hidden inside things like locking primitives, atomics, I/O accessors etc. Another is that , I read cortex-a15 whitepaper , It says : Load/Store cluster § All Load/Store, data transfers and cache maintenance operations § Partially out-of-order, 1 Load and 1 Store executed per cycle § Load cannot bypass a Store, Store cannot bypass a Store It's hard to interpret this out of context. Please can you point me at the whitepaper? Also, I was talking from the perspective of the architecture and you're not using an A15 afaik. !! Store can't bypass store !! , but in this BUG , it seems later store can Be wrote into cache even before the earlier store completed ! Am I right ? The architecture requires stores to be observed in program order from the perspective of the CPU issuing them, but there aren't ordering guarantees for stores to different locations when observed by a different CPU. Will
