From: "Ed L. Cashin" <ecashin@coraid.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH 09/13] remove race between use and initialization of locks
Date: Wed, 26 Dec 2007 14:28:22 -0500 [thread overview]
Message-ID: <20071226192822.GG9411@coraid.com> (raw)
In-Reply-To: <20071221220040.5862b43b.akpm@linux-foundation.org>
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" <ecashin@coraid.com> 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 <ecashin@coraid.com>
next prev parent reply other threads:[~2007-12-26 19:30 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-12-20 22:15 [PATCH 01/13] bring driver version number to 47 Ed L. Cashin
2007-12-20 22:15 ` [PATCH 02/13] handle multiple network paths to AoE device Ed L. Cashin
2007-12-20 22:15 ` [PATCH 03/13] mac_addr: avoid 64-bit arch compiler warnings Ed L. Cashin
2007-12-20 22:15 ` [PATCH 04/13] clean up udev configuration example Ed L. Cashin
2007-12-20 22:15 ` [PATCH 05/13] eliminate goto and improve readability Ed L. Cashin
2007-12-20 22:15 ` [PATCH 06/13] user can ask driver to forget previously detected devices Ed L. Cashin
2007-12-20 22:15 ` [PATCH 07/13] dynamically allocate a capped number of skbs when necessary Ed L. Cashin
2007-12-20 22:15 ` [PATCH 08/13] only install new AoE device once Ed L. Cashin
2007-12-20 22:15 ` [PATCH 09/13] remove race between use and initialization of locks Ed L. Cashin
2007-12-22 6:00 ` Andrew Morton
2007-12-26 19:28 ` Ed L. Cashin [this message]
2007-12-26 20:25 ` [PATCH] aoe: initialize locking structures before registering char devices Ed L. Cashin
2007-12-26 20:35 ` [PATCH] aoe: document the behavior of /dev/etherd/err Ed L. Cashin
2007-12-20 22:15 ` [PATCH 10/13] add module parameter for users who need more outstanding I/O Ed L. Cashin
2007-12-20 22:15 ` [PATCH 11/13] the aoeminor doesn't need a long format Ed L. Cashin
2007-12-20 22:16 ` [PATCH 12/13] make error messages more specific Ed L. Cashin
2007-12-20 22:16 ` [PATCH 13/13] update copyright date Ed L. Cashin
2007-12-22 6:03 ` Andrew Morton
-- strict thread matches above, loose matches on Subject: below --
2007-12-07 17:47 [PATCH 01/13] bring driver version number to 47 Ed L. Cashin
2007-12-07 17:48 ` [PATCH 09/13] remove race between use and initialization of locks Ed L. Cashin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20071226192822.GG9411@coraid.com \
--to=ecashin@coraid.com \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox