[LU-5453] mdt_attr_set() uses EX lock for slave 0, PW for master and other slaves

• Status: Resolved
• Project: Lustre
• Affects Version/s: Lustre 2.7.0
• Fix Version/s: Lustre 2.7.0
• Type: Bug
• Priority: Minor
• Reporter: John Hammond
• Assignee: John Hammond
• Resolution: Fixed
• Labels: dne2, mdt
• Severity: 3

Description

In the striped directory case of mdt_attr_set() we use a EX lock on slave 0 while we use a PW lock on the master inode and the other slaves:

mdt_attr_set()
	__u64 lockpart = MDS_INODELOCK_UPDATE;

	if (ma->ma_attr.la_valid & (LA_MODE|LA_UID|LA_GID))
		lockpart |= MDS_INODELOCK_LOOKUP | MDS_INODELOCK_PERM;

        lh = &info->mti_lh[MDT_LH_PARENT];
	mdt_lock_reg_init(lh, LCK_PW);

        rc = mdt_object_lock(info, mo, lh, lockpart, MDT_LOCAL_LOCK);
        if (rc != 0)
        	RETURN(rc);

        s0_lh = &info->mti_lh[MDT_LH_LOCAL];
        mdt_lock_reg_init(s0_lh, LCK_EX);
        rc = mdt_lock_slaves(info, mo, LCK_PW, lockpart, s0_lh, &s0_obj, einfo);
        if (rc != 0)
        	GOTO(out_unlock, rc);

Because the locking order is master, slave0, then other slaves this seems prone to causing deadlocks.

Comments

[John Hammond]

Please see http://review.whamcloud.com/11353.

[Oleg Drokin]

I think for all intents and purposes EX and PW locs are the same, everything else conflicts with them? So it's a pure cosmetic issue I would think

[John Hammond]

LCK_CR?

[Di Wang]

I agree all of slave lock mode should keep consistency with the master. Could you please explain what deadlock issue you concerned here? The lock order will always be master, then slave (from 0 to n), and each slave can only belong to 1 master.

[John Hammond]

> Could you please explain what deadlock issue you concerned here?

Nothing concrete. I just meant that even with a defined order for objects the ordering of lock modes (weak mode, strong mode, weak mode) seems like trouble.

[Andreas Dilger]

Di, is this still an issue with the new striped directory layout where the master object has separate shard objects 0..N?

[Andreas Dilger]

It looks like this was fixed as part of http://review.whamcloud.com/11353 anyway, which landed in 2.7.0.

[Gerrit Updater]

Wang Shilong ([e-mail]) uploaded a new patch: https://review.whamcloud.com/26790
Subject: LU-5453 mdt: mdt_reint_setattr() and mdt_attr_set() fixes
Project: fs/lustre-release
Branch: b2_5
Current Patch Set: 1
Commit: 139d70f1587e9058c76f1cbcbd268e8d6d9c6faf