* [Cluster-devel] [PATCH] gfs2: Don't withdraw under a spin lock
@ 2018-06-07 11:09 Andreas Gruenbacher
2018-06-13 16:32 ` Andrew Price
0 siblings, 1 reply; 3+ messages in thread
From: Andreas Gruenbacher @ 2018-06-07 11:09 UTC (permalink / raw)
To: cluster-devel.redhat.com
In two places, the gfs2_io_error_bh macro is called while holding the
sd_ail_lock spin lock. This isn't allowed because gfs2_io_error_bh
withdraws the filesystem, which can sleep because it issues a uevent.
To fix that, add a __gfs2_io_error_bh macro that doesn't withdraw the
filesystem, and withdraw the filesystem after releasing sd_ail_lock
where necessary.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/gfs2/log.c | 30 +++++++++++++++++++++---------
fs/gfs2/util.c | 38 ++++++++++++++++++++------------------
fs/gfs2/util.h | 10 +++++++---
3 files changed, 48 insertions(+), 30 deletions(-)
diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index 0248835625f1..b8e937190212 100644
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -92,7 +92,8 @@ static void gfs2_remove_from_ail(struct gfs2_bufdata *bd)
static int gfs2_ail1_start_one(struct gfs2_sbd *sdp,
struct writeback_control *wbc,
- struct gfs2_trans *tr)
+ struct gfs2_trans *tr,
+ bool *withdraw)
__releases(&sdp->sd_ail_lock)
__acquires(&sdp->sd_ail_lock)
{
@@ -107,8 +108,10 @@ __acquires(&sdp->sd_ail_lock)
gfs2_assert(sdp, bd->bd_tr == tr);
if (!buffer_busy(bh)) {
- if (!buffer_uptodate(bh))
- gfs2_io_error_bh(sdp, bh);
+ if (!buffer_uptodate(bh)) {
+ __gfs2_io_error_bh(sdp, bh);
+ *withdraw = true;
+ }
list_move(&bd->bd_ail_st_list, &tr->tr_ail2_list);
continue;
}
@@ -148,6 +151,7 @@ void gfs2_ail1_flush(struct gfs2_sbd *sdp, struct writeback_control *wbc)
struct list_head *head = &sdp->sd_ail1_list;
struct gfs2_trans *tr;
struct blk_plug plug;
+ bool withdraw = false;
trace_gfs2_ail_flush(sdp, wbc, 1);
blk_start_plug(&plug);
@@ -156,11 +160,13 @@ void gfs2_ail1_flush(struct gfs2_sbd *sdp, struct writeback_control *wbc)
list_for_each_entry_reverse(tr, head, tr_list) {
if (wbc->nr_to_write <= 0)
break;
- if (gfs2_ail1_start_one(sdp, wbc, tr))
+ if (gfs2_ail1_start_one(sdp, wbc, tr, &withdraw))
goto restart;
}
spin_unlock(&sdp->sd_ail_lock);
blk_finish_plug(&plug);
+ if (withdraw)
+ gfs2_lm_withdraw(sdp, NULL);
trace_gfs2_ail_flush(sdp, wbc, 0);
}
@@ -188,7 +194,8 @@ static void gfs2_ail1_start(struct gfs2_sbd *sdp)
*
*/
-static void gfs2_ail1_empty_one(struct gfs2_sbd *sdp, struct gfs2_trans *tr)
+static void gfs2_ail1_empty_one(struct gfs2_sbd *sdp, struct gfs2_trans *tr,
+ bool *withdraw)
{
struct gfs2_bufdata *bd, *s;
struct buffer_head *bh;
@@ -199,11 +206,12 @@ static void gfs2_ail1_empty_one(struct gfs2_sbd *sdp, struct gfs2_trans *tr)
gfs2_assert(sdp, bd->bd_tr == tr);
if (buffer_busy(bh))
continue;
- if (!buffer_uptodate(bh))
- gfs2_io_error_bh(sdp, bh);
+ if (!buffer_uptodate(bh)) {
+ __gfs2_io_error_bh(sdp, bh);
+ *withdraw = true;
+ }
list_move(&bd->bd_ail_st_list, &tr->tr_ail2_list);
}
-
}
/**
@@ -218,10 +226,11 @@ static int gfs2_ail1_empty(struct gfs2_sbd *sdp)
struct gfs2_trans *tr, *s;
int oldest_tr = 1;
int ret;
+ bool withdraw = false;
spin_lock(&sdp->sd_ail_lock);
list_for_each_entry_safe_reverse(tr, s, &sdp->sd_ail1_list, tr_list) {
- gfs2_ail1_empty_one(sdp, tr);
+ gfs2_ail1_empty_one(sdp, tr, &withdraw);
if (list_empty(&tr->tr_ail1_list) && oldest_tr)
list_move(&tr->tr_list, &sdp->sd_ail2_list);
else
@@ -230,6 +239,9 @@ static int gfs2_ail1_empty(struct gfs2_sbd *sdp)
ret = list_empty(&sdp->sd_ail1_list);
spin_unlock(&sdp->sd_ail_lock);
+ if (withdraw)
+ gfs2_lm_withdraw(sdp, "fatal: I/O error(s)\n");
+
return ret;
}
diff --git a/fs/gfs2/util.c b/fs/gfs2/util.c
index 763d659db91b..59c811de0dc7 100644
--- a/fs/gfs2/util.c
+++ b/fs/gfs2/util.c
@@ -46,14 +46,16 @@ int gfs2_lm_withdraw(struct gfs2_sbd *sdp, const char *fmt, ...)
test_and_set_bit(SDF_SHUTDOWN, &sdp->sd_flags))
return 0;
- va_start(args, fmt);
+ if (fmt) {
+ va_start(args, fmt);
- vaf.fmt = fmt;
- vaf.va = &args;
+ vaf.fmt = fmt;
+ vaf.va = &args;
- fs_err(sdp, "%pV", &vaf);
+ fs_err(sdp, "%pV", &vaf);
- va_end(args);
+ va_end(args);
+ }
if (sdp->sd_args.ar_errors == GFS2_ERRORS_WITHDRAW) {
fs_err(sdp, "about to withdraw this file system\n");
@@ -246,21 +248,21 @@ int gfs2_io_error_i(struct gfs2_sbd *sdp, const char *function, char *file,
}
/**
- * gfs2_io_error_bh_i - Flag a buffer I/O error and withdraw
- * Returns: -1 if this call withdrew the machine,
- * 0 if it was already withdrawn
+ * gfs2_io_error_bh_i - Flag a buffer I/O error
+ * @withdraw: withdraw the filesystem
*/
-int gfs2_io_error_bh_i(struct gfs2_sbd *sdp, struct buffer_head *bh,
- const char *function, char *file, unsigned int line)
+void gfs2_io_error_bh_i(struct gfs2_sbd *sdp, struct buffer_head *bh,
+ const char *function, char *file, unsigned int line,
+ bool withdraw)
{
- int rv;
- rv = gfs2_lm_withdraw(sdp,
- "fatal: I/O error\n"
- " block = %llu\n"
- " function = %s, file = %s, line = %u\n",
- (unsigned long long)bh->b_blocknr,
- function, file, line);
- return rv;
+ fs_err(sdp,
+ "fatal: I/O error\n"
+ " block = %llu\n"
+ " function = %s, file = %s, line = %u\n",
+ (unsigned long long)bh->b_blocknr,
+ function, file, line);
+ if (withdraw)
+ gfs2_lm_withdraw(sdp, NULL);
}
diff --git a/fs/gfs2/util.h b/fs/gfs2/util.h
index 3926f95a6eb7..9d9583e842ac 100644
--- a/fs/gfs2/util.h
+++ b/fs/gfs2/util.h
@@ -136,11 +136,15 @@ int gfs2_io_error_i(struct gfs2_sbd *sdp, const char *function,
gfs2_io_error_i((sdp), __func__, __FILE__, __LINE__);
-int gfs2_io_error_bh_i(struct gfs2_sbd *sdp, struct buffer_head *bh,
- const char *function, char *file, unsigned int line);
+void gfs2_io_error_bh_i(struct gfs2_sbd *sdp, struct buffer_head *bh,
+ const char *function, char *file, unsigned int line,
+ bool withdraw);
#define gfs2_io_error_bh(sdp, bh) \
-gfs2_io_error_bh_i((sdp), (bh), __func__, __FILE__, __LINE__);
+gfs2_io_error_bh_i((sdp), (bh), __func__, __FILE__, __LINE__, true);
+
+#define __gfs2_io_error_bh(sdp, bh) \
+gfs2_io_error_bh_i((sdp), (bh), __func__, __FILE__, __LINE__, false);
extern struct kmem_cache *gfs2_glock_cachep;
--
2.17.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* [Cluster-devel] [PATCH] gfs2: Don't withdraw under a spin lock
2018-06-07 11:09 [Cluster-devel] [PATCH] gfs2: Don't withdraw under a spin lock Andreas Gruenbacher
@ 2018-06-13 16:32 ` Andrew Price
2018-06-14 13:41 ` Andreas Gruenbacher
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Price @ 2018-06-13 16:32 UTC (permalink / raw)
To: cluster-devel.redhat.com
On 07/06/18 12:09, Andreas Gruenbacher wrote:
> In two places, the gfs2_io_error_bh macro is called while holding the
> sd_ail_lock spin lock. This isn't allowed because gfs2_io_error_bh
> withdraws the filesystem, which can sleep because it issues a uevent.
> To fix that, add a __gfs2_io_error_bh macro that doesn't withdraw the
> filesystem, and withdraw the filesystem after releasing sd_ail_lock
> where necessary.
>
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
> fs/gfs2/log.c | 30 +++++++++++++++++++++---------
> fs/gfs2/util.c | 38 ++++++++++++++++++++------------------
> fs/gfs2/util.h | 10 +++++++---
> 3 files changed, 48 insertions(+), 30 deletions(-)
>
> diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
> index 0248835625f1..b8e937190212 100644
> --- a/fs/gfs2/log.c
> +++ b/fs/gfs2/log.c
> @@ -92,7 +92,8 @@ static void gfs2_remove_from_ail(struct gfs2_bufdata *bd)
>
> static int gfs2_ail1_start_one(struct gfs2_sbd *sdp,
> struct writeback_control *wbc,
> - struct gfs2_trans *tr)
> + struct gfs2_trans *tr,
> + bool *withdraw)
> __releases(&sdp->sd_ail_lock)
> __acquires(&sdp->sd_ail_lock)
> {
> @@ -107,8 +108,10 @@ __acquires(&sdp->sd_ail_lock)
> gfs2_assert(sdp, bd->bd_tr == tr);
>
> if (!buffer_busy(bh)) {
> - if (!buffer_uptodate(bh))
> - gfs2_io_error_bh(sdp, bh);
> + if (!buffer_uptodate(bh)) {
> + __gfs2_io_error_bh(sdp, bh);
> + *withdraw = true;
> + }
> list_move(&bd->bd_ail_st_list, &tr->tr_ail2_list);
> continue;
> }
> @@ -148,6 +151,7 @@ void gfs2_ail1_flush(struct gfs2_sbd *sdp, struct writeback_control *wbc)
> struct list_head *head = &sdp->sd_ail1_list;
> struct gfs2_trans *tr;
> struct blk_plug plug;
> + bool withdraw = false;
>
> trace_gfs2_ail_flush(sdp, wbc, 1);
> blk_start_plug(&plug);
> @@ -156,11 +160,13 @@ void gfs2_ail1_flush(struct gfs2_sbd *sdp, struct writeback_control *wbc)
> list_for_each_entry_reverse(tr, head, tr_list) {
> if (wbc->nr_to_write <= 0)
> break;
> - if (gfs2_ail1_start_one(sdp, wbc, tr))
> + if (gfs2_ail1_start_one(sdp, wbc, tr, &withdraw))
> goto restart;
> }
> spin_unlock(&sdp->sd_ail_lock);
> blk_finish_plug(&plug);
> + if (withdraw)
> + gfs2_lm_withdraw(sdp, NULL);
> trace_gfs2_ail_flush(sdp, wbc, 0);
> }
>
> @@ -188,7 +194,8 @@ static void gfs2_ail1_start(struct gfs2_sbd *sdp)
> *
> */
>
> -static void gfs2_ail1_empty_one(struct gfs2_sbd *sdp, struct gfs2_trans *tr)
> +static void gfs2_ail1_empty_one(struct gfs2_sbd *sdp, struct gfs2_trans *tr,
> + bool *withdraw)
> {
> struct gfs2_bufdata *bd, *s;
> struct buffer_head *bh;
> @@ -199,11 +206,12 @@ static void gfs2_ail1_empty_one(struct gfs2_sbd *sdp, struct gfs2_trans *tr)
> gfs2_assert(sdp, bd->bd_tr == tr);
> if (buffer_busy(bh))
> continue;
> - if (!buffer_uptodate(bh))
> - gfs2_io_error_bh(sdp, bh);
> + if (!buffer_uptodate(bh)) {
> + __gfs2_io_error_bh(sdp, bh);
> + *withdraw = true;
> + }
> list_move(&bd->bd_ail_st_list, &tr->tr_ail2_list);
> }
> -
> }
>
> /**
> @@ -218,10 +226,11 @@ static int gfs2_ail1_empty(struct gfs2_sbd *sdp)
> struct gfs2_trans *tr, *s;
> int oldest_tr = 1;
> int ret;
> + bool withdraw = false;
>
> spin_lock(&sdp->sd_ail_lock);
> list_for_each_entry_safe_reverse(tr, s, &sdp->sd_ail1_list, tr_list) {
> - gfs2_ail1_empty_one(sdp, tr);
> + gfs2_ail1_empty_one(sdp, tr, &withdraw);
> if (list_empty(&tr->tr_ail1_list) && oldest_tr)
> list_move(&tr->tr_list, &sdp->sd_ail2_list);
> else
> @@ -230,6 +239,9 @@ static int gfs2_ail1_empty(struct gfs2_sbd *sdp)
> ret = list_empty(&sdp->sd_ail1_list);
> spin_unlock(&sdp->sd_ail_lock);
>
> + if (withdraw)
> + gfs2_lm_withdraw(sdp, "fatal: I/O error(s)\n");
> +
> return ret;
> }
>
> diff --git a/fs/gfs2/util.c b/fs/gfs2/util.c
> index 763d659db91b..59c811de0dc7 100644
> --- a/fs/gfs2/util.c
> +++ b/fs/gfs2/util.c
> @@ -46,14 +46,16 @@ int gfs2_lm_withdraw(struct gfs2_sbd *sdp, const char *fmt, ...)
> test_and_set_bit(SDF_SHUTDOWN, &sdp->sd_flags))
> return 0;
>
> - va_start(args, fmt);
> + if (fmt) {
> + va_start(args, fmt);
>
> - vaf.fmt = fmt;
> - vaf.va = &args;
> + vaf.fmt = fmt;
> + vaf.va = &args;
>
> - fs_err(sdp, "%pV", &vaf);
> + fs_err(sdp, "%pV", &vaf);
>
> - va_end(args);
> + va_end(args);
> + }
>
> if (sdp->sd_args.ar_errors == GFS2_ERRORS_WITHDRAW) {
> fs_err(sdp, "about to withdraw this file system\n");
> @@ -246,21 +248,21 @@ int gfs2_io_error_i(struct gfs2_sbd *sdp, const char *function, char *file,
> }
>
> /**
> - * gfs2_io_error_bh_i - Flag a buffer I/O error and withdraw
> - * Returns: -1 if this call withdrew the machine,
> - * 0 if it was already withdrawn
> + * gfs2_io_error_bh_i - Flag a buffer I/O error
> + * @withdraw: withdraw the filesystem
> */
>
> -int gfs2_io_error_bh_i(struct gfs2_sbd *sdp, struct buffer_head *bh,
> - const char *function, char *file, unsigned int line)
> +void gfs2_io_error_bh_i(struct gfs2_sbd *sdp, struct buffer_head *bh,
> + const char *function, char *file, unsigned int line,
> + bool withdraw)
> {
> - int rv;
> - rv = gfs2_lm_withdraw(sdp,
> - "fatal: I/O error\n"
> - " block = %llu\n"
> - " function = %s, file = %s, line = %u\n",
> - (unsigned long long)bh->b_blocknr,
> - function, file, line);
> - return rv;
> + fs_err(sdp,
> + "fatal: I/O error\n"
> + " block = %llu\n"
> + " function = %s, file = %s, line = %u\n",
> + (unsigned long long)bh->b_blocknr,
> + function, file, line);
> + if (withdraw)
> + gfs2_lm_withdraw(sdp, NULL);
> }
>
> diff --git a/fs/gfs2/util.h b/fs/gfs2/util.h
> index 3926f95a6eb7..9d9583e842ac 100644
> --- a/fs/gfs2/util.h
> +++ b/fs/gfs2/util.h
> @@ -136,11 +136,15 @@ int gfs2_io_error_i(struct gfs2_sbd *sdp, const char *function,
> gfs2_io_error_i((sdp), __func__, __FILE__, __LINE__);
>
>
> -int gfs2_io_error_bh_i(struct gfs2_sbd *sdp, struct buffer_head *bh,
> - const char *function, char *file, unsigned int line);
> +void gfs2_io_error_bh_i(struct gfs2_sbd *sdp, struct buffer_head *bh,
> + const char *function, char *file, unsigned int line,
> + bool withdraw);
>
> #define gfs2_io_error_bh(sdp, bh) \
> -gfs2_io_error_bh_i((sdp), (bh), __func__, __FILE__, __LINE__);
> +gfs2_io_error_bh_i((sdp), (bh), __func__, __FILE__, __LINE__, true);
> +
> +#define __gfs2_io_error_bh(sdp, bh) \
> +gfs2_io_error_bh_i((sdp), (bh), __func__, __FILE__, __LINE__, false);
Perhaps these could be named something like gfs2_io_error_bh_wd and
gfs2_io_error_bh to indicate the difference.
Otherwise, this looks good to me.
Andy
>
>
> extern struct kmem_cache *gfs2_glock_cachep;
>
^ permalink raw reply [flat|nested] 3+ messages in thread* [Cluster-devel] [PATCH] gfs2: Don't withdraw under a spin lock
2018-06-13 16:32 ` Andrew Price
@ 2018-06-14 13:41 ` Andreas Gruenbacher
0 siblings, 0 replies; 3+ messages in thread
From: Andreas Gruenbacher @ 2018-06-14 13:41 UTC (permalink / raw)
To: cluster-devel.redhat.com
On 13 June 2018 at 18:32, Andrew Price <anprice@redhat.com> wrote:
> On 07/06/18 12:09, Andreas Gruenbacher wrote:
>>
>> In two places, the gfs2_io_error_bh macro is called while holding the
>> sd_ail_lock spin lock. This isn't allowed because gfs2_io_error_bh
>> withdraws the filesystem, which can sleep because it issues a uevent.
>> To fix that, add a __gfs2_io_error_bh macro that doesn't withdraw the
>> filesystem, and withdraw the filesystem after releasing sd_ail_lock
>> where necessary.
>>
>> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
>> ---
>> fs/gfs2/log.c | 30 +++++++++++++++++++++---------
>> fs/gfs2/util.c | 38 ++++++++++++++++++++------------------
>> fs/gfs2/util.h | 10 +++++++---
>> 3 files changed, 48 insertions(+), 30 deletions(-)
>>
>> diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
>> index 0248835625f1..b8e937190212 100644
>> --- a/fs/gfs2/log.c
>> +++ b/fs/gfs2/log.c
>> @@ -92,7 +92,8 @@ static void gfs2_remove_from_ail(struct gfs2_bufdata
>> *bd)
>> static int gfs2_ail1_start_one(struct gfs2_sbd *sdp,
>> struct writeback_control *wbc,
>> - struct gfs2_trans *tr)
>> + struct gfs2_trans *tr,
>> + bool *withdraw)
>> __releases(&sdp->sd_ail_lock)
>> __acquires(&sdp->sd_ail_lock)
>> {
>> @@ -107,8 +108,10 @@ __acquires(&sdp->sd_ail_lock)
>> gfs2_assert(sdp, bd->bd_tr == tr);
>> if (!buffer_busy(bh)) {
>> - if (!buffer_uptodate(bh))
>> - gfs2_io_error_bh(sdp, bh);
>> + if (!buffer_uptodate(bh)) {
>> + __gfs2_io_error_bh(sdp, bh);
>> + *withdraw = true;
>> + }
>> list_move(&bd->bd_ail_st_list, &tr->tr_ail2_list);
>> continue;
>> }
>> @@ -148,6 +151,7 @@ void gfs2_ail1_flush(struct gfs2_sbd *sdp, struct
>> writeback_control *wbc)
>> struct list_head *head = &sdp->sd_ail1_list;
>> struct gfs2_trans *tr;
>> struct blk_plug plug;
>> + bool withdraw = false;
>> trace_gfs2_ail_flush(sdp, wbc, 1);
>> blk_start_plug(&plug);
>> @@ -156,11 +160,13 @@ void gfs2_ail1_flush(struct gfs2_sbd *sdp, struct
>> writeback_control *wbc)
>> list_for_each_entry_reverse(tr, head, tr_list) {
>> if (wbc->nr_to_write <= 0)
>> break;
>> - if (gfs2_ail1_start_one(sdp, wbc, tr))
>> + if (gfs2_ail1_start_one(sdp, wbc, tr, &withdraw))
>> goto restart;
>> }
>> spin_unlock(&sdp->sd_ail_lock);
>> blk_finish_plug(&plug);
>> + if (withdraw)
>> + gfs2_lm_withdraw(sdp, NULL);
>> trace_gfs2_ail_flush(sdp, wbc, 0);
>> }
>> @@ -188,7 +194,8 @@ static void gfs2_ail1_start(struct gfs2_sbd *sdp)
>> *
>> */
>> -static void gfs2_ail1_empty_one(struct gfs2_sbd *sdp, struct gfs2_trans
>> *tr)
>> +static void gfs2_ail1_empty_one(struct gfs2_sbd *sdp, struct gfs2_trans
>> *tr,
>> + bool *withdraw)
>> {
>> struct gfs2_bufdata *bd, *s;
>> struct buffer_head *bh;
>> @@ -199,11 +206,12 @@ static void gfs2_ail1_empty_one(struct gfs2_sbd
>> *sdp, struct gfs2_trans *tr)
>> gfs2_assert(sdp, bd->bd_tr == tr);
>> if (buffer_busy(bh))
>> continue;
>> - if (!buffer_uptodate(bh))
>> - gfs2_io_error_bh(sdp, bh);
>> + if (!buffer_uptodate(bh)) {
>> + __gfs2_io_error_bh(sdp, bh);
>> + *withdraw = true;
>> + }
>> list_move(&bd->bd_ail_st_list, &tr->tr_ail2_list);
>> }
>> -
>> }
>> /**
>> @@ -218,10 +226,11 @@ static int gfs2_ail1_empty(struct gfs2_sbd *sdp)
>> struct gfs2_trans *tr, *s;
>> int oldest_tr = 1;
>> int ret;
>> + bool withdraw = false;
>> spin_lock(&sdp->sd_ail_lock);
>> list_for_each_entry_safe_reverse(tr, s, &sdp->sd_ail1_list,
>> tr_list) {
>> - gfs2_ail1_empty_one(sdp, tr);
>> + gfs2_ail1_empty_one(sdp, tr, &withdraw);
>> if (list_empty(&tr->tr_ail1_list) && oldest_tr)
>> list_move(&tr->tr_list, &sdp->sd_ail2_list);
>> else
>> @@ -230,6 +239,9 @@ static int gfs2_ail1_empty(struct gfs2_sbd *sdp)
>> ret = list_empty(&sdp->sd_ail1_list);
>> spin_unlock(&sdp->sd_ail_lock);
>> + if (withdraw)
>> + gfs2_lm_withdraw(sdp, "fatal: I/O error(s)\n");
>> +
>> return ret;
>> }
>> diff --git a/fs/gfs2/util.c b/fs/gfs2/util.c
>> index 763d659db91b..59c811de0dc7 100644
>> --- a/fs/gfs2/util.c
>> +++ b/fs/gfs2/util.c
>> @@ -46,14 +46,16 @@ int gfs2_lm_withdraw(struct gfs2_sbd *sdp, const char
>> *fmt, ...)
>> test_and_set_bit(SDF_SHUTDOWN, &sdp->sd_flags))
>> return 0;
>> - va_start(args, fmt);
>> + if (fmt) {
>> + va_start(args, fmt);
>> - vaf.fmt = fmt;
>> - vaf.va = &args;
>> + vaf.fmt = fmt;
>> + vaf.va = &args;
>> - fs_err(sdp, "%pV", &vaf);
>> + fs_err(sdp, "%pV", &vaf);
>> - va_end(args);
>> + va_end(args);
>> + }
>> if (sdp->sd_args.ar_errors == GFS2_ERRORS_WITHDRAW) {
>> fs_err(sdp, "about to withdraw this file system\n");
>> @@ -246,21 +248,21 @@ int gfs2_io_error_i(struct gfs2_sbd *sdp, const char
>> *function, char *file,
>> }
>> /**
>> - * gfs2_io_error_bh_i - Flag a buffer I/O error and withdraw
>> - * Returns: -1 if this call withdrew the machine,
>> - * 0 if it was already withdrawn
>> + * gfs2_io_error_bh_i - Flag a buffer I/O error
>> + * @withdraw: withdraw the filesystem
>> */
>> -int gfs2_io_error_bh_i(struct gfs2_sbd *sdp, struct buffer_head *bh,
>> - const char *function, char *file, unsigned int
>> line)
>> +void gfs2_io_error_bh_i(struct gfs2_sbd *sdp, struct buffer_head *bh,
>> + const char *function, char *file, unsigned int
>> line,
>> + bool withdraw)
>> {
>> - int rv;
>> - rv = gfs2_lm_withdraw(sdp,
>> - "fatal: I/O error\n"
>> - " block = %llu\n"
>> - " function = %s, file = %s, line = %u\n",
>> - (unsigned long long)bh->b_blocknr,
>> - function, file, line);
>> - return rv;
>> + fs_err(sdp,
>> + "fatal: I/O error\n"
>> + " block = %llu\n"
>> + " function = %s, file = %s, line = %u\n",
>> + (unsigned long long)bh->b_blocknr,
>> + function, file, line);
>> + if (withdraw)
>> + gfs2_lm_withdraw(sdp, NULL);
>> }
>> diff --git a/fs/gfs2/util.h b/fs/gfs2/util.h
>> index 3926f95a6eb7..9d9583e842ac 100644
>> --- a/fs/gfs2/util.h
>> +++ b/fs/gfs2/util.h
>> @@ -136,11 +136,15 @@ int gfs2_io_error_i(struct gfs2_sbd *sdp, const char
>> *function,
>> gfs2_io_error_i((sdp), __func__, __FILE__, __LINE__);
>> -int gfs2_io_error_bh_i(struct gfs2_sbd *sdp, struct buffer_head *bh,
>> - const char *function, char *file, unsigned int
>> line);
>> +void gfs2_io_error_bh_i(struct gfs2_sbd *sdp, struct buffer_head *bh,
>> + const char *function, char *file, unsigned int
>> line,
>> + bool withdraw);
>> #define gfs2_io_error_bh(sdp, bh) \
>> -gfs2_io_error_bh_i((sdp), (bh), __func__, __FILE__, __LINE__);
>> +gfs2_io_error_bh_i((sdp), (bh), __func__, __FILE__, __LINE__, true);
>> +
>> +#define __gfs2_io_error_bh(sdp, bh) \
>> +gfs2_io_error_bh_i((sdp), (bh), __func__, __FILE__, __LINE__, false);
>
>
> Perhaps these could be named something like gfs2_io_error_bh_wd and
> gfs2_io_error_bh to indicate the difference.
Yes, makes sense. I've pushed this to the new for-later branch on
kernel.org now.
Thanks,
Andreas
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-06-14 13:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-07 11:09 [Cluster-devel] [PATCH] gfs2: Don't withdraw under a spin lock Andreas Gruenbacher
2018-06-13 16:32 ` Andrew Price
2018-06-14 13:41 ` Andreas Gruenbacher
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).