From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sunil Mushran Date: Tue, 02 Dec 2008 13:31:06 -0800 Subject: [Ocfs2-devel] [nov 28] question of dlm_get_lock_resource() In-Reply-To: <49358998.9040401@oracle.com> References: <492FEC31.5010907@suse.de> <4934A0C4.2090404@oracle.com> <4935672E.6080800@suse.de> <49358998.9040401@oracle.com> Message-ID: <4935A91A.7090300@oracle.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: ocfs2-devel@oss.oracle.com On second thoughts, I should elaborate further. I'll do so sometime later today. Sunil Mushran wrote: > Read the purge_lockres code, the only place where we set this flag. > > Coly Li wrote: > >> 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 ? >>>> >>>> >> >> > > > _______________________________________________ > Ocfs2-devel mailing list > Ocfs2-devel at oss.oracle.com > http://oss.oracle.com/mailman/listinfo/ocfs2-devel >