diff options
author | Andreas Gruenbacher <agruenba@redhat.com> | 2018-06-07 06:56:46 -0400 |
---|---|---|
committer | Bob Peterson <rpeterso@redhat.com> | 2018-06-21 08:39:44 -0400 |
commit | 9e1a9ecd13b9bb421c88135b178577caf4d54f6a (patch) | |
tree | be518fb982b7f8f47c3ed3fa864772d4dd974985 | |
parent | f85c10e24ab9fd8ccb6de3d6061a3110ff3581df (diff) |
gfs2: Don't withdraw under a spin lock
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_wd macro that does withdraw the
filesystem and change gfs2_io_error_bh to not withdraw the filesystem.
In those places where the new gfs2_io_error_bh is used, withdraw the
filesystem after releasing sd_ail_lock.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Signed-off-by: Bob Peterson <rpeterso@redhat.com>
Reviewed-by: Andrew Price <anprice@redhat.com>
-rw-r--r-- | fs/gfs2/log.c | 26 | ||||
-rw-r--r-- | fs/gfs2/lops.c | 2 | ||||
-rw-r--r-- | fs/gfs2/meta_io.c | 4 | ||||
-rw-r--r-- | fs/gfs2/util.c | 38 | ||||
-rw-r--r-- | fs/gfs2/util.h | 10 |
5 files changed, 49 insertions, 31 deletions
diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c index 0248835625f1..a767fad02386 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) | |||
92 | 92 | ||
93 | static int gfs2_ail1_start_one(struct gfs2_sbd *sdp, | 93 | static int gfs2_ail1_start_one(struct gfs2_sbd *sdp, |
94 | struct writeback_control *wbc, | 94 | struct writeback_control *wbc, |
95 | struct gfs2_trans *tr) | 95 | struct gfs2_trans *tr, |
96 | bool *withdraw) | ||
96 | __releases(&sdp->sd_ail_lock) | 97 | __releases(&sdp->sd_ail_lock) |
97 | __acquires(&sdp->sd_ail_lock) | 98 | __acquires(&sdp->sd_ail_lock) |
98 | { | 99 | { |
@@ -107,8 +108,10 @@ __acquires(&sdp->sd_ail_lock) | |||
107 | gfs2_assert(sdp, bd->bd_tr == tr); | 108 | gfs2_assert(sdp, bd->bd_tr == tr); |
108 | 109 | ||
109 | if (!buffer_busy(bh)) { | 110 | if (!buffer_busy(bh)) { |
110 | if (!buffer_uptodate(bh)) | 111 | if (!buffer_uptodate(bh)) { |
111 | gfs2_io_error_bh(sdp, bh); | 112 | gfs2_io_error_bh(sdp, bh); |
113 | *withdraw = true; | ||
114 | } | ||
112 | list_move(&bd->bd_ail_st_list, &tr->tr_ail2_list); | 115 | list_move(&bd->bd_ail_st_list, &tr->tr_ail2_list); |
113 | continue; | 116 | continue; |
114 | } | 117 | } |
@@ -148,6 +151,7 @@ void gfs2_ail1_flush(struct gfs2_sbd *sdp, struct writeback_control *wbc) | |||
148 | struct list_head *head = &sdp->sd_ail1_list; | 151 | struct list_head *head = &sdp->sd_ail1_list; |
149 | struct gfs2_trans *tr; | 152 | struct gfs2_trans *tr; |
150 | struct blk_plug plug; | 153 | struct blk_plug plug; |
154 | bool withdraw = false; | ||
151 | 155 | ||
152 | trace_gfs2_ail_flush(sdp, wbc, 1); | 156 | trace_gfs2_ail_flush(sdp, wbc, 1); |
153 | blk_start_plug(&plug); | 157 | blk_start_plug(&plug); |
@@ -156,11 +160,13 @@ restart: | |||
156 | list_for_each_entry_reverse(tr, head, tr_list) { | 160 | list_for_each_entry_reverse(tr, head, tr_list) { |
157 | if (wbc->nr_to_write <= 0) | 161 | if (wbc->nr_to_write <= 0) |
158 | break; | 162 | break; |
159 | if (gfs2_ail1_start_one(sdp, wbc, tr)) | 163 | if (gfs2_ail1_start_one(sdp, wbc, tr, &withdraw)) |
160 | goto restart; | 164 | goto restart; |
161 | } | 165 | } |
162 | spin_unlock(&sdp->sd_ail_lock); | 166 | spin_unlock(&sdp->sd_ail_lock); |
163 | blk_finish_plug(&plug); | 167 | blk_finish_plug(&plug); |
168 | if (withdraw) | ||
169 | gfs2_lm_withdraw(sdp, NULL); | ||
164 | trace_gfs2_ail_flush(sdp, wbc, 0); | 170 | trace_gfs2_ail_flush(sdp, wbc, 0); |
165 | } | 171 | } |
166 | 172 | ||
@@ -188,7 +194,8 @@ static void gfs2_ail1_start(struct gfs2_sbd *sdp) | |||
188 | * | 194 | * |
189 | */ | 195 | */ |
190 | 196 | ||
191 | static void gfs2_ail1_empty_one(struct gfs2_sbd *sdp, struct gfs2_trans *tr) | 197 | static void gfs2_ail1_empty_one(struct gfs2_sbd *sdp, struct gfs2_trans *tr, |
198 | bool *withdraw) | ||
192 | { | 199 | { |
193 | struct gfs2_bufdata *bd, *s; | 200 | struct gfs2_bufdata *bd, *s; |
194 | struct buffer_head *bh; | 201 | struct buffer_head *bh; |
@@ -199,11 +206,12 @@ static void gfs2_ail1_empty_one(struct gfs2_sbd *sdp, struct gfs2_trans *tr) | |||
199 | gfs2_assert(sdp, bd->bd_tr == tr); | 206 | gfs2_assert(sdp, bd->bd_tr == tr); |
200 | if (buffer_busy(bh)) | 207 | if (buffer_busy(bh)) |
201 | continue; | 208 | continue; |
202 | if (!buffer_uptodate(bh)) | 209 | if (!buffer_uptodate(bh)) { |
203 | gfs2_io_error_bh(sdp, bh); | 210 | gfs2_io_error_bh(sdp, bh); |
211 | *withdraw = true; | ||
212 | } | ||
204 | list_move(&bd->bd_ail_st_list, &tr->tr_ail2_list); | 213 | list_move(&bd->bd_ail_st_list, &tr->tr_ail2_list); |
205 | } | 214 | } |
206 | |||
207 | } | 215 | } |
208 | 216 | ||
209 | /** | 217 | /** |
@@ -218,10 +226,11 @@ static int gfs2_ail1_empty(struct gfs2_sbd *sdp) | |||
218 | struct gfs2_trans *tr, *s; | 226 | struct gfs2_trans *tr, *s; |
219 | int oldest_tr = 1; | 227 | int oldest_tr = 1; |
220 | int ret; | 228 | int ret; |
229 | bool withdraw = false; | ||
221 | 230 | ||
222 | spin_lock(&sdp->sd_ail_lock); | 231 | spin_lock(&sdp->sd_ail_lock); |
223 | list_for_each_entry_safe_reverse(tr, s, &sdp->sd_ail1_list, tr_list) { | 232 | list_for_each_entry_safe_reverse(tr, s, &sdp->sd_ail1_list, tr_list) { |
224 | gfs2_ail1_empty_one(sdp, tr); | 233 | gfs2_ail1_empty_one(sdp, tr, &withdraw); |
225 | if (list_empty(&tr->tr_ail1_list) && oldest_tr) | 234 | if (list_empty(&tr->tr_ail1_list) && oldest_tr) |
226 | list_move(&tr->tr_list, &sdp->sd_ail2_list); | 235 | list_move(&tr->tr_list, &sdp->sd_ail2_list); |
227 | else | 236 | else |
@@ -230,6 +239,9 @@ static int gfs2_ail1_empty(struct gfs2_sbd *sdp) | |||
230 | ret = list_empty(&sdp->sd_ail1_list); | 239 | ret = list_empty(&sdp->sd_ail1_list); |
231 | spin_unlock(&sdp->sd_ail_lock); | 240 | spin_unlock(&sdp->sd_ail_lock); |
232 | 241 | ||
242 | if (withdraw) | ||
243 | gfs2_lm_withdraw(sdp, "fatal: I/O error(s)\n"); | ||
244 | |||
233 | return ret; | 245 | return ret; |
234 | } | 246 | } |
235 | 247 | ||
diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c index 4d6567990baf..f2567f958d00 100644 --- a/fs/gfs2/lops.c +++ b/fs/gfs2/lops.c | |||
@@ -49,7 +49,7 @@ void gfs2_pin(struct gfs2_sbd *sdp, struct buffer_head *bh) | |||
49 | if (test_set_buffer_pinned(bh)) | 49 | if (test_set_buffer_pinned(bh)) |
50 | gfs2_assert_withdraw(sdp, 0); | 50 | gfs2_assert_withdraw(sdp, 0); |
51 | if (!buffer_uptodate(bh)) | 51 | if (!buffer_uptodate(bh)) |
52 | gfs2_io_error_bh(sdp, bh); | 52 | gfs2_io_error_bh_wd(sdp, bh); |
53 | bd = bh->b_private; | 53 | bd = bh->b_private; |
54 | /* If this buffer is in the AIL and it has already been written | 54 | /* If this buffer is in the AIL and it has already been written |
55 | * to in-place disk block, remove it from the AIL. | 55 | * to in-place disk block, remove it from the AIL. |
diff --git a/fs/gfs2/meta_io.c b/fs/gfs2/meta_io.c index 52de1036d9f9..be9c0bf697fe 100644 --- a/fs/gfs2/meta_io.c +++ b/fs/gfs2/meta_io.c | |||
@@ -293,7 +293,7 @@ int gfs2_meta_read(struct gfs2_glock *gl, u64 blkno, int flags, | |||
293 | if (unlikely(!buffer_uptodate(bh))) { | 293 | if (unlikely(!buffer_uptodate(bh))) { |
294 | struct gfs2_trans *tr = current->journal_info; | 294 | struct gfs2_trans *tr = current->journal_info; |
295 | if (tr && test_bit(TR_TOUCHED, &tr->tr_flags)) | 295 | if (tr && test_bit(TR_TOUCHED, &tr->tr_flags)) |
296 | gfs2_io_error_bh(sdp, bh); | 296 | gfs2_io_error_bh_wd(sdp, bh); |
297 | brelse(bh); | 297 | brelse(bh); |
298 | *bhp = NULL; | 298 | *bhp = NULL; |
299 | return -EIO; | 299 | return -EIO; |
@@ -320,7 +320,7 @@ int gfs2_meta_wait(struct gfs2_sbd *sdp, struct buffer_head *bh) | |||
320 | if (!buffer_uptodate(bh)) { | 320 | if (!buffer_uptodate(bh)) { |
321 | struct gfs2_trans *tr = current->journal_info; | 321 | struct gfs2_trans *tr = current->journal_info; |
322 | if (tr && test_bit(TR_TOUCHED, &tr->tr_flags)) | 322 | if (tr && test_bit(TR_TOUCHED, &tr->tr_flags)) |
323 | gfs2_io_error_bh(sdp, bh); | 323 | gfs2_io_error_bh_wd(sdp, bh); |
324 | return -EIO; | 324 | return -EIO; |
325 | } | 325 | } |
326 | if (unlikely(test_bit(SDF_SHUTDOWN, &sdp->sd_flags))) | 326 | if (unlikely(test_bit(SDF_SHUTDOWN, &sdp->sd_flags))) |
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, ...) | |||
46 | test_and_set_bit(SDF_SHUTDOWN, &sdp->sd_flags)) | 46 | test_and_set_bit(SDF_SHUTDOWN, &sdp->sd_flags)) |
47 | return 0; | 47 | return 0; |
48 | 48 | ||
49 | va_start(args, fmt); | 49 | if (fmt) { |
50 | va_start(args, fmt); | ||
50 | 51 | ||
51 | vaf.fmt = fmt; | 52 | vaf.fmt = fmt; |
52 | vaf.va = &args; | 53 | vaf.va = &args; |
53 | 54 | ||
54 | fs_err(sdp, "%pV", &vaf); | 55 | fs_err(sdp, "%pV", &vaf); |
55 | 56 | ||
56 | va_end(args); | 57 | va_end(args); |
58 | } | ||
57 | 59 | ||
58 | if (sdp->sd_args.ar_errors == GFS2_ERRORS_WITHDRAW) { | 60 | if (sdp->sd_args.ar_errors == GFS2_ERRORS_WITHDRAW) { |
59 | fs_err(sdp, "about to withdraw this file system\n"); | 61 | 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, | |||
246 | } | 248 | } |
247 | 249 | ||
248 | /** | 250 | /** |
249 | * gfs2_io_error_bh_i - Flag a buffer I/O error and withdraw | 251 | * gfs2_io_error_bh_i - Flag a buffer I/O error |
250 | * Returns: -1 if this call withdrew the machine, | 252 | * @withdraw: withdraw the filesystem |
251 | * 0 if it was already withdrawn | ||
252 | */ | 253 | */ |
253 | 254 | ||
254 | int gfs2_io_error_bh_i(struct gfs2_sbd *sdp, struct buffer_head *bh, | 255 | void gfs2_io_error_bh_i(struct gfs2_sbd *sdp, struct buffer_head *bh, |
255 | const char *function, char *file, unsigned int line) | 256 | const char *function, char *file, unsigned int line, |
257 | bool withdraw) | ||
256 | { | 258 | { |
257 | int rv; | 259 | fs_err(sdp, |
258 | rv = gfs2_lm_withdraw(sdp, | 260 | "fatal: I/O error\n" |
259 | "fatal: I/O error\n" | 261 | " block = %llu\n" |
260 | " block = %llu\n" | 262 | " function = %s, file = %s, line = %u\n", |
261 | " function = %s, file = %s, line = %u\n", | 263 | (unsigned long long)bh->b_blocknr, |
262 | (unsigned long long)bh->b_blocknr, | 264 | function, file, line); |
263 | function, file, line); | 265 | if (withdraw) |
264 | return rv; | 266 | gfs2_lm_withdraw(sdp, NULL); |
265 | } | 267 | } |
266 | 268 | ||
diff --git a/fs/gfs2/util.h b/fs/gfs2/util.h index 3926f95a6eb7..96ac4aba4738 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, | |||
136 | gfs2_io_error_i((sdp), __func__, __FILE__, __LINE__); | 136 | gfs2_io_error_i((sdp), __func__, __FILE__, __LINE__); |
137 | 137 | ||
138 | 138 | ||
139 | int gfs2_io_error_bh_i(struct gfs2_sbd *sdp, struct buffer_head *bh, | 139 | void gfs2_io_error_bh_i(struct gfs2_sbd *sdp, struct buffer_head *bh, |
140 | const char *function, char *file, unsigned int line); | 140 | const char *function, char *file, unsigned int line, |
141 | bool withdraw); | ||
142 | |||
143 | #define gfs2_io_error_bh_wd(sdp, bh) \ | ||
144 | gfs2_io_error_bh_i((sdp), (bh), __func__, __FILE__, __LINE__, true); | ||
141 | 145 | ||
142 | #define gfs2_io_error_bh(sdp, bh) \ | 146 | #define gfs2_io_error_bh(sdp, bh) \ |
143 | gfs2_io_error_bh_i((sdp), (bh), __func__, __FILE__, __LINE__); | 147 | gfs2_io_error_bh_i((sdp), (bh), __func__, __FILE__, __LINE__, false); |
144 | 148 | ||
145 | 149 | ||
146 | extern struct kmem_cache *gfs2_glock_cachep; | 150 | extern struct kmem_cache *gfs2_glock_cachep; |