Commit c503a62103c46d56447f56306b52be6f844689ba
1 parent
6d768177c2
Exists in
smarc-l5.0.0_1.0.0-ga
and in
5 other branches
dlm: fix conversion deadlock from recovery
The process of rebuilding locks on a new master during recovery could re-order the locks on the convert queue, creating an "in place" conversion deadlock that would not be resolved. Fix this by not considering queue order when granting conversions after recovery. Signed-off-by: David Teigland <teigland@redhat.com>
Showing 2 changed files with 48 additions and 17 deletions Side-by-side Diff
fs/dlm/lock.c
... | ... | @@ -2279,10 +2279,14 @@ |
2279 | 2279 | * immediate request, it is 0 if called later, after the lock has been |
2280 | 2280 | * queued. |
2281 | 2281 | * |
2282 | + * recover is 1 if dlm_recover_grant() is trying to grant conversions | |
2283 | + * after recovery. | |
2284 | + * | |
2282 | 2285 | * References are from chapter 6 of "VAXcluster Principles" by Roy Davis |
2283 | 2286 | */ |
2284 | 2287 | |
2285 | -static int _can_be_granted(struct dlm_rsb *r, struct dlm_lkb *lkb, int now) | |
2288 | +static int _can_be_granted(struct dlm_rsb *r, struct dlm_lkb *lkb, int now, | |
2289 | + int recover) | |
2286 | 2290 | { |
2287 | 2291 | int8_t conv = (lkb->lkb_grmode != DLM_LOCK_IV); |
2288 | 2292 | |
... | ... | @@ -2314,7 +2318,7 @@ |
2314 | 2318 | */ |
2315 | 2319 | |
2316 | 2320 | if (queue_conflict(&r->res_grantqueue, lkb)) |
2317 | - goto out; | |
2321 | + return 0; | |
2318 | 2322 | |
2319 | 2323 | /* |
2320 | 2324 | * 6-3: By default, a conversion request is immediately granted if the |
2321 | 2325 | |
... | ... | @@ -2323,9 +2327,26 @@ |
2323 | 2327 | */ |
2324 | 2328 | |
2325 | 2329 | if (queue_conflict(&r->res_convertqueue, lkb)) |
2326 | - goto out; | |
2330 | + return 0; | |
2327 | 2331 | |
2328 | 2332 | /* |
2333 | + * The RECOVER_GRANT flag means dlm_recover_grant() is granting | |
2334 | + * locks for a recovered rsb, on which lkb's have been rebuilt. | |
2335 | + * The lkb's may have been rebuilt on the queues in a different | |
2336 | + * order than they were in on the previous master. So, granting | |
2337 | + * queued conversions in order after recovery doesn't make sense | |
2338 | + * since the order hasn't been preserved anyway. The new order | |
2339 | + * could also have created a new "in place" conversion deadlock. | |
2340 | + * (e.g. old, failed master held granted EX, with PR->EX, NL->EX. | |
2341 | + * After recovery, there would be no granted locks, and possibly | |
2342 | + * NL->EX, PR->EX, an in-place conversion deadlock.) So, after | |
2343 | + * recovery, grant conversions without considering order. | |
2344 | + */ | |
2345 | + | |
2346 | + if (conv && recover) | |
2347 | + return 1; | |
2348 | + | |
2349 | + /* | |
2329 | 2350 | * 6-5: But the default algorithm for deciding whether to grant or |
2330 | 2351 | * queue conversion requests does not by itself guarantee that such |
2331 | 2352 | * requests are serviced on a "first come first serve" basis. This, in |
... | ... | @@ -2360,7 +2381,7 @@ |
2360 | 2381 | if (list_empty(&r->res_convertqueue)) |
2361 | 2382 | return 1; |
2362 | 2383 | else |
2363 | - goto out; | |
2384 | + return 0; | |
2364 | 2385 | } |
2365 | 2386 | |
2366 | 2387 | /* |
2367 | 2388 | |
... | ... | @@ -2406,12 +2427,12 @@ |
2406 | 2427 | if (!now && !conv && list_empty(&r->res_convertqueue) && |
2407 | 2428 | first_in_list(lkb, &r->res_waitqueue)) |
2408 | 2429 | return 1; |
2409 | - out: | |
2430 | + | |
2410 | 2431 | return 0; |
2411 | 2432 | } |
2412 | 2433 | |
2413 | 2434 | static int can_be_granted(struct dlm_rsb *r, struct dlm_lkb *lkb, int now, |
2414 | - int *err) | |
2435 | + int recover, int *err) | |
2415 | 2436 | { |
2416 | 2437 | int rv; |
2417 | 2438 | int8_t alt = 0, rqmode = lkb->lkb_rqmode; |
... | ... | @@ -2420,7 +2441,7 @@ |
2420 | 2441 | if (err) |
2421 | 2442 | *err = 0; |
2422 | 2443 | |
2423 | - rv = _can_be_granted(r, lkb, now); | |
2444 | + rv = _can_be_granted(r, lkb, now, recover); | |
2424 | 2445 | if (rv) |
2425 | 2446 | goto out; |
2426 | 2447 | |
... | ... | @@ -2461,7 +2482,7 @@ |
2461 | 2482 | |
2462 | 2483 | if (alt) { |
2463 | 2484 | lkb->lkb_rqmode = alt; |
2464 | - rv = _can_be_granted(r, lkb, now); | |
2485 | + rv = _can_be_granted(r, lkb, now, 0); | |
2465 | 2486 | if (rv) |
2466 | 2487 | lkb->lkb_sbflags |= DLM_SBF_ALTMODE; |
2467 | 2488 | else |
... | ... | @@ -2485,6 +2506,7 @@ |
2485 | 2506 | unsigned int *count) |
2486 | 2507 | { |
2487 | 2508 | struct dlm_lkb *lkb, *s; |
2509 | + int recover = rsb_flag(r, RSB_RECOVER_GRANT); | |
2488 | 2510 | int hi, demoted, quit, grant_restart, demote_restart; |
2489 | 2511 | int deadlk; |
2490 | 2512 | |
... | ... | @@ -2498,7 +2520,7 @@ |
2498 | 2520 | demoted = is_demoted(lkb); |
2499 | 2521 | deadlk = 0; |
2500 | 2522 | |
2501 | - if (can_be_granted(r, lkb, 0, &deadlk)) { | |
2523 | + if (can_be_granted(r, lkb, 0, recover, &deadlk)) { | |
2502 | 2524 | grant_lock_pending(r, lkb); |
2503 | 2525 | grant_restart = 1; |
2504 | 2526 | if (count) |
... | ... | @@ -2542,7 +2564,7 @@ |
2542 | 2564 | struct dlm_lkb *lkb, *s; |
2543 | 2565 | |
2544 | 2566 | list_for_each_entry_safe(lkb, s, &r->res_waitqueue, lkb_statequeue) { |
2545 | - if (can_be_granted(r, lkb, 0, NULL)) { | |
2567 | + if (can_be_granted(r, lkb, 0, 0, NULL)) { | |
2546 | 2568 | grant_lock_pending(r, lkb); |
2547 | 2569 | if (count) |
2548 | 2570 | (*count)++; |
... | ... | @@ -3042,7 +3064,7 @@ |
3042 | 3064 | { |
3043 | 3065 | int error = 0; |
3044 | 3066 | |
3045 | - if (can_be_granted(r, lkb, 1, NULL)) { | |
3067 | + if (can_be_granted(r, lkb, 1, 0, NULL)) { | |
3046 | 3068 | grant_lock(r, lkb); |
3047 | 3069 | queue_cast(r, lkb, 0); |
3048 | 3070 | goto out; |
... | ... | @@ -3082,7 +3104,7 @@ |
3082 | 3104 | |
3083 | 3105 | /* changing an existing lock may allow others to be granted */ |
3084 | 3106 | |
3085 | - if (can_be_granted(r, lkb, 1, &deadlk)) { | |
3107 | + if (can_be_granted(r, lkb, 1, 0, &deadlk)) { | |
3086 | 3108 | grant_lock(r, lkb); |
3087 | 3109 | queue_cast(r, lkb, 0); |
3088 | 3110 | goto out; |
... | ... | @@ -3108,7 +3130,7 @@ |
3108 | 3130 | |
3109 | 3131 | if (is_demoted(lkb)) { |
3110 | 3132 | grant_pending_convert(r, DLM_LOCK_IV, NULL, NULL); |
3111 | - if (_can_be_granted(r, lkb, 1)) { | |
3133 | + if (_can_be_granted(r, lkb, 1, 0)) { | |
3112 | 3134 | grant_lock(r, lkb); |
3113 | 3135 | queue_cast(r, lkb, 0); |
3114 | 3136 | goto out; |
3115 | 3137 | |
... | ... | @@ -5373,9 +5395,10 @@ |
5373 | 5395 | |
5374 | 5396 | if (!rsb_flag(r, RSB_RECOVER_GRANT)) |
5375 | 5397 | continue; |
5376 | - rsb_clear_flag(r, RSB_RECOVER_GRANT); | |
5377 | - if (!is_master(r)) | |
5398 | + if (!is_master(r)) { | |
5399 | + rsb_clear_flag(r, RSB_RECOVER_GRANT); | |
5378 | 5400 | continue; |
5401 | + } | |
5379 | 5402 | hold_rsb(r); |
5380 | 5403 | spin_unlock(&ls->ls_rsbtbl[bucket].lock); |
5381 | 5404 | return r; |
5382 | 5405 | |
... | ... | @@ -5420,7 +5443,9 @@ |
5420 | 5443 | rsb_count++; |
5421 | 5444 | count = 0; |
5422 | 5445 | lock_rsb(r); |
5446 | + /* the RECOVER_GRANT flag is checked in the grant path */ | |
5423 | 5447 | grant_pending_locks(r, &count); |
5448 | + rsb_clear_flag(r, RSB_RECOVER_GRANT); | |
5424 | 5449 | lkb_count += count; |
5425 | 5450 | confirm_master(r, 0); |
5426 | 5451 | unlock_rsb(r); |
fs/dlm/recover.c
... | ... | @@ -804,6 +804,7 @@ |
804 | 804 | |
805 | 805 | static void recover_conversion(struct dlm_rsb *r) |
806 | 806 | { |
807 | + struct dlm_ls *ls = r->res_ls; | |
807 | 808 | struct dlm_lkb *lkb; |
808 | 809 | int grmode = -1; |
809 | 810 | |
810 | 811 | |
811 | 812 | |
... | ... | @@ -818,10 +819,15 @@ |
818 | 819 | list_for_each_entry(lkb, &r->res_convertqueue, lkb_statequeue) { |
819 | 820 | if (lkb->lkb_grmode != DLM_LOCK_IV) |
820 | 821 | continue; |
821 | - if (grmode == -1) | |
822 | + if (grmode == -1) { | |
823 | + log_debug(ls, "recover_conversion %x set gr to rq %d", | |
824 | + lkb->lkb_id, lkb->lkb_rqmode); | |
822 | 825 | lkb->lkb_grmode = lkb->lkb_rqmode; |
823 | - else | |
826 | + } else { | |
827 | + log_debug(ls, "recover_conversion %x set gr %d", | |
828 | + lkb->lkb_id, grmode); | |
824 | 829 | lkb->lkb_grmode = grmode; |
830 | + } | |
825 | 831 | } |
826 | 832 | } |
827 | 833 |