The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.50.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git after rh7-3.10.0-693.21.1.vz7.50.4 ------> commit a35df0c623c9ccf81a75e0755c883c524aed6780 Author: Kirill Tkhai <ktk...@virtuozzo.com> Date: Mon Jun 4 23:24:13 2018 +0300
fuse kio: Introduce pcs_cs_list_of_cs_link() Pack repeating patterns in a separate function. Signed-off-by: Kirill Tkhai <ktk...@virtuozzo.com> ===================== Patchset description: Fix deadlock during change of CS address This is not a complete patchset, but I meet the situation when it's necessary to change original logic in small way, so this is a request for comments. [1-5/7] are mostly preparations and fixes, so my question is about [6-7/7]. 1) Patch 6 changes order of actions: pcs_map_notify_addr_change() is called after assigning of rpc addr. Can we do that? As I understand this results in new maps are created with new address, while in pcs_map_notify_addr_change() we invalidate old ones. So, for me it seems there is no a problem. This is needed for possibility to unlock cs->lock in pcs_map_notify_addr_change(). Theoretically, two pcs_cs_find_create() may happen in parallel, so we want they assign addr in the order they happen. Otherwise, the first one with the old addr_serno may overwrite the addr. 2) Patch 7 uses the preparations from previous patches and makes pcs_map_notify_addr_change() to unlock cs->lock for a while. New elements are added to head of cs->map_list, so we skip them on iterations. But it seems, they must be correct because we already updated rpc addr in pcs_cs_find_create(). Is there a reason we can't do this? Kirill Tkhai (7): fuse kio: Introduce pcs_cs_list_of_cs_link() fuse kio: Fix potential use after free fuse kio: Fix possible use after free in cslist_destroy() fuse kio: Introduce pcs_cs::use_count instead of ::is_probing fuse kio: Wait till cs is unused in pcs_csset_fini() fuse kio: Change order around pcs_map_notify_addr_change() fuse kio: Fix fix deadlock during change CS address --- fs/fuse/kio/pcs/pcs_map.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/fs/fuse/kio/pcs/pcs_map.c b/fs/fuse/kio/pcs/pcs_map.c index e649a9b5efb5..56b660849a80 100644 --- a/fs/fuse/kio/pcs/pcs_map.c +++ b/fs/fuse/kio/pcs/pcs_map.c @@ -26,6 +26,16 @@ */ #define MAP_BATCH 16 +static struct pcs_cs_list *cs_link_to_cs_list(struct pcs_cs_link *csl) +{ + struct pcs_cs_record *cs_rec; + struct pcs_cs_list *cs_list; + + cs_rec = container_of(csl, struct pcs_cs_record, cslink); + cs_list = container_of(cs_rec - csl->index, struct pcs_cs_list, cs[0]); + return cs_list; +} + static void pcs_ireq_queue_fail(struct list_head *queue, int error); abs_time_t get_real_time_ms(void) @@ -550,12 +560,10 @@ static void map_recalc_maps(struct pcs_cs * cs) assert_spin_locked(&cs->lock); list_for_each_entry(csl, &cs->map_list, link) { - struct pcs_cs_record * cs_rec; - struct pcs_cs_list * cs_list; + struct pcs_cs_list *cs_list; int read_idx; - cs_rec = container_of(csl, struct pcs_cs_record, cslink); - cs_list = container_of(cs_rec - csl->index, struct pcs_cs_list, cs[0]); + cs_list = cs_link_to_cs_list(csl); read_idx = READ_ONCE(cs_list->read_index); if (read_idx >= 0 && (!cs_is_blacklisted(cs) || @@ -570,12 +578,10 @@ void pcs_map_force_reselect(struct pcs_cs * cs) assert_spin_locked(&cs->lock); list_for_each_entry(csl, &cs->map_list, link) { - struct pcs_cs_record * cs_rec; - struct pcs_cs_list * cs_list; + struct pcs_cs_list *cs_list; int read_idx; - cs_rec = container_of(csl, struct pcs_cs_record, cslink); - cs_list = container_of(cs_rec - csl->index, struct pcs_cs_list, cs[0]); + cs_list = cs_link_to_cs_list(csl); read_idx = READ_ONCE(cs_list->read_index); if (read_idx >= 0 && cs_list->cs[read_idx].cslink.cs == cs) @@ -606,11 +612,9 @@ static int urgent_whitelist(struct pcs_cs * cs) assert_spin_locked(&cs->lock); list_for_each_entry(csl, &cs->map_list, link) { - struct pcs_cs_record * cs_rec; - struct pcs_cs_list * cs_list; + struct pcs_cs_list *cs_list; - cs_rec = container_of(csl, struct pcs_cs_record, cslink); - cs_list = container_of(cs_rec - csl->index, struct pcs_cs_list, cs[0]); + cs_list = cs_link_to_cs_list(csl); if (cs_list->map == NULL) continue; @@ -694,16 +698,12 @@ void pcs_map_notify_addr_change(struct pcs_cs * cs) cs_whitelist(cs, "addr update"); list_for_each_entry(csl, &cs->map_list, link) { - struct pcs_cs_record * cs_rec; - struct pcs_cs_list * cs_list; - struct pcs_map_entry * m; - - cs_rec = container_of(csl, struct pcs_cs_record, cslink); - cs_list = container_of(cs_rec - csl->index, struct pcs_cs_list, cs[0]); + struct pcs_cs_list *cs_list; + struct pcs_map_entry *m; if (csl->addr_serno == cs->addr_serno) continue; - + cs_list = cs_link_to_cs_list(csl); if ((m = cs_list->map) == NULL) continue; _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel