From: Linus Torvalds on


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

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
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


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
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/