Commit ca0246bb authored by Vitaly Wool's avatar Vitaly Wool Committed by Linus Torvalds

z3fold: fix possible reclaim races

Reclaim and free can race on an object which is basically fine but in
order for reclaim to be able to map "freed" object we need to encode
object length in the handle.  handle_to_chunks() is then introduced to
extract object length from a handle and use it during mapping.

Moreover, to avoid racing on a z3fold "headless" page release, we should
not try to free that page in z3fold_free() if the reclaim bit is set.
Also, in the unlikely case of trying to reclaim a page being freed, we
should not proceed with that page.

While at it, fix the page accounting in reclaim function.

This patch supersedes "[PATCH] z3fold: fix reclaim lock-ups".

Link: http://lkml.kernel.org/r/20181105162225.74e8837d03583a9b707cf559@gmail.comSigned-off-by: default avatarVitaly Wool <vitaly.vul@sony.com>
Signed-off-by: default avatarJongseok Kim <ks77sj@gmail.com>
Reported-by-by: default avatarJongseok Kim <ks77sj@gmail.com>
Reviewed-by: default avatarSnild Dolkow <snild@sony.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 1ce80e0f
...@@ -99,6 +99,7 @@ struct z3fold_header { ...@@ -99,6 +99,7 @@ struct z3fold_header {
#define NCHUNKS ((PAGE_SIZE - ZHDR_SIZE_ALIGNED) >> CHUNK_SHIFT) #define NCHUNKS ((PAGE_SIZE - ZHDR_SIZE_ALIGNED) >> CHUNK_SHIFT)
#define BUDDY_MASK (0x3) #define BUDDY_MASK (0x3)
#define BUDDY_SHIFT 2
/** /**
* struct z3fold_pool - stores metadata for each z3fold pool * struct z3fold_pool - stores metadata for each z3fold pool
...@@ -145,7 +146,7 @@ enum z3fold_page_flags { ...@@ -145,7 +146,7 @@ enum z3fold_page_flags {
MIDDLE_CHUNK_MAPPED, MIDDLE_CHUNK_MAPPED,
NEEDS_COMPACTING, NEEDS_COMPACTING,
PAGE_STALE, PAGE_STALE,
UNDER_RECLAIM PAGE_CLAIMED, /* by either reclaim or free */
}; };
/***************** /*****************
...@@ -174,7 +175,7 @@ static struct z3fold_header *init_z3fold_page(struct page *page, ...@@ -174,7 +175,7 @@ static struct z3fold_header *init_z3fold_page(struct page *page,
clear_bit(MIDDLE_CHUNK_MAPPED, &page->private); clear_bit(MIDDLE_CHUNK_MAPPED, &page->private);
clear_bit(NEEDS_COMPACTING, &page->private); clear_bit(NEEDS_COMPACTING, &page->private);
clear_bit(PAGE_STALE, &page->private); clear_bit(PAGE_STALE, &page->private);
clear_bit(UNDER_RECLAIM, &page->private); clear_bit(PAGE_CLAIMED, &page->private);
spin_lock_init(&zhdr->page_lock); spin_lock_init(&zhdr->page_lock);
kref_init(&zhdr->refcount); kref_init(&zhdr->refcount);
...@@ -223,8 +224,11 @@ static unsigned long encode_handle(struct z3fold_header *zhdr, enum buddy bud) ...@@ -223,8 +224,11 @@ static unsigned long encode_handle(struct z3fold_header *zhdr, enum buddy bud)
unsigned long handle; unsigned long handle;
handle = (unsigned long)zhdr; handle = (unsigned long)zhdr;
if (bud != HEADLESS) if (bud != HEADLESS) {
handle += (bud + zhdr->first_num) & BUDDY_MASK; handle |= (bud + zhdr->first_num) & BUDDY_MASK;
if (bud == LAST)
handle |= (zhdr->last_chunks << BUDDY_SHIFT);
}
return handle; return handle;
} }
...@@ -234,6 +238,12 @@ static struct z3fold_header *handle_to_z3fold_header(unsigned long handle) ...@@ -234,6 +238,12 @@ static struct z3fold_header *handle_to_z3fold_header(unsigned long handle)
return (struct z3fold_header *)(handle & PAGE_MASK); return (struct z3fold_header *)(handle & PAGE_MASK);
} }
/* only for LAST bud, returns zero otherwise */
static unsigned short handle_to_chunks(unsigned long handle)
{
return (handle & ~PAGE_MASK) >> BUDDY_SHIFT;
}
/* /*
* (handle & BUDDY_MASK) < zhdr->first_num is possible in encode_handle * (handle & BUDDY_MASK) < zhdr->first_num is possible in encode_handle
* but that doesn't matter. because the masking will result in the * but that doesn't matter. because the masking will result in the
...@@ -720,37 +730,39 @@ static void z3fold_free(struct z3fold_pool *pool, unsigned long handle) ...@@ -720,37 +730,39 @@ static void z3fold_free(struct z3fold_pool *pool, unsigned long handle)
page = virt_to_page(zhdr); page = virt_to_page(zhdr);
if (test_bit(PAGE_HEADLESS, &page->private)) { if (test_bit(PAGE_HEADLESS, &page->private)) {
/* HEADLESS page stored */ /* if a headless page is under reclaim, just leave.
bud = HEADLESS; * NB: we use test_and_set_bit for a reason: if the bit
} else { * has not been set before, we release this page
z3fold_page_lock(zhdr); * immediately so we don't care about its value any more.
bud = handle_to_buddy(handle); */
if (!test_and_set_bit(PAGE_CLAIMED, &page->private)) {
switch (bud) { spin_lock(&pool->lock);
case FIRST: list_del(&page->lru);
zhdr->first_chunks = 0; spin_unlock(&pool->lock);
break; free_z3fold_page(page);
case MIDDLE: atomic64_dec(&pool->pages_nr);
zhdr->middle_chunks = 0;
zhdr->start_middle = 0;
break;
case LAST:
zhdr->last_chunks = 0;
break;
default:
pr_err("%s: unknown bud %d\n", __func__, bud);
WARN_ON(1);
z3fold_page_unlock(zhdr);
return;
} }
return;
} }
if (bud == HEADLESS) { /* Non-headless case */
spin_lock(&pool->lock); z3fold_page_lock(zhdr);
list_del(&page->lru); bud = handle_to_buddy(handle);
spin_unlock(&pool->lock);
free_z3fold_page(page); switch (bud) {
atomic64_dec(&pool->pages_nr); case FIRST:
zhdr->first_chunks = 0;
break;
case MIDDLE:
zhdr->middle_chunks = 0;
break;
case LAST:
zhdr->last_chunks = 0;
break;
default:
pr_err("%s: unknown bud %d\n", __func__, bud);
WARN_ON(1);
z3fold_page_unlock(zhdr);
return; return;
} }
...@@ -758,7 +770,7 @@ static void z3fold_free(struct z3fold_pool *pool, unsigned long handle) ...@@ -758,7 +770,7 @@ static void z3fold_free(struct z3fold_pool *pool, unsigned long handle)
atomic64_dec(&pool->pages_nr); atomic64_dec(&pool->pages_nr);
return; return;
} }
if (test_bit(UNDER_RECLAIM, &page->private)) { if (test_bit(PAGE_CLAIMED, &page->private)) {
z3fold_page_unlock(zhdr); z3fold_page_unlock(zhdr);
return; return;
} }
...@@ -836,20 +848,30 @@ static int z3fold_reclaim_page(struct z3fold_pool *pool, unsigned int retries) ...@@ -836,20 +848,30 @@ static int z3fold_reclaim_page(struct z3fold_pool *pool, unsigned int retries)
} }
list_for_each_prev(pos, &pool->lru) { list_for_each_prev(pos, &pool->lru) {
page = list_entry(pos, struct page, lru); page = list_entry(pos, struct page, lru);
/* this bit could have been set by free, in which case
* we pass over to the next page in the pool.
*/
if (test_and_set_bit(PAGE_CLAIMED, &page->private))
continue;
zhdr = page_address(page);
if (test_bit(PAGE_HEADLESS, &page->private)) if (test_bit(PAGE_HEADLESS, &page->private))
/* candidate found */
break; break;
zhdr = page_address(page); if (!z3fold_page_trylock(zhdr)) {
if (!z3fold_page_trylock(zhdr)) zhdr = NULL;
continue; /* can't evict at this point */ continue; /* can't evict at this point */
}
kref_get(&zhdr->refcount); kref_get(&zhdr->refcount);
list_del_init(&zhdr->buddy); list_del_init(&zhdr->buddy);
zhdr->cpu = -1; zhdr->cpu = -1;
set_bit(UNDER_RECLAIM, &page->private);
break; break;
} }
if (!zhdr)
break;
list_del_init(&page->lru); list_del_init(&page->lru);
spin_unlock(&pool->lock); spin_unlock(&pool->lock);
...@@ -898,6 +920,7 @@ static int z3fold_reclaim_page(struct z3fold_pool *pool, unsigned int retries) ...@@ -898,6 +920,7 @@ static int z3fold_reclaim_page(struct z3fold_pool *pool, unsigned int retries)
if (test_bit(PAGE_HEADLESS, &page->private)) { if (test_bit(PAGE_HEADLESS, &page->private)) {
if (ret == 0) { if (ret == 0) {
free_z3fold_page(page); free_z3fold_page(page);
atomic64_dec(&pool->pages_nr);
return 0; return 0;
} }
spin_lock(&pool->lock); spin_lock(&pool->lock);
...@@ -905,7 +928,7 @@ static int z3fold_reclaim_page(struct z3fold_pool *pool, unsigned int retries) ...@@ -905,7 +928,7 @@ static int z3fold_reclaim_page(struct z3fold_pool *pool, unsigned int retries)
spin_unlock(&pool->lock); spin_unlock(&pool->lock);
} else { } else {
z3fold_page_lock(zhdr); z3fold_page_lock(zhdr);
clear_bit(UNDER_RECLAIM, &page->private); clear_bit(PAGE_CLAIMED, &page->private);
if (kref_put(&zhdr->refcount, if (kref_put(&zhdr->refcount,
release_z3fold_page_locked)) { release_z3fold_page_locked)) {
atomic64_dec(&pool->pages_nr); atomic64_dec(&pool->pages_nr);
...@@ -964,7 +987,7 @@ static void *z3fold_map(struct z3fold_pool *pool, unsigned long handle) ...@@ -964,7 +987,7 @@ static void *z3fold_map(struct z3fold_pool *pool, unsigned long handle)
set_bit(MIDDLE_CHUNK_MAPPED, &page->private); set_bit(MIDDLE_CHUNK_MAPPED, &page->private);
break; break;
case LAST: case LAST:
addr += PAGE_SIZE - (zhdr->last_chunks << CHUNK_SHIFT); addr += PAGE_SIZE - (handle_to_chunks(handle) << CHUNK_SHIFT);
break; break;
default: default:
pr_err("unknown buddy id %d\n", buddy); pr_err("unknown buddy id %d\n", buddy);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment