g_eli: disambiguate CPU-bound worker creation

[mhorne]

Summary

• split worker creation logic into a helper function
• keep the loops separate
• debug message for absent CPUs is dropped
• add a short explanatory comment
• rename local var to 'nthreads'

No functional change intended.

[kib]

sys/geom/eli/g_eli.c

1033

Can geli be loaded at runtime? What protects sc_workers against races?

1142

Extra ()

1144–1148

hm, in which sense the workers are bound?
Why it matters which CPUs are absent?

[mhorne]

sys/geom/eli/g_eli.c

1033

We are at the beginning of the sc lifecycle, allocated in g_eli_create(). This is per-provider.

Yes, during load/creation at runtime the worker threads could race against this insertion. In practice, their single access to the list is protected by the G_ELI_FLAG_DESTROY flag, which cannot be issued at this point.

But I think it would be wise to surround it with acquisition of sc->sc_queue_mtx. Thanks for asking.

1144–1148

Early in g_eli_worker() we have:

	thread_lock(curthread);
	sched_prio(curthread, PUSER);
	if (sc->sc_cpubind)
		sched_bind(curthread, wr->w_number % mp_ncpus);
	thread_unlock(curthread);

I am not sure how sched_bind() will behave when the requested CPU is absent. Do you know?

At best, it will fail gracefully, and we will have one extra "floating" worker thread for each absent CPU. An extra thread is not harmful, but is clearly not the intent of the existing g_eli_cpu_is_disable(i) check, the starting point of these changes.

[kib]

sys/geom/eli/g_eli.c

1144–1148

On the first look, sched_ule_bind() would work in the sense that it should not immediately panic or follow dandling pointer. But I think the consequence would be that the thread lock is forever in the blocked state, and any attempt to touch that lock would result either in the system lock-up or panic due to spinlock timeout.

[mhorne]

Obtain mutex around sc_workers list.

Remove unnecessary braces.