public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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

  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