[Cluster-devel] [PATCH 13/18] [try #2] DLM: fix conversion deadlock when DLM_LKF_NODLCKWT flag is set

[David Teigland <teigland@redhat.com>]

On Tue, Sep 12, 2017 at 09:01:31AM +0000, tsutomu.owa at toshiba.co.jp wrote:
> When the DLM_LKF_NODLCKWT flag was set, even if conversion deadlock
> was detected, the caller of can_be_granted() was unknown.
> We change the behavior of can_be_granted() and change it to detect
> conversion deadlock regardless of whether the DLM_LKF_NODLCKWT flag
> is set or not. And depending on whether the DLM_LKF_NODLCKWT flag
> is set or not, we change the behavior at the caller of can_be_granted().

Thanks for sorting this out.  Could we expand this with a description of
the externally visible effects from the change?

> @@ -2549,6 +2548,17 @@ static int grant_pending_convert(struct dlm_rsb *r, int high, int *cw,
>  			log_print("WARN: pending deadlock %x node %d %s",
>  				  lkb->lkb_id, lkb->lkb_nodeid, r->res_name);
>  			dlm_dump_rsb(r);
> +			/*
> +			 * If DLM_LKB_NODLKWT flag is set and conversion
> +			 * deadlock is detected, we request blocking AST and
> +			 * down (or cancel) conversion.
> +			 */
> +			if (lkb->lkb_exflags & DLM_LKF_NODLCKWT &&
> +			    lkb->lkb_highbast < lkb->lkb_rqmode) {
> +				queue_bast(r, lkb, lkb->lkb_rqmode);
> +				lkb->lkb_highbast = lkb->lkb_rqmode;
> +			}

Is this the main change in behavior you are looking for?  It seems to make
sense.  Do you find that this case is occurring during normal operation?
I did not previously expect that this code path would be used, but if it
is, then we should change the FIXME comment to explain how and when it
happens, and the warning and dump_rsb should be suppresssed.

> +
>  			continue;
>  		}
>  
> @@ -3124,7 +3134,7 @@ static int do_convert(struct dlm_rsb *r, struct dlm_lkb *lkb)
>  	   deadlock, so we leave it on the granted queue and return EDEADLK in
>  	   the ast for the convert. */
>  
> -	if (deadlk) {
> +	if (deadlk && !(lkb->lkb_exflags & DLM_LKF_NODLCKWT)) {
>  		/* it's left on the granted queue */
>  		revert_lock(r, lkb);
>  		queue_cast(r, lkb, -EDEADLK);
> -- 
> 2.7.4