linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 3/8] ext3: remove lock/unlock super
@ 2012-08-16 10:00 Marco Stornelli
  2012-08-16 16:39 ` Jan Kara
  0 siblings, 1 reply; 6+ messages in thread
From: Marco Stornelli @ 2012-08-16 10:00 UTC (permalink / raw)
  To: bharrosh, bhalevy, jack, Andrew Morton, adilger.kernel, tytso,
	hirofumi, mikulas, Al Viro, hch, dushistov, osd-dev, Linux Kernel,
	linux-ext4, Linux FS Devel

From: Marco Stornelli <marco.stornelli@gmail.com>

Replaced lock and unlock super with a new fs mutex s_lock.

Signed-off-by: Marco Stornelli <marco.stornelli@gmail.com>
---

diff -Nurp linux-3.6-rc1-orig/fs/ext3/ext3.h linux-3.6-rc1/fs/ext3/ext3.h
--- linux-3.6-rc1-orig/fs/ext3/ext3.h	2012-08-16 09:37:31.000000000 +0200
+++ linux-3.6-rc1/fs/ext3/ext3.h	2012-08-16 09:46:39.000000000 +0200
@@ -672,6 +672,7 @@ struct ext3_sb_info {
 	char *s_qf_names[MAXQUOTAS];		/* Names of quota files with journalled quota */
 	int s_jquota_fmt;			/* Format of quota to use */
 #endif
+	struct mutex s_lock;
 };
  static inline spinlock_t *
diff -Nurp linux-3.6-rc1-orig/fs/ext3/super.c linux-3.6-rc1/fs/ext3/super.c
--- linux-3.6-rc1-orig/fs/ext3/super.c	2012-08-16 09:37:31.000000000 +0200
+++ linux-3.6-rc1/fs/ext3/super.c	2012-08-16 09:49:11.000000000 +0200
@@ -1939,6 +1939,7 @@ static int ext3_fill_super (struct super
 	sbi->s_gdb_count = db_count;
 	get_random_bytes(&sbi->s_next_generation, sizeof(u32));
 	spin_lock_init(&sbi->s_next_gen_lock);
+	mutex_init(&sbi->s_lock);
  	/* per fileystem reservation list head & lock */
 	spin_lock_init(&sbi->s_rsv_window_lock);
@@ -2582,11 +2583,11 @@ out:
 static int ext3_unfreeze(struct super_block *sb)
 {
 	if (!(sb->s_flags & MS_RDONLY)) {
-		lock_super(sb);
+		mutex_lock(&EXT3_SB(sb)->s_lock);
 		/* Reser the needs_recovery flag before the fs is unlocked. */
 		EXT3_SET_INCOMPAT_FEATURE(sb, EXT3_FEATURE_INCOMPAT_RECOVER);
 		ext3_commit_super(sb, EXT3_SB(sb)->s_es, 1);
-		unlock_super(sb);
+		mutex_unlock(&EXT3_SB(sb)->s_lock);
 		journal_unlock_updates(EXT3_SB(sb)->s_journal);
 	}
 	return 0;
@@ -2606,7 +2607,7 @@ static int ext3_remount (struct super_bl
 #endif
  	/* Store the original options */
-	lock_super(sb);
+	mutex_lock(&EXT3_SB(sb)->s_lock);
 	old_sb_flags = sb->s_flags;
 	old_opts.s_mount_opt = sbi->s_mount_opt;
 	old_opts.s_resuid = sbi->s_resuid;
@@ -2712,7 +2713,7 @@ static int ext3_remount (struct super_bl
 		    old_opts.s_qf_names[i] != sbi->s_qf_names[i])
 			kfree(old_opts.s_qf_names[i]);
 #endif
-	unlock_super(sb);
+	mutex_unlock(&EXT3_SB(sb)->s_lock);
  	if (enable_quota)
 		dquot_resume(sb, -1);
@@ -2732,7 +2733,7 @@ restore_opts:
 		sbi->s_qf_names[i] = old_opts.s_qf_names[i];
 	}
 #endif
-	unlock_super(sb);
+	mutex_unlock(&EXT3_SB(sb)->s_lock);
 	return err;
 }
 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/8] ext3: remove lock/unlock super
  2012-08-16 10:00 [PATCH 3/8] ext3: remove lock/unlock super Marco Stornelli
@ 2012-08-16 16:39 ` Jan Kara
  2012-08-16 19:19   ` Theodore Ts'o
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Kara @ 2012-08-16 16:39 UTC (permalink / raw)
  To: Marco Stornelli
  Cc: bharrosh, bhalevy, jack, Andrew Morton, adilger.kernel, tytso,
	hirofumi, mikulas, Al Viro, hch, dushistov, osd-dev, Linux Kernel,
	linux-ext4, Linux FS Devel

On Thu 16-08-12 12:00:26, Marco Stornelli wrote:
> From: Marco Stornelli <marco.stornelli@gmail.com>
> 
> Replaced lock and unlock super with a new fs mutex s_lock.
  Hum, is the lock needed at all? Remount & unfreeze both run with s_umount
held for writing. Thus we already have exclusion between these two calls.
The same seems to hold for ext4 BTW.

								Honza
> 
> Signed-off-by: Marco Stornelli <marco.stornelli@gmail.com>
> ---
> 
> diff -Nurp linux-3.6-rc1-orig/fs/ext3/ext3.h linux-3.6-rc1/fs/ext3/ext3.h
> --- linux-3.6-rc1-orig/fs/ext3/ext3.h	2012-08-16 09:37:31.000000000 +0200
> +++ linux-3.6-rc1/fs/ext3/ext3.h	2012-08-16 09:46:39.000000000 +0200
> @@ -672,6 +672,7 @@ struct ext3_sb_info {
>  	char *s_qf_names[MAXQUOTAS];		/* Names of quota files with journalled quota */
>  	int s_jquota_fmt;			/* Format of quota to use */
>  #endif
> +	struct mutex s_lock;
>  };
>   static inline spinlock_t *
> diff -Nurp linux-3.6-rc1-orig/fs/ext3/super.c linux-3.6-rc1/fs/ext3/super.c
> --- linux-3.6-rc1-orig/fs/ext3/super.c	2012-08-16 09:37:31.000000000 +0200
> +++ linux-3.6-rc1/fs/ext3/super.c	2012-08-16 09:49:11.000000000 +0200
> @@ -1939,6 +1939,7 @@ static int ext3_fill_super (struct super
>  	sbi->s_gdb_count = db_count;
>  	get_random_bytes(&sbi->s_next_generation, sizeof(u32));
>  	spin_lock_init(&sbi->s_next_gen_lock);
> +	mutex_init(&sbi->s_lock);
>   	/* per fileystem reservation list head & lock */
>  	spin_lock_init(&sbi->s_rsv_window_lock);
> @@ -2582,11 +2583,11 @@ out:
>  static int ext3_unfreeze(struct super_block *sb)
>  {
>  	if (!(sb->s_flags & MS_RDONLY)) {
> -		lock_super(sb);
> +		mutex_lock(&EXT3_SB(sb)->s_lock);
>  		/* Reser the needs_recovery flag before the fs is unlocked. */
>  		EXT3_SET_INCOMPAT_FEATURE(sb, EXT3_FEATURE_INCOMPAT_RECOVER);
>  		ext3_commit_super(sb, EXT3_SB(sb)->s_es, 1);
> -		unlock_super(sb);
> +		mutex_unlock(&EXT3_SB(sb)->s_lock);
>  		journal_unlock_updates(EXT3_SB(sb)->s_journal);
>  	}
>  	return 0;
> @@ -2606,7 +2607,7 @@ static int ext3_remount (struct super_bl
>  #endif
>   	/* Store the original options */
> -	lock_super(sb);
> +	mutex_lock(&EXT3_SB(sb)->s_lock);
>  	old_sb_flags = sb->s_flags;
>  	old_opts.s_mount_opt = sbi->s_mount_opt;
>  	old_opts.s_resuid = sbi->s_resuid;
> @@ -2712,7 +2713,7 @@ static int ext3_remount (struct super_bl
>  		    old_opts.s_qf_names[i] != sbi->s_qf_names[i])
>  			kfree(old_opts.s_qf_names[i]);
>  #endif
> -	unlock_super(sb);
> +	mutex_unlock(&EXT3_SB(sb)->s_lock);
>   	if (enable_quota)
>  		dquot_resume(sb, -1);
> @@ -2732,7 +2733,7 @@ restore_opts:
>  		sbi->s_qf_names[i] = old_opts.s_qf_names[i];
>  	}
>  #endif
> -	unlock_super(sb);
> +	mutex_unlock(&EXT3_SB(sb)->s_lock);
>  	return err;
>  }
>  
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/8] ext3: remove lock/unlock super
  2012-08-16 16:39 ` Jan Kara
@ 2012-08-16 19:19   ` Theodore Ts'o
  2012-08-17  6:47     ` Marco Stornelli
  0 siblings, 1 reply; 6+ messages in thread
From: Theodore Ts'o @ 2012-08-16 19:19 UTC (permalink / raw)
  To: Jan Kara
  Cc: Marco Stornelli, bharrosh, bhalevy, Andrew Morton, adilger.kernel,
	hirofumi, mikulas, Al Viro, hch, dushistov, osd-dev, Linux Kernel,
	linux-ext4, Linux FS Devel

On Thu, Aug 16, 2012 at 06:39:04PM +0200, Jan Kara wrote:
> On Thu 16-08-12 12:00:26, Marco Stornelli wrote:
> > From: Marco Stornelli <marco.stornelli@gmail.com>
> > 
> > Replaced lock and unlock super with a new fs mutex s_lock.
>   Hum, is the lock needed at all? Remount & unfreeze both run with s_umount
> held for writing. Thus we already have exclusion between these two calls.
> The same seems to hold for ext4 BTW.

Agreed, it's not clear lock_super() is needed at all for ext4 at this
point.

	     	       		       	      - Ted

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/8] ext3: remove lock/unlock super
  2012-08-16 19:19   ` Theodore Ts'o
@ 2012-08-17  6:47     ` Marco Stornelli
  2012-08-17 23:08       ` Theodore Ts'o
  0 siblings, 1 reply; 6+ messages in thread
From: Marco Stornelli @ 2012-08-17  6:47 UTC (permalink / raw)
  To: Theodore Ts'o, Jan Kara, Andrew Morton, adilger.kernel,
	dushistov
  Cc: bharrosh, bhalevy, hirofumi, mikulas, Al Viro, hch, osd-dev,
	Linux Kernel, linux-ext4, Linux FS Devel

Il 16/08/2012 21:19, Theodore Ts'o ha scritto:
> On Thu, Aug 16, 2012 at 06:39:04PM +0200, Jan Kara wrote:
>> On Thu 16-08-12 12:00:26, Marco Stornelli wrote:
>>> From: Marco Stornelli <marco.stornelli@gmail.com>
>>>
>>> Replaced lock and unlock super with a new fs mutex s_lock.
>>    Hum, is the lock needed at all? Remount & unfreeze both run with s_umount
>> held for writing. Thus we already have exclusion between these two calls.
>> The same seems to hold for ext4 BTW.
>
> Agreed, it's not clear lock_super() is needed at all for ext4 at this
> point.
>
> 	     	       		       	      - Ted
>

Great. I'll remove the calls for ext3/ext4 when I'll submit the second 
version of the patch.

Thanks for your feedback,

Marco

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/8] ext3: remove lock/unlock super
  2012-08-17  6:47     ` Marco Stornelli
@ 2012-08-17 23:08       ` Theodore Ts'o
  2012-08-18  7:06         ` Marco Stornelli
  0 siblings, 1 reply; 6+ messages in thread
From: Theodore Ts'o @ 2012-08-17 23:08 UTC (permalink / raw)
  To: Marco Stornelli
  Cc: Jan Kara, Andrew Morton, adilger.kernel, dushistov, bharrosh,
	bhalevy, hirofumi, mikulas, Al Viro, hch, osd-dev, Linux Kernel,
	linux-ext4, Linux FS Devel

On Fri, Aug 17, 2012 at 08:47:04AM +0200, Marco Stornelli wrote:
> Great. I'll remove the calls for ext3/ext4 when I'll submit the
> second version of the patch.

FYI, I have the following patch my ext4 tree, so I could do more
intensive testing.  I'll let folks know if anything goes horribly
wrong, but I'm pretty sure this should be safe.

       	   	     	      	   	    - Ted

>From 81f74e743344217487792e4190f9478963d1bb21 Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <tytso@mit.edu>
Date: Fri, 17 Aug 2012 19:00:34 -0400
Subject: [PATCH] ext4: drop lock_super()/unlock_super()

We don't need lock_super()/unlock_super() any more, since the places
where it is used, we are protected by the s_umount r/w semaphore.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Cc: Marco Stornelli <marco.stornelli@gmail.com>
---
 fs/ext4/super.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 3ab798d..bae4124 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -861,7 +861,6 @@ static void ext4_put_super(struct super_block *sb)
 	flush_workqueue(sbi->dio_unwritten_wq);
 	destroy_workqueue(sbi->dio_unwritten_wq);
 
-	lock_super(sb);
 	if (sbi->s_journal) {
 		err = jbd2_journal_destroy(sbi->s_journal);
 		sbi->s_journal = NULL;
@@ -928,7 +927,6 @@ static void ext4_put_super(struct super_block *sb)
 	 * Now that we are completely done shutting down the
 	 * superblock, we need to actually destroy the kobject.
 	 */
-	unlock_super(sb);
 	kobject_put(&sbi->s_kobj);
 	wait_for_completion(&sbi->s_kobj_unregister);
 	if (sbi->s_chksum_driver)
@@ -4535,11 +4533,9 @@ static int ext4_unfreeze(struct super_block *sb)
 	if (sb->s_flags & MS_RDONLY)
 		return 0;
 
-	lock_super(sb);
 	/* Reset the needs_recovery flag before the fs is unlocked. */
 	EXT4_SET_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_RECOVER);
 	ext4_commit_super(sb, 1);
-	unlock_super(sb);
 	return 0;
 }
 
@@ -4575,7 +4571,6 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
 	char *orig_data = kstrdup(data, GFP_KERNEL);
 
 	/* Store the original options */
-	lock_super(sb);
 	old_sb_flags = sb->s_flags;
 	old_opts.s_mount_opt = sbi->s_mount_opt;
 	old_opts.s_mount_opt2 = sbi->s_mount_opt2;
@@ -4717,7 +4712,6 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
 	if (sbi->s_journal == NULL)
 		ext4_commit_super(sb, 1);
 
-	unlock_super(sb);
 #ifdef CONFIG_QUOTA
 	/* Release old quota file names */
 	for (i = 0; i < MAXQUOTAS; i++)
@@ -4730,10 +4724,8 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
 		else if (EXT4_HAS_RO_COMPAT_FEATURE(sb,
 					EXT4_FEATURE_RO_COMPAT_QUOTA)) {
 			err = ext4_enable_quotas(sb);
-			if (err) {
-				lock_super(sb);
+			if (err)
 				goto restore_opts;
-			}
 		}
 	}
 #endif
@@ -4760,7 +4752,6 @@ restore_opts:
 		sbi->s_qf_names[i] = old_opts.s_qf_names[i];
 	}
 #endif
-	unlock_super(sb);
 	kfree(orig_data);
 	return err;
 }
-- 
1.7.12.rc0.22.gcdd159b


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/8] ext3: remove lock/unlock super
  2012-08-17 23:08       ` Theodore Ts'o
@ 2012-08-18  7:06         ` Marco Stornelli
  0 siblings, 0 replies; 6+ messages in thread
From: Marco Stornelli @ 2012-08-18  7:06 UTC (permalink / raw)
  To: Theodore Ts'o, Al Viro
  Cc: Jan Kara, Andrew Morton, adilger.kernel, dushistov, bharrosh,
	bhalevy, hirofumi, mikulas, hch, osd-dev, Linux Kernel,
	linux-ext4, Linux FS Devel

Il 18/08/2012 01:08, Theodore Ts'o ha scritto:
> On Fri, Aug 17, 2012 at 08:47:04AM +0200, Marco Stornelli wrote:
>> Great. I'll remove the calls for ext3/ext4 when I'll submit the
>> second version of the patch.
>
> FYI, I have the following patch my ext4 tree, so I could do more
> intensive testing.  I'll let folks know if anything goes horribly
> wrong, but I'm pretty sure this should be safe.

Yeah, I'm agree. It should be safe.

>
>         	   	     	      	   	    - Ted
>
>>From 81f74e743344217487792e4190f9478963d1bb21 Mon Sep 17 00:00:00 2001
> From: Theodore Ts'o <tytso@mit.edu>
> Date: Fri, 17 Aug 2012 19:00:34 -0400
> Subject: [PATCH] ext4: drop lock_super()/unlock_super()
>
> We don't need lock_super()/unlock_super() any more, since the places
> where it is used, we are protected by the s_umount r/w semaphore.
>
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> Cc: Marco Stornelli <marco.stornelli@gmail.com>
> ---

I think the patches will go via Al's tree so if Al is agree I'll submit 
the patches without modify ext4 at this point. Al, let me know if we 
want to proceed in a different way.

Marco

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-08-18  7:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-16 10:00 [PATCH 3/8] ext3: remove lock/unlock super Marco Stornelli
2012-08-16 16:39 ` Jan Kara
2012-08-16 19:19   ` Theodore Ts'o
2012-08-17  6:47     ` Marco Stornelli
2012-08-17 23:08       ` Theodore Ts'o
2012-08-18  7:06         ` Marco Stornelli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).