On Fri, 24 May 2019 17:49:18 +0200 Vitaly Wool <vitalywool@xxxxxxxxx> 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).
