Skip to content

Commit

Permalink
Revert "Wrap smp_processor_id in kpreempt_[dis|en]able" and "Fix CPU_…
Browse files Browse the repository at this point in the history
…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 <ryao@gentoo.org>
  • Loading branch information
ryao committed Feb 16, 2016
1 parent 5b2ddbd commit 7c52fff
Show file tree
Hide file tree
Showing 4 changed files with 3 additions and 28 deletions.
4 changes: 0 additions & 4 deletions include/sys/zfs_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 1 addition & 7 deletions module/zfs/fm.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 1 addition & 11 deletions module/zfs/txg.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
7 changes: 1 addition & 6 deletions module/zfs/zio.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 7c52fff

Please sign in to comment.