[RHEL7,COMMIT] fuse kio: Wait till cs is unused in pcs_csset_fini()

Submitted by Konstantin Khorenko on June 4, 2018, 8:24 p.m.

Details

Message ID 201806042024.w54KOFun005660@finist_ce7.work
State New
Series "fuse kio: Wait till cs is unused in pcs_csset_fini()"
Headers show

Commit Message

Konstantin Khorenko June 4, 2018, 8:24 p.m.
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 2c1c2d3e8162cc6e17afccc82a33a40481da6d5e
Author: Kirill Tkhai <ktkhai@virtuozzo.com>
Date:   Mon Jun 4 23:24:15 2018 +0300

    fuse kio: Wait till cs is unused in pcs_csset_fini()
    
    This is the second function, which can isolate a cs.
    Despite it's unused now, I add waiting use_count here
    for uniformity and for the future use.
    
    After this, all the code will know we can't isolate
    a cs if someone uses it.
    
    Signed-off-by: Kirill Tkhai <ktkhai@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 | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Patch hide | download patch | download mbox

diff --git a/fs/fuse/kio/pcs/pcs_cs.c b/fs/fuse/kio/pcs/pcs_cs.c
index 5cf7d73de67b..9614f2473629 100644
--- a/fs/fuse/kio/pcs/pcs_cs.c
+++ b/fs/fuse/kio/pcs/pcs_cs.c
@@ -1059,6 +1059,18 @@  void pcs_csset_init(struct pcs_cs_set *css)
 	atomic64_set(&css->csl_serno_gen, 0);
 }
 
+static void pcs_cs_wait_unused(struct pcs_cs *cs)
+{
+	assert_spin_locked(&cs->lock);
+	cs->use_count++;
+	while (cs->use_count != 1) {
+		spin_unlock(&cs->lock);
+		schedule_timeout(1);
+		spin_lock(&cs->lock);
+	}
+	cs->use_count--;
+}
+
 void pcs_csset_fini(struct pcs_cs_set *css)
 {
 	unsigned int i;
@@ -1082,6 +1094,7 @@  void pcs_csset_fini(struct pcs_cs_set *css)
 				continue;
 			}
 			rcu_read_unlock();
+			pcs_cs_wait_unused(cs);
 			pcs_cs_isolate(cs, &to_resubmit);
 			spin_unlock(&cs->lock);
 			pcs_cs_destroy(cs);