Re: mpt2sas: BUG: using smp_processor_id() in preemptible

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2012-02-21 at 10:01 +0100, Jan Schmidt wrote:
> Hi Eric,
> 
> On 21.02.2012 00:06, Moore, Eric wrote:
> > Jan - the smp_processor_id() is part of NUMA support which I added over a year ago. I don't see any problem with the driver running on sles11sp2 which has btrfs support ( I'm not sure  if CONFIG_DEBUG_PREEMPT is turned on as I have not checked).   Any theory why its oopsing in smp_processor_id() ?
> 
> -- excerpt from include/linux/smp.h --
> /*
>  * smp_processor_id(): get the current CPU ID.
>  *
>  * if DEBUG_PREEMPT is enabled then we check whether it is
>  * used in a preemption-safe way. (smp_processor_id() is safe
>  * if it's used in a preemption-off critical section, or in
>  * a thread that is bound to the current CPU.)
> [...]
>  */
> #ifdef CONFIG_DEBUG_PREEMPT
>   extern unsigned int debug_smp_processor_id(void);
> # define smp_processor_id() debug_smp_processor_id()
> #else
> # define smp_processor_id() raw_smp_processor_id()
> #endif
>  
> #define get_cpu()               ({ preempt_disable(); smp_processor_id(); })
> #define put_cpu()               preempt_enable()
> -- end --
> 
> ... and ...
> 
> -- excerpt from drivers/scsi/mpt2sas/mpt2sas_base.c --
> static inline u8 
> _base_get_msix_index(struct MPT2SAS_ADAPTER *ioc)
> {
>         return ioc->cpu_msix_table[smp_processor_id()]; 
> }
> -- end --
> 
> Suggestion: Use get_cpu() and put_cpu().

you mean

diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c b/drivers/scsi/mpt2sas/mpt2sas_base.c
index 0b2c955..5235068 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -1785,7 +1785,9 @@ static inline void _base_writeq(__u64 b, volatile void __iomem *addr,
 static inline u8
 _base_get_msix_index(struct MPT2SAS_ADAPTER *ioc)
 {
-	return ioc->cpu_msix_table[smp_processor_id()];
+	int cpu = get_cpu();
+	put_cpu();
+	return ioc->cpu_msix_table[cpu];
 }
 
 /**


This is the I/O hot path we're talking about.  The point about this use
of smp_processor_id() is that if the block layer doesn't specify a CPU
affinity for the request and we're using MSI interrupt steering, we have
to (because the MSIs steer to different CPUs), so we just fill in the
approximation of the current CPU ... we don't care that it be exactly
accurate at the time.  We need the lightest weight way of getting
current CPU (and we don't care if it changes because of pre-empt at the
next instruction).

I suppose doing the above is only a couple of extra instruction cycles,
but it is pretty pointless and all to silence a spurious warning.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux