zram and zswap always call zs_obj_write() right after zs_malloc(), and zs_obj_write() has to take pool->lock and look up object location again, which zs_malloc() just did. Replace zs_malloc() and zs_obj_write() with one single zs_store() call. zs_store() takes zspage read lock under class->lock, before it drops class->lock to copy the data, while zs_obj_write() used to take it under pool->lock. Compaction and page migration already take zspage lock under class->lock, so this follows the existing pool->lock -> class->lock -> zspage->lock order. We don't need pool->lock there, zs_store() doesn't look up the object via the handle, it already has the zspage from the allocation. Signed-off-by: Sergey Senozhatsky --- Documentation/core-api/memory-allocation.rst | 2 +- Documentation/mm/zsmalloc.rst | 4 +- drivers/block/zram/zram_drv.c | 42 +++++--------- include/linux/zsmalloc.h | 6 +- mm/zsmalloc.c | 58 +++++++++----------- mm/zswap.c | 5 +- 6 files changed, 47 insertions(+), 70 deletions(-) diff --git a/Documentation/core-api/memory-allocation.rst b/Documentation/core-api/memory-allocation.rst index 648222b6c857..5069ac72b857 100644 --- a/Documentation/core-api/memory-allocation.rst +++ b/Documentation/core-api/memory-allocation.rst @@ -9,7 +9,7 @@ allocate small chunks using `kmalloc` or `kmem_cache_alloc` families, large virtually contiguous areas using `vmalloc` and its derivatives, or you can directly request pages from the page allocator with `alloc_pages`. It is also possible to use more specialized allocators, -for instance `cma_alloc` or `zs_malloc`. +for instance `cma_alloc` or `zs_store`. Most of the memory allocation APIs use GFP flags to express how that memory should be allocated. The GFP acronym stands for "get free diff --git a/Documentation/mm/zsmalloc.rst b/Documentation/mm/zsmalloc.rst index d6b21a0c0694..034daae0d37e 100644 --- a/Documentation/mm/zsmalloc.rst +++ b/Documentation/mm/zsmalloc.rst @@ -20,9 +20,9 @@ For simplicity, zsmalloc can only allocate objects of size up to PAGE_SIZE since this satisfies the requirements of all its current users (in the worst case, page is incompressible and is thus stored "as-is" i.e. in uncompressed form). For allocation requests larger than this size, failure -is returned (see zs_malloc). +is returned (see zs_store). -Additionally, zs_malloc() does not return a pointer; instead, it returns 0 +Additionally, zs_store() does not return a pointer; instead, it returns 0 (or a negative error code) and stores the encoded location of the allocated object in the caller-provided ``struct zs_handle``. The reason for this indirection is that zsmalloc does not keep zspages permanently mapped since diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index 7c3d04d9c5bc..42852a4116e0 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -2261,9 +2261,11 @@ static int write_incompressible_page(struct zram *zram, struct page *page, slot_lock(zram, index); slot_free(zram, index); - ret = zs_malloc(zram->mem_pool, &zram->table[index].handle, PAGE_SIZE, - GFP_NOIO | __GFP_NOWARN | - __GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page)); + src = kmap_local_page(page); + ret = zs_store(zram->mem_pool, &zram->table[index].handle, src, + PAGE_SIZE, GFP_NOIO | __GFP_NOWARN | + __GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page)); + kunmap_local(src); if (ret) { slot_unlock(zram, index); return ret; @@ -2271,11 +2273,6 @@ static int write_incompressible_page(struct zram *zram, struct page *page, update_used_max(zram); - src = kmap_local_page(page); - zs_obj_write(zram->mem_pool, &zram->table[index].handle, src, - PAGE_SIZE); - kunmap_local(src); - set_slot_flag(zram, index, ZRAM_HUGE); set_slot_size(zram, index, PAGE_SIZE); mark_slot_accessed(zram, index); @@ -2327,21 +2324,18 @@ static int zram_write_page(struct zram *zram, struct page *page, slot_lock(zram, index); slot_free(zram, index); - ret = zs_malloc(zram->mem_pool, &zram->table[index].handle, comp_len, - GFP_NOIO | __GFP_NOWARN | - __GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page)); + ret = zs_store(zram->mem_pool, &zram->table[index].handle, + zstrm->buffer, comp_len, + GFP_NOIO | __GFP_NOWARN | + __GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page)); + zcomp_cstrm_put(zstrm); if (ret) { - zcomp_cstrm_put(zstrm); slot_unlock(zram, index); return ret; } update_used_max(zram); - zs_obj_write(zram->mem_pool, &zram->table[index].handle, zstrm->buffer, - comp_len); - zcomp_cstrm_put(zstrm); - set_slot_size(zram, index, comp_len); mark_slot_accessed(zram, index); slot_unlock(zram, index); @@ -2526,18 +2520,12 @@ static int recompress_slot(struct zram *zram, unsigned long index, * modify zsmalloc API to return this information. For now, we will * make do with the node of the page allocated for recompression. */ - ret = zs_malloc(zram->mem_pool, &zram->recomp_handle, comp_len_new, - GFP_NOIO | __GFP_NOWARN | - __GFP_HIGHMEM | __GFP_MOVABLE, - page_to_nid(page)); - if (ret) { - zcomp_cstrm_put(zstrm); - return ret; - } - - zs_obj_write(zram->mem_pool, &zram->recomp_handle, zstrm->buffer, - comp_len_new); + ret = zs_store(zram->mem_pool, &zram->recomp_handle, zstrm->buffer, + comp_len_new, GFP_NOIO | __GFP_NOWARN | + __GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page)); zcomp_cstrm_put(zstrm); + if (ret) + return ret; slot_free(zram, index); zs_rebind_handle(zram->mem_pool, &zram->table[index].handle, diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h index 8273ae924153..8971eef333ba 100644 --- a/include/linux/zsmalloc.h +++ b/include/linux/zsmalloc.h @@ -49,8 +49,8 @@ struct scatterlist; struct zs_pool *zs_create_pool(const char *name); void zs_destroy_pool(struct zs_pool *pool); -int zs_malloc(struct zs_pool *pool, struct zs_handle *handle, size_t size, - gfp_t flags, const int nid); +int zs_store(struct zs_pool *pool, struct zs_handle *handle, const void *src, + size_t size, gfp_t flags, const int nid); void zs_free(struct zs_pool *pool, struct zs_handle *handle); void zs_rebind_handle(struct zs_pool *pool, struct zs_handle *dst, struct zs_handle *src); @@ -67,8 +67,6 @@ void zs_pool_stats(struct zs_pool *pool, struct zs_pool_stats *stats); void zs_obj_read_sg_begin(struct zs_pool *pool, const struct zs_handle *handle, struct scatterlist *sg, size_t mem_len); void zs_obj_read_sg_end(struct zs_pool *pool, const struct zs_handle *handle); -void zs_obj_write(struct zs_pool *pool, const struct zs_handle *handle, - void *handle_mem, size_t mem_len); extern const struct movable_operations zsmalloc_mops; diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c index fd466139e158..09ce8c294c5e 100644 --- a/mm/zsmalloc.c +++ b/mm/zsmalloc.c @@ -1178,54 +1178,37 @@ void zs_obj_read_sg_end(struct zs_pool *pool, const struct zs_handle *handle) } EXPORT_SYMBOL_GPL(zs_obj_read_sg_end); -void zs_obj_write(struct zs_pool *pool, const struct zs_handle *handle, - void *handle_mem, size_t mem_len) +static void obj_write(struct size_class *class, struct zspage *zspage, + unsigned long obj, const void *src, size_t size) { - struct zspage *zspage; struct zpdesc *zpdesc; - unsigned long obj, off; + unsigned long off; unsigned int obj_idx; - struct size_class *class; - /* Guarantee we can get zspage from handle safely */ - read_lock(&pool->lock); - obj = handle_to_obj(handle); obj_to_location(obj, &zpdesc, &obj_idx); - zspage = get_zspage(zpdesc); - - /* Make sure migration doesn't move any pages in this zspage */ - zspage_read_lock(zspage); - read_unlock(&pool->lock); - - class = zspage_class(pool, zspage); off = offset_in_page(class->size * obj_idx); if (!ZsHugePage(zspage)) off += ZS_HANDLE_SIZE; - if (off + mem_len <= PAGE_SIZE) { + if (off + size <= PAGE_SIZE) { /* this object is contained entirely within a page */ void *dst = kmap_local_zpdesc(zpdesc); - memcpy(dst + off, handle_mem, mem_len); + memcpy(dst + off, src, size); kunmap_local(dst); } else { /* this object spans two pages */ size_t sizes[2]; sizes[0] = PAGE_SIZE - off; - sizes[1] = mem_len - sizes[0]; + sizes[1] = size - sizes[0]; - memcpy_to_page(zpdesc_page(zpdesc), off, - handle_mem, sizes[0]); + memcpy_to_page(zpdesc_page(zpdesc), off, src, sizes[0]); zpdesc = get_next_zpdesc(zpdesc); - memcpy_to_page(zpdesc_page(zpdesc), 0, - handle_mem + sizes[0], sizes[1]); + memcpy_to_page(zpdesc_page(zpdesc), 0, src + sizes[0], sizes[1]); } - - zspage_read_unlock(zspage); } -EXPORT_SYMBOL_GPL(zs_obj_write); /** * zs_huge_class_size() - Returns the size (in bytes) of the first huge @@ -1290,9 +1273,10 @@ static unsigned long obj_malloc(struct zs_pool *pool, /** - * zs_malloc - Allocate block of given size from pool. + * zs_store() - Allocate block of given size from pool and copy data into it. * @pool: pool to allocate from * @handle: handle to store allocated object in + * @src: data to copy into the allocated object * @size: size of block to allocate * @gfp: gfp flags when allocating object * @nid: The preferred node id to allocate new zspage (if needed) @@ -1305,10 +1289,11 @@ static unsigned long obj_malloc(struct zs_pool *pool, * * Return: 0 on success, negative error code otherwise. */ -int zs_malloc(struct zs_pool *pool, struct zs_handle *handle, size_t size, - gfp_t gfp, const int nid) +int zs_store(struct zs_pool *pool, struct zs_handle *handle, const void *src, + size_t size, gfp_t gfp, const int nid) { struct size_class *class; + unsigned long obj; int newfg; struct zspage *zspage; @@ -1330,7 +1315,7 @@ int zs_malloc(struct zs_pool *pool, struct zs_handle *handle, size_t size, spin_lock(&class->lock); zspage = find_get_zspage(class); if (likely(zspage)) { - obj_malloc(pool, zspage, handle); + obj = obj_malloc(pool, zspage, handle); /* Now move the zspage to another fullness group, if required */ fix_fullness_group(class, zspage); class_stat_add(class, ZS_OBJS_INUSE, 1); @@ -1345,7 +1330,7 @@ int zs_malloc(struct zs_pool *pool, struct zs_handle *handle, size_t size, return -ENOMEM; spin_lock(&class->lock); - obj_malloc(pool, zspage, handle); + obj = obj_malloc(pool, zspage, handle); newfg = get_fullness_group(class, zspage); insert_zspage(class, zspage, newfg); atomic_long_add(class->pages_per_zspage, &pool->pages_allocated); @@ -1355,11 +1340,15 @@ int zs_malloc(struct zs_pool *pool, struct zs_handle *handle, size_t size, /* We completely set up zspage so mark them as movable */ SetZsPageMovable(pool, zspage); out: + zspage_read_lock(zspage); spin_unlock(&class->lock); + obj_write(class, zspage, obj, src, size); + zspage_read_unlock(zspage); + return 0; } -EXPORT_SYMBOL_GPL(zs_malloc); +EXPORT_SYMBOL_GPL(zs_store); static void obj_free(int class_size, unsigned long obj) { @@ -1844,7 +1833,10 @@ static int zs_page_migrate(struct page *newpage, struct page *page, * the class lock protects zpage alloc/free in the zspage. */ spin_lock(&class->lock); - /* the zspage write_lock protects zpage access via zs_obj_read/write() */ + /* + * The zspage write_lock protects zpage access via zs_obj_read_sg_*() + * and zs_store(). + */ if (!zspage_write_trylock(zspage)) { spin_unlock(&class->lock); write_unlock(&pool->lock); @@ -2238,7 +2230,7 @@ struct zs_pool *zs_create_pool(const char *name) huge_class_size = size; /* * The object uses ZS_HANDLE_SIZE bytes to store the - * handle. We need to subtract it, because zs_malloc() + * handle. We need to subtract it, because zs_store() * unconditionally adds handle size before it performs * size class search - so object may be smaller than * huge class size, yet it still can end up in the huge diff --git a/mm/zswap.c b/mm/zswap.c index 241f9fdebaad..a2e0314db1cf 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -950,12 +950,11 @@ static bool zswap_compress(struct folio *folio, long index, } gfp = GFP_NOWAIT | __GFP_NORETRY | __GFP_HIGHMEM | __GFP_MOVABLE; - alloc_ret = zs_malloc(pool->zs_pool, &entry->handle, dlen, gfp, - folio_nid(folio)); + alloc_ret = zs_store(pool->zs_pool, &entry->handle, dst, dlen, gfp, + folio_nid(folio)); if (alloc_ret) goto unlock; - zs_obj_write(pool->zs_pool, &entry->handle, dst, dlen); entry->length = dlen; unlock: -- 2.56.0.506.g506a7eabc5-goog