From: Joseph Qi <jiangqi903@gmail.com>
To: Eric Ren <zren@suse.com>
Cc: ocfs2-devel@oss.oracle.com, linux-kernel@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>,
Joel Becker <jlbec@evilplan.org>, Gang He <ghe@suse.com>,
Junxiao Bi <junxiao.bi@oracle.com>,
mfasheh@versity.com
Subject: Re: [Ocfs2-devel] [PATCH 2/2] ocfs2: fix deadlocks when taking inode lock at vfs entry points
Date: Thu, 12 Jan 2017 19:36:27 +0800 [thread overview]
Message-ID: <836cc2c3-f00e-adf2-922e-d4d7a8b3286d@gmail.com> (raw)
In-Reply-To: <340218da-57ae-dcdb-2ad3-b934632b0ab9@suse.com>
On 17/1/12 19:24, Eric Ren wrote:
> Hi Joseph,
>
> On 01/09/2017 10:13 AM, Eric Ren wrote:
>>>>> So you are trying to fix it by making phase3 finish without really
>>>>> doing
>>>> Phase3 can go ahead because this node is already under protection
>>>> of cluster lock.
>>> You said it was blocked...
>> Oh, sorry, I meant phase3 can go ahead if this patch set is applied;-)
>>
>>> "Another hand, the recursive cluster lock (the second one) will be
>>> blocked in
>>> __ocfs2_cluster_lock() because of OCFS2_LOCK_BLOCKED."
>>>>> __ocfs2_cluster_lock, then Process B can continue either.
>>>>> Let us bear in mind that phase1 and phase3 are in the same context
>>>>> and
>>>>> executed in order. That's why I think there is no need to check if
>>>>> locked
>>>>> by myself in phase1.
>> Sorry, I still cannot see it. Without keeping track of the first
>> cluster lock, how can we
>> know if
>> we are under a context that has already been in the protecting of
>> cluster lock? How can we
>> handle
>> the recursive locking (the second cluster lock) if we don't have this
>> information?
>>>>> If phase1 finds it is already locked by myself, that means the holder
>>>>> is left by last operation without dec holder. That's why I think
>>>>> it is a bug
>>>>> instead of a recursive lock case.
>> I think I got your point here. Do you mean that we should just add
>> the lock holder at the
>> first locking position
>> without checking before that? Unfortunately, it's tricky here to know
>> exactly which ocfs2
>> routine will be the first vfs
>> entry point, such as ocfs2_get_acl() which can be both the first vfs
>> entry point and the
>> second vfs entry point after
>> ocfs2_permission(), right?
>>
>> It will be a coding bug if the problem you concern about happens. I
>> think we don't need to
>> worry about this much because
>> the code logic here is quite simple;-)
> Ping...
>
> Did I clear your doubts by the last email? I really want to get your
> point, if not.
>
> If there's any problem, I will fix them in the next version;-)
Yes, but I still worry about the code bug case will be hidden behind
recursive lock...
Anyway, It depends on others...
Thanks,
Joseph
>
> Thanks,
> Eric
>
>>
>> Thanks for your patience!
>> Eric
>>
>>>> D
>
next prev parent reply other threads:[~2017-01-12 11:36 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-05 15:31 [PATCH 0/2] fix deadlock caused by recursive cluster locking Eric Ren
2017-01-05 15:31 ` [PATCH 1/2] ocfs2/dlmglue: prepare tracking logic to avoid recursive cluster lock Eric Ren
2017-01-06 6:07 ` Joseph Qi
2017-01-06 7:03 ` Eric Ren
2017-01-06 7:24 ` Joseph Qi
2017-01-06 8:04 ` Eric Ren
2017-01-13 3:59 ` Junxiao Bi
2017-01-13 6:12 ` Eric Ren
2017-01-16 2:42 ` Junxiao Bi
2017-01-16 3:31 ` Eric Ren
2017-01-05 15:31 ` [PATCH 2/2] ocfs2: fix deadlocks when taking inode lock at vfs entry points Eric Ren
2017-01-06 6:09 ` Joseph Qi
2017-01-06 6:56 ` Eric Ren
2017-01-06 7:14 ` Joseph Qi
2017-01-06 8:21 ` Eric Ren
2017-01-06 9:03 ` Joseph Qi
2017-01-06 9:13 ` Eric Ren
2017-01-06 9:55 ` Joseph Qi
2017-01-06 11:56 ` Eric Ren
2017-01-09 1:13 ` Joseph Qi
2017-01-09 2:13 ` Eric Ren
2017-01-12 11:24 ` [Ocfs2-devel] " Eric Ren
2017-01-12 11:36 ` Joseph Qi [this message]
2017-01-06 14:52 ` kbuild test robot
2017-01-09 5:24 ` Eric Ren
2017-01-06 17:53 ` kbuild test robot
2017-01-13 4:22 ` Junxiao Bi
2017-01-13 6:19 ` Eric Ren
2017-01-16 2:46 ` Junxiao Bi
2017-01-16 3:06 ` Eric Ren
2017-01-16 3:13 ` Junxiao Bi
2017-01-16 3:17 ` Eric Ren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=836cc2c3-f00e-adf2-922e-d4d7a8b3286d@gmail.com \
--to=jiangqi903@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=ghe@suse.com \
--cc=jlbec@evilplan.org \
--cc=junxiao.bi@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mfasheh@versity.com \
--cc=ocfs2-devel@oss.oracle.com \
--cc=zren@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).