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: Linus Torvalds on 24 Apr 2010 14:40 On Sat, 24 Apr 2010, Frederic Weisbecker wrote: > > This is the second version of this pull request and you don't seem to be > pulling it either. I don't see the point, frankly. Especially not outside the merge window, but quite frankly, I don't see it in general. The whole thing seems to be designed to be inconvenient, and to have a config option that I fundamentally don't believe in (CONFIG_BKL). 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: Linus Torvalds on 24 Apr 2010 15:00 On Sat, 24 Apr 2010, Linus Torvalds wrote: > > I don't see the point, frankly. Especially not outside the merge window, > but quite frankly, I don't see it in general. The whole thing seems to be > designed to be inconvenient, and to have a config option that I > fundamentally don't believe in (CONFIG_BKL). More detail: it still leaves that old "ioctl" function pointer that needs the BKL and is ungreppable. So the whole and only point of the patch is to make our current mess even _more_ complex, with three different cases. No, thank you. Quite frankly, if you want to get rid of the BKL in ioctl's and make them easily greppable, then I would suggest a simple renaming: rename the current '->ioctl()' thing to '->bkl_ioctl()', and mark it deprecated. No new config options, no new games. Just rename it. No need to mark things with CONFIG_BKL, when you can just see it by virtue of them using 'bkl_ioctl'. 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: Arnd Bergmann on 24 Apr 2010 16:00 On Saturday 24 April 2010 20:47:17 Linus Torvalds wrote: > On Sat, 24 Apr 2010, Linus Torvalds wrote: > > > > I don't see the point, frankly. Especially not outside the merge window, > > but quite frankly, I don't see it in general. The whole thing seems to be > > designed to be inconvenient, and to have a config option that I > > fundamentally don't believe in (CONFIG_BKL). > > More detail: it still leaves that old "ioctl" function pointer that needs > the BKL and is ungreppable. So the whole and only point of the patch is to > make our current mess even more complex, with three different cases. No, > thank you. > > Quite frankly, if you want to get rid of the BKL in ioctl's and make them > easily greppable, then I would suggest a simple renaming: rename the > current '->ioctl()' thing to '->bkl_ioctl()', and mark it deprecated. No > new config options, no new games. Just rename it. No need to mark things > with CONFIG_BKL, when you can just see it by virtue of them using > 'bkl_ioctl'. We want to do the rename in the next merge window and remove the old ->ioctl(), this patch is just a preparation for this so we can start queuing the patches for the rename in maintainer trees. The addition of the deprecated_ioctl() helper is not essential but let's us move all BKL users into loadable modules next. The CONFIG_BKL stuff is not a requirement for doing this, but something we /also/ want to do in the next merge window, i.e. mark all BKL users as CONFIG_BKL, not just the ones that use the locked_ioctl (or bkl_ioctl). I agree that it's now a bit late for this, but when I initially suggested this (before -rc3, IIRC), that would have given us the chance to queue up all the patches with a dependency on this for the next merge window. Now the current outlook is probably that we do a lot of the preparation work like this patch in 2.6.35-rc1, which moves the merge of the interesting parts out to the 2.6.36 timeframe. Arnd -- 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 24 Apr 2010 18:20 On Sat, 24 Apr 2010, Arnd Bergmann wrote: > > With CONFIG_BKL disabled, we gain a few cyles in the scheduler, That has _nothing_ to do with the ioctl's though. Stop mixing things up. There are two totally independent issues: - making the BKL ioctl's be explicit and findable - eventually getting rid of the BKL entirely and I think you guys are totally mixing things up, and making things WORSE in the process. The notion of having _three_ different "ioctl()" function pointers just makes me want to gag. And there is absolutely _zero_ reason for it. Tjhere is no way in hell that we want to have every subsystem maintainer try to independently do their own ioctl's. Most of the drivers that have those things are basically unmaintained or on the back burner anyway. So don't make the current ugly ioctl situation worse. Not even as a stop-gap, because there is absolutely _zero_ upside to making yet another new crazy temporary ioctl interface. And don't try to conflate the issue of ioctl and BKL. There are still code-paths that do lock_kernel() without the ioctl's, so the whole ioctl renaming has _zero_ to do with CONFIG_BKL. 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 25 Apr 2010 13:40
On Sat, Apr 24, 2010 at 03:15:23PM -0700, Linus Torvalds wrote: > > > On Sat, 24 Apr 2010, Arnd Bergmann wrote: > > > > With CONFIG_BKL disabled, we gain a few cyles in the scheduler, > > That has _nothing_ to do with the ioctl's though. > > Stop mixing things up. > > There are two totally independent issues: > > - making the BKL ioctl's be explicit and findable > > - eventually getting rid of the BKL entirely > > and I think you guys are totally mixing things up, and making things WORSE > in the process. > > The notion of having _three_ different "ioctl()" function pointers just > makes me want to gag. And there is absolutely _zero_ reason for it. Tjhere > is no way in hell that we want to have every subsystem maintainer try to > independently do their own ioctl's. Most of the drivers that have those > things are basically unmaintained or on the back burner anyway. > > So don't make the current ugly ioctl situation worse. Not even as a > stop-gap, because there is absolutely _zero_ upside to making yet another > new crazy temporary ioctl interface. Our final goal was not to have three different ioctl interfaces. This state was only deemed to be temporary. This was the only way to make the change smoother and don't conflict with other trees with a single monolithic patch. But if you are ok with a single one, then we are going this way and we'll send it for the next merge window. > And don't try to conflate the issue of ioctl and BKL. There are still > code-paths that do lock_kernel() without the ioctl's, so the whole ioctl > renaming has _zero_ to do with CONFIG_BKL. It's true, but once it gets pushed down/dropped from every core parts (which is what we are working on currently in parallel), lock_kernel() and .bkl_ioctl is only going to be used by unmaintained drivers. This is the time where having a CONFIG_BKL is going to make sense. And it won't be a question of saving some bytes but improve efficiency of schedule() for those who don't need such old or unmaintained drivers. May be we should only start to focus on this new config once this state is reached. And to prepare for that, are you ok with this scheme of: - .ioctl = foo, + .unlocked_ioctl = bkl_ioctl, + .bkl_ioctl = foo, ....done at the same time as the big rename patch. This will prepare to remove the bkl from vfs and build it conditionally from the bkl lib, once the bkl is out the core? -- 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/ |