Re: [PATCH -tip] remove the BKL: Replace BKL in mount/umountsyscalls with a mutex

[Christoph Hellwig]

On Thu, Apr 16, 2009 at 04:27:58PM +0200, Alessio Igor Bogani wrote:
> Replace ths BKL in sys_mount()/sys_umount() syscalls with a regular mutex.

Could you try to explain what these actuall try to protect?

>  	unsigned long tmp = ((unsigned long)mnt / L1_CACHE_BYTES);
> @@ -1073,9 +1075,9 @@ static int do_umount(struct vfsmount *mnt, int flags)
>  	 */
>  
>  	if (flags & MNT_FORCE && sb->s_op->umount_begin) {
> -		lock_kernel();
> +		mutex_lock(&mount_lock);
>  		sb->s_op->umount_begin(sb);
> -		unlock_kernel();
> +		mutex_unlock(&mount_lock);

This is a very easy case, just move the lock into ->umount_begin.  And
then ping the maintainers of the 5 instances actually making use of it -
I suspect none of them actually require it.

> @@ -1094,9 +1096,9 @@ static int do_umount(struct vfsmount *mnt, int flags)
>  		 */
>  		down_write(&sb->s_umount);
>  		if (!(sb->s_flags & MS_RDONLY)) {
> -			lock_kernel();
> +			mutex_lock(&mount_lock);
>  			retval = do_remount_sb(sb, MS_RDONLY, NULL, 0);
> -			unlock_kernel();
> +			mutex_unlock(&mount_lock);

I suspect moving lock_kernel down into ->remount_fs is the much better
option.  Will require some audit of do_remount_sb, though.

> -	lock_kernel();
> +	mutex_lock(&mount_lock);
>  	retval = do_mount((char *)dev_page, dir_page, (char *)type_page,
>  			  flags, (void *)data_page);
> -	unlock_kernel();
> +	mutex_unlock(&mount_lock);

Again, much better to push it down and probably eliminate it completely
for all sane filesystems.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/