| Message ID | 20211117025956.79616-2-dave@stgolabs.net (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | scsi/fcoe: Play nicer with PREEMPT_RT | expand |
On 2021-11-16 18:59:54 [-0800], Davidlohr Bueso wrote: > The get_cpu() in fc_exch_em_alloc() was introduced in: > > f018b73af6db ([SCSI] libfc, libfcoe, fcoe: use smp_processor_id() only when preempt disabled) > > for no other reason than to simply use smp_processor_id() > without getting a warning, because everything is done with > the pool->lock held anyway. However, get_cpu(), by disabling > preemption, does not play well with PREEMPT_RT, particularly > when acquiring a regular (and thus sleepable) spinlock. > > Therefore remove the get_cpu() and just use the unstable value > as we will have CPU locality guarantees next by taking the lock. > The window of migration, as noted by Sebastian, is small and > even if it happens the result is correct. > > Signed-off-by: Davidlohr Bueso <dbueso@suse.de> Acked-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c > index 841000445b9a..be37bfb2814d 100644 > --- a/drivers/scsi/libfc/fc_exch.c > +++ b/drivers/scsi/libfc/fc_exch.c > @@ -825,10 +825,9 @@ static struct fc_exch *fc_exch_em_alloc(struct fc_lport *lport, > } > memset(ep, 0, sizeof(*ep)); > > - cpu = get_cpu(); > + cpu = raw_smp_processor_id(); > pool = per_cpu_ptr(mp->pool, cpu); > spin_lock_bh(&pool->lock); > - put_cpu(); The `cpu' variable is later ORed into ep->oxid. I haven't figured out why this is important/ required. The ep variable/ `fc_exch' is allocated from a per-CPU memory pool and is later released to the same pool but the pool's CPU number is not obtained from fc_exch::oxid but there is a pointer to its pool stored in fc_exch::em. So it remains a mystery to why the CPU number is stored here. > /* peek cache of free slot */ > if (pool->left != FC_XID_UNKNOWN) { Sebastian
diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c index 841000445b9a..be37bfb2814d 100644 --- a/drivers/scsi/libfc/fc_exch.c +++ b/drivers/scsi/libfc/fc_exch.c @@ -825,10 +825,9 @@ static struct fc_exch *fc_exch_em_alloc(struct fc_lport *lport, } memset(ep, 0, sizeof(*ep)); - cpu = get_cpu(); + cpu = raw_smp_processor_id(); pool = per_cpu_ptr(mp->pool, cpu); spin_lock_bh(&pool->lock); - put_cpu(); /* peek cache of free slot */ if (pool->left != FC_XID_UNKNOWN) {
The get_cpu() in fc_exch_em_alloc() was introduced in: f018b73af6db ([SCSI] libfc, libfcoe, fcoe: use smp_processor_id() only when preempt disabled) for no other reason than to simply use smp_processor_id() without getting a warning, because everything is done with the pool->lock held anyway. However, get_cpu(), by disabling preemption, does not play well with PREEMPT_RT, particularly when acquiring a regular (and thus sleepable) spinlock. Therefore remove the get_cpu() and just use the unstable value as we will have CPU locality guarantees next by taking the lock. The window of migration, as noted by Sebastian, is small and even if it happens the result is correct. Signed-off-by: Davidlohr Bueso <dbueso@suse.de> --- drivers/scsi/libfc/fc_exch.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)