[Devel] [PATCH RHEL7 COMMIT] fuse kio: Introduce pcs_cs::use_count instead of ::is_probing

Konstantin Khorenko khorenko at virtuozzo.com
Mon Jun 4 23:24:15 MSK 2018


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 e6b0bf08eaa2b09c9d560c8db76641087c596f88
Author: Kirill Tkhai <ktkhai at virtuozzo.com>
Date:   Mon Jun 4 23:24:14 2018 +0300

    fuse kio: Introduce pcs_cs::use_count instead of ::is_probing
    
    This patch generalizes is_probing and this allows to prohibit
    cs isolation after unlocking of cs::lock, not only in bl_timer_work().
    
    Signed-off-by: Kirill Tkhai <ktkhai at 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 | 10 +++++-----
 fs/fuse/kio/pcs/pcs_cs.h |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/fuse/kio/pcs/pcs_cs.c b/fs/fuse/kio/pcs/pcs_cs.c
index 0b6c4f248251..5cf7d73de67b 100644
--- a/fs/fuse/kio/pcs/pcs_cs.c
+++ b/fs/fuse/kio/pcs/pcs_cs.c
@@ -82,7 +82,7 @@ struct pcs_cs *pcs_cs_alloc(struct pcs_cs_set *css,
 	cs->nflows = 0;
 
 	cs->state = 0;
-	cs->is_probing = 0;
+	cs->use_count = 0;
 	cs->is_dead = 0;
 	INIT_LIST_HEAD(&cs->bl_link);
 
@@ -957,7 +957,7 @@ static void cs_probe_done(struct pcs_msg *msg)
 			TRACE("probe error %d", msg->error.value);
 			cs_blacklist(cs, msg->error.value, "probe");
 		}
-		cs->is_probing = 0;
+		cs->use_count--;
 	}
 	spin_unlock(&cs->lock);
 	pcs_free_msg(msg);
@@ -1015,7 +1015,7 @@ static void bl_timer_work(struct work_struct *w)
 		spin_lock(&cs->lock);
 		BUG_ON(cs->is_dead);
 		list_move(&cs->bl_link, &to_blacklist);
-		if (cs->is_probing) {
+		if (cs->use_count) {
 			spin_unlock(&cs->lock);
 			continue;
 		}
@@ -1025,14 +1025,14 @@ static void bl_timer_work(struct work_struct *w)
 			pcs_cs_destroy(cs);
 			continue;
 		}
-		cs->is_probing = 1;
+		cs->use_count++;
 		spin_unlock(&cs->lock);
 		msg = cs_prep_probe(cs);
 		if (msg)
 			pcs_rpc_call(cs->rpc, msg);
 		spin_lock(&cs->lock);
 		if (!msg)
-			cs->is_probing = 0;
+			cs->use_count--;
 		spin_unlock(&cs->lock);
 	}
 	spin_lock(&css->lock);
diff --git a/fs/fuse/kio/pcs/pcs_cs.h b/fs/fuse/kio/pcs/pcs_cs.h
index eb81ac51f3ae..1fb40936d046 100644
--- a/fs/fuse/kio/pcs/pcs_cs.h
+++ b/fs/fuse/kio/pcs/pcs_cs.h
@@ -76,8 +76,8 @@ struct pcs_cs {
 
 	unsigned long		state;
 	int			blacklist_reason;
+	unsigned int		use_count; /* Protects cs against isolation */
 	struct list_head	bl_link;
-	unsigned		is_probing:1;
 	unsigned		is_dead:1;
 
 


More information about the Devel mailing list