linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Vitaly Wool <vitalywool@gmail.com>
Cc: Linux-MM <linux-mm@kvack.org>,
	linux-kernel@vger.kernel.org, Dan Streetman <ddstreet@ieee.org>,
	Oleksiy.Avramchenko@sony.com,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Uladzislau Rezki <urezki@gmail.com>
Subject: Re: [PATCH] z3fold: add inter-page compaction
Date: Sat, 25 May 2019 15:09:48 -0700	[thread overview]
Message-ID: <20190525150948.e1ff1a2a894ca8110abc8183@linux-foundation.org> (raw)
In-Reply-To: <20190524174918.71074b358001bdbf1c23cd77@gmail.com>

On Fri, 24 May 2019 17:49:18 +0200 Vitaly Wool <vitalywool@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	other threads:[~2019-05-25 22:10 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-24 15:49 Vitaly Wool
2019-05-25 22:09 ` Andrew Morton [this message]
2019-05-27 10:54   ` Vitaly Wool
2019-10-06  4:14 Vitaly Wool

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190525150948.e1ff1a2a894ca8110abc8183@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=Oleksiy.Avramchenko@sony.com \
    --cc=b.zolnierkie@samsung.com \
    --cc=ddstreet@ieee.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=urezki@gmail.com \
    --cc=vitalywool@gmail.com \
    --subject='Re: [PATCH] z3fold: add inter-page compaction' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).