public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Frederic Weisbecker <fweisbec@gmail.com>
To: Ingo Molnar <mingo@elte.hu>
Cc: Arnd Bergmann <arnd@arndb.de>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Al Viro <viro@zeniv.linux.org.uk>, Jan Blunck <jblunck@suse.de>,
	John Kacur <jkacur@redhat.com>
Subject: Re: [GIT PULL v2] Preparation for BKL'ed ioctl removal
Date: Wed, 28 Apr 2010 15:21:15 +0200	[thread overview]
Message-ID: <20100428132112.GC12017@nowhere> (raw)
In-Reply-To: <20100427092530.GD11348@elte.hu>

On Tue, Apr 27, 2010 at 11:25:30AM +0200, Ingo Molnar wrote:
> 
> * Arnd Bergmann <arnd@arndb.de> wrote:
> 
> > On Monday 26 April 2010, Ingo Molnar wrote:
> > > This could be done all automated for a hundred old drivers if need to be. 
> > > There would be no bkl_ioctl's left.
> > 
> > I don't think it can be fully automated. [...]
> 
> Corner cases are not a problem as long as the risk of them going unnoticed is 
> lower than the risk of a manual conversion introducing bugs.
> 
> > [...] For the majority of the modules, your approach would work fine, but 
> > there are still the well-known pitfalls in corner cases:
> > 
> > - recursive uses in functions outside of ioctl (possibly none left
> >   after the TTY layer is done, but who knows)
> 
> Not a problem even if there's any such usage left: lockdep will sort those out 
> very quickly.
> 
> > - lock-order problems with other mutexes (see DRM)
> 
> This too will be mapped out very quickly via lockdep.


And the hung task detector too which is the last resort to detect
uncovered resource dependencies (was really useful for reiserfs).

But the problem is among those people who may use such ancient drivers,
I guess few of them will have those debug config enabled.

And because there are almost no testers of these drivers, nobody/few will ever
notice the problem.

 
> > - code that depends on autorelease to allow one ioctl while another
> >   is sleeping. (a small number of drivers)
> 
> This is a real issue, and in fact it's an unknown: there may be an unknown 
> number of random sleep points within BKL codepaths that is being relied on in 
> creative ways.
> 
> Note that by introducing a mutex we (in most cases) make the locking 
> _stricter_, so the biggest risk from that is a lockup - which will be 
> debuggable via lockdep.


So, as explained above, lockdep won't even help here.

I mean, for callsites that are obvious, say when it is clear that
the bkl is leaf lock or doesn't introduce uncovered resource dependencies due
to non-release on sleep, we should do such conversion. And I guess most
drivers that use the bkl follow this scheme.

But for the others (rares I think), the operation looks unsafe to me.
If we don't have the hardware to test the driver, then lockdep and hung
task detectors are going to be useless.

That said, once we reach that point with 4 users of bkl remaining, may
be that will be time to buy such hardware for a symbolic $1 in obscure
places and do the tests. Or just git-rm if we are too lazy.


  reply	other threads:[~2010-04-28 13:21 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-16  3:56 [GIT PULL] Preparation for BKL'ed ioctl removal Frederic Weisbecker
2010-04-22  0:48 ` [GIT PULL v2] " Frederic Weisbecker
2010-04-24 15:25   ` Frederic Weisbecker
2010-04-24 18:36     ` Linus Torvalds
2010-04-24 18:47       ` Linus Torvalds
2010-04-24 19:54         ` Arnd Bergmann
2010-04-24 20:01           ` Linus Torvalds
2010-04-24 20:40             ` Arnd Bergmann
2010-04-24 22:15               ` Linus Torvalds
2010-04-25 17:39                 ` Frederic Weisbecker
2010-04-25 17:49                   ` Linus Torvalds
2010-04-25 18:05                     ` Frederic Weisbecker
2010-04-26  8:30                     ` Arnd Bergmann
2010-04-26 18:08                       ` Linus Torvalds
2010-04-26 19:12                         ` Arnd Bergmann
2010-04-26 20:36                           ` Linus Torvalds
2010-04-26 22:23                           ` [PATCH 0/6] Push down BKL into device drivers Arnd Bergmann
2010-04-27  9:14                             ` John Kacur
2010-04-26 22:24                           ` [PATCH 1/6] dvb: push down BKL into ioctl functions Arnd Bergmann
2010-04-26 22:24                           ` [PATCH 2/6] scsi: " Arnd Bergmann
2010-04-26 22:24                           ` [PATCH 3/6] isdn: " Arnd Bergmann
2010-04-26 22:24                           ` [PATCH 4/6] staging: " Arnd Bergmann
2010-04-27 18:15                             ` Frederic Weisbecker
2010-04-27 18:33                               ` Greg KH
2010-04-26 22:24                           ` [PATCH 5/6] v4l: always use unlocked_ioctl Arnd Bergmann
2010-04-26 22:24                           ` [PATCH 6/6] drivers: push down BKL into various drivers Arnd Bergmann
2010-04-26 20:42                         ` [GIT PULL v2] Preparation for BKL'ed ioctl removal David Miller
2010-04-26 22:09                           ` Frederic Weisbecker
2010-04-26 22:32                             ` Linus Torvalds
2010-04-26 23:04                               ` Frederic Weisbecker
2010-04-26  7:25                   ` Ingo Molnar
2010-04-26 11:29                     ` Arnd Bergmann
2010-04-27  9:25                       ` Ingo Molnar
2010-04-28 13:21                         ` Frederic Weisbecker [this message]
2010-04-28 13:37                           ` Ingo Molnar
2010-04-28 14:05                         ` Arnd Bergmann

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=20100428132112.GC12017@nowhere \
    --to=fweisbec@gmail.com \
    --cc=arnd@arndb.de \
    --cc=jblunck@suse.de \
    --cc=jkacur@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.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