public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: corbet@lwn.net (Jonathan Corbet)
To: Ingo Molnar <mingo@elte.hu>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Thomas Gleixner <tglx@linutronix.de>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>,
	Alexander Viro <viro@ftp.linux.org.uk>,
	linux-kernel@vger.kernel.org
Subject: Re: [announce] "kill the Big Kernel Lock (BKL)" tree
Date: Wed, 14 May 2008 15:45:15 -0600	[thread overview]
Message-ID: <6457.1210801515@vena.lwn.net> (raw)
In-Reply-To: Your message of "Wed, 14 May 2008 19:49:55 +0200." <20080514174955.GA515@elte.hu>

Sez Ingo:

> This task is not easy at all. 12 years after Linux has been converted to 
> an SMP OS we still have 1300+ legacy BKL using sites. There are 400+ 
> lock_kernel() critical sections and 800+ ioctls.

There's also every char device open() method - a rather long list in its
own right.  I'd be surprised if one in ten of them really needs it, but
one has to look...

I've been looking at the chrdev code anyway, and pondering on how this
might be addressed.  Here's some thoughts on alternatives, I'd be
curious what people think:

1: We could add an unlocked_open() to the file_operations structure;
   drivers could be converted over as they are verified not to need the
   BKL on open.  Disadvantages are that it grows this structure for a
   relatively rare case - most open() calls already don't need the BKL.
   But it's a relatively easy path without flag days.

2: Create a char_dev_ops structure for char devs and use it instead of
   file_operations.  I vaguely remember seeing Al mutter about that a
   while back.  Quite a while back.  This mirrors what was done with
   block devices, and makes some sense - there's a lot of stuff in
   struct file_operations which is not really applicable to char devs.
   Then struct char_dev_ops could have open() and locked_open(), with
   the latter destined for removal sometime around 2015 or so.

   Advantages are that it's cleaner and separates out some things which
   perhaps shouldn't be mixed anyway.  Disadvantage is...well...a fair
   amount of code churn.  It would also require chrdev-specific wrappers
   to map straight file_operations calls in the VFS to the new
   callbacks.

3: Provide a new form of cdev_add() which lets the driver indicate
   that the BKL is not needed on open (or anything else?).  At a
   minimum, it could just be a new parameter on cdev_add which has a
   value of zero or FIXME_I_STILL_NEED_BKL.  Still some churn but easier
   to script and smaller because a lot of drivers are still using
   register_chrdev() - something else worth fixing.

   A more involved form might provide a new chardev_add() which takes
   the new char_dev_ops structure too.  Mapping between new and old
   operations vectors would be done internally to avoid breaking older
   drivers before they can be fixed.

4: Just find every char dev open() function and shove in lock_kernel()
   calls, then remove the call from chrdev_open().  The disadvantage
   here is that, beyond lots of work and churn, there's no way to know
   which ones you missed.

I kind of like the combination of 2 and 3, done in such a way that
there's no "every driver must change" flag day.  This could be an
interesting project, even...  Thoughts?

jon

  parent reply	other threads:[~2008-05-14 21:45 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-14 17:49 [announce] "kill the Big Kernel Lock (BKL)" tree Ingo Molnar
2008-05-14 18:30 ` Andi Kleen
2008-05-14 21:00   ` Alan Cox
2008-05-14 21:13     ` Andi Kleen
2008-05-14 21:16       ` H. Peter Anvin
2008-05-14 21:17         ` Alan Cox
2008-05-14 21:19       ` Alan Cox
2008-05-14 21:45         ` Linus Torvalds
2008-05-14 22:03           ` Andi Kleen
2008-05-15 13:34             ` Alan Cox
2008-05-15 14:27               ` Andi Kleen
2008-05-15 15:36                 ` Alan Cox
2008-05-16 10:21                   ` Andi Kleen
2008-05-15  8:02           ` Ingo Molnar
2008-05-14 18:41 ` Linus Torvalds
2008-05-14 19:41   ` Ingo Molnar
2008-05-14 20:05     ` Frederik Deweerdt
2008-05-14 21:45 ` Jonathan Corbet [this message]
2008-05-14 21:39   ` Alan Cox
2008-05-14 21:56   ` Linus Torvalds
2008-05-14 22:07     ` Jonathan Corbet
2008-05-14 22:14       ` Linus Torvalds
2008-05-22 20:20       ` Alan Cox
2008-05-16 15:44     ` [PATCH, RFC] char dev BKL pushdown Jonathan Corbet
2008-05-16 15:49       ` Christoph Hellwig
2008-05-16 16:03         ` [PATCH] kill empty chardev open/release methods Christoph Hellwig
2008-05-16 16:24           ` Alan Cox
2008-05-16 20:55           ` Alan Cox
2008-05-18 19:46             ` Jonathan Corbet
2008-05-18 19:58               ` Alan Cox
2008-05-16 16:22       ` [PATCH, RFC] char dev BKL pushdown Alan Cox
2008-05-16 16:30       ` Linus Torvalds
2008-05-16 16:43         ` Jonathan Corbet
2008-05-17 21:15       ` Arnd Bergmann
2008-05-18 20:26         ` Jonathan Corbet
2008-05-19 23:07           ` Arnd Bergmann
     [not found]             ` <200805200111.47275.arnd@arndb.de>
2008-05-19 23:14               ` [PATCH 2/3, RFC] watchdog " Arnd Bergmann
2008-05-20  6:20                 ` Christoph Hellwig
2008-05-20  8:30                   ` Arnd Bergmann
2008-05-20 15:47                     ` Wim Van Sebroeck
2008-05-20 18:31                       ` Alan Cox
2008-05-20 21:00                         ` Arnd Bergmann
2008-05-22  9:34                           ` Alan Cox
2008-05-20  9:08                   ` Alan Cox
2008-05-20  8:42                 ` Alan Cox
2008-05-19 23:26             ` [PATCH 1/3, RFC] misc char " Arnd Bergmann
2008-05-20  0:07               ` Mike Frysinger
2008-05-20  0:21                 ` Jonathan Corbet
2008-05-20  0:46                   ` Mike Frysinger
2008-05-20  8:46               ` Alan Cox
2008-05-20 23:01               ` Mike Frysinger
2008-05-20 23:25                 ` Jonathan Corbet
2008-05-21 16:22                   ` Mike Frysinger
2008-05-19 23:34             ` [PATCH 3/3, RFC] remove BKL from misc_open() Arnd Bergmann
2008-05-20 15:13             ` [PATCH, RFC] char dev BKL pushdown Jonathan Corbet
2008-05-20 17:21               ` Arnd Bergmann
2008-05-20 18:51                 ` Alan Cox
2008-05-17 21:58       ` Linus Torvalds
2008-05-18 20:07         ` Jonathan Corbet
2008-05-14 22:11   ` [announce] "kill the Big Kernel Lock (BKL)" tree Andi Kleen
2008-05-14 22:16     ` Linus Torvalds
2008-05-14 22:21       ` Andi Kleen
2008-05-15 13:30         ` Alan Cox
2008-05-15 15:05         ` John Stoffel
2008-05-15 15:10           ` Andi Kleen
2008-05-15 15:18             ` John Stoffel
2008-05-15 15:45               ` Andi Kleen
2008-05-15  8:44   ` Jan Engelhardt
2008-05-15 14:54     ` Diego Calleja
2008-05-14 21:46 ` Alan Cox
2008-05-14 22:11   ` Linus Torvalds
2008-05-14 22:15   ` Andi Kleen
2008-05-15 17:41 ` Linus Torvalds
2008-05-15 20:27   ` Arjan van de Ven
2008-05-15 20:45     ` Peter Zijlstra
2008-05-15 21:22       ` Arjan van de Ven
2008-05-17  0:14 ` Kevin Winchester
2008-05-17  0:37   ` Kevin Winchester

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=6457.1210801515@vena.lwn.net \
    --to=corbet@lwn.net \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@ftp.linux.org.uk \
    /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