Re: [PATCH] [SCSI] mpt2sas : Fix unsafe using smp_processor_id() in preemptible

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Tue, 2012-04-17 at 22:13 -0600, Matthew Wilcox wrote:
> On Tue, Apr 17, 2012 at 09:35:49PM -0600, Moore, Eric wrote:
> >  Jan Schmidt suggested using raw_smp_processor_id()  back in February, see this: http://marc.info/?t=132974687100003&r=1&w=2
> > 
> > Alex Shi recently suggested using preempt_disable() and preempt_enable() to solve the same issue, see this: http://marc.info/?t=133274303900003&r=1&w=2
> > 
> > I believe the stack traces are there in both email discussion, they occur when CONFIG_DEBUG_PREEMPT is enabled.
> > 
> > I would rather go with the solution giving the best performance.   James Bottomley is there on the discussion with  Jan Schmidt, he suggested using get_cpu() and put_cpu().
> 
> I use get_cpu() / put_cpu() in the NVMe driver in similar circumstances.
> It's not noticable in the profiles :-)
> 
> Where my usage differs from the patch for mpt2sas is that I hold a
> reference to the CPU over the submission.  This works out well for
> me because I have per-CPU state.  Might be worth considering for your
> driver ...

Right, so in this case mpt2sas doesn't care.  It literally only needs a
notion of the current CPU for cache hotness of MSI queue return.  It
doesn't need pinning or anything else.  I think the use of
raw_smp_processor_id() in this instance is ideal, because it actually is
a "need to think about how to do this better" flag, which may mean
actually doing it in a more per-CPU state type way.

Mind you, something like

cpu = get_cpu(); put_cpu();

Is also an instant smack in the eyeballs too because it looks so daft,
so I'm not incredibly bothered.  I lean towards raw_smp_processor_id()
because it's what's actually wanted, but not strongly.

James