Diff 27519

usr.bin/grep/grep.c

Show First 20 Lines • Show All 52 Lines • ▼ Show 20 Lines
#endif		#endif
#include <vm/vm.h>		#include <vm/vm.h>
#include <vm/vm_extern.h>		#include <vm/vm_extern.h>
#include <vm/vm_kern.h>		#include <vm/vm_kern.h>
#include <vm/uma.h>		#include <vm/uma.h>

#include <ck_epoch.h>		#include <ck_epoch.h>

static MALLOC_DEFINE(M_EPOCH, "epoch", "epoch based reclamation");

#ifdef __amd64__		#ifdef __amd64__
#define EPOCH_ALIGN CACHE_LINE_SIZE*2		#define EPOCH_ALIGN CACHE_LINE_SIZE*2
#else		#else
#define EPOCH_ALIGN CACHE_LINE_SIZE		#define EPOCH_ALIGN CACHE_LINE_SIZE
#endif		#endif

TAILQ_HEAD (epoch_tdlist, epoch_tracker);		TAILQ_HEAD (epoch_tdlist, epoch_tracker);
typedef struct epoch_record {		typedef struct epoch_record {
ck_epoch_record_t er_record;		ck_epoch_record_t er_record;
struct epoch_context er_drain_ctx;		struct epoch_context er_drain_ctx;
struct epoch *er_parent;		struct epoch *er_parent;
volatile struct epoch_tdlist er_tdlist;		volatile struct epoch_tdlist er_tdlist;
volatile uint32_t er_gen;		volatile uint32_t er_gen;
uint32_t er_cpuid;		uint32_t er_cpuid;
} __aligned(EPOCH_ALIGN) *epoch_record_t;		} __aligned(EPOCH_ALIGN) *epoch_record_t;

struct epoch {		struct epoch {
struct ck_epoch e_epoch __aligned(EPOCH_ALIGN);		struct ck_epoch e_epoch __aligned(EPOCH_ALIGN);
epoch_record_t e_pcpu_record;		epoch_record_t e_pcpu_record;
int e_idx;
int e_flags;		int e_flags;
struct sx e_drain_sx;		struct sx e_drain_sx;
struct mtx e_drain_mtx;		struct mtx e_drain_mtx;
volatile int e_drain_count;		volatile int e_drain_count;
const char *e_name;		const char *e_name;
};		};

		markjUnsubmitted Done Inline Actions Why is it defined like this? I can't see the purpose of `val`. markj: Why is it defined like this? I can't see the purpose of `val`.
		hselaskyAuthorUnsubmitted Done Inline Actions The atomic functions don't accept "const char ptr", instead of casting it in the code, I make a cast in the union ... hselasky:* The atomic functions don't accept "const char *ptr", instead of casting it in the code, I make…
		markjUnsubmitted Done Inline Actions I don't object but personally I'd prefer the ugliness of the casts to the union. atomic_(load\|store)_ptr were changed to not require casts recently, it's only the MD atomic_(load\|store)_(acq\|rel)_* macros that still need it for now. Please add a comment explaining why it's a union. markj: I don't object but personally I'd prefer the ugliness of the casts to the union. atomic_…
/* arbitrary --- needs benchmarking */		/* arbitrary --- needs benchmarking */
#define MAX_ADAPTIVE_SPIN 100		#define MAX_ADAPTIVE_SPIN 100
#define MAX_EPOCHS 64		#define MAX_EPOCHS 64

CTASSERT(sizeof(ck_epoch_entry_t) == sizeof(struct epoch_context));		CTASSERT(sizeof(ck_epoch_entry_t) == sizeof(struct epoch_context));
SYSCTL_NODE(_kern, OID_AUTO, epoch, CTLFLAG_RW \| CTLFLAG_MPSAFE, 0,		SYSCTL_NODE(_kern, OID_AUTO, epoch, CTLFLAG_RW \| CTLFLAG_MPSAFE, 0,
"epoch information");		"epoch information");
SYSCTL_NODE(_kern_epoch, OID_AUTO, stats, CTLFLAG_RW \| CTLFLAG_MPSAFE, 0,		SYSCTL_NODE(_kern_epoch, OID_AUTO, stats, CTLFLAG_RW \| CTLFLAG_MPSAFE, 0,
Show All 25 Lines
SYSCTL_COUNTER_U64(_kern_epoch_stats, OID_AUTO, epoch_call_tasks, CTLFLAG_RW,		SYSCTL_COUNTER_U64(_kern_epoch_stats, OID_AUTO, epoch_call_tasks, CTLFLAG_RW,
&epoch_call_task_count, "# of times a callback task was run");		&epoch_call_task_count, "# of times a callback task was run");

TAILQ_HEAD (threadlist, thread);		TAILQ_HEAD (threadlist, thread);

CK_STACK_CONTAINER(struct ck_epoch_entry, stack_entry,		CK_STACK_CONTAINER(struct ck_epoch_entry, stack_entry,
ck_epoch_entry_container)		ck_epoch_entry_container)

epoch_t allepochs[MAX_EPOCHS];		static struct epoch epoch_array[MAX_EPOCHS];

DPCPU_DEFINE(struct grouptask, epoch_cb_task);		DPCPU_DEFINE(struct grouptask, epoch_cb_task);
DPCPU_DEFINE(int, epoch_cb_count);		DPCPU_DEFINE(int, epoch_cb_count);

static __read_mostly int inited;		static __read_mostly int inited;
static __read_mostly int epoch_count;		static __read_mostly int epoch_count;
__read_mostly epoch_t global_epoch;		__read_mostly epoch_t global_epoch;
__read_mostly epoch_t global_epoch_preempt;		__read_mostly epoch_t global_epoch_preempt;

static void epoch_call_task(void *context __unused);		static void epoch_call_task(void *context __unused);
static uma_zone_t pcpu_zone_record;		static uma_zone_t pcpu_zone_record;

		static struct sx epoch_sx;

		#define EPOCH_LOCK() sx_xlock(&epoch_sx)
		#define EPOCH_UNLOCK() sx_xunlock(&epoch_sx)

#ifdef EPOCH_TRACE		#ifdef EPOCH_TRACE
struct stackentry {		struct stackentry {
RB_ENTRY(stackentry) se_node;		RB_ENTRY(stackentry) se_node;
struct stack se_stack;		struct stack se_stack;
};		};

static int		static int
stackentry_compare(struct stackentry a, struct stackentry b)		stackentry_compare(struct stackentry a, struct stackentry b)
▲ Show 20 Lines • Show All 124 Lines • ▼ Show 20 Lines	GROUPTASK_INIT(DPCPU_ID_PTR(cpu, epoch_cb_task), 0,
epoch_call_task, NULL);		epoch_call_task, NULL);
taskqgroup_attach_cpu(qgroup_softirq,		taskqgroup_attach_cpu(qgroup_softirq,
DPCPU_ID_PTR(cpu, epoch_cb_task), NULL, cpu, NULL, NULL,		DPCPU_ID_PTR(cpu, epoch_cb_task), NULL, cpu, NULL, NULL,
"epoch call task");		"epoch call task");
}		}
#ifdef EPOCH_TRACE		#ifdef EPOCH_TRACE
SLIST_INIT(&thread0.td_epochs);		SLIST_INIT(&thread0.td_epochs);
#endif		#endif
		sx_init(&epoch_sx, "epoch-sx");
inited = 1;		inited = 1;
global_epoch = epoch_alloc("Global", 0);		global_epoch = epoch_alloc("Global", 0);
global_epoch_preempt = epoch_alloc("Global preemptible", EPOCH_PREEMPT);		global_epoch_preempt = epoch_alloc("Global preemptible", EPOCH_PREEMPT);
}		}
SYSINIT(epoch, SI_SUB_EPOCH, SI_ORDER_FIRST, epoch_init, NULL);		SYSINIT(epoch, SI_SUB_EPOCH, SI_ORDER_FIRST, epoch_init, NULL);

#if !defined(EARLY_AP_STARTUP)		#if !defined(EARLY_AP_STARTUP)
static void		static void
Show All 28 Lines	epoch_adjust_prio(struct thread *td, u_char prio)
thread_lock(td);		thread_lock(td);
sched_prio(td, prio);		sched_prio(td, prio);
thread_unlock(td);		thread_unlock(td);
}		}

epoch_t		epoch_t
epoch_alloc(const char *name, int flags)		epoch_alloc(const char *name, int flags)
{		{
epoch_t epoch;		epoch_t epoch;
		markjUnsubmitted Done Inline Actions Can't we just use a global `sx` to synchronize epoch allocation and free? markj: Can't we just use a global `sx` to synchronize epoch allocation and free?
		int i;

if (__predict_false(!inited))		if (__predict_false(!inited))
panic("%s called too early in boot", __func__);		panic("%s called too early in boot", __func__);
epoch = malloc(sizeof(struct epoch), M_EPOCH, M_ZERO \| M_WAITOK);
		MPASS(name != NULL);

		EPOCH_LOCK();

		/* look for free epoch entry */
		kibUnsubmitted Done Inline Actions Comments should be a proper sentences. kib: Comments should be a proper sentences.
		for (i = 0; i != epoch_count; i++) {
		kibUnsubmitted Done Inline Actions I do not quite understand how this loop works. You only loop up to the top allocated index, not to MAX_EPOCH, so if there is no holes, you cannot allocate anything. kib: I do not quite understand how this loop works. You only loop up to the top allocated index…
		hselaskyAuthorUnsubmitted Done Inline Actions See the epoch_count++ below. hselasky: See the epoch_count++ below.
		if (epoch_array[i].e_name == NULL)
		break;
		}

		/* check if too many epochs are allocated */
		if (i >= MAX_EPOCHS) {
		epoch = NULL;
		goto done;
		}

		epoch = epoch_array + i;
		kibUnsubmitted Done Inline Actions This blank line is not needed. kib: This blank line is not needed.
ck_epoch_init(&epoch->e_epoch);		ck_epoch_init(&epoch->e_epoch);
epoch_ctor(epoch);		epoch_ctor(epoch);
MPASS(epoch_count < MAX_EPOCHS - 2);
epoch->e_flags = flags;		epoch->e_flags = flags;
epoch->e_idx = epoch_count;
epoch->e_name = name;		epoch->e_name = name;
sx_init(&epoch->e_drain_sx, "epoch-drain-sx");		sx_init(&epoch->e_drain_sx, "epoch-drain-sx");
mtx_init(&epoch->e_drain_mtx, "epoch-drain-mtx", NULL, MTX_DEF);		mtx_init(&epoch->e_drain_mtx, "epoch-drain-mtx", NULL, MTX_DEF);
allepochs[epoch_count++] = epoch;
		/* track upper epoch index */
		if (i > epoch_count)
		epoch_count = i;
		kibUnsubmitted Done Inline Actions I suggest renaming this variable into something like epoch_array_max_slot. It is not epoch_count. kib: I suggest renaming this variable into something like epoch_array_max_slot. It is not…
		done:
		EPOCH_UNLOCK();
return (epoch);		return (epoch);
		kibUnsubmitted Done Inline Actions May be we should bite the bullet and return NULL if there is no space in the array ? kib: May be we should bite the bullet and return NULL if there is no space in the array ?
		markjUnsubmitted Not Done Inline Actions Please use atomic(9) instead of `volatile`. markj: Please use atomic(9) instead of `volatile`.
}		}

void		void
epoch_free(epoch_t epoch)		epoch_free(epoch_t epoch)
{		{

		EPOCH_LOCK();

		MPASS(epoch->e_name != NULL);

epoch_drain_callbacks(epoch);		epoch_drain_callbacks(epoch);
allepochs[epoch->e_idx] = NULL;
epoch_wait(global_epoch);		epoch_wait(global_epoch);
uma_zfree_pcpu(pcpu_zone_record, epoch->e_pcpu_record);		uma_zfree_pcpu(pcpu_zone_record, epoch->e_pcpu_record);
mtx_destroy(&epoch->e_drain_mtx);		mtx_destroy(&epoch->e_drain_mtx);
sx_destroy(&epoch->e_drain_sx);		sx_destroy(&epoch->e_drain_sx);
free(epoch, M_EPOCH);		memset(epoch, 0, sizeof(*epoch));

		EPOCH_UNLOCK();
}		}

static epoch_record_t		static epoch_record_t
epoch_currecord(epoch_t epoch)		epoch_currecord(epoch_t epoch)
{		{

return (zpcpu_get(epoch->e_pcpu_record));		return (zpcpu_get(epoch->e_pcpu_record));
}		}
▲ Show 20 Lines • Show All 337 Lines • ▼ Show 20 Lines	epoch_call_task(void *arg __unused)
epoch_t epoch;		epoch_t epoch;
ck_stack_t cb_stack;		ck_stack_t cb_stack;
int i, npending, total;		int i, npending, total;

ck_stack_init(&cb_stack);		ck_stack_init(&cb_stack);
critical_enter();		critical_enter();
epoch_enter(global_epoch);		epoch_enter(global_epoch);
for (total = i = 0; i < epoch_count; i++) {		for (total = i = 0; i < epoch_count; i++) {
if (__predict_false((epoch = allepochs[i]) == NULL))		epoch = epoch_array + i;
		if (__predict_false(epoch->e_name == NULL))
		kibUnsubmitted Done Inline Actions Is it safe to iterate to epoch_array_max_count instead of MAX_EPOCHS ? Could we miss an update to max_count there and then skip some epoch ? kib: Is it safe to iterate to epoch_array_max_count instead of MAX_EPOCHS ? Could we miss an update…
		hselaskyAuthorUnsubmitted Done Inline Actions e_name is protected by global_epoch. hselasky: e_name is protected by global_epoch.
		kibUnsubmitted Done Inline Actions From what I see, global epoch ensures that e_name is not cleared until epoch_call_task() ends it global section. I asked about different situation, where the update from epoch_alloc (most likely, to epoch_array_max_count) is missed by epoch_call_task and then corresponding epoch is not drained by the task. But I see that you changed the loop limit to MAX_EPOCHS already. kib: From what I see, global epoch ensures that e_name is not cleared until epoch_call_task() ends…
continue;		continue;
er = epoch_currecord(epoch);		er = epoch_currecord(epoch);
record = &er->er_record;		record = &er->er_record;
if ((npending = record->n_pending) == 0)		if ((npending = record->n_pending) == 0)
continue;		continue;
ck_epoch_poll_deferred(record, &cb_stack);		ck_epoch_poll_deferred(record, &cb_stack);
total += npending - record->n_pending;		total += npending - record->n_pending;
}		}
▲ Show 20 Lines • Show All 131 Lines • Show Last 20 Lines

bsdgrep trivial improvement: define REG_NOSPEC to be a no-op if not defined, use REG_NOSPEC instead of magic number
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 27519

usr.bin/grep/grep.c

bsdgrep trivial improvement: define REG_NOSPEC to be a no-op if not defined, use REG_NOSPEC instead of magic numberClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 27519

usr.bin/grep/grep.c

bsdgrep trivial improvement: define REG_NOSPEC to be a no-op if not defined, use REG_NOSPEC instead of magic number
ClosedPublic
Actions

Revision Contents
Changeset List