diff options
author | Joyce <xuejiufei@huawei.com> | 2013-09-11 17:20:03 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2013-09-11 18:56:51 -0400 |
commit | 6f8648e894498f769832b79399b1cfabd2973ea9 (patch) | |
tree | 4ea339363940a3412d8be1dfe668f66eda03214f /fs | |
parent | 03dbe88aa9cd0d7b0a876b38bd75ce73b4522454 (diff) |
ocfs2: fix a tiny race case when firing callbacks
In o2hb_shutdown_slot() and o2hb_check_slot(), since event is defined as
local, it is only valid during the call stack. So the following tiny race
case may happen in a multi-volumes mounted environment:
o2hb-vol1 o2hb-vol2
1) o2hb_shutdown_slot
allocate local event1
2) queue_node_event
add event1 to global o2hb_node_events
3) o2hb_shutdown_slot
allocate local event2
4) queue_node_event
add event2 to global o2hb_node_events
5) o2hb_run_event_list
delete event1 from o2hb_node_events
6) o2hb_run_event_list
event1 empty, return
7) o2hb_shutdown_slot
event1 lifecycle ends
8) o2hb_fire_callbacks
event1 is already *invalid*
This patch lets it wait on o2hb_callback_sem when another thread is firing
callbacks. And for performance consideration, we only call
o2hb_run_event_list when there is an event queued.
Signed-off-by: Joyce <xuejiufei@huawei.com>
Signed-off-by: Joseph Qi <joseph.qi@huawei.com>
Cc: Joel Becker <jlbec@evilplan.org>
Cc: Mark Fasheh <mfasheh@suse.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'fs')
-rw-r--r-- | fs/ocfs2/cluster/heartbeat.c | 18 |
1 files changed, 9 insertions, 9 deletions
diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c index 25b72e82b8fa..363f0dcc924f 100644 --- a/fs/ocfs2/cluster/heartbeat.c +++ b/fs/ocfs2/cluster/heartbeat.c | |||
@@ -639,16 +639,9 @@ static void o2hb_fire_callbacks(struct o2hb_callback *hbcall, | |||
639 | /* Will run the list in order until we process the passed event */ | 639 | /* Will run the list in order until we process the passed event */ |
640 | static void o2hb_run_event_list(struct o2hb_node_event *queued_event) | 640 | static void o2hb_run_event_list(struct o2hb_node_event *queued_event) |
641 | { | 641 | { |
642 | int empty; | ||
643 | struct o2hb_callback *hbcall; | 642 | struct o2hb_callback *hbcall; |
644 | struct o2hb_node_event *event; | 643 | struct o2hb_node_event *event; |
645 | 644 | ||
646 | spin_lock(&o2hb_live_lock); | ||
647 | empty = list_empty(&queued_event->hn_item); | ||
648 | spin_unlock(&o2hb_live_lock); | ||
649 | if (empty) | ||
650 | return; | ||
651 | |||
652 | /* Holding callback sem assures we don't alter the callback | 645 | /* Holding callback sem assures we don't alter the callback |
653 | * lists when doing this, and serializes ourselves with other | 646 | * lists when doing this, and serializes ourselves with other |
654 | * processes wanting callbacks. */ | 647 | * processes wanting callbacks. */ |
@@ -707,6 +700,7 @@ static void o2hb_shutdown_slot(struct o2hb_disk_slot *slot) | |||
707 | struct o2hb_node_event event = | 700 | struct o2hb_node_event event = |
708 | { .hn_item = LIST_HEAD_INIT(event.hn_item), }; | 701 | { .hn_item = LIST_HEAD_INIT(event.hn_item), }; |
709 | struct o2nm_node *node; | 702 | struct o2nm_node *node; |
703 | int queued = 0; | ||
710 | 704 | ||
711 | node = o2nm_get_node_by_num(slot->ds_node_num); | 705 | node = o2nm_get_node_by_num(slot->ds_node_num); |
712 | if (!node) | 706 | if (!node) |
@@ -724,11 +718,13 @@ static void o2hb_shutdown_slot(struct o2hb_disk_slot *slot) | |||
724 | 718 | ||
725 | o2hb_queue_node_event(&event, O2HB_NODE_DOWN_CB, node, | 719 | o2hb_queue_node_event(&event, O2HB_NODE_DOWN_CB, node, |
726 | slot->ds_node_num); | 720 | slot->ds_node_num); |
721 | queued = 1; | ||
727 | } | 722 | } |
728 | } | 723 | } |
729 | spin_unlock(&o2hb_live_lock); | 724 | spin_unlock(&o2hb_live_lock); |
730 | 725 | ||
731 | o2hb_run_event_list(&event); | 726 | if (queued) |
727 | o2hb_run_event_list(&event); | ||
732 | 728 | ||
733 | o2nm_node_put(node); | 729 | o2nm_node_put(node); |
734 | } | 730 | } |
@@ -788,6 +784,7 @@ static int o2hb_check_slot(struct o2hb_region *reg, | |||
788 | unsigned int dead_ms = o2hb_dead_threshold * O2HB_REGION_TIMEOUT_MS; | 784 | unsigned int dead_ms = o2hb_dead_threshold * O2HB_REGION_TIMEOUT_MS; |
789 | unsigned int slot_dead_ms; | 785 | unsigned int slot_dead_ms; |
790 | int tmp; | 786 | int tmp; |
787 | int queued = 0; | ||
791 | 788 | ||
792 | memcpy(hb_block, slot->ds_raw_block, reg->hr_block_bytes); | 789 | memcpy(hb_block, slot->ds_raw_block, reg->hr_block_bytes); |
793 | 790 | ||
@@ -881,6 +878,7 @@ fire_callbacks: | |||
881 | slot->ds_node_num); | 878 | slot->ds_node_num); |
882 | 879 | ||
883 | changed = 1; | 880 | changed = 1; |
881 | queued = 1; | ||
884 | } | 882 | } |
885 | 883 | ||
886 | list_add_tail(&slot->ds_live_item, | 884 | list_add_tail(&slot->ds_live_item, |
@@ -932,6 +930,7 @@ fire_callbacks: | |||
932 | node, slot->ds_node_num); | 930 | node, slot->ds_node_num); |
933 | 931 | ||
934 | changed = 1; | 932 | changed = 1; |
933 | queued = 1; | ||
935 | } | 934 | } |
936 | 935 | ||
937 | /* We don't clear this because the node is still | 936 | /* We don't clear this because the node is still |
@@ -947,7 +946,8 @@ fire_callbacks: | |||
947 | out: | 946 | out: |
948 | spin_unlock(&o2hb_live_lock); | 947 | spin_unlock(&o2hb_live_lock); |
949 | 948 | ||
950 | o2hb_run_event_list(&event); | 949 | if (queued) |
950 | o2hb_run_event_list(&event); | ||
951 | 951 | ||
952 | if (node) | 952 | if (node) |
953 | o2nm_node_put(node); | 953 | o2nm_node_put(node); |