[PATCH] exofs: remove BKL from super operations

[PATCH] exofs: remove BKL from super operations

[Boaz Harrosh]

the two places inside exofs that where taking the BKL were:
exofs_put_super() - .put_super
and
exofs_sync_fs() - which is .sync_fs and is also called from
                  .write_super.

Now exofs_sync_fs() is protected from itself by also taking
the sb_lock.

exofs_put_super() directly calls exofs_sync_fs() so there is no
danger between these two either.

In anyway there is absolutely nothing dangerous been done
inside exofs_sync_fs().

Unless there is some subtle race with the actual lifetime of
the super_block in regard to .put_super and some other parts
of the VFS. Which is highly unlikely.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 fs/exofs/super.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/fs/exofs/super.c b/fs/exofs/super.c
index 5ab10c3..9f500de 100644
--- a/fs/exofs/super.c
+++ b/fs/exofs/super.c
@@ -214,7 +214,6 @@ int exofs_sync_fs(struct super_block *sb, int wait)
 	}
 
 	lock_super(sb);
-	lock_kernel();
 	sbi = sb->s_fs_info;
 	fscb->s_nextid = cpu_to_le64(sbi->s_nextid);
 	fscb->s_numfiles = cpu_to_le32(sbi->s_numfiles);
@@ -245,7 +244,6 @@ int exofs_sync_fs(struct super_block *sb, int wait)
 out:
 	if (or)
 		osd_end_request(or);
-	unlock_kernel();
 	unlock_super(sb);
 	kfree(fscb);
 	return ret;
@@ -268,8 +266,6 @@ static void exofs_put_super(struct super_block *sb)
 	int num_pend;
 	struct exofs_sb_info *sbi = sb->s_fs_info;
 
-	lock_kernel();
-
 	if (sb->s_dirt)
 		exofs_write_super(sb);
 
@@ -286,8 +282,6 @@ static void exofs_put_super(struct super_block *sb)
 	osduld_put_device(sbi->s_dev);
 	kfree(sb->s_fs_info);
 	sb->s_fs_info = NULL;
-
-	unlock_kernel();
 }
 
 /*
-- 
1.6.2.1

Re: [PATCH] exofs: remove BKL from super operations

[Christoph Hellwig]

Any chance you could also remove the lock_super usage once your start
revisiting the lock?  lock_super is never taken by the VFS anymore, so
you can easily replace it with fs-local locking.

Re: [PATCH] exofs: remove BKL from super operations

[Boaz Harrosh]

On 08/20/2009 02:49 AM, Christoph Hellwig wrote:
> Any chance you could also remove the lock_super usage once your start
> revisiting the lock?  lock_super is never taken by the VFS anymore, so
> you can easily replace it with fs-local locking.
> 

OK Sure, thanks.

One question please?

I need a mutex_lock I can sleep on. Could I use the inode_lock associate
with the root_inode. Or that could lead to dead-locks with the VFS?

I'll test it out anyway, but out of your head could it lead to problems?
All I need is sleepable serialization of exofs_sync_fs() from itself.

(Or should I just allocate another mutex at the fs-sb-data?)

Boaz

Re: [PATCH] exofs: remove BKL from super operations

[Christoph Hellwig]

On Sun, Aug 23, 2009 at 03:04:46PM +0300, Boaz Harrosh wrote:
> On 08/20/2009 02:49 AM, Christoph Hellwig wrote:
> > Any chance you could also remove the lock_super usage once your start
> > revisiting the lock?  lock_super is never taken by the VFS anymore, so
> > you can easily replace it with fs-local locking.
> > 
> 
> OK Sure, thanks.
> 
> One question please?
> 
> I need a mutex_lock I can sleep on. Could I use the inode_lock associate
> with the root_inode. Or that could lead to dead-locks with the VFS?

It could lead to all kinds of lock dependency problems no one even
thinks about.

> (Or should I just allocate another mutex at the fs-sb-data?)

Yes.

Re: [PATCH] exofs: remove BKL from super operations

[Boaz Harrosh]

On 08/23/2009 04:31 PM, Christoph Hellwig wrote:
> On Sun, Aug 23, 2009 at 03:04:46PM +0300, Boaz Harrosh wrote:
>> On 08/20/2009 02:49 AM, Christoph Hellwig wrote:
>>> Any chance you could also remove the lock_super usage once your start
>>> revisiting the lock?  lock_super is never taken by the VFS anymore, so
>>> you can easily replace it with fs-local locking.
>>>
>>
>> OK Sure, thanks.
>>
>> One question please?
>>
>> I need a mutex_lock I can sleep on. Could I use the inode_lock associate
>> with the root_inode. Or that could lead to dead-locks with the VFS?
> 
> It could lead to all kinds of lock dependency problems no one even
> thinks about.
> 
>> (Or should I just allocate another mutex at the fs-sb-data?)
> 
> Yes.
> 

Thanks, I'll do that