This patch eliminates the sd_statfs_mutex variable when doing
updates to local statfs change files. The new code uses the
sd_statfs_spin spin_lock that's already taken to get the same
effect.
On Tue, 2007-12-11 at 19:04 -0600, Bob Peterson wrote:
> Hi,
>
> This patch eliminates the sd_statfs_mutex variable when doing
> updates to local statfs change files. The new code uses the
> sd_statfs_spin spin_lock that's already taken to get the same
> effect.
>
> Regards,
>
> Bob Peterson
> Red Hat GFS
>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> --
> .../fs/gfs2/incore.h | 1 -
> .../fs/gfs2/ops_fstype.c | 1 -
> .../fs/gfs2/super.c | 10 ++--------
> 3 files changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/gfs2-2.6.git.patch3/fs/gfs2/incore.h b/gfs2-2.6.git.patch4/fs/gfs2/incore.h
> index 14862d1..9a83429 100644
> --- a/gfs2-2.6.git.patch3/fs/gfs2/incore.h
> +++ b/gfs2-2.6.git.patch4/fs/gfs2/incore.h
> @@ -527,7 +527,6 @@ struct gfs2_sbd {
> /* StatFS stuff */
>
> spinlock_t sd_statfs_spin;
> - struct mutex sd_statfs_mutex;
> struct gfs2_statfs_change_host sd_statfs_master;
> struct gfs2_statfs_change_host sd_statfs_local;
> unsigned long sd_statfs_sync_time;
> diff --git a/gfs2-2.6.git.patch3/fs/gfs2/ops_fstype.c b/gfs2-2.6.git.patch4/fs/gfs2/ops_fstype.c
> index 35ec630..22e260e 100644
> --- a/gfs2-2.6.git.patch3/fs/gfs2/ops_fstype.c
> +++ b/gfs2-2.6.git.patch4/fs/gfs2/ops_fstype.c
> @@ -60,7 +60,6 @@ static struct gfs2_sbd *init_sbd(struct super_block *sb)
>
> mutex_init(&sdp->sd_inum_mutex);
> spin_lock_init(&sdp->sd_statfs_spin);
> - mutex_init(&sdp->sd_statfs_mutex);
>
> spin_lock_init(&sdp->sd_rindex_spin);
> mutex_init(&sdp->sd_rindex_mutex);
> diff --git a/gfs2-2.6.git.patch3/fs/gfs2/super.c b/gfs2-2.6.git.patch4/fs/gfs2/super.c
> index dda7747..da5b29c 100644
> --- a/gfs2-2.6.git.patch3/fs/gfs2/super.c
> +++ b/gfs2-2.6.git.patch4/fs/gfs2/super.c
> @@ -686,11 +686,8 @@ void gfs2_statfs_change(struct gfs2_sbd *sdp, s64 total, s64 free,
> if (error)
> return;
>
> - mutex_lock(&sdp->sd_statfs_mutex);
> - gfs2_trans_add_bh(l_ip->i_gl, l_bh, 1);
> - mutex_unlock(&sdp->sd_statfs_mutex);
> -
> spin_lock(&sdp->sd_statfs_spin);
> + gfs2_trans_add_bh(l_ip->i_gl, l_bh, 1);
You can't do gfs2_trans_add_bh under a spinlock, but there is no reason
why you can't just reverse the order of these two statements to fix it,
Steve.
12-12-2007, 02:31 PM
Bob Peterson
Get rid of sd_statfs_mutex
On Wed, 2007-12-12 at 09:18 +0000, Steven Whitehouse wrote:
> You can't do gfs2_trans_add_bh under a spinlock, but there is no reason
> why you can't just reverse the order of these two statements to fix it,
>
> Steve.
Hi Steve,
If we reverse the two statements, the trans_add_bh is not protected at
all, which I assume was the purpose of the mutex in the first place.
I'm not sure this is buying us much anyway, so perhaps we should forget
it.
Regards,
Bob Peterson
12-12-2007, 02:56 PM
Steven Whitehouse
Get rid of sd_statfs_mutex
Hi,
On Wed, 2007-12-12 at 09:31 -0600, Bob Peterson wrote:
> On Wed, 2007-12-12 at 09:18 +0000, Steven Whitehouse wrote:
> > You can't do gfs2_trans_add_bh under a spinlock, but there is no reason
> > why you can't just reverse the order of these two statements to fix it,
> >
> > Steve.
>
> Hi Steve,
>
> If we reverse the two statements, the trans_add_bh is not protected at
> all, which I assume was the purpose of the mutex in the first place.
> I'm not sure this is buying us much anyway, so perhaps we should forget
> it.
>
> Regards,
>
> Bob Peterson
>
>
I don't see why it needs to be "protected", I think that mutex is just a
bug in that case unless you can prove otherwise... it seems to be doing
nothing,
Steve.
12-12-2007, 03:44 PM
Bob Peterson
Get rid of sd_statfs_mutex
On Wed, 2007-12-12 at 15:56 +0000, Steven Whitehouse wrote:
> I don't see why it needs to be "protected", I think that mutex is just a
> bug in that case unless you can prove otherwise... it seems to be doing
> nothing,
>
> Steve.