afs: Split the usage count on struct afs_server
authorDavid Howells <dhowells@redhat.com>
Fri, 17 Apr 2020 16:31:26 +0000 (17:31 +0100)
committerDavid Howells <dhowells@redhat.com>
Sun, 31 May 2020 14:19:51 +0000 (15:19 +0100)
Split the usage count on the afs_server struct to have an active count that
registers who's actually using it separately from the reference count on
the object.

This allows a future patch to dispatch polling probes without advancing the
"unuse" time into the future each time we emit a probe, which would
otherwise prevent unused server records from expiring.

Included in this:

 (1) The latter part of afs_destroy_server() in which the RCU destruction
     of afs_server objects is invoked and the outstanding server count is
     decremented is split out into __afs_put_server().

 (2) afs_put_server() now calls __afs_put_server() rather then setting the
     management timer.

 (3) The calls begun by afs_fs_give_up_all_callbacks() and
     afs_fs_get_capabilities() can now take a ref on the server record, so
     afs_destroy_server() can just drop its ref and needn't wait for the
     completion of these calls.  They'll put the ref when they're done.

 (4) Because of (3), afs_fs_probe_done() no longer needs to wake up
     afs_destroy_server() with server->probe_outstanding.

 (5) afs_gc_servers can be simplified.  It only needs to check if
     server->active is 0 rather than playing games with the refcount.

 (6) afs_manage_servers() can propose a server for gc if usage == 0 rather
     than if ref == 1.  The gc is effected by (5).

Signed-off-by: David Howells <dhowells@redhat.com>
fs/afs/cmservice.c
fs/afs/fs_probe.c
fs/afs/fsclient.c
fs/afs/internal.h
fs/afs/proc.c
fs/afs/rxrpc.c
fs/afs/server.c
fs/afs/server_list.c
include/trace/events/afs.h

index 380ad5a..7dcbca3 100644 (file)
@@ -268,7 +268,9 @@ static void SRXAFSCB_CallBack(struct work_struct *work)
         * to maintain cache coherency.
         */
        if (call->server) {
-               trace_afs_server(call->server, atomic_read(&call->server->usage),
+               trace_afs_server(call->server,
+                                atomic_read(&call->server->ref),
+                                atomic_read(&call->server->active),
                                 afs_server_trace_callback);
                afs_break_callbacks(call->server, call->count, call->request);
        }
index 37d1bba..d37d78e 100644 (file)
@@ -16,7 +16,6 @@ static bool afs_fs_probe_done(struct afs_server *server)
        if (!atomic_dec_and_test(&server->probe_outstanding))
                return false;
 
-       wake_up_var(&server->probe_outstanding);
        clear_bit_unlock(AFS_SERVER_FL_PROBING, &server->flags);
        wake_up_bit(&server->flags, AFS_SERVER_FL_PROBING);
        return true;
index d2b3798..3854d16 100644 (file)
@@ -1842,7 +1842,7 @@ int afs_fs_give_up_all_callbacks(struct afs_net *net,
        bp = call->request;
        *bp++ = htonl(FSGIVEUPALLCALLBACKS);
 
-       /* Can't take a ref on server */
+       call->server = afs_use_server(server, afs_server_trace_give_up_cb);
        afs_make_call(ac, call, GFP_NOFS);
        return afs_wait_for_call_to_complete(call, ac);
 }
@@ -1924,7 +1924,7 @@ struct afs_call *afs_fs_get_capabilities(struct afs_net *net,
                return ERR_PTR(-ENOMEM);
 
        call->key = key;
-       call->server = afs_get_server(server, afs_server_trace_get_caps);
+       call->server = afs_use_server(server, afs_server_trace_get_caps);
        call->server_index = server_index;
        call->upgrade = true;
        call->async = true;
@@ -1934,7 +1934,6 @@ struct afs_call *afs_fs_get_capabilities(struct afs_net *net,
        bp = call->request;
        *bp++ = htonl(FSGETCAPABILITIES);
 
-       /* Can't take a ref on server */
        trace_afs_make_fs_call(call, NULL);
        afs_make_call(ac, call, GFP_NOFS);
        return call;
index ee17c86..cb70e1c 100644 (file)
@@ -498,7 +498,7 @@ struct afs_server {
        struct hlist_node       addr6_link;     /* Link in net->fs_addresses6 */
        struct hlist_node       proc_link;      /* Link in net->fs_proc */
        struct afs_server       *gc_next;       /* Next server in manager's list */
-       time64_t                put_time;       /* Time at which last put */
+       time64_t                unuse_time;     /* Time at which last unused */
        unsigned long           flags;
 #define AFS_SERVER_FL_NOT_READY        1               /* The record is not ready for use */
 #define AFS_SERVER_FL_NOT_FOUND        2               /* VL server says no such server */
@@ -512,7 +512,8 @@ struct afs_server {
 #define AFS_SERVER_FL_NO_RM2   10              /* Fileserver doesn't support YFS.RemoveFile2 */
 #define AFS_SERVER_FL_HAVE_EPOCH 11            /* ->epoch is valid */
 #define AFS_SERVER_FL_NEEDS_UPDATE 12          /* Fileserver address list is out of date */
-       atomic_t                usage;
+       atomic_t                ref;            /* Object refcount */
+       atomic_t                active;         /* Active user count */
        u32                     addr_version;   /* Address list version */
        u32                     cm_epoch;       /* Server RxRPC epoch */
        unsigned int            debug_id;       /* Debugging ID for traces */
@@ -1244,6 +1245,9 @@ extern struct afs_server *afs_find_server(struct afs_net *,
 extern struct afs_server *afs_find_server_by_uuid(struct afs_net *, const uuid_t *);
 extern struct afs_server *afs_lookup_server(struct afs_cell *, struct key *, const uuid_t *, u32);
 extern struct afs_server *afs_get_server(struct afs_server *, enum afs_server_trace);
+extern struct afs_server *afs_use_server(struct afs_server *, enum afs_server_trace);
+extern void afs_unuse_server(struct afs_net *, struct afs_server *, enum afs_server_trace);
+extern void afs_unuse_server_notime(struct afs_net *, struct afs_server *, enum afs_server_trace);
 extern void afs_put_server(struct afs_net *, struct afs_server *, enum afs_server_trace);
 extern void afs_manage_servers(struct work_struct *);
 extern void afs_servers_timer(struct timer_list *);
index 468e171..9bce789 100644 (file)
@@ -378,19 +378,20 @@ static int afs_proc_servers_show(struct seq_file *m, void *v)
        int i;
 
        if (v == SEQ_START_TOKEN) {
-               seq_puts(m, "UUID                                 USE ADDR\n");
+               seq_puts(m, "UUID                                 REF ACT ADDR\n");
                return 0;
        }
 
        server = list_entry(v, struct afs_server, proc_link);
        alist = rcu_dereference(server->addresses);
-       seq_printf(m, "%pU %3d %pISpc%s\n",
+       seq_printf(m, "%pU %3d %3d %pISpc%s\n",
                   &server->uuid,
-                  atomic_read(&server->usage),
+                  atomic_read(&server->ref),
+                  atomic_read(&server->active),
                   &alist->addrs[0].transport,
                   alist->preferred == 0 ? "*" : "");
        for (i = 1; i < alist->nr_addrs; i++)
-               seq_printf(m, "                                         %pISpc%s\n",
+               seq_printf(m, "                                             %pISpc%s\n",
                           &alist->addrs[i].transport,
                           alist->preferred == i ? "*" : "");
        return 0;
index 1ecc67d..ab2962f 100644 (file)
@@ -183,7 +183,7 @@ void afs_put_call(struct afs_call *call)
                if (call->type->destructor)
                        call->type->destructor(call);
 
-               afs_put_server(call->net, call->server, afs_server_trace_put_call);
+               afs_unuse_server_notime(call->net, call->server, afs_server_trace_put_call);
                afs_put_cb_interest(call->net, call->cbi);
                afs_put_addrlist(call->alist);
                kfree(call->request);
index 9e50ccd..4969a68 100644 (file)
@@ -25,6 +25,10 @@ static void afs_dec_servers_outstanding(struct afs_net *net)
                wake_up_var(&net->servers_outstanding);
 }
 
+static struct afs_server *afs_maybe_use_server(struct afs_server *,
+                                              enum afs_server_trace);
+static void __afs_put_server(struct afs_net *, struct afs_server *);
+
 /*
  * Find a server by one of its addresses.
  */
@@ -40,7 +44,7 @@ struct afs_server *afs_find_server(struct afs_net *net,
 
        do {
                if (server)
-                       afs_put_server(net, server, afs_server_trace_put_find_rsq);
+                       afs_unuse_server_notime(net, server, afs_server_trace_put_find_rsq);
                server = NULL;
                read_seqbegin_or_lock(&net->fs_addr_lock, &seq);
 
@@ -78,9 +82,9 @@ struct afs_server *afs_find_server(struct afs_net *net,
                }
 
                server = NULL;
+               continue;
        found:
-               if (server && !atomic_inc_not_zero(&server->usage))
-                       server = NULL;
+               server = afs_maybe_use_server(server, afs_server_trace_get_by_addr);
 
        } while (need_seqretry(&net->fs_addr_lock, seq));
 
@@ -91,7 +95,7 @@ struct afs_server *afs_find_server(struct afs_net *net,
 }
 
 /*
- * Look up a server by its UUID
+ * Look up a server by its UUID and mark it active.
  */
 struct afs_server *afs_find_server_by_uuid(struct afs_net *net, const uuid_t *uuid)
 {
@@ -107,7 +111,7 @@ struct afs_server *afs_find_server_by_uuid(struct afs_net *net, const uuid_t *uu
                 * changes.
                 */
                if (server)
-                       afs_put_server(net, server, afs_server_trace_put_uuid_rsq);
+                       afs_unuse_server(net, server, afs_server_trace_put_uuid_rsq);
                server = NULL;
 
                read_seqbegin_or_lock(&net->fs_lock, &seq);
@@ -122,7 +126,7 @@ struct afs_server *afs_find_server_by_uuid(struct afs_net *net, const uuid_t *uu
                        } else if (diff > 0) {
                                p = p->rb_right;
                        } else {
-                               afs_get_server(server, afs_server_trace_get_by_uuid);
+                               afs_use_server(server, afs_server_trace_get_by_uuid);
                                break;
                        }
 
@@ -198,7 +202,7 @@ exists:
 }
 
 /*
- * allocate a new server record
+ * Allocate a new server record and mark it active.
  */
 static struct afs_server *afs_alloc_server(struct afs_net *net,
                                           const uuid_t *uuid,
@@ -212,7 +216,8 @@ static struct afs_server *afs_alloc_server(struct afs_net *net,
        if (!server)
                goto enomem;
 
-       atomic_set(&server->usage, 1);
+       atomic_set(&server->ref, 1);
+       atomic_set(&server->active, 1);
        server->debug_id = atomic_inc_return(&afs_server_debug_id);
        RCU_INIT_POINTER(server->addresses, alist);
        server->addr_version = alist->version;
@@ -224,7 +229,7 @@ static struct afs_server *afs_alloc_server(struct afs_net *net,
        spin_lock_init(&server->probe_lock);
 
        afs_inc_servers_outstanding(net);
-       trace_afs_server(server, 1, afs_server_trace_alloc);
+       trace_afs_server(server, 1, 1, afs_server_trace_alloc);
        _leave(" = %p", server);
        return server;
 
@@ -292,7 +297,6 @@ struct afs_server *afs_lookup_server(struct afs_cell *cell, struct key *key,
                kfree(candidate);
        }
 
-       _leave(" = %p{%d}", server, atomic_read(&server->usage));
        return server;
 }
 
@@ -328,9 +332,38 @@ void afs_servers_timer(struct timer_list *timer)
 struct afs_server *afs_get_server(struct afs_server *server,
                                  enum afs_server_trace reason)
 {
-       unsigned int u = atomic_inc_return(&server->usage);
+       unsigned int u = atomic_inc_return(&server->ref);
+
+       trace_afs_server(server, u, atomic_read(&server->active), reason);
+       return server;
+}
+
+/*
+ * Try to get a reference on a server object.
+ */
+static struct afs_server *afs_maybe_use_server(struct afs_server *server,
+                                              enum afs_server_trace reason)
+{
+       unsigned int r = atomic_fetch_add_unless(&server->ref, 1, 0);
+       unsigned int a;
+
+       if (r == 0)
+               return NULL;
+
+       a = atomic_inc_return(&server->active);
+       trace_afs_server(server, r, a, reason);
+       return server;
+}
+
+/*
+ * Get an active count on a server object.
+ */
+struct afs_server *afs_use_server(struct afs_server *server, enum afs_server_trace reason)
+{
+       unsigned int r = atomic_inc_return(&server->ref);
+       unsigned int a = atomic_inc_return(&server->active);
 
-       trace_afs_server(server, u, reason);
+       trace_afs_server(server, r, a, reason);
        return server;
 }
 
@@ -345,28 +378,56 @@ void afs_put_server(struct afs_net *net, struct afs_server *server,
        if (!server)
                return;
 
-       server->put_time = ktime_get_real_seconds();
-
-       usage = atomic_dec_return(&server->usage);
+       usage = atomic_dec_return(&server->ref);
+       trace_afs_server(server, usage, atomic_read(&server->active), reason);
+       if (unlikely(usage == 0))
+               __afs_put_server(net, server);
+}
 
-       trace_afs_server(server, usage, reason);
+/*
+ * Drop an active count on a server object without updating the last-unused
+ * time.
+ */
+void afs_unuse_server_notime(struct afs_net *net, struct afs_server *server,
+                            enum afs_server_trace reason)
+{
+       if (server) {
+               unsigned int active = atomic_dec_return(&server->active);
 
-       if (likely(usage > 0))
-               return;
+               if (active == 0)
+                       afs_set_server_timer(net, afs_server_gc_delay);
+               afs_put_server(net, server, reason);
+       }
+}
 
-       afs_set_server_timer(net, afs_server_gc_delay);
+/*
+ * Drop an active count on a server object.
+ */
+void afs_unuse_server(struct afs_net *net, struct afs_server *server,
+                     enum afs_server_trace reason)
+{
+       if (server) {
+               server->unuse_time = ktime_get_real_seconds();
+               afs_unuse_server_notime(net, server, reason);
+       }
 }
 
 static void afs_server_rcu(struct rcu_head *rcu)
 {
        struct afs_server *server = container_of(rcu, struct afs_server, rcu);
 
-       trace_afs_server(server, atomic_read(&server->usage),
-                        afs_server_trace_free);
+       trace_afs_server(server, atomic_read(&server->ref),
+                        atomic_read(&server->active), afs_server_trace_free);
        afs_put_addrlist(rcu_access_pointer(server->addresses));
        kfree(server);
 }
 
+static void __afs_put_server(struct afs_net *net, struct afs_server *server)
+{
+       call_rcu(&server->rcu, afs_server_rcu);
+       afs_dec_servers_outstanding(net);
+}
+
 /*
  * destroy a dead server
  */
@@ -379,19 +440,10 @@ static void afs_destroy_server(struct afs_net *net, struct afs_server *server)
                .error  = 0,
        };
 
-       trace_afs_server(server, atomic_read(&server->usage),
-                        afs_server_trace_give_up_cb);
-
        if (test_bit(AFS_SERVER_FL_MAY_HAVE_CB, &server->flags))
                afs_fs_give_up_all_callbacks(net, server, &ac, NULL);
 
-       wait_var_event(&server->probe_outstanding,
-                      atomic_read(&server->probe_outstanding) == 0);
-
-       trace_afs_server(server, atomic_read(&server->usage),
-                        afs_server_trace_destroy);
-       call_rcu(&server->rcu, afs_server_rcu);
-       afs_dec_servers_outstanding(net);
+       afs_put_server(net, server, afs_server_trace_destroy);
 }
 
 /*
@@ -400,31 +452,28 @@ static void afs_destroy_server(struct afs_net *net, struct afs_server *server)
 static void afs_gc_servers(struct afs_net *net, struct afs_server *gc_list)
 {
        struct afs_server *server;
-       bool deleted;
-       int usage;
+       int active;
 
        while ((server = gc_list)) {
                gc_list = server->gc_next;
 
                write_seqlock(&net->fs_lock);
-               usage = 1;
-               deleted = atomic_try_cmpxchg(&server->usage, &usage, 0);
-               trace_afs_server(server, usage, afs_server_trace_gc);
-               if (deleted) {
+
+               active = atomic_read(&server->active);
+               if (active == 0) {
+                       trace_afs_server(server, atomic_read(&server->ref),
+                                        active, afs_server_trace_gc);
                        rb_erase(&server->uuid_rb, &net->fs_servers);
                        hlist_del_rcu(&server->proc_link);
-               }
-               write_sequnlock(&net->fs_lock);
-
-               if (deleted) {
-                       write_seqlock(&net->fs_addr_lock);
                        if (!hlist_unhashed(&server->addr4_link))
                                hlist_del_rcu(&server->addr4_link);
                        if (!hlist_unhashed(&server->addr6_link))
                                hlist_del_rcu(&server->addr6_link);
-                       write_sequnlock(&net->fs_addr_lock);
-                       afs_destroy_server(net, server);
                }
+               write_sequnlock(&net->fs_lock);
+
+               if (active == 0)
+                       afs_destroy_server(net, server);
        }
 }
 
@@ -453,15 +502,14 @@ void afs_manage_servers(struct work_struct *work)
        for (cursor = rb_first(&net->fs_servers); cursor; cursor = rb_next(cursor)) {
                struct afs_server *server =
                        rb_entry(cursor, struct afs_server, uuid_rb);
-               int usage = atomic_read(&server->usage);
+               int active = atomic_read(&server->active);
 
-               _debug("manage %pU %u", &server->uuid, usage);
+               _debug("manage %pU %u", &server->uuid, active);
 
-               ASSERTCMP(usage, >=, 1);
-               ASSERTIFCMP(purging, usage, ==, 1);
+               ASSERTIFCMP(purging, active, ==, 0);
 
-               if (usage == 1) {
-                       time64_t expire_at = server->put_time;
+               if (active == 0) {
+                       time64_t expire_at = server->unuse_time;
 
                        if (!test_bit(AFS_SERVER_FL_VL_FAIL, &server->flags) &&
                            !test_bit(AFS_SERVER_FL_NOT_FOUND, &server->flags))
@@ -532,7 +580,8 @@ static noinline bool afs_update_server_record(struct afs_fs_cursor *fc, struct a
 
        _enter("");
 
-       trace_afs_server(server, atomic_read(&server->usage), afs_server_trace_update);
+       trace_afs_server(server, atomic_read(&server->ref), atomic_read(&server->active),
+                        afs_server_trace_update);
 
        alist = afs_vl_lookup_addrs(fc->vnode->volume->cell, fc->key,
                                    &server->uuid);
index f567732..b77e50f 100644 (file)
@@ -16,8 +16,8 @@ void afs_put_serverlist(struct afs_net *net, struct afs_server_list *slist)
        if (slist && refcount_dec_and_test(&slist->usage)) {
                for (i = 0; i < slist->nr_servers; i++) {
                        afs_put_cb_interest(net, slist->servers[i].cb_interest);
-                       afs_put_server(net, slist->servers[i].server,
-                                      afs_server_trace_put_slist);
+                       afs_unuse_server(net, slist->servers[i].server,
+                                        afs_server_trace_put_slist);
                }
                kfree(slist);
        }
index c612cab..f9691f6 100644 (file)
@@ -33,6 +33,7 @@ enum afs_server_trace {
        afs_server_trace_destroy,
        afs_server_trace_free,
        afs_server_trace_gc,
+       afs_server_trace_get_by_addr,
        afs_server_trace_get_by_uuid,
        afs_server_trace_get_caps,
        afs_server_trace_get_install,
@@ -241,6 +242,7 @@ enum afs_cb_break_reason {
        EM(afs_server_trace_destroy,            "DESTROY  ") \
        EM(afs_server_trace_free,               "FREE     ") \
        EM(afs_server_trace_gc,                 "GC       ") \
+       EM(afs_server_trace_get_by_addr,        "GET addr ") \
        EM(afs_server_trace_get_by_uuid,        "GET uuid ") \
        EM(afs_server_trace_get_caps,           "GET caps ") \
        EM(afs_server_trace_get_install,        "GET inst ") \
@@ -1271,26 +1273,30 @@ TRACE_EVENT(afs_cb_miss,
            );
 
 TRACE_EVENT(afs_server,
-           TP_PROTO(struct afs_server *server, int usage, enum afs_server_trace reason),
+           TP_PROTO(struct afs_server *server, int ref, int active,
+                    enum afs_server_trace reason),
 
-           TP_ARGS(server, usage, reason),
+           TP_ARGS(server, ref, active, reason),
 
            TP_STRUCT__entry(
                    __field(unsigned int,               server          )
-                   __field(int,                        usage           )
+                   __field(int,                        ref             )
+                   __field(int,                        active          )
                    __field(int,                        reason          )
                             ),
 
            TP_fast_assign(
                    __entry->server = server->debug_id;
-                   __entry->usage = usage;
+                   __entry->ref = ref;
+                   __entry->active = active;
                    __entry->reason = reason;
                           ),
 
-           TP_printk("s=%08x %s u=%d",
+           TP_printk("s=%08x %s u=%d a=%d",
                      __entry->server,
                      __print_symbolic(__entry->reason, afs_server_traces),
-                     __entry->usage)
+                     __entry->ref,
+                     __entry->active)
            );
 
 #endif /* _TRACE_AFS_H */