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(-)
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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