* [Ocfs2-devel] [PATCH v2] ocfs2/cluster: unlock the o2hb_live_lock before the o2nm_depend_item()
@ 2017-11-02 6:00 alex chen
2017-11-06 9:37 ` Joseph Qi
0 siblings, 1 reply; 3+ messages in thread
From: alex chen @ 2017-11-02 6:00 UTC (permalink / raw)
To: ocfs2-devel
In the following situation, the down_write() will be called under
the spin_lock(), which may lead a soft lockup:
o2hb_region_inc_user
spin_lock(&o2hb_live_lock)
o2hb_region_pin
o2nm_depend_item
configfs_depend_item
inode_lock
down_write
-->here may sleep and reschedule
So we should unlock the o2hb_live_lock before the o2nm_depend_item(), and
get item reference in advance to prevent the region to be released.
Signed-off-by: Alex Chen <alex.chen@huawei.com>
Reviewed-by: Yiwen Jiang <jiangyiwen@huawei.com>
Reviewed-by: Jun Piao <piaojun@huawei.com>
---
fs/ocfs2/cluster/heartbeat.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
index d020604..07b2fdc 100644
--- a/fs/ocfs2/cluster/heartbeat.c
+++ b/fs/ocfs2/cluster/heartbeat.c
@@ -2399,8 +2399,15 @@ static int o2hb_region_pin(const char *region_uuid)
if (reg->hr_item_pinned || reg->hr_item_dropped)
goto skip_pin;
+ config_item_get(®->hr_item);
+ spin_unlock(&o2hb_live_lock);
+
/* Ignore ENOENT only for local hb (userdlm domain) */
ret = o2nm_depend_item(®->hr_item);
+
+ config_item_put(®->hr_item);
+ spin_lock(&o2hb_live_lock);
+
if (!ret) {
mlog(ML_CLUSTER, "Pin region %s\n", uuid);
reg->hr_item_pinned = 1;
--
1.9.5.msysgit.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [Ocfs2-devel] [PATCH v2] ocfs2/cluster: unlock the o2hb_live_lock before the o2nm_depend_item()
2017-11-02 6:00 [Ocfs2-devel] [PATCH v2] ocfs2/cluster: unlock the o2hb_live_lock before the o2nm_depend_item() alex chen
@ 2017-11-06 9:37 ` Joseph Qi
2017-11-07 2:30 ` alex chen
0 siblings, 1 reply; 3+ messages in thread
From: Joseph Qi @ 2017-11-06 9:37 UTC (permalink / raw)
To: ocfs2-devel
Hi Alex,
For local heartbeat mode, I understand that if the region is not found,
o2hb_live_lock won't be released. And if the region is found, it doesn't
have to iterate the next region. So I agree with your fix in this case.
But I still don't get how to make sure the safe iteration in case of
global heartbeat mode.
Thanks,
Joseph
On 17/11/2 14:00, alex chen wrote:
> In the following situation, the down_write() will be called under
> the spin_lock(), which may lead a soft lockup:
> o2hb_region_inc_user
> spin_lock(&o2hb_live_lock)
> o2hb_region_pin
> o2nm_depend_item
> configfs_depend_item
> inode_lock
> down_write
> -->here may sleep and reschedule
>
> So we should unlock the o2hb_live_lock before the o2nm_depend_item(), and
> get item reference in advance to prevent the region to be released.
>
> Signed-off-by: Alex Chen <alex.chen@huawei.com>
> Reviewed-by: Yiwen Jiang <jiangyiwen@huawei.com>
> Reviewed-by: Jun Piao <piaojun@huawei.com>
> ---
> fs/ocfs2/cluster/heartbeat.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
> index d020604..07b2fdc 100644
> --- a/fs/ocfs2/cluster/heartbeat.c
> +++ b/fs/ocfs2/cluster/heartbeat.c
> @@ -2399,8 +2399,15 @@ static int o2hb_region_pin(const char *region_uuid)
> if (reg->hr_item_pinned || reg->hr_item_dropped)
> goto skip_pin;
>
> + config_item_get(®->hr_item);
> + spin_unlock(&o2hb_live_lock);
> +
> /* Ignore ENOENT only for local hb (userdlm domain) */
> ret = o2nm_depend_item(®->hr_item);
> +
> + config_item_put(®->hr_item);
> + spin_lock(&o2hb_live_lock);
> +
> if (!ret) {
> mlog(ML_CLUSTER, "Pin region %s\n", uuid);
> reg->hr_item_pinned = 1;
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* [Ocfs2-devel] [PATCH v2] ocfs2/cluster: unlock the o2hb_live_lock before the o2nm_depend_item()
2017-11-06 9:37 ` Joseph Qi
@ 2017-11-07 2:30 ` alex chen
0 siblings, 0 replies; 3+ messages in thread
From: alex chen @ 2017-11-07 2:30 UTC (permalink / raw)
To: ocfs2-devel
Hi Joseph,
Thanks for your reply.
For the global heartbeat mode, the concurrently access to reg->hr_item_pinned
may be happened after unlocking. I will modify this patch and send the patch v3.
Thanks,
Alex
On 2017/11/6 17:37, Joseph Qi wrote:
> Hi Alex,
>
> For local heartbeat mode, I understand that if the region is not found,
> o2hb_live_lock won't be released. And if the region is found, it doesn't
> have to iterate the next region. So I agree with your fix in this case.
>
> But I still don't get how to make sure the safe iteration in case of
> global heartbeat mode.
>
> Thanks,
> Joseph
>
> On 17/11/2 14:00, alex chen wrote:
>> In the following situation, the down_write() will be called under
>> the spin_lock(), which may lead a soft lockup:
>> o2hb_region_inc_user
>> spin_lock(&o2hb_live_lock)
>> o2hb_region_pin
>> o2nm_depend_item
>> configfs_depend_item
>> inode_lock
>> down_write
>> -->here may sleep and reschedule
>>
>> So we should unlock the o2hb_live_lock before the o2nm_depend_item(), and
>> get item reference in advance to prevent the region to be released.
>>
>> Signed-off-by: Alex Chen <alex.chen@huawei.com>
>> Reviewed-by: Yiwen Jiang <jiangyiwen@huawei.com>
>> Reviewed-by: Jun Piao <piaojun@huawei.com>
>> ---
>> fs/ocfs2/cluster/heartbeat.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
>> index d020604..07b2fdc 100644
>> --- a/fs/ocfs2/cluster/heartbeat.c
>> +++ b/fs/ocfs2/cluster/heartbeat.c
>> @@ -2399,8 +2399,15 @@ static int o2hb_region_pin(const char *region_uuid)
>> if (reg->hr_item_pinned || reg->hr_item_dropped)
>> goto skip_pin;
>>
>> + config_item_get(®->hr_item);
>> + spin_unlock(&o2hb_live_lock);
>> +
>> /* Ignore ENOENT only for local hb (userdlm domain) */
>> ret = o2nm_depend_item(®->hr_item);
>> +
>> + config_item_put(®->hr_item);
>> + spin_lock(&o2hb_live_lock);
>> +
>> if (!ret) {
>> mlog(ML_CLUSTER, "Pin region %s\n", uuid);
>> reg->hr_item_pinned = 1;
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-11-07 2:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-02 6:00 [Ocfs2-devel] [PATCH v2] ocfs2/cluster: unlock the o2hb_live_lock before the o2nm_depend_item() alex chen
2017-11-06 9:37 ` Joseph Qi
2017-11-07 2:30 ` alex chen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.