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 885971b6a2d878fff83b5709a96b5d6171fbab7d Author: Kirill Tkhai <ktk...@virtuozzo.com> Date: Mon Jun 4 23:24:15 2018 +0300
fuse kio: Change order around pcs_map_notify_addr_change() This unblacklistes a cs after all maps are marked as stale, while rpc addr becomes assigned earlier. Since pcs_rpc::addr is dereferenced with cs unlocked, it seems, we can may such the change. This will be used in next patch. 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_cs.c | 7 +++++-- fs/fuse/kio/pcs/pcs_map.c | 2 -- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/fs/fuse/kio/pcs/pcs_cs.c b/fs/fuse/kio/pcs/pcs_cs.c index 9614f2473629..27f0bc3754d9 100644 --- a/fs/fuse/kio/pcs/pcs_cs.c +++ b/fs/fuse/kio/pcs/pcs_cs.c @@ -193,11 +193,14 @@ struct pcs_cs *pcs_cs_find_create(struct pcs_cs_set *csset, PCS_NODE_ID_T *id, P if (pcs_netaddr_cmp(&cs->addr, addr)) { cs->addr = *addr; cs->addr_serno++; - if (!(flags & CS_FL_INACTIVE)) - pcs_map_notify_addr_change(cs); + TRACE("Port change CS" NODE_FMT " seq=%d", NODE_ARGS(*id), cs->addr_serno); pcs_rpc_set_address(cs->rpc, addr); + if (!(flags & CS_FL_INACTIVE)) { + pcs_map_notify_addr_change(cs); + cs_whitelist(cs, "addr update"); + } } } /* TODO: (flags & PCS_RPC_F_LOCAL) should be checker here */ diff --git a/fs/fuse/kio/pcs/pcs_map.c b/fs/fuse/kio/pcs/pcs_map.c index 56a1118af255..49af8b961478 100644 --- a/fs/fuse/kio/pcs/pcs_map.c +++ b/fs/fuse/kio/pcs/pcs_map.c @@ -699,8 +699,6 @@ void pcs_map_notify_addr_change(struct pcs_cs * cs) struct pcs_cs_link * csl; assert_spin_locked(&cs->lock); - cs_whitelist(cs, "addr update"); - rcu_read_lock(); list_for_each_entry(csl, &cs->map_list, link) { struct pcs_cs_list *cs_list; _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel