| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20090428160236.GI21950@atrey.karlin.mff.cuni.cz>
Date: Tue, 28 Apr 2009 18:02:37 +0200
From: Jan Kara <jack@...e.cz>
To: Theodore Ts'o <tytso@....edu>
Cc: Ext4 Developers List <linux-ext4@...r.kernel.org>,
Linux Kernel Developers List <linux-kernel@...r.kernel.org>,
Christoph Hellwig <hch@...radead.org>,
Al Viro <viro@...IV.linux.org.uk>
Subject: Re: [PATCH 5/5] ext4: Replace lock/unlock_super() with an explicit lock for resizing
> Use a separate lock to protect s_groups_count and the other block
> group descriptors which get changed via an on-line resize operation,
> so we can stop overloading the use of lock_super().
This patch looks good as well. You can add
Acked-by: Jan Kara <jack@...e.cz>
>
> Signed-off-by: "Theodore Ts'o" <tytso@....edu>
> ---
> fs/ext4/ext4_sb.h | 1 +
> fs/ext4/resize.c | 35 ++++++++++++++++++-----------------
> fs/ext4/super.c | 1 +
> 3 files changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/fs/ext4/ext4_sb.h b/fs/ext4/ext4_sb.h
> index 4bda2f7..2d36223 100644
> --- a/fs/ext4/ext4_sb.h
> +++ b/fs/ext4/ext4_sb.h
> @@ -72,6 +72,7 @@ struct ext4_sb_info {
> struct journal_s *s_journal;
> struct list_head s_orphan;
> struct mutex s_orphan_lock;
> + struct mutex s_resize_lock;
> unsigned long s_commit_interval;
> u32 s_max_batch_time;
> u32 s_min_batch_time;
> diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
> index 546c7dd..e8ded13 100644
> --- a/fs/ext4/resize.c
> +++ b/fs/ext4/resize.c
> @@ -193,7 +193,7 @@ static int setup_new_group_blocks(struct super_block *sb,
> if (IS_ERR(handle))
> return PTR_ERR(handle);
>
> - lock_super(sb);
> + mutex_lock(&sbi->s_resize_lock);
> if (input->group != sbi->s_groups_count) {
> err = -EBUSY;
> goto exit_journal;
> @@ -302,7 +302,7 @@ exit_bh:
> brelse(bh);
>
> exit_journal:
> - unlock_super(sb);
> + mutex_unlock(&sbi->s_resize_lock);
> if ((err2 = ext4_journal_stop(handle)) && !err)
> err = err2;
>
> @@ -643,11 +643,12 @@ exit_free:
> * important part is that the new block and inode counts are in the backup
> * superblocks, and the location of the new group metadata in the GDT backups.
> *
> - * We do not need lock_super() for this, because these blocks are not
> - * otherwise touched by the filesystem code when it is mounted. We don't
> - * need to worry about last changing from sbi->s_groups_count, because the
> - * worst that can happen is that we do not copy the full number of backups
> - * at this time. The resize which changed s_groups_count will backup again.
> + * We do not need take the s_resize_lock for this, because these
> + * blocks are not otherwise touched by the filesystem code when it is
> + * mounted. We don't need to worry about last changing from
> + * sbi->s_groups_count, because the worst that can happen is that we
> + * do not copy the full number of backups at this time. The resize
> + * which changed s_groups_count will backup again.
> */
> static void update_backups(struct super_block *sb,
> int blk_off, char *data, int size)
> @@ -809,7 +810,7 @@ int ext4_group_add(struct super_block *sb, struct ext4_new_group_data *input)
> goto exit_put;
> }
>
> - lock_super(sb);
> + mutex_lock(&sbi->s_resize_lock);
> if (input->group != sbi->s_groups_count) {
> ext4_warning(sb, __func__,
> "multiple resizers run on filesystem!");
> @@ -840,7 +841,7 @@ int ext4_group_add(struct super_block *sb, struct ext4_new_group_data *input)
> /*
> * OK, now we've set up the new group. Time to make it active.
> *
> - * Current kernels don't lock all allocations via lock_super(),
> + * We do not lock all allocations via s_resize_lock
> * so we have to be safe wrt. concurrent accesses the group
> * data. So we need to be careful to set all of the relevant
> * group descriptor data etc. *before* we enable the group.
> @@ -900,12 +901,12 @@ int ext4_group_add(struct super_block *sb, struct ext4_new_group_data *input)
> *
> * The precise rules we use are:
> *
> - * * Writers of s_groups_count *must* hold lock_super
> + * * Writers of s_groups_count *must* hold s_resize_lock
> * AND
> * * Writers must perform a smp_wmb() after updating all dependent
> * data and before modifying the groups count
> *
> - * * Readers must hold lock_super() over the access
> + * * Readers must hold s_resize_lock over the access
> * OR
> * * Readers must perform an smp_rmb() after reading the groups count
> * and before reading any dependent data.
> @@ -948,7 +949,7 @@ int ext4_group_add(struct super_block *sb, struct ext4_new_group_data *input)
> sb->s_dirt = 1;
>
> exit_journal:
> - unlock_super(sb);
> + mutex_unlock(&sbi->s_resize_lock);
> if ((err2 = ext4_journal_stop(handle)) && !err)
> err = err2;
> if (!err) {
> @@ -986,7 +987,7 @@ int ext4_group_extend(struct super_block *sb, struct ext4_super_block *es,
>
> /* We don't need to worry about locking wrt other resizers just
> * yet: we're going to revalidate es->s_blocks_count after
> - * taking lock_super() below. */
> + * taking the s_resize_lock below. */
> o_blocks_count = ext4_blocks_count(es);
> o_groups_count = EXT4_SB(sb)->s_groups_count;
>
> @@ -1056,11 +1057,11 @@ int ext4_group_extend(struct super_block *sb, struct ext4_super_block *es,
> goto exit_put;
> }
>
> - lock_super(sb);
> + mutex_lock(&EXT4_SB(sb)->s_resize_lock);
> if (o_blocks_count != ext4_blocks_count(es)) {
> ext4_warning(sb, __func__,
> "multiple resizers run on filesystem!");
> - unlock_super(sb);
> + mutex_unlock(&EXT4_SB(sb)->s_resize_lock);
> ext4_journal_stop(handle);
> err = -EBUSY;
> goto exit_put;
> @@ -1070,14 +1071,14 @@ int ext4_group_extend(struct super_block *sb, struct ext4_super_block *es,
> EXT4_SB(sb)->s_sbh))) {
> ext4_warning(sb, __func__,
> "error %d on journal write access", err);
> - unlock_super(sb);
> + mutex_unlock(&EXT4_SB(sb)->s_resize_lock);
> ext4_journal_stop(handle);
> goto exit_put;
> }
> ext4_blocks_count_set(es, o_blocks_count + add);
> ext4_handle_dirty_metadata(handle, NULL, EXT4_SB(sb)->s_sbh);
> sb->s_dirt = 1;
> - unlock_super(sb);
> + mutex_unlock(&EXT4_SB(sb)->s_resize_lock);
> ext4_debug("freeing blocks %llu through %llu\n", o_blocks_count,
> o_blocks_count + add);
> /* We add the blocks to the bitmap and set the group need init bit */
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index c23e82c..b4735e4 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -2624,6 +2624,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
> #endif
> INIT_LIST_HEAD(&sbi->s_orphan); /* unlinked but open files */
> mutex_init(&sbi->s_orphan_lock);
> + mutex_init(&sbi->s_resize_lock);
>
> sb->s_root = NULL;
>
> --
> 1.5.6.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Jan Kara <jack@...e.cz>
SuSE CR Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists