On Wed, Nov 30, 2016 at 05:07:22PM +0800, Eric Ren wrote:
> @@ -852,12 +868,19 @@ void dlm_recover_rsbs(struct dlm_ls *ls)
> if (is_master(r)) {
> if (rsb_flag(r, RSB_RECOVER_CONVERT))
> recover_conversion(r);
> +
> + /* recover lvb before granting locks so the updated
> + lvb/VALNOTVALID is presented in the completion */
> + recover_lvb(r);
> +
> if (rsb_flag(r, RSB_NEW_MASTER2))
> recover_grant(r);
> - recover_lvb(r);
> count++;
> + } else {
> + rsb_clear_flag(r, RSB_VALNOTVALID);
> }
> """
>
> 3) Questions:
>
> a. Should we put recover_lvb() even before recover_conversion()? if not, why?
Yes, I think you're right. The lvb decision should be made using the
original lock modes, not the modified lock modes from recover_conversion.
> b. Why should we clear fag RSB_VALNOTVALID in the else branch?
That looks incorrect also. I think VALNOTVALID should only be cleared
when the lvb is written by the application. The else condition should
probably just be removed. That does raise the question of whether it
could be masking another problem (e.g. some case where the flag is not
being cleared when it should be, or a case where it's being set and
shouldn't be.)
Dave
