netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steffen Klassert <steffen.klassert@secunet.com>
To: Shan Wei <shanwei88@gmail.com>
Cc: Christoph Lameter <cl@linux.com>,
	David Miller <davem@davemloft.net>,
	NetDev <netdev@vger.kernel.org>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	Kernel-Maillist <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/9] net: xfrm: use this_cpu_ptr per-cpu helper
Date: Thu, 1 Nov 2012 10:18:42 +0100	[thread overview]
Message-ID: <20121101091842.GA2719@secunet.com> (raw)
In-Reply-To: <50923956.5090206@gmail.com>

On Thu, Nov 01, 2012 at 04:56:54PM +0800, Shan Wei wrote:
> Christoph Lameter said, at 2012/11/1 1:35:
> > It would be better to use
> > 
> > 	this_cpu_read(tfms)
> > 
> > since that would also make it atomic vs interrupts. The above code (both
> > original and modified) could determine a pointer to a per cpu structure
> > and then take an interrupt which would move the task. On return we would
> > be accessing the per cpu variable of another processor.
> 
> this_cpu_read
> |-----_this_cpu_generic_read
> 
> #define _this_cpu_generic_read(pcp)                                     \
> ({      typeof(pcp) ret__;                                              \
>         preempt_disable();                                              \
>         ret__ = *this_cpu_ptr(&(pcp));                                  \
>         preempt_enable();                                               \
>         ret__;                                                          \
> })
> 
> 
> this_cpu_read operations locate per-cpu variable with preemption safe, not
> disable interrupts. why is it atomic vs interrupts?
> 
> I have no idea whether we need to disable preemption for this code?
> At least, xfrm code run well with per_cpu_ptr which don't disable preemption.

We compare the name of the newly allocated crypto transform against the
existing ones. The name is the same on every percpu transform and it
does not change after the transform is allocated. So we don't care
if we get migrated or not, we can just take the transform from an
arbitrary cpu to read the name from it.

  reply	other threads:[~2012-11-01  9:18 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-31 11:22 [PATCH 3/9] net: xfrm: use this_cpu_ptr per-cpu helper Shan Wei
2012-10-31 17:35 ` Christoph Lameter
2012-11-01  3:41   ` Herbert Xu
2012-11-01  9:00     ` Shan Wei
2012-11-01  8:56   ` Shan Wei
2012-11-01  9:18     ` Steffen Klassert [this message]
2012-11-01 12:15     ` David Laight

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121101091842.GA2719@secunet.com \
    --to=steffen.klassert@secunet.com \
    --cc=cl@linux.com \
    --cc=davem@davemloft.net \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=shanwei88@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).