lkml.org 
[lkml]   [2009]   [Apr]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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@suse.cz>
    >
    > Signed-off-by: "Theodore Ts'o" <tytso@mit.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@vger.kernel.org
    > More majordomo info at http://vger.kernel.org/majordomo-info.html
    --
    Jan Kara <jack@suse.cz>
    SuSE CR Labs


    \
     
     \ /
      Last update: 2009-04-28 18:07    [W:0.035 / U:30.432 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site