Revert "Wrap smp_processor_id in kpreempt_[dis|en]able" and "Fix CPU_…

…SEQID use in preemptible context" This reverts commit 15a9e03 and commit 8878261. Wrapping CPU_SEQID with kpreempt_disable() and kpreempt_enable() causes us to call preempt_schedule(). This allows the thread to be rescheduled on another CPU, allowing the vacated CPU to begin using the per-CPU structure that we still reference on the CPU we just vacated. This is an unnecessary source of contention that is a regression from the illumos behavior. We can turn off the Linux warnings by calling `raw_smp_processor_id()` instead of wrapping things with kpreempt_disable() and kpreempt_enable(), so lets do that. Requires-spl: refs/pull/532/head Signed-off-by: Richard Yao <[email protected]>
openzfs · Feb 18, 2016 · 4159569 · 4159569
1 parent 3ac9d91
commit 4159569
Show file tree

Hide file tree

Showing 4 changed files with 3 additions and 28 deletions.
diff --git a/include/sys/zfs_context.h b/include/sys/zfs_context.h
@@ -67,7 +67,6 @@
 #include <sys/fm/fs/zfs.h>
 #include <sys/sunddi.h>
 #include <sys/ctype.h>
-#include <sys/disp.h>
 #include <sys/trace.h>
 #include <linux/dcache_compat.h>
 #include <linux/utsname_compat.h>
@@ -262,9 +261,6 @@ extern kthread_t *zk_thread_create(caddr_t stk, size_t  stksize,
 	proc_t *pp, int state, pri_t pri, int detachstate);
 extern void zk_thread_join(kt_did_t tid);
 
-#define	kpreempt_disable()	((void)0)
-#define	kpreempt_enable()	((void)0)
-
 #define	PS_NONE		-1
 
 #define	issig(why)	(FALSE)

diff --git a/module/zfs/fm.c b/module/zfs/fm.c
@@ -1515,13 +1515,7 @@ fm_ena_generate_cpu(uint64_t timestamp, processorid_t cpuid, uchar_t format)
 uint64_t
 fm_ena_generate(uint64_t timestamp, uchar_t format)
 {
-	uint64_t ena;
-
-	kpreempt_disable();
-	ena = fm_ena_generate_cpu(timestamp, getcpuid(), format);
-	kpreempt_enable();
-
-	return (ena);
+	return (fm_ena_generate_cpu(timestamp, getcpuid(), format));
 }
 
 uint64_t

diff --git a/module/zfs/txg.c b/module/zfs/txg.c
@@ -294,19 +294,9 @@ uint64_t
 txg_hold_open(dsl_pool_t *dp, txg_handle_t *th)
 {
 	tx_state_t *tx = &dp->dp_tx;
-	tx_cpu_t *tc;
+	tx_cpu_t *tc = &tx->tx_cpu[CPU_SEQID];
 	uint64_t txg;
 
-	/*
-	 * It appears the processor id is simply used as a "random"
-	 * number to index into the array, and there isn't any other
-	 * significance to the chosen tx_cpu. Because.. Why not use
-	 * the current cpu to index into the array?
-	 */
-	kpreempt_disable();
-	tc = &tx->tx_cpu[CPU_SEQID];
-	kpreempt_enable();
-
 	mutex_enter(&tc->tc_open_lock);
 	txg = tx->tx_open_txg;
 

diff --git a/module/zfs/zio.c b/module/zfs/zio.c
@@ -1542,19 +1542,14 @@ zio_nowait(zio_t *zio)
 
 	if (zio->io_child_type == ZIO_CHILD_LOGICAL &&
 	    zio_unique_parent(zio) == NULL) {
-		zio_t *pio;
-
 		/*
 		 * This is a logical async I/O with no parent to wait for it.
 		 * We add it to the spa_async_root_zio "Godfather" I/O which
 		 * will ensure they complete prior to unloading the pool.
 		 */
 		spa_t *spa = zio->io_spa;
-		kpreempt_disable();
-		pio = spa->spa_async_zio_root[CPU_SEQID];
-		kpreempt_enable();
 
-		zio_add_child(pio, zio);
+		zio_add_child(spa->spa_async_zio_root[CPU_SEQID], zio);
 	}
 
 	__zio_execute(zio);