ocfs2/cluster: unlock the o2hb_live_lock before the o2nm_depend_item()
PATCH v2 1/1 ocfs2/cluster: unlock the o2hb_live_lock before the o2nm_depend_item()
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 rescheduleSo 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.
--- fs/ocfs2/cluster/heartbeat.c | 7 +++++++ 1 file changed, 7 insertions(+)
fs/ocfs2/cluster/heartbeat.c +7 -0
@@ -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 ;
Signature · 1 line
1.9.5.msysgit.1
PATCH v2 1/1 ocfs2/cluster: unlock the o2hb_live_lock before the o2nm_depend_item()
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
Quoted 42 lines · alex chen
In the following situation, the
down_write()will be called under thespin_lock(), which may lead a soft lockup:o2hb_region_inc_user
spin_lock(&o2hb_live_lock)
o2hb_region_pin
o2nm_depend_itemconfigfs_depend_item inode_lock down_write -->here may sleep and rescheduleSo we should unlock the
o2hb_live_lockbefore theo2nm_depend_item(), and get item reference in advance to prevent the region to be released.Signed-off-by Alex Chen <[email protected]>Reviewed-by Yiwen Jiang <[email protected]>Reviewed-by Jun Piao <[email protected]>--- fs/ocfs2/cluster/heartbeat.c | 7 +++++++ 1 file changed, 7 insertions(+)fs/ocfs2/cluster/heartbeat.c +7
-0@@ -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 ;
PATCH v2 1/1 ocfs2/cluster: unlock the o2hb_live_lock before the o2nm_depend_item()
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
Quoted 55 lines · Joseph Qi
Hi Alex,
For local heartbeat mode, I understand that if the region is not found,
o2hb_live_lockwon'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,
JosephQuoted 42 lines · alex chen
In the following situation, the
down_write()will be called under thespin_lock(), which may lead a soft lockup:o2hb_region_inc_user
spin_lock(&o2hb_live_lock)
o2hb_region_pin
o2nm_depend_itemconfigfs_depend_item inode_lock down_write -->here may sleep and rescheduleSo we should unlock the
o2hb_live_lockbefore theo2nm_depend_item(), and get item reference in advance to prevent the region to be released.Signed-off-by Alex Chen <[email protected]>Reviewed-by Yiwen Jiang <[email protected]>Reviewed-by Jun Piao <[email protected]>--- fs/ocfs2/cluster/heartbeat.c | 7 +++++++ 1 file changed, 7 insertions(+)fs/ocfs2/cluster/heartbeat.c +7
-0@@ -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 ;