On Fri, Dec 21, 2007 at 10:00:40PM -0800, Andrew Morton wrote: > On Thu, 20 Dec 2007 17:15:57 -0500 "Ed L. Cashin" <[EMAIL PROTECTED]> wrote: ... > > +static __DECLARE_SEMAPHORE_GENERIC(emsgs_sema, 0); ... > > - sema_init(&emsgs_sema, 0); > > - spin_lock_init(&emsgs_lock); > > aoe_class = class_create(THIS_MODULE, "aoe"); > > if (IS_ERR(aoe_class)) { > > unregister_chrdev(AOE_MAJOR, "aoechr"); > > I think it would be better to go back to initialising emsgs_lock at runtime > rather than fattening the exported semaphore API like this.
I don't think there is anything wrong with having a complete set of initialization routines for a semaphore, but it's certainly easy enough to go back to Alexey Dobriyan's original suggestion, which was to simply move the initialization calls before register_chardev. I will follow this email with a patch that does that. > emssgs_sema is a weird-looking thing. There really should be some comments > in there because it is unobvious what the code is attempting to do. > > What is the code attempting to do? There is a ring buffer of error messages. Userland processes can read these error messages by reading /dev/etherd/err, blocking if there are no error messages to read yet. The emsgs_sema semaphore is used to manage the reader(s) waiting for error messages. When there are sleepers waiting, "up" is used to wake one up when a new error message is produced. A reader gets a single message, not just some text with a mixture of different errors. If I do, cat /dev/etherd/err > /my/log/file ... then I can hit control-c or send a SIGTERM to stop it. > It appears to me that nblocked_emsgs_readers gets incorrectly > decremented if the down_interruptible() got interrupted, btw. The counter will be incremented again if the process goes back to sleep waiting for an error message, but the process might be getting killed. The counter is really just for sleeping readers. -- Ed L Cashin <[EMAIL PROTECTED]> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/