Whamcloud - gitweb
LU-181 obdclass: fix portal_handle memory wastage
authorLiang Zhen <liang@whamcloud.com>
Fri, 9 Dec 2011 06:33:15 +0000 (14:33 +0800)
committerOleg Drokin <green@whamcloud.com>
Tue, 22 May 2012 05:29:57 +0000 (01:29 -0400)
This patch is the first step of fixing memory wastage in Lustre.
We can save 20 bytes for each portals_handle(on 64bits system)
by reusing members, which could be over a hundred megabytes
on system with millions of ldlm_locks because portals_handle is
embedded in ldlm_lock.

Signed-off-by: Liang Zhen <liang@whamcloud.com>
Change-Id: I401c0c0c2fc8d2624fd48c714dc3d06fc0e4e21e
Reviewed-on: http://review.whamcloud.com/1827
Tested-by: Hudson
Tested-by: Maloo <whamcloud.maloo@gmail.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
lustre/include/lustre_export.h
lustre/include/lustre_handles.h
lustre/include/obd_support.h
lustre/ldlm/ldlm_lock.c
lustre/lov/lov_internal.h
lustre/lov/lov_request.c
lustre/mdt/mdt_internal.h
lustre/mdt/mdt_open.c
lustre/obdclass/genops.c
lustre/obdclass/lustre_handles.c

index 772a798..b424358 100644 (file)
@@ -179,13 +179,13 @@ struct obd_export {
          */
         cfs_atomic_t              exp_rpc_count; /* RPC references */
         cfs_atomic_t              exp_cb_count; /* Commit callback references */
+       /** Number of queued replay requests to be processes */
+       cfs_atomic_t              exp_replay_count;
         cfs_atomic_t              exp_locks_count; /** Lock references */
 #if LUSTRE_TRACKS_LOCK_EXP_REFS
         cfs_list_t                exp_locks_list;
         cfs_spinlock_t            exp_locks_list_guard;
 #endif
-        /** Number of queued replay requests to be processes */
-        cfs_atomic_t              exp_replay_count;
         /** UUID of client connected to this export */
         struct obd_uuid           exp_client_uuid;
         /** To link all exports on an obd device */
index 85e9e6d..eddb04a 100644 (file)
@@ -58,7 +58,10 @@ typedef struct {
 } cfs_rcu_head_t;
 #endif
 
-typedef void (*portals_handle_addref_cb)(void *object);
+struct portals_handle_ops {
+       void (*hop_addref)(void *object);
+       void (*hop_free)(void *object, int size);
+};
 
 /* These handles are most easily used by having them appear at the very top of
  * whatever object that you want to make handles for.  ie:
@@ -69,29 +72,26 @@ typedef void (*portals_handle_addref_cb)(void *object);
  * };
  *
  * Now you're able to assign the results of cookie2handle directly to an
- * ldlm_lock.  If it's not at the top, you'll want to hack up a macro that
- * uses some offsetof() magic. */
-
+ * ldlm_lock.  If it's not at the top, you'll want to use container_of()
+ * to compute the start of the structure based on the handle field. */
 struct portals_handle {
-        cfs_list_t h_link;
-        __u64 h_cookie;
-        portals_handle_addref_cb h_addref;
+       cfs_list_t                      h_link;
+       __u64                           h_cookie;
+       struct portals_handle_ops       *h_ops;
 
-        /* newly added fields to handle the RCU issue. -jxiong */
-        cfs_spinlock_t h_lock;
-        void *h_ptr;
-        void (*h_free_cb)(void *, size_t);
-        cfs_rcu_head_t h_rcu;
-        unsigned int h_size;
-        __u8 h_in:1;
-        __u8 h_unused[3];
+       /* newly added fields to handle the RCU issue. -jxiong */
+       cfs_rcu_head_t                  h_rcu;
+       cfs_spinlock_t                  h_lock;
+       unsigned int                    h_size:31;
+       unsigned int                    h_in:1;
 };
 #define RCU2HANDLE(rcu)    container_of(rcu, struct portals_handle, h_rcu)
 
 /* handles.c */
 
 /* Add a handle to the hash table */
-void class_handle_hash(struct portals_handle *, portals_handle_addref_cb);
+void class_handle_hash(struct portals_handle *,
+                      struct portals_handle_ops *ops);
 void class_handle_unhash(struct portals_handle *);
 void class_handle_hash_back(struct portals_handle *);
 void *class_handle2object(__u64 cookie);
index 9e5b6e3..4243085 100644 (file)
@@ -667,22 +667,20 @@ do {                                                                          \
 # define my_call_rcu(rcu, cb)             (cb)(rcu)
 #endif
 
-#define OBD_FREE_RCU_CB(ptr, size, handle, free_cb)                           \
-do {                                                                          \
-        struct portals_handle *__h = (handle);                                \
-        LASSERT(handle);                                                      \
-        __h->h_ptr = (ptr);                                                   \
-        __h->h_size = (size);                                                 \
-        __h->h_free_cb = (void (*)(void *, size_t))(free_cb);                 \
-        my_call_rcu(&__h->h_rcu, class_handle_free_cb);                       \
-        POISON_PTR(ptr);                                                      \
+#define OBD_FREE_RCU(ptr, size, handle)                                              \
+do {                                                                         \
+       struct portals_handle *__h = (handle);                                \
+                                                                             \
+       LASSERT(handle != NULL);                                              \
+       __h->h_cookie = (unsigned long)(ptr);                                 \
+       __h->h_size = (size);                                                 \
+       my_call_rcu(&__h->h_rcu, class_handle_free_cb);                       \
+       POISON_PTR(ptr);                                                      \
 } while(0)
-#define OBD_FREE_RCU(ptr, size, handle) OBD_FREE_RCU_CB(ptr, size, handle, NULL)
 
 #else
 #define OBD_FREE(ptr, size) ((void)(size), free((ptr)))
 #define OBD_FREE_RCU(ptr, size, handle) (OBD_FREE(ptr, size))
-#define OBD_FREE_RCU_CB(ptr, size, handle, cb)     ((*(cb))(ptr, size))
 #endif /* ifdef __KERNEL__ */
 
 #ifdef __arch_um__
index 3c9b052..2b4392f 100644 (file)
@@ -193,12 +193,6 @@ struct ldlm_lock *ldlm_lock_get(struct ldlm_lock *lock)
         return lock;
 }
 
-static void ldlm_lock_free(struct ldlm_lock *lock, size_t size)
-{
-        LASSERT(size == sizeof(*lock));
-        OBD_SLAB_FREE(lock, ldlm_lock_slab, sizeof(*lock));
-}
-
 void ldlm_lock_put(struct ldlm_lock *lock)
 {
         ENTRY;
@@ -231,8 +225,7 @@ void ldlm_lock_put(struct ldlm_lock *lock)
 
                 ldlm_interval_free(ldlm_interval_detach(lock));
                 lu_ref_fini(&lock->l_reference);
-                OBD_FREE_RCU_CB(lock, sizeof(*lock), &lock->l_handle,
-                                ldlm_lock_free);
+               OBD_FREE_RCU(lock, sizeof(*lock), &lock->l_handle);
         }
 
         EXIT;
@@ -399,6 +392,17 @@ static void lock_handle_addref(void *lock)
         LDLM_LOCK_GET((struct ldlm_lock *)lock);
 }
 
+static void lock_handle_free(void *lock, int size)
+{
+       LASSERT(size == sizeof(struct ldlm_lock));
+       OBD_SLAB_FREE(lock, ldlm_lock_slab, size);
+}
+
+struct portals_handle_ops lock_handle_ops = {
+       .hop_addref = lock_handle_addref,
+       .hop_free   = lock_handle_free,
+};
+
 /*
  * usage: pass in a resource on which you have done ldlm_resource_get
  *        new lock will take over the refcount.
@@ -436,7 +440,7 @@ static struct ldlm_lock *ldlm_lock_new(struct ldlm_resource *resource)
         lprocfs_counter_incr(ldlm_res_to_ns(resource)->ns_stats,
                              LDLM_NSS_LOCKS);
         CFS_INIT_LIST_HEAD(&lock->l_handle.h_link);
-        class_handle_hash(&lock->l_handle, lock_handle_addref);
+       class_handle_hash(&lock->l_handle, &lock_handle_ops);
 
         lu_ref_init(&lock->l_reference);
         lu_ref_add(&lock->l_reference, "hash", lock);
index 2c2214e..73e894a 100644 (file)
@@ -88,29 +88,6 @@ struct lov_request_set {
 
 extern cfs_mem_cache_t *lov_oinfo_slab;
 
-static inline void lov_llh_addref(void *llhp)
-{
-        struct lov_lock_handles *llh = llhp;
-        cfs_atomic_inc(&llh->llh_refcount);
-        CDEBUG(D_INFO, "GETting llh %p : new refcount %d\n", llh,
-               cfs_atomic_read(&llh->llh_refcount));
-}
-
-static inline struct lov_lock_handles *lov_llh_new(struct lov_stripe_md *lsm)
-{
-        struct lov_lock_handles *llh;
-
-        OBD_ALLOC(llh, sizeof *llh +
-                  sizeof(*llh->llh_handles) * lsm->lsm_stripe_count);
-        if (llh == NULL)
-                return NULL;
-        cfs_atomic_set(&llh->llh_refcount, 2);
-        llh->llh_stripe_count = lsm->lsm_stripe_count;
-        CFS_INIT_LIST_HEAD(&llh->llh_handle.h_link);
-        class_handle_hash(&llh->llh_handle, lov_llh_addref);
-        return llh;
-}
-
 void lov_finish_set(struct lov_request_set *set);
 
 static inline void lov_get_reqset(struct lov_request_set *set)
index 87204c3..c7293e5 100644 (file)
@@ -272,6 +272,37 @@ int lov_fini_enqueue_set(struct lov_request_set *set, __u32 mode, int rc,
         RETURN(rc ? rc : ret);
 }
 
+static void lov_llh_addref(void *llhp)
+{
+       struct lov_lock_handles *llh = llhp;
+
+       cfs_atomic_inc(&llh->llh_refcount);
+       CDEBUG(D_INFO, "GETting llh %p : new refcount %d\n", llh,
+              cfs_atomic_read(&llh->llh_refcount));
+}
+
+static struct portals_handle_ops lov_handle_ops = {
+       .hop_addref = lov_llh_addref,
+       .hop_free   = NULL,
+};
+
+static struct lov_lock_handles *lov_llh_new(struct lov_stripe_md *lsm)
+{
+       struct lov_lock_handles *llh;
+
+       OBD_ALLOC(llh, sizeof *llh +
+                 sizeof(*llh->llh_handles) * lsm->lsm_stripe_count);
+       if (llh == NULL)
+               return NULL;
+
+       cfs_atomic_set(&llh->llh_refcount, 2);
+       llh->llh_stripe_count = lsm->lsm_stripe_count;
+       CFS_INIT_LIST_HEAD(&llh->llh_handle.h_link);
+       class_handle_hash(&llh->llh_handle, &lov_handle_ops);
+
+       return llh;
+}
+
 int lov_prep_enqueue_set(struct obd_export *exp, struct obd_info *oinfo,
                          struct ldlm_enqueue_info *einfo,
                          struct lov_request_set **reqset)
index 80332e9..a78eb91 100644 (file)
@@ -84,10 +84,10 @@ struct mdt_object;
 /* file data for open files on MDS */
 struct mdt_file_data {
         struct portals_handle mfd_handle; /* must be first */
+       int                   mfd_mode;   /* open mode provided by client */
         cfs_list_t            mfd_list;   /* protected by med_open_lock */
         __u64                 mfd_xid;    /* xid of the open request */
         struct lustre_handle  mfd_old_handle; /* old handle in replay case */
-        int                   mfd_mode;   /* open mode provided by client */
         struct mdt_object    *mfd_object; /* point to opened object */
 };
 
index 38853b6..16fbdd9 100644 (file)
@@ -54,6 +54,11 @@ static void mdt_mfd_get(void *mfdp)
 {
 }
 
+static struct portals_handle_ops mfd_handle_ops = {
+       .hop_addref = mdt_mfd_get,
+       .hop_free   = NULL,
+};
+
 /* Create a new mdt_file_data struct, initialize it,
  * and insert it to global hash table */
 struct mdt_file_data *mdt_mfd_new(void)
@@ -65,7 +70,7 @@ struct mdt_file_data *mdt_mfd_new(void)
         if (mfd != NULL) {
                 CFS_INIT_LIST_HEAD(&mfd->mfd_handle.h_link);
                 CFS_INIT_LIST_HEAD(&mfd->mfd_list);
-                class_handle_hash(&mfd->mfd_handle, mdt_mfd_get);
+               class_handle_hash(&mfd->mfd_handle, &mfd_handle_ops);
         }
         RETURN(mfd);
 }
index 9839fcd..081b55e 100644 (file)
@@ -798,6 +798,11 @@ static void export_handle_addref(void *export)
         class_export_get(export);
 }
 
+static struct portals_handle_ops export_handle_ops = {
+       .hop_addref = export_handle_addref,
+       .hop_free   = NULL,
+};
+
 struct obd_export *class_export_get(struct obd_export *exp)
 {
         cfs_atomic_inc(&exp->exp_refcount);
@@ -863,8 +868,8 @@ struct obd_export *class_new_export(struct obd_device *obd,
         CFS_INIT_LIST_HEAD(&export->exp_req_replay_queue);
         CFS_INIT_LIST_HEAD(&export->exp_handle.h_link);
         CFS_INIT_LIST_HEAD(&export->exp_hp_rpcs);
-        class_handle_hash(&export->exp_handle, export_handle_addref);
-        export->exp_last_request_time = cfs_time_current_sec();
+       class_handle_hash(&export->exp_handle, &export_handle_ops);
+       export->exp_last_request_time = cfs_time_current_sec();
         cfs_spin_lock_init(&export->exp_lock);
         cfs_spin_lock_init(&export->exp_rpc_lock);
         CFS_INIT_HLIST_NODE(&export->exp_uuid_hash);
@@ -976,6 +981,11 @@ static void import_handle_addref(void *import)
         class_import_get(import);
 }
 
+static struct portals_handle_ops import_handle_ops = {
+       .hop_addref = import_handle_addref,
+       .hop_free   = NULL,
+};
+
 struct obd_import *class_import_get(struct obd_import *import)
 {
         cfs_atomic_inc(&import->imp_refcount);
@@ -1044,7 +1054,7 @@ struct obd_import *class_new_import(struct obd_device *obd)
         cfs_atomic_set(&imp->imp_inval_count, 0);
         CFS_INIT_LIST_HEAD(&imp->imp_conn_list);
         CFS_INIT_LIST_HEAD(&imp->imp_handle.h_link);
-        class_handle_hash(&imp->imp_handle, import_handle_addref);
+       class_handle_hash(&imp->imp_handle, &import_handle_ops);
         init_imp_at(&imp->imp_at);
 
         /* the default magic is V2, will be used in connect RPC, and
index 835cbda..4776b27 100644 (file)
@@ -82,7 +82,8 @@ static struct handle_bucket {
  * Generate a unique 64bit cookie (hash) for a handle and insert it into
  * global (per-node) hash-table.
  */
-void class_handle_hash(struct portals_handle *h, portals_handle_addref_cb cb)
+void class_handle_hash(struct portals_handle *h,
+                      struct portals_handle_ops *ops)
 {
         struct handle_bucket *bucket;
         ENTRY;
@@ -108,8 +109,8 @@ void class_handle_hash(struct portals_handle *h, portals_handle_addref_cb cb)
                 handle_base += HANDLE_INCR;
         }
         cfs_spin_unlock(&handle_base_lock);
-        h->h_addref = cb;
+
+       h->h_ops = ops;
         cfs_spin_lock_init(&h->h_lock);
 
         bucket = &handle_hash[h->h_cookie & HANDLE_HASH_MASK];
@@ -189,7 +190,7 @@ void *class_handle2object(__u64 cookie)
 
                 cfs_spin_lock(&h->h_lock);
                 if (likely(h->h_in != 0)) {
-                        h->h_addref(h);
+                       h->h_ops->hop_addref(h);
                         retval = h;
                 }
                 cfs_spin_unlock(&h->h_lock);
@@ -202,14 +203,13 @@ void *class_handle2object(__u64 cookie)
 
 void class_handle_free_cb(cfs_rcu_head_t *rcu)
 {
-        struct portals_handle *h = RCU2HANDLE(rcu);
-        if (h->h_free_cb) {
-                h->h_free_cb(h->h_ptr, h->h_size);
-        } else {
-                void *ptr = h->h_ptr;
-                unsigned int size = h->h_size;
-                OBD_FREE(ptr, size);
-        }
+       struct portals_handle *h = RCU2HANDLE(rcu);
+       void *ptr = (void *)(unsigned long)h->h_cookie;
+
+       if (h->h_ops->hop_free != NULL)
+               h->h_ops->hop_free(ptr, h->h_size);
+       else
+               OBD_FREE(ptr, h->h_size);
 }
 
 int class_handle_init(void)
@@ -252,8 +252,8 @@ static int cleanup_all_handles(void)
 
                 cfs_spin_lock(&handle_hash[i].lock);
                 list_for_each_entry_rcu(h, &(handle_hash[i].head), h_link) {
-                        CERROR("force clean handle "LPX64" addr %p addref %p\n",
-                               h->h_cookie, h, h->h_addref);
+                       CERROR("force clean handle "LPX64" addr %p ops %p\n",
+                              h->h_cookie, h, h->h_ops);
 
                         class_handle_unhash_nolock(h);
                         rc++;