Re: [PATCH] eCryptfs: Remove unnecessary mux fromecryptfs_init_ecryptfs_miscdev()

[Cyrill Gorcunov]

[Michael Halcrow - Thu, Jun 26, 2008 at 11:03:20AM -0500]
| On Thu, Jun 26, 2008 at 05:59:04PM +0400, Cyrill Gorcunov wrote:
| > Some analisys shows the following interesting things -
| > 
| > Call chain (if being compiled into the kernel)
| > ----------------------------------------------
| > 
| > ecryptfs_init
| > 	ecryptfs_init_messaging
| > 		ecryptfs_init_ecryptfs_miscdev
| > 			mutex_lock(&ecryptfs_daemon_hash_mux);
| > 			misc_register(&ecryptfs_miscdev);
| > 				mutex_lock(&misc_mtx);
| > 				mutex_unlock(&misc_mtx);
| > 1 --->
| > 			mutex_unlock(&ecryptfs_daemon_hash_mux);
| > 
| > As only we have misc device registered it is valid to open it.
| > And that is happened by another proccess
| > 
| > chrdev_open
| > 	misc_open
| > 		mutex_lock(&misc_mtx);
| > 			ecryptfs_miscdev_open
| > 				mutex_lock(&ecryptfs_daemon_hash_mux);
| > 			
| > 
| > If this happens at point (1) /ie chrdev_open called at this point/
| > we are getting to circular warning as on top.
| 
| It appears to be only a warning and that deadlock cannot occur due to
| the fact that ecryptfs_miscdev_open() cannot be called until after
| mutex_unlock(&misc_mtx) occurs while under
| ecryptfs_init_ecryptfs_miscdev(). However, on second glance, given the
| call sequence detail you provided here, I think I am being
| over-zealous in my hash table locking with ecryptfs_daemon_hash_mux in
| ecryptfs_init_ecryptfs_miscdev(). The misc_mtx should provide all the
| protection required to keep the daemon hash table sane during miscdev
| registration.
| 
| ---
| 
| The misc_mtx should provide all the protection required to keep the
| daemon hash table sane during miscdev registration. Since this mutex
| is causing gratuitous lockdep warnings, this patch removes it.
| 
| Signed-off-by: Michael Halcrow <mhalcrow@xxxxxxxxxx>
| ---
|  fs/ecryptfs/miscdev.c |    2 --
|  1 files changed, 0 insertions(+), 2 deletions(-)
| 
| diff --git a/fs/ecryptfs/miscdev.c b/fs/ecryptfs/miscdev.c
| index 6560da1..a9926a4 100644
| --- a/fs/ecryptfs/miscdev.c
| +++ b/fs/ecryptfs/miscdev.c
| @@ -577,13 +577,11 @@ int ecryptfs_init_ecryptfs_miscdev(void)
|  	int rc;
|  
|  	atomic_set(&ecryptfs_num_miscdev_opens, 0);
| -	mutex_lock(&ecryptfs_daemon_hash_mux);
|  	rc = misc_register(&ecryptfs_miscdev);
|  	if (rc)
|  		printk(KERN_ERR "%s: Failed to register miscellaneous device "
|  		       "for communications with userspace daemons; rc = [%d]\n",
|  		       __func__, rc);
| -	mutex_unlock(&ecryptfs_daemon_hash_mux);
|  	return rc;
|  }
|  
| -- 
| 1.5.3.7
| 

Yes, it was only warning, but it was a bit annoying you know :)
Thanks a lot, Michael!

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