On Fri, 24 May 2019 17:49:18 +0200 Vitaly Wool <vitalyw...@gmail.com> wrote:

> For each page scheduled for compaction (e. g. by z3fold_free()),
> try to apply inter-page compaction before running the traditional/
> existing intra-page compaction. That means, if the page has only one
> buddy, we treat that buddy as a new object that we aim to place into
> an existing z3fold page. If such a page is found, that object is
> transferred and the old page is freed completely. The transferred
> object is named "foreign" and treated slightly differently thereafter.
> 
> Namely, we increase "foreign handle" counter for the new page. Pages
> with non-zero "foreign handle" count become unmovable. This patch
> implements "foreign handle" detection when a handle is freed to
> decrement the foreign handle counter accordingly, so a page may as
> well become movable again as the time goes by.
> 
> As a result, we almost always have exactly 3 objects per page and
> significantly better average compression ratio.
> 
> ...
>
> +static inline struct z3fold_header *handle_to_z3fold_header(unsigned long);
> +static inline struct z3fold_pool *zhdr_to_pool(struct z3fold_header *);

Forward-declaring inline functions is peculiar, but it does appear to work.

z3fold is quite inline-happy.  Fortunately the compiler will ignore the
inline hint if it seems a bad idea.  Even then, the below shrinks
z3fold.o text from 30k to 27k.  Which might even make it faster....

--- a/mm/z3fold.c~a
+++ a/mm/z3fold.c
@@ -185,8 +185,8 @@ enum z3fold_handle_flags {
        HANDLES_ORPHANED = 0,
 };
 
-static inline struct z3fold_header *handle_to_z3fold_header(unsigned long);
-static inline struct z3fold_pool *zhdr_to_pool(struct z3fold_header *);
+static struct z3fold_header *handle_to_z3fold_header(unsigned long);
+static struct z3fold_pool *zhdr_to_pool(struct z3fold_header *);
 static struct z3fold_header *__z3fold_alloc(struct z3fold_pool *, size_t, 
bool);
 static void add_to_unbuddied(struct z3fold_pool *, struct z3fold_header *);
 
@@ -205,7 +205,7 @@ static int size_to_chunks(size_t size)
 
 static void compact_page_work(struct work_struct *w);
 
-static inline struct z3fold_buddy_slots *alloc_slots(struct z3fold_pool *pool,
+static struct z3fold_buddy_slots *alloc_slots(struct z3fold_pool *pool,
                                                        gfp_t gfp)
 {
        struct z3fold_buddy_slots *slots = kmem_cache_alloc(pool->c_handle,
@@ -220,17 +220,17 @@ static inline struct z3fold_buddy_slots
        return slots;
 }
 
-static inline struct z3fold_pool *slots_to_pool(struct z3fold_buddy_slots *s)
+static struct z3fold_pool *slots_to_pool(struct z3fold_buddy_slots *s)
 {
        return (struct z3fold_pool *)(s->pool & ~HANDLE_FLAG_MASK);
 }
 
-static inline struct z3fold_buddy_slots *handle_to_slots(unsigned long handle)
+static struct z3fold_buddy_slots *handle_to_slots(unsigned long handle)
 {
        return (struct z3fold_buddy_slots *)(handle & ~(SLOTS_ALIGN - 1));
 }
 
-static inline void free_handle(unsigned long handle)
+static void free_handle(unsigned long handle)
 {
        struct z3fold_buddy_slots *slots;
        struct z3fold_header *zhdr;
@@ -423,7 +423,7 @@ static unsigned long encode_handle(struc
        return (unsigned long)&slots->slot[idx];
 }
 
-static inline struct z3fold_header *__get_z3fold_header(unsigned long handle,
+static struct z3fold_header *__get_z3fold_header(unsigned long handle,
                                                        bool lock)
 {
        struct z3fold_buddy_slots *slots;
@@ -648,7 +648,7 @@ static int num_free_chunks(struct z3fold
 }
 
 /* Add to the appropriate unbuddied list */
-static inline void add_to_unbuddied(struct z3fold_pool *pool,
+static void add_to_unbuddied(struct z3fold_pool *pool,
                                struct z3fold_header *zhdr)
 {
        if (zhdr->first_chunks == 0 || zhdr->last_chunks == 0 ||
@@ -664,7 +664,7 @@ static inline void add_to_unbuddied(stru
        }
 }
 
-static inline void *mchunk_memmove(struct z3fold_header *zhdr,
+static void *mchunk_memmove(struct z3fold_header *zhdr,
                                unsigned short dst_chunk)
 {
        void *beg = zhdr;
@@ -673,7 +673,7 @@ static inline void *mchunk_memmove(struc
                       zhdr->middle_chunks << CHUNK_SHIFT);
 }
 
-static inline bool buddy_single(struct z3fold_header *zhdr)
+static bool buddy_single(struct z3fold_header *zhdr)
 {
        return !((zhdr->first_chunks && zhdr->middle_chunks) ||
                        (zhdr->first_chunks && zhdr->last_chunks) ||
@@ -884,7 +884,7 @@ static void compact_page_work(struct wor
 }
 
 /* returns _locked_ z3fold page header or NULL */
-static inline struct z3fold_header *__z3fold_alloc(struct z3fold_pool *pool,
+static struct z3fold_header *__z3fold_alloc(struct z3fold_pool *pool,
                                                size_t size, bool can_sleep)
 {
        struct z3fold_header *zhdr = NULL;
_


>
> ...
>
> +static inline struct z3fold_header *__get_z3fold_header(unsigned long handle,
> +                                                     bool lock)
> +{
> +     struct z3fold_buddy_slots *slots;
> +     struct z3fold_header *zhdr;
> +     unsigned int seq;
> +     bool is_valid;
> +
> +     if (!(handle & (1 << PAGE_HEADLESS))) {
> +             slots = handle_to_slots(handle);
> +             do {
> +                     unsigned long addr;
> +
> +                     seq = read_seqbegin(&slots->seqlock);
> +                     addr = *(unsigned long *)handle;
> +                     zhdr = (struct z3fold_header *)(addr & PAGE_MASK);
> +                     preempt_disable();

Why is this done?

> +                     is_valid = !read_seqretry(&slots->seqlock, seq);
> +                     if (!is_valid) {
> +                             preempt_enable();
> +                             continue;
> +                     }
> +                     /*
> +                      * if we are here, zhdr is a pointer to a valid z3fold
> +                      * header. Lock it! And then re-check if someone has
> +                      * changed which z3fold page this handle points to
> +                      */
> +                     if (lock)
> +                             z3fold_page_lock(zhdr);
> +                     preempt_enable();
> +                     /*
> +                      * we use is_valid as a "cached" value: if it's false,
> +                      * no other checks needed, have to go one more round
> +                      */
> +             } while (!is_valid || (read_seqretry(&slots->seqlock, seq) &&
> +                     (lock ? ({ z3fold_page_unlock(zhdr); 1; }) : 1)));
> +     } else {
> +             zhdr = (struct z3fold_header *)(handle & PAGE_MASK);
> +     }
> +
> +     return zhdr;
> +}
>
> ...
>
>  static unsigned short handle_to_chunks(unsigned long handle)
>  {
> -     unsigned long addr = *(unsigned long *)handle;
> +     unsigned long addr;
> +     struct z3fold_buddy_slots *slots = handle_to_slots(handle);
> +     unsigned int seq;
> +
> +     do {
> +             seq = read_seqbegin(&slots->seqlock);
> +             addr = *(unsigned long *)handle;
> +     } while (read_seqretry(&slots->seqlock, seq));

It isn't done here (I think).

Reply via email to