From: Paul Clements <paul.clements@steeleye.com>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: akpm@osdl.org, djani22@dynamicweb.hu,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [NBD] Use per-device semaphore instead of BKL
Date: Sun, 20 Nov 2005 12:19:17 -0500 [thread overview]
Message-ID: <4380B015.9060005@steeleye.com> (raw)
In-Reply-To: <20051120015807.GA3593@gondor.apana.org.au>
Herbert Xu wrote:
> On Sun, Nov 20, 2005 at 09:34:19AM +1100, herbert wrote:
>
>>This is intentional actually. nbd_clear_queue never races against
>>nbd_find_request because the ioctl is protected by the BKL. If it
>>weren't, then we have much worse problems to worry about (e.g.,
>>while you're clearing the queue someone else could have set the
>>socket again and started queueing requests).
>
>
> Actually, we do have bigger problems :) The BKL is dropped every
> time you sleep, and nbd_do_it is definitely a frequent sleeper :)
The dropping of the lock in nbd_do_it is actually critical to the way
nbd functions. nbd_do_it runs for the lifetime of the nbd device, so if
nbd_do_it were holding some lock (BKL or otherwise), we'd have big problems.
> This isn't really an issue in practice though because the NBD
> client program is single-threaded and doesn't share its file
> descriptors with anyone else.
Right, there's no problem in practice.
> However, we shouldn't make it too easy for the user to shoot himself
> in the foot. If he's going to do that, let him at least pay for the
> bullet :)
>
> So here is a patch to use a per-device semaphore instead of the
> BKL to protect the ioctl's against each other.
The problem with this patch is that no ioctls can come in once nbd_do_it
starts because nbd_do_it runs for the lifetime of the device.
I think we really just need to add the acquiring of queue_lock in
nbd_clear_que to your previous patch and leave it at that. I'll code
that up and test it.
Thanks,
Paul
next prev parent reply other threads:[~2005-11-20 17:19 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <200511190345.jAJ3jFC3016406@shell0.pdx.osdl.net>
[not found] ` <437F4C85.3070108@steeleye.com>
2005-11-19 22:34 ` + nbd-fix-tx-rx-race-condition.patch added to -mm tree Herbert Xu
2005-11-20 1:58 ` [NBD] Use per-device semaphore instead of BKL Herbert Xu
2005-11-20 17:19 ` Paul Clements [this message]
2005-11-20 20:43 ` Herbert Xu
2005-11-20 21:42 ` Paul Clements
2005-11-20 22:08 ` Herbert Xu
2005-11-21 1:12 ` Paul Clements
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=4380B015.9060005@steeleye.com \
--to=paul.clements@steeleye.com \
--cc=akpm@osdl.org \
--cc=djani22@dynamicweb.hu \
--cc=herbert@gondor.apana.org.au \
--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