[patch 0/2] spufs: Minor cleanups

[patch 0/2] spufs: Minor cleanups

[jblunck]

Arnd,

here are some minor cleanups that I think are worth doing. I had them in my
union mount queue because I had some changes to lookup_create() planned.

Cheers,
Jan

[patch 1/2] spufs: make dentry reference count and locking symmetric

[jblunck]

[-- Attachment #1: spufs-make-dput-and-locking-symmetric.diff --]
[-- Type: text/plain, Size: 1753 bytes --]

This patch moves the dput() and the parent inode locking to the same function.

Signed-off-by: Jan Blunck <jblunck@suse.de>
---
 arch/powerpc/platforms/cell/spufs/inode.c |   14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

Index: b/arch/powerpc/platforms/cell/spufs/inode.c
===================================================================
--- a/arch/powerpc/platforms/cell/spufs/inode.c
+++ b/arch/powerpc/platforms/cell/spufs/inode.c
@@ -450,16 +450,16 @@ spufs_create_context(struct inode *inode
 	ret = -EPERM;
 	if ((flags & SPU_CREATE_NOSCHED) &&
 	    !capable(CAP_SYS_NICE))
-		goto out_unlock;
+		goto out;
 
 	ret = -EINVAL;
 	if ((flags & (SPU_CREATE_NOSCHED | SPU_CREATE_ISOLATE))
 	    == SPU_CREATE_ISOLATE)
-		goto out_unlock;
+		goto out;
 
 	ret = -ENODEV;
 	if ((flags & SPU_CREATE_ISOLATE) && !isolated_loader)
-		goto out_unlock;
+		goto out;
 
 	gang = NULL;
 	neighbor = NULL;
@@ -468,7 +468,7 @@ spufs_create_context(struct inode *inode
 		gang = SPUFS_I(inode)->i_gang;
 		ret = -EINVAL;
 		if (!gang)
-			goto out_unlock;
+			goto out;
 		mutex_lock(&gang->aff_mutex);
 		neighbor = spufs_assert_affinity(flags, gang, aff_filp);
 		if (IS_ERR(neighbor)) {
@@ -505,10 +505,7 @@ spufs_create_context(struct inode *inode
 out_aff_unlock:
 	if (affinity)
 		mutex_unlock(&gang->aff_mutex);
-out_unlock:
-	mutex_unlock(&inode->i_mutex);
 out:
-	dput(dentry);
 	return ret;
 }
 
@@ -595,8 +592,6 @@ static int spufs_create_gang(struct inod
 	}
 
 out:
-	mutex_unlock(&inode->i_mutex);
-	dput(dentry);
 	return ret;
 }
 
@@ -645,7 +640,6 @@ long spufs_create(struct nameidata *nd,
 					    filp);
 	if (ret >= 0)
 		fsnotify_mkdir(nd->path.dentry->d_inode, dentry);
-	return ret;
 
 out_dput:
 	dput(dentry);

[patch 2/2] spufs: remove double check for non-negative dentry

[jblunck]

[-- Attachment #1: spufs-remove-unnecessary-EEXIST-check.patch --]
[-- Type: text/plain, Size: 777 bytes --]

This patch removes an unnecessary double check if the dentry returned by
lookup_create() is actually non-negative. Since lookup_create() itself returns
an error in this case just remove the check.

Signed-off-by: Jan Blunck <jblunck@suse.de>
---
 arch/powerpc/platforms/cell/spufs/inode.c |    4 ----
 1 file changed, 4 deletions(-)

Index: b/arch/powerpc/platforms/cell/spufs/inode.c
===================================================================
--- a/arch/powerpc/platforms/cell/spufs/inode.c
+++ b/arch/powerpc/platforms/cell/spufs/inode.c
@@ -625,10 +625,6 @@ long spufs_create(struct nameidata *nd,
 	if (IS_ERR(dentry))
 		goto out_dir;
 
-	ret = -EEXIST;
-	if (dentry->d_inode)
-		goto out_dput;
-
 	mode &= ~current->fs->umask;
 
 	if (flags & SPU_CREATE_GANG)

Re: [patch 1/2] spufs: make dentry reference count and locking symmetric

[Arnd Bergmann]

On Tuesday 05 May 2009, jblunck@suse.de wrote:
> @@ -505,10 +505,7 @@ spufs_create_context(struct inode *inode
>  out_aff_unlock:
>         if (affinity)
>                 mutex_unlock(&gang->aff_mutex);
> -out_unlock:
> -       mutex_unlock(&inode->i_mutex);
>  out:
> -       dput(dentry);
>         return ret;
>  }
>  

There is a nasty deadlock problem with spu_forget(), which we
must call after unlocking i_mutex but before the final dput, IIRC.

The path down there is something like 

spu_forget -> mmput -> exit_mmap -> remove_vma -> 
	fput -> dput -> dentry_iput -> iput -> iput_final ->
	spufs_delete_inode -> destroy_spu_context.

	Arnd <><

Re: [patch 2/2] spufs: remove double check for non-negative dentry

[Arnd Bergmann]

On Tuesday 05 May 2009, jblunck@suse.de wrote:
> This patch removes an unnecessary double check if the dentry returned by
> lookup_create() is actually non-negative. Since lookup_create() itself returns
> an error in this case just remove the check.
> 
> Signed-off-by: Jan Blunck <jblunck@suse.de>

Looks good, I'll push this one on, unless Jeremy has objections.

Thanks,

	Arnd <><

> --- a/arch/powerpc/platforms/cell/spufs/inode.c
> +++ b/arch/powerpc/platforms/cell/spufs/inode.c
> @@ -625,10 +625,6 @@ long spufs_create(struct nameidata *nd,
>  	if (IS_ERR(dentry))
>  		goto out_dir;
>  
> -	ret = -EEXIST;
> -	if (dentry->d_inode)
> -		goto out_dput;
> -
>  	mode &= ~current->fs->umask;
>  
>  	if (flags & SPU_CREATE_GANG)
>