public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@suse.de>
To: "Peter T. Breuer" <ptb@it.uc3m.es>
Cc: David Woodhouse <dwmw2@infradead.org>,
	William Lee Irwin III <wli@holomorphy.com>,
	"Martin J. Bligh" <mbligh@aracnet.com>,
	linux-kernel@vger.kernel.org
Subject: Re: recursive spinlocks. Shoot.
Date: Mon, 19 May 2003 15:45:50 +0200	[thread overview]
Message-ID: <20030519134550.GA812@suse.de> (raw)
In-Reply-To: <200305191337.h4JDbf311387@oboe.it.uc3m.es>

On Mon, May 19 2003, Peter T. Breuer wrote:
> "David Woodhouse wrote:"
> > To be honest, if any programmer is capable of committing this error and
> > not finding and fixing it for themselves, then they're also capable, and
> > arguably _likely_, to introduce subtle lock ordering discrepancies which
> > will cause deadlock once in a blue moon.
> > 
> > I don't _want_ you to make life easier for this hypothetical programmer.
> > 
> > I want them to either learn to comprehend locking _properly_, or take up
> > gardening instead.
> 
> Let's quote the example from rubini & corbet of the sbull block device
> driver. The request function ends like so:
> 
> 
>         spin_unlock_irq (&io_request_lock);
>         spin_lock(&device->lock);
> 
>     /* Process all of the buffers in this (possibly clustered) request.  */
>         do {
>             status = sbull_transfer(device, req);
>         } while (end_that_request_first(req, status, DEVICE_NAME));
>         spin_unlock(&device->lock);
>         spin_lock_irq (&io_request_lock);
>         end_that_request_last(req);
>     }
>     device->busy = 0;
> }
> 
> 
> Notice that he runs end_that_request_first outside the io_request_lock
> and end_that_request_last under the lock. Do you know which is right?
> (if any :-).

Both are right, as it so happens.

> And he takes a device lock before calling the "transfer" routine.
> Yes, he's ok because his transfer function is trivial and doesn't
> take the lock, but anyone following his recipe is heading for
> trouble.

In 2.5, the device lock most likely would be the queue lock as well so
no confusion there. But what are you talking about? I'd assume that the
device lock must be held in the transfer function, why else would you
grab it in the first place in the function you quote? Please, if you are
going to find examples to support the recursive locking idea, find some
decent ones...

I think you are trying to make up problems that do not exist. I'd hate
to see recursive locks being used just because someone can't be bothered
to write to code correctly (recursive locks have its uses, the one you
are advocating definitely isn't one). Recursive locks are _not_ a remedy
for someone who doesn't understand locking or isn't capable enough to
design his locking correctly. Period.

-- 
Jens Axboe


  reply	other threads:[~2003-05-19 13:33 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-05-18  9:21 recursive spinlocks. Shoot Peter T. Breuer
2003-05-18 16:30 ` Martin J. Bligh
2003-05-18 16:35   ` William Lee Irwin III
2003-05-18 16:49     ` Arjan van de Ven
2003-05-18 16:54       ` William Lee Irwin III
2003-05-18 17:14         ` Martin J. Bligh
2003-05-18 17:24     ` Peter T. Breuer
2003-05-18 22:34       ` David Woodhouse
2003-05-19 13:37         ` Peter T. Breuer
2003-05-19 13:45           ` Jens Axboe [this message]
2003-05-19 13:47           ` Arjan van de Ven
     [not found]           ` <mailman.1053352200.24653.linux-kernel2news@redhat.com>
2003-05-19 23:54             ` Pete Zaitcev
2003-05-20  0:03               ` viro
2003-05-20  0:03               ` Johannes Erdfelt
2003-05-20  3:12         ` Robert White
2003-05-20 11:59           ` Helge Hafting
2003-05-20 12:23             ` Richard B. Johnson
2003-05-20 21:05               ` Robert White
2003-05-20 21:42                 ` Richard B. Johnson
2003-05-20 23:06                   ` Robert White
2003-05-21 14:01                     ` Richard B. Johnson
2003-05-21 21:56                       ` Robert White
2003-05-22  0:13                         ` viro
2003-05-22  0:32                           ` Robert White
2003-05-22  0:46                         ` Carl-Daniel Hailfinger
2003-05-21  5:48                   ` Nikita Danilov
2003-05-22  1:00           ` Rik van Riel
2003-05-22  3:11             ` Robert White
2003-05-22  4:04               ` Nick Piggin
2003-05-22  4:42                 ` Peter T. Breuer
2003-05-22  5:09                   ` Nick Piggin
2003-05-23  0:19                 ` Robert White
2003-05-23  7:22                   ` Nikita Danilov
2003-05-23  9:07                     ` Helge Hafting
2003-05-23 12:18                     ` William Lee Irwin III
2003-05-24  2:39                       ` Robert White
2003-05-28 16:50                         ` Timothy Miller
2003-05-19  2:05       ` Kevin O'Connor
2003-05-19  6:19       ` Jan Hudec
2003-05-19 10:29       ` Helge Hafting
2003-05-19 11:37         ` Nikita Danilov
2003-05-22  1:21           ` Daniel Phillips
2003-05-19 14:28       ` Martin J. Bligh
2003-05-18 18:13 ` Davide Libenzi
     [not found] <20030518182010$0541@gated-at.bofh.it>
2003-05-18 19:09 ` Peter T. Breuer
2003-05-18 19:31   ` Davide Libenzi
2003-05-18 19:49     ` Peter T. Breuer
2003-05-18 20:13       ` Davide Libenzi
2003-05-19 20:47   ` Jan Hudec
     [not found] <20030518202013$5297@gated-at.bofh.it>
2003-05-18 23:15 ` Peter T. Breuer
2003-05-18 23:26   ` Davide Libenzi
2003-05-19 12:48     ` Peter T. Breuer
2003-05-19 17:15       ` Davide Libenzi
2003-05-19 17:27         ` Peter T. Breuer
2003-05-19 17:57           ` Alan Cox
2003-05-19 19:51         ` Peter T. Breuer
2003-05-19 20:22   ` Robert White
     [not found] <20030520231013$3d77@gated-at.bofh.it>
2003-05-21 14:16 ` Peter T. Breuer

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=20030519134550.GA812@suse.de \
    --to=axboe@suse.de \
    --cc=dwmw2@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mbligh@aracnet.com \
    --cc=ptb@it.uc3m.es \
    --cc=wli@holomorphy.com \
    /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