[Devel] [PATCH RHEL7 COMMIT] fuse kio: Introduce pcs_cs_list_of_cs_link()

Konstantin Khorenko khorenko at virtuozzo.com
Mon Jun 4 23:24:13 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 a35df0c623c9ccf81a75e0755c883c524aed6780
Author: Kirill Tkhai <ktkhai at 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 <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_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;
 


More information about the Devel mailing list