linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] z3fold fixes for intra-page compaction
@ 2019-11-27 14:20 Vitaly Wool
  2019-11-27 14:21 ` [PATCH 1/3] z3fold: avoid subtle race when freeing slots Vitaly Wool
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Vitaly Wool @ 2019-11-27 14:20 UTC (permalink / raw)
  To: linux-mm, linux-kernel; +Cc: Andrew Morton

This is a consolidation of z3fold stability fixes for the new intra-page
compaction functionality.

Vitaly Wool (3):
    z3fold: avoid subtle race when freeing slots
    z3fold: compact objects more accurately
    z3fold: protect handle reads

 mm/z3fold.c |   30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/3] z3fold: avoid subtle race when freeing slots
  2019-11-27 14:20 [PATCH 0/3] z3fold fixes for intra-page compaction Vitaly Wool
@ 2019-11-27 14:21 ` Vitaly Wool
  2019-11-27 14:22 ` [PATCH 2/3] z3fold: compact objects more accurately Vitaly Wool
  2019-11-27 14:23 ` [PATCH 3/3] z3fold: protect handle reads Vitaly Wool
  2 siblings, 0 replies; 4+ messages in thread
From: Vitaly Wool @ 2019-11-27 14:21 UTC (permalink / raw)
  To: linux-mm, linux-kernel; +Cc: Andrew Morton

There is a subtle race between freeing slots and setting the last
slot to zero since the OPRPHANED flag was set after the rwlock
had been released. Fix that to avoid rare memory leaks caused by
this race.

Signed-off-by: Vitaly Wool <vitaly.vul@sony.com>
---
 mm/z3fold.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/mm/z3fold.c b/mm/z3fold.c
index d48d0ec3bcdd..36bd2612f609 100644
--- a/mm/z3fold.c
+++ b/mm/z3fold.c
@@ -327,6 +327,10 @@ static inline void free_handle(unsigned long handle)
 	zhdr->foreign_handles--;
 	is_free = true;
 	read_lock(&slots->lock);
+	if (!test_bit(HANDLES_ORPHANED, &slots->pool)) {
+		read_unlock(&slots->lock);
+		return;
+	}
 	for (i = 0; i <= BUDDY_MASK; i++) {
 		if (slots->slot[i]) {
 			is_free = false;
@@ -335,7 +339,7 @@ static inline void free_handle(unsigned long handle)
 	}
 	read_unlock(&slots->lock);
 
-	if (is_free && test_and_clear_bit(HANDLES_ORPHANED, &slots->pool)) {
+	if (is_free) {
 		struct z3fold_pool *pool = slots_to_pool(slots);
 
 		kmem_cache_free(pool->c_handle, slots);
@@ -531,12 +535,12 @@ static void __release_z3fold_page(struct z3fold_header *zhdr, bool locked)
 			break;
 		}
 	}
+	if (!is_free)
+		set_bit(HANDLES_ORPHANED, &zhdr->slots->pool);
 	read_unlock(&zhdr->slots->lock);
 
 	if (is_free)
 		kmem_cache_free(pool->c_handle, zhdr->slots);
-	else
-		set_bit(HANDLES_ORPHANED, &zhdr->slots->pool);
 
 	if (locked)
 		z3fold_page_unlock(zhdr);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/3] z3fold: compact objects more accurately
  2019-11-27 14:20 [PATCH 0/3] z3fold fixes for intra-page compaction Vitaly Wool
  2019-11-27 14:21 ` [PATCH 1/3] z3fold: avoid subtle race when freeing slots Vitaly Wool
@ 2019-11-27 14:22 ` Vitaly Wool
  2019-11-27 14:23 ` [PATCH 3/3] z3fold: protect handle reads Vitaly Wool
  2 siblings, 0 replies; 4+ messages in thread
From: Vitaly Wool @ 2019-11-27 14:22 UTC (permalink / raw)
  To: linux-mm, linux-kernel; +Cc: Andrew Morton

There are several small things to be considered regarding the
new inter-page compaction mechanism. First, we better set the
relevant size in chunks to 0 in the old z3fold header for the
object that has been moved to another z3fold page. Then, we
shouldn't do inter-page compaction if an object is mapped.
Lastly, free_handle should happen before release_z3fold_page
(but not in case the page is under reclaim, it will the handle
will be freed by reclaim then).

This patch addresses all three issues.

Signed-off-by: Vitaly Wool <vitaly.vul@sony.com>
---
 mm/z3fold.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/mm/z3fold.c b/mm/z3fold.c
index 36bd2612f609..f2a75418e248 100644
--- a/mm/z3fold.c
+++ b/mm/z3fold.c
@@ -670,6 +670,7 @@ static struct z3fold_header *compact_single_buddy(struct z3fold_header *zhdr)
 	int first_idx = __idx(zhdr, FIRST);
 	int middle_idx = __idx(zhdr, MIDDLE);
 	int last_idx = __idx(zhdr, LAST);
+	unsigned short *moved_chunks = NULL;
 
 	/*
 	 * No need to protect slots here -- all the slots are "local" and
@@ -679,14 +680,17 @@ static struct z3fold_header *compact_single_buddy(struct z3fold_header *zhdr)
 		p += ZHDR_SIZE_ALIGNED;
 		sz = zhdr->first_chunks << CHUNK_SHIFT;
 		old_handle = (unsigned long)&zhdr->slots->slot[first_idx];
+		moved_chunks = &zhdr->first_chunks;
 	} else if (zhdr->middle_chunks && zhdr->slots->slot[middle_idx]) {
 		p += zhdr->start_middle << CHUNK_SHIFT;
 		sz = zhdr->middle_chunks << CHUNK_SHIFT;
 		old_handle = (unsigned long)&zhdr->slots->slot[middle_idx];
+		moved_chunks = &zhdr->middle_chunks;
 	} else if (zhdr->last_chunks && zhdr->slots->slot[last_idx]) {
 		p += PAGE_SIZE - (zhdr->last_chunks << CHUNK_SHIFT);
 		sz = zhdr->last_chunks << CHUNK_SHIFT;
 		old_handle = (unsigned long)&zhdr->slots->slot[last_idx];
+		moved_chunks = &zhdr->last_chunks;
 	}
 
 	if (sz > 0) {
@@ -743,6 +747,8 @@ static struct z3fold_header *compact_single_buddy(struct z3fold_header *zhdr)
 		write_unlock(&zhdr->slots->lock);
 		add_to_unbuddied(pool, new_zhdr);
 		z3fold_page_unlock(new_zhdr);
+
+		*moved_chunks = 0;
 	}
 
 	return new_zhdr;
@@ -840,7 +846,7 @@ static void do_compact_page(struct z3fold_header *zhdr, bool locked)
 	}
 
 	if (!zhdr->foreign_handles && buddy_single(zhdr) &&
-			compact_single_buddy(zhdr)) {
+	    zhdr->mapped_count == 0 && compact_single_buddy(zhdr)) {
 		if (kref_put(&zhdr->refcount, release_z3fold_page_locked))
 			atomic64_dec(&pool->pages_nr);
 		else
@@ -1254,6 +1260,8 @@ static void z3fold_free(struct z3fold_pool *pool, unsigned long handle)
 		return;
 	}
 
+	if (!page_claimed)
+		free_handle(handle);
 	if (kref_put(&zhdr->refcount, release_z3fold_page_locked_list)) {
 		atomic64_dec(&pool->pages_nr);
 		return;
@@ -1263,7 +1271,6 @@ static void z3fold_free(struct z3fold_pool *pool, unsigned long handle)
 		z3fold_page_unlock(zhdr);
 		return;
 	}
-	free_handle(handle);
 	if (unlikely(PageIsolated(page)) ||
 	    test_and_set_bit(NEEDS_COMPACTING, &page->private)) {
 		put_z3fold_header(zhdr);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 3/3] z3fold: protect handle reads
  2019-11-27 14:20 [PATCH 0/3] z3fold fixes for intra-page compaction Vitaly Wool
  2019-11-27 14:21 ` [PATCH 1/3] z3fold: avoid subtle race when freeing slots Vitaly Wool
  2019-11-27 14:22 ` [PATCH 2/3] z3fold: compact objects more accurately Vitaly Wool
@ 2019-11-27 14:23 ` Vitaly Wool
  2 siblings, 0 replies; 4+ messages in thread
From: Vitaly Wool @ 2019-11-27 14:23 UTC (permalink / raw)
  To: linux-mm, linux-kernel; +Cc: Andrew Morton

We need to make sure slots are protected on reading a handle. Since
handles are modified by free_handle() which only takes slot rwlock,
that lock should be used when handles are read.

Signed-off-by: Vitaly Wool <vitaly.vul@sony.com>
---
 mm/z3fold.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/mm/z3fold.c b/mm/z3fold.c
index f2a75418e248..43754d8ebce8 100644
--- a/mm/z3fold.c
+++ b/mm/z3fold.c
@@ -486,8 +486,12 @@ static unsigned long encode_handle(struct z3fold_header *zhdr, enum buddy bud)
 /* only for LAST bud, returns zero otherwise */
 static unsigned short handle_to_chunks(unsigned long handle)
 {
-	unsigned long addr = *(unsigned long *)handle;
+	struct z3fold_buddy_slots *slots = handle_to_slots(handle);
+	unsigned long addr;
 
+	read_lock(&slots->lock);
+	addr = *(unsigned long *)handle;
+	read_unlock(&slots->lock);
 	return (addr & ~PAGE_MASK) >> BUDDY_SHIFT;
 }
 
@@ -499,10 +503,13 @@ static unsigned short handle_to_chunks(unsigned long handle)
 static enum buddy handle_to_buddy(unsigned long handle)
 {
 	struct z3fold_header *zhdr;
+	struct z3fold_buddy_slots *slots = handle_to_slots(handle);
 	unsigned long addr;
 
+	read_lock(&slots->lock);
 	WARN_ON(handle & (1 << PAGE_HEADLESS));
 	addr = *(unsigned long *)handle;
+	read_unlock(&slots->lock);
 	zhdr = (struct z3fold_header *)(addr & PAGE_MASK);
 	return (addr - zhdr->first_num) & BUDDY_MASK;
 }
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-11-27 14:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-27 14:20 [PATCH 0/3] z3fold fixes for intra-page compaction Vitaly Wool
2019-11-27 14:21 ` [PATCH 1/3] z3fold: avoid subtle race when freeing slots Vitaly Wool
2019-11-27 14:22 ` [PATCH 2/3] z3fold: compact objects more accurately Vitaly Wool
2019-11-27 14:23 ` [PATCH 3/3] z3fold: protect handle reads Vitaly Wool

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).