Message ID | 50A1A7C9.3060703@gmail.com |
---|---|
State | Not Applicable, archived |
Delegated to: | David Miller |
Headers | show |
On Tue, Nov 13, 2012 at 09:52:09AM +0800, Shan Wei wrote: > From: Shan Wei <davidshan@tencent.com> > Please add a proper commit message, explaining why you do this change. > > Signed-off-by: Shan Wei <davidshan@tencent.com> > --- > v4: > derefrence pointer before reading to avoid compile warning. > --- > net/xfrm/xfrm_ipcomp.c | 8 +++----- > 1 files changed, 3 insertions(+), 5 deletions(-) > > diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c > index e5246fb..2906d52 100644 > --- a/net/xfrm/xfrm_ipcomp.c > +++ b/net/xfrm/xfrm_ipcomp.c > @@ -276,18 +276,16 @@ static struct crypto_comp * __percpu *ipcomp_alloc_tfms(const char *alg_name) > struct crypto_comp * __percpu *tfms; > int cpu; > > - /* This can be any valid CPU ID so we don't need locking. */ > - cpu = raw_smp_processor_id(); > > list_for_each_entry(pos, &ipcomp_tfms_list, list) { > struct crypto_comp *tfm; > > - tfms = pos->tfms; > - tfm = *per_cpu_ptr(tfms, cpu); > + /* This can be any valid CPU ID so we don't need locking. */ > + tfm = __this_cpu_read(*pos->tfms); This should just fetch the tfm pointer, so why exactly __this_cpu_read is better than __this_cpu_ptr? Please keep in mind that performance is not the most important thing here. It's much more important that it works in any case. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Steffen Klassert said, at 2012/11/13 15:21: > > This should just fetch the tfm pointer, so why exactly __this_cpu_read > is better than __this_cpu_ptr? Please keep in mind that performance is > not the most important thing here. It's much more important that it > works in any case. [0/9] describes why we should submit this series patches. you are not included in original mail. this_cpu_ptr relocates and address. this_cpu_read() relocates the address and performs the fetch. If you want to operate on rda(defined as per_cpu) then you can only use this_cpu_ptr. this_cpu_read() saves you more instructions since it can do the relocation and the fetch in one instruction. More info, please refer to http://www.spinics.net/lists/kernel/msg1435153.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 13, 2012 at 05:33:19PM +0800, Shan Wei wrote: > Steffen Klassert said, at 2012/11/13 15:21: > > > > This should just fetch the tfm pointer, so why exactly __this_cpu_read > > is better than __this_cpu_ptr? Please keep in mind that performance is > > not the most important thing here. It's much more important that it > > works in any case. > > [0/9] describes why we should submit this series patches. > you are not included in original mail. > > this_cpu_ptr relocates and address. this_cpu_read() relocates the address > and performs the fetch. If you want to operate on rda(defined as per_cpu) > then you can only use this_cpu_ptr. this_cpu_read() saves you more instructions > since it can do the relocation and the fetch in one instruction. > Ok, so please add a commit message to describe your changes. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c index e5246fb..2906d52 100644 --- a/net/xfrm/xfrm_ipcomp.c +++ b/net/xfrm/xfrm_ipcomp.c @@ -276,18 +276,16 @@ static struct crypto_comp * __percpu *ipcomp_alloc_tfms(const char *alg_name) struct crypto_comp * __percpu *tfms; int cpu; - /* This can be any valid CPU ID so we don't need locking. */ - cpu = raw_smp_processor_id(); list_for_each_entry(pos, &ipcomp_tfms_list, list) { struct crypto_comp *tfm; - tfms = pos->tfms; - tfm = *per_cpu_ptr(tfms, cpu); + /* This can be any valid CPU ID so we don't need locking. */ + tfm = __this_cpu_read(*pos->tfms); if (!strcmp(crypto_comp_name(tfm), alg_name)) { pos->users++; - return tfms; + return pos->tfms; } }