Re: [PATCH 6/6] procfs: Kill the bkl in ioctl

[Frederic Weisbecker <fweisbec@gmail.com>]

On Thu, Apr 01, 2010 at 02:42:38PM +0200, Arnd Bergmann wrote:
> On Wednesday 31 March 2010, Frederic Weisbecker wrote:
> > On Wed, Mar 31, 2010 at 10:21:23PM +0200, Arnd Bergmann wrote:
> > 
> > > In the meantime, we can move the declaration of the .locked_ioctl callback
> > > into an #ifdef CONFIG_BKL, to make sure nobody builds a driver with an
> > > ioctl function that does not get called.
> > 
> > 
> > Ok, now how to get this all merged? A single monolithic patch is probably
> > not appropriate.
> > 
> > The simplest is to have a single branch with the default_ioctl implemented,
> > and then attributed to drivers in a set cut by subsystems/drivers. And
> > push the whole for the next -rc1.
> > 
> > The other solution is to push default_ioctl for this release and get
> > the driver changes to each concerned tree. That said, I suspect a good
> > part of them are unmaintained, hence the other solution looks better
> > to me.
> 
> I don't care much about the unmaintained parts, we can always have a
> tree collecting all the patches for those drivers and merge it in -rc1.
> 
> I'd say the nicest way would be to get Linus to merge the patch
> below now, so we can start queuing stuff in maintainer trees on top
> of it, and check against linux-next what is still missing and push
> all of them from our branch.
> 
> 	Arnd
> 
> ---
> Subject: [PATCH] introduce CONFIG_BKL and default_ioctl
> 
> This is a preparation for the removal of the big kernel lock that
> introduces new interfaces for device drivers still using it.
> 
> We can start marking those device drivers as 'depends on CONFIG_BKL'
> now, and make that symbol optional later, when the point has come
> at which we are able to build a kernel without the BKL.
> 
> Similarly, device drivers that currently make use of the implicit
> BKL locking around the ioctl function can now get annotated by
> changing
> 
> 	.ioctl = foo_ioctl,
> 
> to
> 
> 	.locked_ioctl = foo_ioctl,
> 	.unlocked_ioctl = default_ioctl,
> 
> As soon as no driver remains using the old ioctl callback, it can
> get removed.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  fs/ioctl.c               |   22 ++++++++++++++++++++++
>  include/linux/fs.h       |    3 +++
>  include/linux/smp_lock.h |    4 ++++
>  lib/Kconfig.debug        |   10 ++++++++++
>  4 files changed, 39 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index 6c75110..52c2698 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -58,6 +58,28 @@ static long vfs_ioctl(struct file *filp, unsigned int cmd,
>  	return error;
>  }
>  
> +#ifdef CONFIG_BKL
> +/*
> + * default_ioctl - call unlocked_ioctl with BKL held
> + *
> + * Setting only the the ioctl operation but not unlocked_ioctl will become
> + * invalid in the future, all drivers that are not converted to unlocked_ioctl
> + * should set .unlocked_ioctl = default_ioctl now.
> + */
> +long default_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> +{



Do you mind if I rename this to deprecated_ioctl()?
This "default" naming suggests a fallback everyone that don't
need tricky things should use.