Page MenuHomeFreeBSD

kthread: Add a wrapper macro KPROC_START
Needs ReviewPublic

Authored by zlei on Fri, Oct 17, 10:38 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 6, 10:03 AM
Unknown Object (File)
Mon, Nov 3, 1:09 AM
Unknown Object (File)
Fri, Oct 31, 3:42 PM
Unknown Object (File)
Thu, Oct 30, 4:07 PM
Unknown Object (File)
Thu, Oct 30, 2:15 PM
Unknown Object (File)
Tue, Oct 28, 11:15 PM
Unknown Object (File)
Mon, Oct 27, 3:00 PM
Unknown Object (File)
Sun, Oct 26, 11:39 PM

Details

Reviewers
kib
jeff
olce
Summary
kthread: Add a wrapper macro KPROC_START

The function kproc_start(const void *) requires passing an argument
with type `struct kproc_desc *`. The compilers are not able to report
error when passing wrong typed arguments.

Add a wrapper macro KPROC_START so that we no longer worry about that
when using the macro instead of plain SYSINIT.

MFC after:      2 weeks


kthread: Migrate to use the macro KPROC_START to start kernel threads

No functional change intended.

MFC after:      2 weeks

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

zlei requested review of this revision.Fri, Oct 17, 10:38 AM
sys/kern/kern_alq.c
418

Hi @jeff, I'm not quite sure if the name matters, but I do not find "ALQ Daemon" used elsewhere except here.

Will the change of thread name from "ALQ Daemon" to "alq_daemon" breaks anything ?

olce added inline comments.
sys/kern/kern_alq.c
418

I don't think that name matters, and would prefer the names to have no spaces. Let's see if @jeff says something (based on the past few years, I'd say it's unlikely, but perhaps wait for a few days or a little more).

sys/sys/kthread.h
62

Perhaps expand the generated name to decrease the chances of collision with explicit variables?

67

If changed above, to be changed here.

This revision is now accepted and ready to land.Fri, Oct 17, 12:05 PM
sys/sys/kthread.h
61

The simple macro argument names have high chance of interfere with names locally visible.
C standard reserves the identifiers namespace '^_[A-Z]' (i.e. all identifiers starting with underscore, then upper-case letter) to file-local preprocessor tokens.

I suggest to use that convention for such global macro args, and also I suggest to use much more descriptive names for them. If I need to write another KPROC_START(), I would need to spend more time than I want figuring out what these n/s/o/f/gp mean.

This revision now requires review to proceed.Mon, Oct 20, 2:19 PM
sys/sys/kthread.h
61

What about these names, subsystem and order is ~stolen~ from the defination of SYSINIT. The kname kfunc and kproc should self explain what they are.

62

There're already some macros prefer the same suffix, well I do not have strong opinion here, either the prefix or suffix one should be able to avoid collision.

Well, the struct kproc_desc is used mainly combined with kproc_start(), there is less chance developers want to declare one struct kproc_desc alone. Even the name collision happens, it is still easy to resolve.

sys/sys/kthread.h
61–67

Re-reading this, I now think that the name KPROC_START() is misleading, as it may be understood as implying the macro can be used to launch a kernel proc at any time, whereas this is only at sysinit (kproc_start() is arguably a bad precedent). SYSINIT_KPROC() seems much clearer in this respect.

I don't find kname, kfunc and kproc self-explaining enough: kname should rather be name as it is also the sysinit name (it cannot, e.g., contain a space, so is not a string so not an arbitrary process name), kfunc has no precedent and could introduce some doubt (why the k?) and kproc gives no clue about the type of the expression to pass. Please see my suggestions instead.

Concerning @kib recommendation of prefixing with _, I'm not against but don't see any real benefit of doing that here: The macro arguments are never used to create variables, except for some static structure names (directly or through SYSINIT()) and that's why I asked for some longer prefix (_kproc_desc), so there are no risks of collisions with local names.

sys/sys/kthread.h
61–67

If somebody wrote

void kfunc(void);

and then tried to write

KPROC_START(..., kfunc, ...)

then she gets the trouble with namespace pollution from the arg names.

sys/sys/kthread.h
61–67

(snip)
then she gets the trouble with namespace pollution from the arg names.

I don't think so. Which trouble?

sys/sys/kthread.h
61–67

If the argument with the kfunc value is not the macro argument named kfunc, the problem is there.