[Cluster-devel] [GFS2 PATCH] gfs2: don't hold the sd_jindex_spin during recovery

[Cluster-devel] [GFS2 PATCH] gfs2: don't hold the sd_jindex_spin during recovery

[Bob Peterson]

Hi,

The sd_jindex_spin is used to serialize access to the sd_jindex_list.
Before this patch function gfs2_recover_set would hold the
spin_lock while recovery is running. Since recovery may take a very
long time, other processes needing to use the list would
monopolize a CPU for a very long time, spinning. This patch allows
it to unlock the spin_lock before calling gfs2_recover_journal.
The test_and_set_bit there should prevent multiple processes from
trying to recover the same journal.

This is only a problem when multiple processes attempt recovery,
which is possible via (1) a uevent kicking a 1 into the sysfs file
/sys/fs/gfs2/<table>/lock_module/recover, while the gfs2_control_func
in lock_dlm also calls gfs2_recover_set().

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/sys.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
index 0c2a60fa66d7f..9fcb66d882b45 100644
--- a/fs/gfs2/sys.c
+++ b/fs/gfs2/sys.c
@@ -424,8 +424,8 @@ int gfs2_recover_set(struct gfs2_sbd *sdp, unsigned jid)
 	list_for_each_entry(jd, &sdp->sd_jindex_list, jd_list) {
 		if (jd->jd_jid != jid && !sdp->sd_args.ar_spectator)
 			continue;
-		rv = gfs2_recover_journal(jd, false);
-		break;
+		spin_unlock(&sdp->sd_jindex_spin);
+		return gfs2_recover_journal(jd, false);
 	}
 out:
 	spin_unlock(&sdp->sd_jindex_spin);

[Cluster-devel] [GFS2 PATCH] gfs2: don't hold the sd_jindex_spin during recovery

[Steven Whitehouse]

Hi,


On 17/08/18 16:53, Bob Peterson wrote:
> Hi,
>
> The sd_jindex_spin is used to serialize access to the sd_jindex_list.
> Before this patch function gfs2_recover_set would hold the
> spin_lock while recovery is running. Since recovery may take a very
> long time, other processes needing to use the list would
> monopolize a CPU for a very long time, spinning. This patch allows
> it to unlock the spin_lock before calling gfs2_recover_journal.
> The test_and_set_bit there should prevent multiple processes from
> trying to recover the same journal.
>
> This is only a problem when multiple processes attempt recovery,
> which is possible via (1) a uevent kicking a 1 into the sysfs file
> /sys/fs/gfs2/<table>/lock_module/recover, while the gfs2_control_func
> in lock_dlm also calls gfs2_recover_set().
>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> ---
>   fs/gfs2/sys.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
> index 0c2a60fa66d7f..9fcb66d882b45 100644
> --- a/fs/gfs2/sys.c
> +++ b/fs/gfs2/sys.c
> @@ -424,8 +424,8 @@ int gfs2_recover_set(struct gfs2_sbd *sdp, unsigned jid)
>   	list_for_each_entry(jd, &sdp->sd_jindex_list, jd_list) {
>   		if (jd->jd_jid != jid && !sdp->sd_args.ar_spectator)
>   			continue;
> -		rv = gfs2_recover_journal(jd, false);
> -		break;
> +		spin_unlock(&sdp->sd_jindex_spin);
> +		return gfs2_recover_journal(jd, false);
Since the wait parameter is false here, all gfs2_recover_journal does is 
queue some work, and that should not block. Also it breaks the locking 
between the JDF_RECOVERY flag and the queuing of the work too,

Steve.

>   	}
>   out:
>   	spin_unlock(&sdp->sd_jindex_spin);
>