Preparation for BKL'ed ioctl removal
in [Kernel]
|
Prev: mac8390: fix pr_info() calls, was Re: another cleanup patch gone wrong
Next: [PATCH] blkio: Initialize blkg->stats_lock for the root cfqg too
From: Frederic Weisbecker on 26 Apr 2010 18:10 On Mon, Apr 26, 2010 at 01:42:07PM -0700, David Miller wrote: > From: Linus Torvalds <torvalds(a)linux-foundation.org> > Date: Mon, 26 Apr 2010 11:08:39 -0700 (PDT) > > > NOTE! This has gone through a "allmodconfig" (with staging drivers), but > > only on x86-64. There are quite probably missing architecture conversions > > and/or drivers. But this should be the bulk of them. > > "allmodconfig" on sparc64 passes with this patch too, just FYI... Thanks! I've queued it for the next merge window in git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git bkl/ioctl -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Linus Torvalds on 26 Apr 2010 18:40 On Tue, 27 Apr 2010, Frederic Weisbecker wrote: > > I've queued it for the next merge window in > > git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git > bkl/ioctl Btw, I hope you took the second version, that had the two additional fixes from Arnd (and my expansion of his fix to au1550_ac97.c). Looking at the arch code, I doubt there are any big architecture-specific things. But there could easily be some other drivers like au1550_ac97.c that only get enabled on certain architectures and missed the grepping for some reason. That said, because of Arnd's fix, I did end up grepping for 'file_operations' and old-style gcc initializers (ie "ioctl: xyz" rather than the proper ".ioctl = xyz"), and the grep came up empty. But it's possible that there's something hiding: with all of serial, bluetooth, block drivers, sound, socket proto's and v4l2 each having their own 'ioctl' pointers, it's not entirely trivial to grep for it all and be sure.. So there might be one or two cases still hiding, but it looks unlikely. And I'm almost certain that it definitely isn't more than just one or two. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Frederic Weisbecker on 26 Apr 2010 19:10 On Mon, Apr 26, 2010 at 03:32:51PM -0700, Linus Torvalds wrote: > > > On Tue, 27 Apr 2010, Frederic Weisbecker wrote: > > > > I've queued it for the next merge window in > > > > git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git > > bkl/ioctl > > Btw, I hope you took the second version, that had the two additional fixes > from Arnd (and my expansion of his fix to au1550_ac97.c). Yeah. > Looking at the arch code, I doubt there are any big architecture-specific > things. But there could easily be some other drivers like au1550_ac97.c > that only get enabled on certain architectures and missed the grepping for > some reason. > > That said, because of Arnd's fix, I did end up grepping for > 'file_operations' and old-style gcc initializers (ie "ioctl: xyz" rather > than the proper ".ioctl = xyz"), and the grep came up empty. > > But it's possible that there's something hiding: with all of serial, > bluetooth, block drivers, sound, socket proto's and v4l2 each having their > own 'ioctl' pointers, it's not entirely trivial to grep for it all and be > sure.. > > So there might be one or two cases still hiding, but it looks unlikely. > And I'm almost certain that it definitely isn't more than just one or two. Ok, thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Ingo Molnar on 27 Apr 2010 05:30 * Arnd Bergmann <arnd(a)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. > - 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. Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Frederic Weisbecker on 28 Apr 2010 09:30
On Tue, Apr 27, 2010 at 11:25:30AM +0200, Ingo Molnar wrote: > > * Arnd Bergmann <arnd(a)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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |