From mboxrd@z Thu Jan 1 00:00:00 1970 From: Coly Li Date: Wed, 03 Dec 2008 00:49:50 +0800 Subject: [Ocfs2-devel] [nov 28] question of dlm_get_lock_resource() In-Reply-To: <4934A0C4.2090404@oracle.com> References: <492FEC31.5010907@suse.de> <4934A0C4.2090404@oracle.com> Message-ID: <4935672E.6080800@suse.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: ocfs2-devel@oss.oracle.com Sunil Mushran Wrote: > No. dlm->spinlock prevents it racing the dlm_thread. Secondly, lookup > also does a get. > > What scenario are you envisioning? IMHO, locking dlm->spinlock does not mean protecting to tmpres. Even line 727 gets false, how can we make sure in line 749 res->state is not modified to DLM_LOCK_RES_DROPPING_REF by other process ? I can not find the magic which make everything work in order, something important must be missed by me. Thanks. > > Coly Li wrote: >> Hi list, >> >> When I read code of dlm_get_lock_resource(), there is something not >> clear to me. >> >> 719 lookup: >> 720 spin_lock(&dlm->spinlock); >> 721 tmpres = __dlm_lookup_lockres_full(dlm, lockid, namelen, >> hash); >> 722 if (tmpres) { >> 723 int dropping_ref = 0; >> 724 >> 725 spin_lock(&tmpres->spinlock); >> 726 if (tmpres->owner == dlm->node_num) { >> 727 BUG_ON(tmpres->state & >> DLM_LOCK_RES_DROPPING_REF); >> 728 dlm_lockres_grab_inflight_ref(dlm, tmpres); >> 729 } else if (tmpres->state & >> DLM_LOCK_RES_DROPPING_REF) >> 730 dropping_ref = 1; >> 731 spin_unlock(&tmpres->spinlock); >> 732 spin_unlock(&dlm->spinlock); >> 733 >> 734 /* wait until done messaging the master, drop our >> ref to allow >> 735 * the lockres to be purged, start over. */ >> 736 if (dropping_ref) { >> 737 spin_lock(&tmpres->spinlock); >> 738 __dlm_wait_on_lockres_flags(tmpres, >> DLM_LOCK_RES_DROPPING_REF); >> 739 spin_unlock(&tmpres->spinlock); >> 740 dlm_lockres_put(tmpres); >> 741 tmpres = NULL; >> 742 goto lookup; >> 743 } >> 744 >> 745 mlog(0, "found in hash!\n"); >> 746 if (res) >> 747 dlm_lockres_put(res); >> 748 res = tmpres; >> 749 goto leave; >> 750 } >> >> If at line 721 tmpres found from hash, and its state is not >> DLM_LOCK_RES_DROPPING_REF (dropping_ref >> is 0), between line 733 and 748, is it possible to set tmpres->state >> to DLM_LOCK_RES_DROPPING_REF ? >> >> I don't see any protection for this race, maybe there is something I >> missed. Can anybody give me a >> hint ? -- Coly Li SuSE PRC Labs