Re: [PATCH] tools/hotplug: fix locking

[Zhigang Wang <zhigang.x.wang@oracle.com>]

On 06/13/2012 04:11 AM, Ian Campbell wrote:
> On Mon, 2012-06-11 at 14:59 +0100, Zhigang Wang wrote:
>> This patch removes the ownner support and fixed this issue per my test.
> I think the changelog here needs to go into a bit more detail about why
> it is correct to remove the owner support. e.g. why is it no longer
> needed or why is it bogus. Also why does removing it fix the issue?
> Please describe the logic behind the change not just its impact on your
> test case.
Thanks Kouya for the info about Red Hat implement. I also like the flock
implement better.

My plan is to withdraw this patch and test Red Hat implement and if it works
fine, then I will submit the Red Hat patch (as far as it's GPL).

Thanks,

Zhigang
>
>> Kouya: would you please help to confirm this fix is correct?
>>
>> Anyone has encountered this issue please help to test.
>>
>> Signed-off-by: Zhigang Wang <zhigang.x.wang@oracle.com>
>> Cc: Kouya Shimura <kouya@jp.fujitsu.com>
>>
>> diff -r 32034d1914a6 -r eb72d7090b59 tools/hotplug/Linux/locking.sh
>> --- a/tools/hotplug/Linux/locking.sh	Thu Jun 07 19:46:57 2012 +0100
>> +++ b/tools/hotplug/Linux/locking.sh	Mon Jun 11 09:29:43 2012 -0400
>> @@ -48,32 +48,14 @@ sigerr() {
>>  _claim_lock()
>>  {
>>    local lockdir="$1"
>> -  local owner=$(_lock_owner "$lockdir")
>>    local retries=0
>>  
>>    while [ $retries -lt $LOCK_RETRIES ]
>>    do
>> -    mkdir "$lockdir" 2>/dev/null && trap "_release_lock $lockdir; sigerr" ERR &&
>> -      _update_lock_info "$lockdir" && return
>> -
>> -    local new_owner=$(_lock_owner "$lockdir")
>> -    if [ "$new_owner" != "$owner" ]
>> -    then
>> -      owner="$new_owner"
>> -      retries=0
>> -    else
>> -      local pid=$(echo $owner | cut -d : -f 1)
>> -      if [ -n "$pid" -a "$pid" != "unknown" -a ! -f "/proc/$pid/status" ]
>> -      then
>> -        _release_lock $lockdir
>> -      fi
>> -    fi
>> -
>> +    mkdir "$lockdir" 2>/dev/null && trap "_release_lock $lockdir; sigerr" ERR && return
>>      if [ $retries -gt $LOCK_SPINNING_RETRIES ]
>>      then
>>        sleep $LOCK_SLEEPTIME
>> -    else
>> -      sleep 0
>>      fi
>>      retries=$(($retries + 1))
>>    done
>> @@ -91,20 +73,7 @@ _release_lock()
>>  _steal_lock()
>>  {
>>    local lockdir="$1"
>> -  local owner=$(cat "$lockdir/owner" 2>/dev/null || echo "unknown")
>> -  log err "Forced to steal lock on $lockdir from $owner!"
>> +  log err "Forced to steal lock on $lockdir!"
>>    _release_lock "$lockdir"
>>    _claim_lock "$lockdir"
>>  }
>> -
>> -
>> -_lock_owner()
>> -{
>> -  cat "$1/owner" 2>/dev/null || echo "unknown"
>> -}
>> -
>> -
>> -_update_lock_info()
>> -{
>> -  echo "$$: $0" >"$1/owner"
>> -}
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
>