* [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool
@ 2024-12-05 17:49 Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 01/21] mm/zsmalloc: add " Matthew Wilcox (Oracle)
` (22 more replies)
0 siblings, 23 replies; 33+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-12-05 17:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky; +Cc: Matthew Wilcox (Oracle), linux-mm
I've updated this patchset for 6.13-rc1 because it's needed to eliminate
page->index, which I'm hoping to do in the next merge window.
- Renamed mops to movable_ops and changed its type to unsigned long
- Separated out the documentation of the individual page flags
- Removed memcg_data again
- Adapted to memcpy_from_page() API
- Add kmap_local_zpdesc() instead of zpdesc_kmap_atomic()
My actual review of this has been quite limited.
Alex Shi (10):
mm/zsmalloc: add zpdesc memory descriptor for zswap.zpool
mm/zsmalloc: use zpdesc in trylock_zspage()/lock_zspage()
mm/zsmalloc: convert create_page_chain() and its users to use zpdesc
mm/zsmalloc: rename reset_page to reset_zpdesc and use zpdesc in it
mm/zsmalloc: convert SetZsPageMovable and remove unused funcs
mm/zsmalloc: convert get/set_first_obj_offset() to take zpdesc
mm/zsmalloc: introduce __zpdesc_clear_movable
mm/zsmalloc: introduce __zpdesc_clear/set_zsmalloc()
mm/zsmalloc: introduce zpdesc_clear_first() helper
mm/zsmalloc: update comments for page->zpdesc changes
Hyeonggon Yoo (11):
mm/zsmalloc: convert __zs_map_object/__zs_unmap_object to use zpdesc
mm/zsmalloc: add and use pfn/zpdesc seeking funcs
mm/zsmalloc: convert obj_malloc() to use zpdesc
mm/zsmalloc: convert obj_allocated() and related helpers to use zpdesc
mm/zsmalloc: convert init_zspage() to use zpdesc
mm/zsmalloc: convert obj_to_page() and zs_free() to use zpdesc
mm/zsmalloc: add zpdesc_is_isolated()/zpdesc_zone() helper for
zs_page_migrate()
mm/zsmalloc: convert __free_zspage() to use zpdesc
mm/zsmalloc: convert location_to_obj() to take zpdesc
mm/zsmalloc: convert migrate_zspage() to use zpdesc
mm/zsmalloc: convert get_zspage() to take zpdesc
mm/zpdesc.h | 149 ++++++++++++++++
mm/zsmalloc.c | 485 +++++++++++++++++++++++++++-----------------------
2 files changed, 411 insertions(+), 223 deletions(-)
create mode 100644 mm/zpdesc.h
--
2.45.2
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v8 01/21] mm/zsmalloc: add zpdesc memory descriptor for zswap.zpool
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
@ 2024-12-05 17:49 ` Matthew Wilcox (Oracle)
2024-12-10 13:44 ` Hyeonggon Yoo
2024-12-05 17:49 ` [PATCH v8 02/21] mm/zsmalloc: use zpdesc in trylock_zspage()/lock_zspage() Matthew Wilcox (Oracle)
` (21 subsequent siblings)
22 siblings, 1 reply; 33+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-12-05 17:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky; +Cc: Alex Shi, linux-mm, Hyeonggon Yoo
From: Alex Shi <alexs@kernel.org>
The 1st patch introduces new memory descriptor zpdesc and rename
zspage.first_page to zspage.first_zpdesc, no functional change.
We removed PG_owner_priv_1 since it was moved to zspage after
commit a41ec880aa7b ("zsmalloc: move huge compressed obj from
page to zspage").
Originally-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Signed-off-by: Alex Shi <alexs@kernel.org>
---
mm/zpdesc.h | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++
mm/zsmalloc.c | 9 ++++---
2 files changed, 74 insertions(+), 4 deletions(-)
create mode 100644 mm/zpdesc.h
diff --git a/mm/zpdesc.h b/mm/zpdesc.h
new file mode 100644
index 000000000000..9ad232774469
--- /dev/null
+++ b/mm/zpdesc.h
@@ -0,0 +1,69 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* zpdesc.h: zswap.zpool memory descriptor
+ *
+ * Written by Alex Shi <alexs@kernel.org>
+ * Hyeonggon Yoo <42.hyeyoo@gmail.com>
+ */
+#ifndef __MM_ZPDESC_H__
+#define __MM_ZPDESC_H__
+
+/*
+ * struct zpdesc - Memory descriptor for zpool memory
+ * @flags: Page flags, mostly unused.
+ * @lru: Indirectly used by page migration
+ * @movable_ops: Used by page migration
+ * @next: Next zpdesc in a zspage in zsmalloc zpool
+ * @handle: For huge zspage in zsmalloc zpool
+ * @zspage: Points to the zspage this zpdesc is a part of
+ * @first_obj_offset: First object offset in zsmalloc zpool
+ * @_refcount: Indirectly used by page migration
+ * @memcg_data: Memory Control Group data.
+ *
+ * This struct overlays struct page for now. Do not modify without a good
+ * understanding of the issues. In particular, do not expand into
+ * the overlap with memcg_data.
+ *
+ * Page flags used:
+ * * PG_private identifies the first component page
+ * * PG_locked is used by page migration code
+ */
+struct zpdesc {
+ unsigned long flags;
+ struct list_head lru;
+ unsigned long movable_ops;
+ union {
+ struct zpdesc *next;
+ unsigned long handle;
+ };
+ struct zspage *zspage;
+ unsigned int first_obj_offset;
+ atomic_t _refcount;
+};
+#define ZPDESC_MATCH(pg, zp) \
+ static_assert(offsetof(struct page, pg) == offsetof(struct zpdesc, zp))
+
+ZPDESC_MATCH(flags, flags);
+ZPDESC_MATCH(lru, lru);
+ZPDESC_MATCH(mapping, movable_ops);
+ZPDESC_MATCH(index, next);
+ZPDESC_MATCH(index, handle);
+ZPDESC_MATCH(private, zspage);
+ZPDESC_MATCH(page_type, first_obj_offset);
+ZPDESC_MATCH(_refcount, _refcount);
+#undef ZPDESC_MATCH
+static_assert(sizeof(struct zpdesc) <= sizeof(struct page));
+
+#define zpdesc_page(zp) (_Generic((zp), \
+ const struct zpdesc *: (const struct page *)(zp), \
+ struct zpdesc *: (struct page *)(zp)))
+
+/* Using folio conversion to skip compound_head checking */
+#define zpdesc_folio(zp) (_Generic((zp), \
+ const struct zpdesc *: (const struct folio *)(zp), \
+ struct zpdesc *: (struct folio *)(zp)))
+
+#define page_zpdesc(p) (_Generic((p), \
+ const struct page *: (const struct zpdesc *)(p), \
+ struct page *: (struct zpdesc *)(p)))
+
+#endif
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 64b66a4d3e6e..469fda76ed8a 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -67,6 +67,7 @@
#include <linux/pagemap.h>
#include <linux/fs.h>
#include <linux/local_lock.h>
+#include "zpdesc.h"
#define ZSPAGE_MAGIC 0x58
@@ -254,7 +255,7 @@ struct zspage {
};
unsigned int inuse;
unsigned int freeobj;
- struct page *first_page;
+ struct zpdesc *first_zpdesc;
struct list_head list; /* fullness list */
struct zs_pool *pool;
rwlock_t lock;
@@ -459,7 +460,7 @@ static inline void mod_zspage_inuse(struct zspage *zspage, int val)
static inline struct page *get_first_page(struct zspage *zspage)
{
- struct page *first_page = zspage->first_page;
+ struct page *first_page = zpdesc_page(zspage->first_zpdesc);
VM_BUG_ON_PAGE(!is_first_page(first_page), first_page);
return first_page;
@@ -952,7 +953,7 @@ static void create_page_chain(struct size_class *class, struct zspage *zspage,
set_page_private(page, (unsigned long)zspage);
page->index = 0;
if (i == 0) {
- zspage->first_page = page;
+ zspage->first_zpdesc = page_zpdesc(page);
SetPagePrivate(page);
if (unlikely(class->objs_per_zspage == 1 &&
class->pages_per_zspage == 1))
@@ -1318,7 +1319,7 @@ static unsigned long obj_malloc(struct zs_pool *pool,
link->handle = handle | OBJ_ALLOCATED_TAG;
else
/* record handle to page->index */
- zspage->first_page->index = handle | OBJ_ALLOCATED_TAG;
+ zspage->first_zpdesc->handle = handle | OBJ_ALLOCATED_TAG;
kunmap_local(vaddr);
mod_zspage_inuse(zspage, 1);
--
2.45.2
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v8 02/21] mm/zsmalloc: use zpdesc in trylock_zspage()/lock_zspage()
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 01/21] mm/zsmalloc: add " Matthew Wilcox (Oracle)
@ 2024-12-05 17:49 ` Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 03/21] mm/zsmalloc: convert __zs_map_object/__zs_unmap_object to use zpdesc Matthew Wilcox (Oracle)
` (20 subsequent siblings)
22 siblings, 0 replies; 33+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-12-05 17:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky; +Cc: Alex Shi, linux-mm, Hyeonggon Yoo
From: Alex Shi <alexs@kernel.org>
To use zpdesc in trylock_zspage()/lock_zspage() funcs, we add couple of helpers:
zpdesc_lock()/zpdesc_unlock()/zpdesc_trylock()/zpdesc_wait_locked() and
zpdesc_get()/zpdesc_put() for this purpose.
Here we use the folio series func in guts for 2 reasons, one zswap.zpool
only get single page, and use folio could save some compound_head checking;
two, folio_put could bypass devmap checking that we don't need.
BTW, thanks Intel LKP found a build warning on the patch.
Originally-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Signed-off-by: Alex Shi <alexs@kernel.org>
---
mm/zpdesc.h | 30 ++++++++++++++++++++++++
mm/zsmalloc.c | 64 ++++++++++++++++++++++++++++++++++-----------------
2 files changed, 73 insertions(+), 21 deletions(-)
diff --git a/mm/zpdesc.h b/mm/zpdesc.h
index 9ad232774469..4c7feee5ef1a 100644
--- a/mm/zpdesc.h
+++ b/mm/zpdesc.h
@@ -66,4 +66,34 @@ static_assert(sizeof(struct zpdesc) <= sizeof(struct page));
const struct page *: (const struct zpdesc *)(p), \
struct page *: (struct zpdesc *)(p)))
+static inline void zpdesc_lock(struct zpdesc *zpdesc)
+{
+ folio_lock(zpdesc_folio(zpdesc));
+}
+
+static inline bool zpdesc_trylock(struct zpdesc *zpdesc)
+{
+ return folio_trylock(zpdesc_folio(zpdesc));
+}
+
+static inline void zpdesc_unlock(struct zpdesc *zpdesc)
+{
+ folio_unlock(zpdesc_folio(zpdesc));
+}
+
+static inline void zpdesc_wait_locked(struct zpdesc *zpdesc)
+{
+ folio_wait_locked(zpdesc_folio(zpdesc));
+}
+
+static inline void zpdesc_get(struct zpdesc *zpdesc)
+{
+ folio_get(zpdesc_folio(zpdesc));
+}
+
+static inline void zpdesc_put(struct zpdesc *zpdesc)
+{
+ folio_put(zpdesc_folio(zpdesc));
+}
+
#endif
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 469fda76ed8a..1d1dd4578ae3 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -446,13 +446,17 @@ static __maybe_unused int is_first_page(struct page *page)
return PagePrivate(page);
}
+static inline bool is_first_zpdesc(struct zpdesc *zpdesc)
+{
+ return PagePrivate(zpdesc_page(zpdesc));
+}
+
/* Protected by class->lock */
static inline int get_zspage_inuse(struct zspage *zspage)
{
return zspage->inuse;
}
-
static inline void mod_zspage_inuse(struct zspage *zspage, int val)
{
zspage->inuse += val;
@@ -466,6 +470,14 @@ static inline struct page *get_first_page(struct zspage *zspage)
return first_page;
}
+static struct zpdesc *get_first_zpdesc(struct zspage *zspage)
+{
+ struct zpdesc *first_zpdesc = zspage->first_zpdesc;
+
+ VM_BUG_ON_PAGE(!is_first_zpdesc(first_zpdesc), zpdesc_page(first_zpdesc));
+ return first_zpdesc;
+}
+
#define FIRST_OBJ_PAGE_TYPE_MASK 0xffffff
static inline unsigned int get_first_obj_offset(struct page *page)
@@ -752,6 +764,16 @@ static struct page *get_next_page(struct page *page)
return (struct page *)page->index;
}
+static struct zpdesc *get_next_zpdesc(struct zpdesc *zpdesc)
+{
+ struct zspage *zspage = get_zspage(zpdesc_page(zpdesc));
+
+ if (unlikely(ZsHugePage(zspage)))
+ return NULL;
+
+ return zpdesc->next;
+}
+
/**
* obj_to_location - get (<page>, <obj_idx>) from encoded object value
* @obj: the encoded object value
@@ -821,11 +843,11 @@ static void reset_page(struct page *page)
static int trylock_zspage(struct zspage *zspage)
{
- struct page *cursor, *fail;
+ struct zpdesc *cursor, *fail;
- for (cursor = get_first_page(zspage); cursor != NULL; cursor =
- get_next_page(cursor)) {
- if (!trylock_page(cursor)) {
+ for (cursor = get_first_zpdesc(zspage); cursor != NULL; cursor =
+ get_next_zpdesc(cursor)) {
+ if (!zpdesc_trylock(cursor)) {
fail = cursor;
goto unlock;
}
@@ -833,9 +855,9 @@ static int trylock_zspage(struct zspage *zspage)
return 1;
unlock:
- for (cursor = get_first_page(zspage); cursor != fail; cursor =
- get_next_page(cursor))
- unlock_page(cursor);
+ for (cursor = get_first_zpdesc(zspage); cursor != fail; cursor =
+ get_next_zpdesc(cursor))
+ zpdesc_unlock(cursor);
return 0;
}
@@ -1654,7 +1676,7 @@ static int putback_zspage(struct size_class *class, struct zspage *zspage)
*/
static void lock_zspage(struct zspage *zspage)
{
- struct page *curr_page, *page;
+ struct zpdesc *curr_zpdesc, *zpdesc;
/*
* Pages we haven't locked yet can be migrated off the list while we're
@@ -1666,24 +1688,24 @@ static void lock_zspage(struct zspage *zspage)
*/
while (1) {
migrate_read_lock(zspage);
- page = get_first_page(zspage);
- if (trylock_page(page))
+ zpdesc = get_first_zpdesc(zspage);
+ if (zpdesc_trylock(zpdesc))
break;
- get_page(page);
+ zpdesc_get(zpdesc);
migrate_read_unlock(zspage);
- wait_on_page_locked(page);
- put_page(page);
+ zpdesc_wait_locked(zpdesc);
+ zpdesc_put(zpdesc);
}
- curr_page = page;
- while ((page = get_next_page(curr_page))) {
- if (trylock_page(page)) {
- curr_page = page;
+ curr_zpdesc = zpdesc;
+ while ((zpdesc = get_next_zpdesc(curr_zpdesc))) {
+ if (zpdesc_trylock(zpdesc)) {
+ curr_zpdesc = zpdesc;
} else {
- get_page(page);
+ zpdesc_get(zpdesc);
migrate_read_unlock(zspage);
- wait_on_page_locked(page);
- put_page(page);
+ zpdesc_wait_locked(zpdesc);
+ zpdesc_put(zpdesc);
migrate_read_lock(zspage);
}
}
--
2.45.2
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v8 03/21] mm/zsmalloc: convert __zs_map_object/__zs_unmap_object to use zpdesc
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 01/21] mm/zsmalloc: add " Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 02/21] mm/zsmalloc: use zpdesc in trylock_zspage()/lock_zspage() Matthew Wilcox (Oracle)
@ 2024-12-05 17:49 ` Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 04/21] mm/zsmalloc: add and use pfn/zpdesc seeking funcs Matthew Wilcox (Oracle)
` (19 subsequent siblings)
22 siblings, 0 replies; 33+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-12-05 17:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky; +Cc: Hyeonggon Yoo, linux-mm, Alex Shi
From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
These two functions take a pointer to an array of struct page.
Make __zs_{map,unmap}_object() take pointer to an array of zpdesc
instead of page.
Add silly type casting when calling them. Casting will be removed later.
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Signed-off-by: Alex Shi <alexs@kernel.org>
---
mm/zsmalloc.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 1d1dd4578ae3..5e6467c10757 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -1067,7 +1067,7 @@ static inline void __zs_cpu_down(struct mapping_area *area)
}
static void *__zs_map_object(struct mapping_area *area,
- struct page *pages[2], int off, int size)
+ struct zpdesc *zpdescs[2], int off, int size)
{
size_t sizes[2];
char *buf = area->vm_buf;
@@ -1083,14 +1083,14 @@ static void *__zs_map_object(struct mapping_area *area,
sizes[1] = size - sizes[0];
/* copy object to per-cpu buffer */
- memcpy_from_page(buf, pages[0], off, sizes[0]);
- memcpy_from_page(buf + sizes[0], pages[1], 0, sizes[1]);
+ memcpy_from_page(buf, zpdesc_page(zpdescs[0]), off, sizes[0]);
+ memcpy_from_page(buf + sizes[0], zpdesc_page(zpdescs[1]), 0, sizes[1]);
out:
return area->vm_buf;
}
static void __zs_unmap_object(struct mapping_area *area,
- struct page *pages[2], int off, int size)
+ struct zpdesc *zpdescs[2], int off, int size)
{
size_t sizes[2];
char *buf;
@@ -1108,8 +1108,8 @@ static void __zs_unmap_object(struct mapping_area *area,
sizes[1] = size - sizes[0];
/* copy per-cpu buffer to object */
- memcpy_to_page(pages[0], off, buf, sizes[0]);
- memcpy_to_page(pages[1], 0, buf + sizes[0], sizes[1]);
+ memcpy_to_page(zpdesc_page(zpdescs[0]), off, buf, sizes[0]);
+ memcpy_to_page(zpdesc_page(zpdescs[1]), 0, buf + sizes[0], sizes[1]);
out:
/* enable page faults to match kunmap_local() return conditions */
@@ -1248,7 +1248,7 @@ void *zs_map_object(struct zs_pool *pool, unsigned long handle,
pages[1] = get_next_page(page);
BUG_ON(!pages[1]);
- ret = __zs_map_object(area, pages, off, class->size);
+ ret = __zs_map_object(area, (struct zpdesc **)pages, off, class->size);
out:
if (likely(!ZsHugePage(zspage)))
ret += ZS_HANDLE_SIZE;
@@ -1283,7 +1283,7 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
pages[1] = get_next_page(page);
BUG_ON(!pages[1]);
- __zs_unmap_object(area, pages, off, class->size);
+ __zs_unmap_object(area, (struct zpdesc **)pages, off, class->size);
}
local_unlock(&zs_map_area.lock);
--
2.45.2
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v8 04/21] mm/zsmalloc: add and use pfn/zpdesc seeking funcs
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
` (2 preceding siblings ...)
2024-12-05 17:49 ` [PATCH v8 03/21] mm/zsmalloc: convert __zs_map_object/__zs_unmap_object to use zpdesc Matthew Wilcox (Oracle)
@ 2024-12-05 17:49 ` Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 05/21] mm/zsmalloc: convert obj_malloc() to use zpdesc Matthew Wilcox (Oracle)
` (18 subsequent siblings)
22 siblings, 0 replies; 33+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-12-05 17:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky; +Cc: Hyeonggon Yoo, linux-mm, Alex Shi
From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Add pfn_zpdesc(), pfn_zpdesc() and kmap_local_zpdesc(). Convert
obj_to_location() to take zpdesc and also convert its users to use zpdesc.
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Signed-off-by: Alex Shi <alexs@kernel.org>
---
mm/zpdesc.h | 14 ++++++++++
mm/zsmalloc.c | 75 ++++++++++++++++++++++++++-------------------------
2 files changed, 52 insertions(+), 37 deletions(-)
diff --git a/mm/zpdesc.h b/mm/zpdesc.h
index 4c7feee5ef1a..937de815a4ac 100644
--- a/mm/zpdesc.h
+++ b/mm/zpdesc.h
@@ -96,4 +96,18 @@ static inline void zpdesc_put(struct zpdesc *zpdesc)
folio_put(zpdesc_folio(zpdesc));
}
+static inline void *kmap_local_zpdesc(struct zpdesc *zpdesc)
+{
+ return kmap_local_page(zpdesc_page(zpdesc));
+}
+
+static inline unsigned long zpdesc_pfn(struct zpdesc *zpdesc)
+{
+ return page_to_pfn(zpdesc_page(zpdesc));
+}
+
+static inline struct zpdesc *pfn_zpdesc(unsigned long pfn)
+{
+ return page_zpdesc(pfn_to_page(pfn));
+}
#endif
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 5e6467c10757..03b77690c616 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -775,15 +775,15 @@ static struct zpdesc *get_next_zpdesc(struct zpdesc *zpdesc)
}
/**
- * obj_to_location - get (<page>, <obj_idx>) from encoded object value
+ * obj_to_location - get (<zpdesc>, <obj_idx>) from encoded object value
* @obj: the encoded object value
- * @page: page object resides in zspage
+ * @zpdesc: zpdesc object resides in zspage
* @obj_idx: object index
*/
-static void obj_to_location(unsigned long obj, struct page **page,
+static void obj_to_location(unsigned long obj, struct zpdesc **zpdesc,
unsigned int *obj_idx)
{
- *page = pfn_to_page(obj >> OBJ_INDEX_BITS);
+ *zpdesc = pfn_zpdesc(obj >> OBJ_INDEX_BITS);
*obj_idx = (obj & OBJ_INDEX_MASK);
}
@@ -1199,13 +1199,13 @@ void *zs_map_object(struct zs_pool *pool, unsigned long handle,
enum zs_mapmode mm)
{
struct zspage *zspage;
- struct page *page;
+ struct zpdesc *zpdesc;
unsigned long obj, off;
unsigned int obj_idx;
struct size_class *class;
struct mapping_area *area;
- struct page *pages[2];
+ struct zpdesc *zpdescs[2];
void *ret;
/*
@@ -1218,8 +1218,8 @@ void *zs_map_object(struct zs_pool *pool, unsigned long handle,
/* It guarantees it can get zspage from handle safely */
read_lock(&pool->migrate_lock);
obj = handle_to_obj(handle);
- obj_to_location(obj, &page, &obj_idx);
- zspage = get_zspage(page);
+ obj_to_location(obj, &zpdesc, &obj_idx);
+ zspage = get_zspage(zpdesc_page(zpdesc));
/*
* migration cannot move any zpages in this zspage. Here, class->lock
@@ -1238,17 +1238,17 @@ void *zs_map_object(struct zs_pool *pool, unsigned long handle,
area->vm_mm = mm;
if (off + class->size <= PAGE_SIZE) {
/* this object is contained entirely within a page */
- area->vm_addr = kmap_local_page(page);
+ area->vm_addr = kmap_local_zpdesc(zpdesc);
ret = area->vm_addr + off;
goto out;
}
/* this object spans two pages */
- pages[0] = page;
- pages[1] = get_next_page(page);
- BUG_ON(!pages[1]);
+ zpdescs[0] = zpdesc;
+ zpdescs[1] = get_next_zpdesc(zpdesc);
+ BUG_ON(!zpdescs[1]);
- ret = __zs_map_object(area, (struct zpdesc **)pages, off, class->size);
+ ret = __zs_map_object(area, zpdescs, off, class->size);
out:
if (likely(!ZsHugePage(zspage)))
ret += ZS_HANDLE_SIZE;
@@ -1260,7 +1260,7 @@ EXPORT_SYMBOL_GPL(zs_map_object);
void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
{
struct zspage *zspage;
- struct page *page;
+ struct zpdesc *zpdesc;
unsigned long obj, off;
unsigned int obj_idx;
@@ -1268,8 +1268,8 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
struct mapping_area *area;
obj = handle_to_obj(handle);
- obj_to_location(obj, &page, &obj_idx);
- zspage = get_zspage(page);
+ obj_to_location(obj, &zpdesc, &obj_idx);
+ zspage = get_zspage(zpdesc_page(zpdesc));
class = zspage_class(pool, zspage);
off = offset_in_page(class->size * obj_idx);
@@ -1277,13 +1277,13 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
if (off + class->size <= PAGE_SIZE)
kunmap_local(area->vm_addr);
else {
- struct page *pages[2];
+ struct zpdesc *zpdescs[2];
- pages[0] = page;
- pages[1] = get_next_page(page);
- BUG_ON(!pages[1]);
+ zpdescs[0] = zpdesc;
+ zpdescs[1] = get_next_zpdesc(zpdesc);
+ BUG_ON(!zpdescs[1]);
- __zs_unmap_object(area, (struct zpdesc **)pages, off, class->size);
+ __zs_unmap_object(area, zpdescs, off, class->size);
}
local_unlock(&zs_map_area.lock);
@@ -1425,23 +1425,24 @@ static void obj_free(int class_size, unsigned long obj)
{
struct link_free *link;
struct zspage *zspage;
- struct page *f_page;
+ struct zpdesc *f_zpdesc;
unsigned long f_offset;
unsigned int f_objidx;
void *vaddr;
- obj_to_location(obj, &f_page, &f_objidx);
+
+ obj_to_location(obj, &f_zpdesc, &f_objidx);
f_offset = offset_in_page(class_size * f_objidx);
- zspage = get_zspage(f_page);
+ zspage = get_zspage(zpdesc_page(f_zpdesc));
- vaddr = kmap_local_page(f_page);
+ vaddr = kmap_local_zpdesc(f_zpdesc);
link = (struct link_free *)(vaddr + f_offset);
/* Insert this object in containing zspage's freelist */
if (likely(!ZsHugePage(zspage)))
link->next = get_freeobj(zspage) << OBJ_TAG_BITS;
else
- f_page->index = 0;
+ f_zpdesc->next = NULL;
set_freeobj(zspage, f_objidx);
kunmap_local(vaddr);
@@ -1486,7 +1487,7 @@ EXPORT_SYMBOL_GPL(zs_free);
static void zs_object_copy(struct size_class *class, unsigned long dst,
unsigned long src)
{
- struct page *s_page, *d_page;
+ struct zpdesc *s_zpdesc, *d_zpdesc;
unsigned int s_objidx, d_objidx;
unsigned long s_off, d_off;
void *s_addr, *d_addr;
@@ -1495,8 +1496,8 @@ static void zs_object_copy(struct size_class *class, unsigned long dst,
s_size = d_size = class->size;
- obj_to_location(src, &s_page, &s_objidx);
- obj_to_location(dst, &d_page, &d_objidx);
+ obj_to_location(src, &s_zpdesc, &s_objidx);
+ obj_to_location(dst, &d_zpdesc, &d_objidx);
s_off = offset_in_page(class->size * s_objidx);
d_off = offset_in_page(class->size * d_objidx);
@@ -1507,8 +1508,8 @@ static void zs_object_copy(struct size_class *class, unsigned long dst,
if (d_off + class->size > PAGE_SIZE)
d_size = PAGE_SIZE - d_off;
- s_addr = kmap_local_page(s_page);
- d_addr = kmap_local_page(d_page);
+ s_addr = kmap_local_zpdesc(s_zpdesc);
+ d_addr = kmap_local_zpdesc(d_zpdesc);
while (1) {
size = min(s_size, d_size);
@@ -1533,17 +1534,17 @@ static void zs_object_copy(struct size_class *class, unsigned long dst,
if (s_off >= PAGE_SIZE) {
kunmap_local(d_addr);
kunmap_local(s_addr);
- s_page = get_next_page(s_page);
- s_addr = kmap_local_page(s_page);
- d_addr = kmap_local_page(d_page);
+ s_zpdesc = get_next_zpdesc(s_zpdesc);
+ s_addr = kmap_local_zpdesc(s_zpdesc);
+ d_addr = kmap_local_zpdesc(d_zpdesc);
s_size = class->size - written;
s_off = 0;
}
if (d_off >= PAGE_SIZE) {
kunmap_local(d_addr);
- d_page = get_next_page(d_page);
- d_addr = kmap_local_page(d_page);
+ d_zpdesc = get_next_zpdesc(d_zpdesc);
+ d_addr = kmap_local_zpdesc(d_zpdesc);
d_size = class->size - written;
d_off = 0;
}
@@ -1782,7 +1783,7 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
struct zs_pool *pool;
struct size_class *class;
struct zspage *zspage;
- struct page *dummy;
+ struct zpdesc *dummy;
void *s_addr, *d_addr, *addr;
unsigned int offset;
unsigned long handle;
--
2.45.2
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v8 05/21] mm/zsmalloc: convert obj_malloc() to use zpdesc
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
` (3 preceding siblings ...)
2024-12-05 17:49 ` [PATCH v8 04/21] mm/zsmalloc: add and use pfn/zpdesc seeking funcs Matthew Wilcox (Oracle)
@ 2024-12-05 17:49 ` Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 06/21] mm/zsmalloc: convert create_page_chain() and its users " Matthew Wilcox (Oracle)
` (17 subsequent siblings)
22 siblings, 0 replies; 33+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-12-05 17:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky; +Cc: Hyeonggon Yoo, linux-mm, Alex Shi
From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Use get_first_zpdesc/get_next_zpdesc to replace
get_first_page/get_next_page. no functional change.
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Signed-off-by: Alex Shi <alexs@kernel.org>
---
mm/zsmalloc.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 03b77690c616..af8a6cd6b814 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -1313,12 +1313,12 @@ EXPORT_SYMBOL_GPL(zs_huge_class_size);
static unsigned long obj_malloc(struct zs_pool *pool,
struct zspage *zspage, unsigned long handle)
{
- int i, nr_page, offset;
+ int i, nr_zpdesc, offset;
unsigned long obj;
struct link_free *link;
struct size_class *class;
- struct page *m_page;
+ struct zpdesc *m_zpdesc;
unsigned long m_offset;
void *vaddr;
@@ -1326,14 +1326,14 @@ static unsigned long obj_malloc(struct zs_pool *pool,
obj = get_freeobj(zspage);
offset = obj * class->size;
- nr_page = offset >> PAGE_SHIFT;
+ nr_zpdesc = offset >> PAGE_SHIFT;
m_offset = offset_in_page(offset);
- m_page = get_first_page(zspage);
+ m_zpdesc = get_first_zpdesc(zspage);
- for (i = 0; i < nr_page; i++)
- m_page = get_next_page(m_page);
+ for (i = 0; i < nr_zpdesc; i++)
+ m_zpdesc = get_next_zpdesc(m_zpdesc);
- vaddr = kmap_local_page(m_page);
+ vaddr = kmap_local_zpdesc(m_zpdesc);
link = (struct link_free *)vaddr + m_offset / sizeof(*link);
set_freeobj(zspage, link->next >> OBJ_TAG_BITS);
if (likely(!ZsHugePage(zspage)))
@@ -1346,7 +1346,7 @@ static unsigned long obj_malloc(struct zs_pool *pool,
kunmap_local(vaddr);
mod_zspage_inuse(zspage, 1);
- obj = location_to_obj(m_page, obj);
+ obj = location_to_obj(zpdesc_page(m_zpdesc), obj);
record_obj(handle, obj);
return obj;
--
2.45.2
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v8 06/21] mm/zsmalloc: convert create_page_chain() and its users to use zpdesc
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
` (4 preceding siblings ...)
2024-12-05 17:49 ` [PATCH v8 05/21] mm/zsmalloc: convert obj_malloc() to use zpdesc Matthew Wilcox (Oracle)
@ 2024-12-05 17:49 ` Matthew Wilcox (Oracle)
2024-12-10 13:53 ` Hyeonggon Yoo
2024-12-05 17:49 ` [PATCH v8 07/21] mm/zsmalloc: convert obj_allocated() and related helpers " Matthew Wilcox (Oracle)
` (16 subsequent siblings)
22 siblings, 1 reply; 33+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-12-05 17:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky; +Cc: Alex Shi, linux-mm, Hyeonggon Yoo
From: Alex Shi <alexs@kernel.org>
Introduce a few helper functions for conversion to convert create_page_chain()
to use zpdesc, then use zpdesc in replace_sub_page() too.
Originally-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Signed-off-by: Alex Shi <alexs@kernel.org>
---
mm/zpdesc.h | 6 +++
mm/zsmalloc.c | 109 ++++++++++++++++++++++++++++++++------------------
2 files changed, 76 insertions(+), 39 deletions(-)
diff --git a/mm/zpdesc.h b/mm/zpdesc.h
index 937de815a4ac..0387f5771dc6 100644
--- a/mm/zpdesc.h
+++ b/mm/zpdesc.h
@@ -110,4 +110,10 @@ static inline struct zpdesc *pfn_zpdesc(unsigned long pfn)
{
return page_zpdesc(pfn_to_page(pfn));
}
+
+static inline void __zpdesc_set_movable(struct zpdesc *zpdesc,
+ const struct movable_operations *mops)
+{
+ __SetPageMovable(zpdesc_page(zpdesc), mops);
+}
#endif
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index af8a6cd6b814..6beb7cce4c31 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -246,6 +246,35 @@ struct zs_pool {
atomic_t compaction_in_progress;
};
+static inline void zpdesc_set_first(struct zpdesc *zpdesc)
+{
+ SetPagePrivate(zpdesc_page(zpdesc));
+}
+
+static inline void zpdesc_inc_zone_page_state(struct zpdesc *zpdesc)
+{
+ inc_zone_page_state(zpdesc_page(zpdesc), NR_ZSPAGES);
+}
+
+static inline void zpdesc_dec_zone_page_state(struct zpdesc *zpdesc)
+{
+ dec_zone_page_state(zpdesc_page(zpdesc), NR_ZSPAGES);
+}
+
+static inline struct zpdesc *alloc_zpdesc(gfp_t gfp)
+{
+ struct page *page = alloc_page(gfp);
+
+ return page_zpdesc(page);
+}
+
+static inline void free_zpdesc(struct zpdesc *zpdesc)
+{
+ struct page *page = zpdesc_page(zpdesc);
+
+ __free_page(page);
+}
+
struct zspage {
struct {
unsigned int huge:HUGE_BITS;
@@ -955,35 +984,35 @@ static void init_zspage(struct size_class *class, struct zspage *zspage)
}
static void create_page_chain(struct size_class *class, struct zspage *zspage,
- struct page *pages[])
+ struct zpdesc *zpdescs[])
{
int i;
- struct page *page;
- struct page *prev_page = NULL;
- int nr_pages = class->pages_per_zspage;
+ struct zpdesc *zpdesc;
+ struct zpdesc *prev_zpdesc = NULL;
+ int nr_zpdescs = class->pages_per_zspage;
/*
* Allocate individual pages and link them together as:
- * 1. all pages are linked together using page->index
- * 2. each sub-page point to zspage using page->private
+ * 1. all pages are linked together using zpdesc->next
+ * 2. each sub-page point to zspage using zpdesc->zspage
*
- * we set PG_private to identify the first page (i.e. no other sub-page
+ * we set PG_private to identify the first zpdesc (i.e. no other zpdesc
* has this flag set).
*/
- for (i = 0; i < nr_pages; i++) {
- page = pages[i];
- set_page_private(page, (unsigned long)zspage);
- page->index = 0;
+ for (i = 0; i < nr_zpdescs; i++) {
+ zpdesc = zpdescs[i];
+ zpdesc->zspage = zspage;
+ zpdesc->next = NULL;
if (i == 0) {
- zspage->first_zpdesc = page_zpdesc(page);
- SetPagePrivate(page);
+ zspage->first_zpdesc = zpdesc;
+ zpdesc_set_first(zpdesc);
if (unlikely(class->objs_per_zspage == 1 &&
class->pages_per_zspage == 1))
SetZsHugePage(zspage);
} else {
- prev_page->index = (unsigned long)page;
+ prev_zpdesc->next = zpdesc;
}
- prev_page = page;
+ prev_zpdesc = zpdesc;
}
}
@@ -995,7 +1024,7 @@ static struct zspage *alloc_zspage(struct zs_pool *pool,
gfp_t gfp)
{
int i;
- struct page *pages[ZS_MAX_PAGES_PER_ZSPAGE];
+ struct zpdesc *zpdescs[ZS_MAX_PAGES_PER_ZSPAGE];
struct zspage *zspage = cache_alloc_zspage(pool, gfp);
if (!zspage)
@@ -1005,25 +1034,25 @@ static struct zspage *alloc_zspage(struct zs_pool *pool,
migrate_lock_init(zspage);
for (i = 0; i < class->pages_per_zspage; i++) {
- struct page *page;
+ struct zpdesc *zpdesc;
- page = alloc_page(gfp);
- if (!page) {
+ zpdesc = alloc_zpdesc(gfp);
+ if (!zpdesc) {
while (--i >= 0) {
- dec_zone_page_state(pages[i], NR_ZSPAGES);
- __ClearPageZsmalloc(pages[i]);
- __free_page(pages[i]);
+ zpdesc_dec_zone_page_state(zpdescs[i]);
+ __ClearPageZsmalloc(zpdesc_page(zpdescs[i]));
+ free_zpdesc(zpdescs[i]);
}
cache_free_zspage(pool, zspage);
return NULL;
}
- __SetPageZsmalloc(page);
+ __SetPageZsmalloc(zpdesc_page(zpdesc));
- inc_zone_page_state(page, NR_ZSPAGES);
- pages[i] = page;
+ zpdesc_inc_zone_page_state(zpdesc);
+ zpdescs[i] = zpdesc;
}
- create_page_chain(class, zspage, pages);
+ create_page_chain(class, zspage, zpdescs);
init_zspage(class, zspage);
zspage->pool = pool;
zspage->class = class->index;
@@ -1744,26 +1773,28 @@ static void migrate_write_unlock(struct zspage *zspage)
static const struct movable_operations zsmalloc_mops;
static void replace_sub_page(struct size_class *class, struct zspage *zspage,
- struct page *newpage, struct page *oldpage)
+ struct zpdesc *newzpdesc, struct zpdesc *oldzpdesc)
{
- struct page *page;
- struct page *pages[ZS_MAX_PAGES_PER_ZSPAGE] = {NULL, };
+ struct zpdesc *zpdesc;
+ struct zpdesc *zpdescs[ZS_MAX_PAGES_PER_ZSPAGE] = {NULL, };
+ unsigned int first_obj_offset;
int idx = 0;
- page = get_first_page(zspage);
+ zpdesc = get_first_zpdesc(zspage);
do {
- if (page == oldpage)
- pages[idx] = newpage;
+ if (zpdesc == oldzpdesc)
+ zpdescs[idx] = newzpdesc;
else
- pages[idx] = page;
+ zpdescs[idx] = zpdesc;
idx++;
- } while ((page = get_next_page(page)) != NULL);
+ } while ((zpdesc = get_next_zpdesc(zpdesc)) != NULL);
- create_page_chain(class, zspage, pages);
- set_first_obj_offset(newpage, get_first_obj_offset(oldpage));
+ create_page_chain(class, zspage, zpdescs);
+ first_obj_offset = get_first_obj_offset(zpdesc_page(oldzpdesc));
+ set_first_obj_offset(zpdesc_page(newzpdesc), first_obj_offset);
if (unlikely(ZsHugePage(zspage)))
- newpage->index = oldpage->index;
- __SetPageMovable(newpage, &zsmalloc_mops);
+ newzpdesc->handle = oldzpdesc->handle;
+ __zpdesc_set_movable(newzpdesc, &zsmalloc_mops);
}
static bool zs_page_isolate(struct page *page, isolate_mode_t mode)
@@ -1836,7 +1867,7 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
}
kunmap_local(s_addr);
- replace_sub_page(class, zspage, newpage, page);
+ replace_sub_page(class, zspage, page_zpdesc(newpage), page_zpdesc(page));
/*
* Since we complete the data copy and set up new zspage structure,
* it's okay to release migration_lock.
--
2.45.2
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v8 07/21] mm/zsmalloc: convert obj_allocated() and related helpers to use zpdesc
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
` (5 preceding siblings ...)
2024-12-05 17:49 ` [PATCH v8 06/21] mm/zsmalloc: convert create_page_chain() and its users " Matthew Wilcox (Oracle)
@ 2024-12-05 17:49 ` Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 08/21] mm/zsmalloc: convert init_zspage() " Matthew Wilcox (Oracle)
` (15 subsequent siblings)
22 siblings, 0 replies; 33+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-12-05 17:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky; +Cc: Hyeonggon Yoo, linux-mm, Alex Shi
From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Convert obj_allocated(), and related helpers to take zpdesc. Also make
its callers to cast (struct page *) to (struct zpdesc *) when calling them.
The users will be converted gradually as there are many.
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Signed-off-by: Alex Shi <alexs@kernel.org>
---
mm/zsmalloc.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 6beb7cce4c31..894a3131ea2f 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -841,15 +841,15 @@ static unsigned long handle_to_obj(unsigned long handle)
return *(unsigned long *)handle;
}
-static inline bool obj_allocated(struct page *page, void *obj,
+static inline bool obj_allocated(struct zpdesc *zpdesc, void *obj,
unsigned long *phandle)
{
unsigned long handle;
- struct zspage *zspage = get_zspage(page);
+ struct zspage *zspage = get_zspage(zpdesc_page(zpdesc));
if (unlikely(ZsHugePage(zspage))) {
- VM_BUG_ON_PAGE(!is_first_page(page), page);
- handle = page->index;
+ VM_BUG_ON_PAGE(!is_first_zpdesc(zpdesc), zpdesc_page(zpdesc));
+ handle = zpdesc->handle;
} else
handle = *(unsigned long *)obj;
@@ -1588,18 +1588,18 @@ static void zs_object_copy(struct size_class *class, unsigned long dst,
* return handle.
*/
static unsigned long find_alloced_obj(struct size_class *class,
- struct page *page, int *obj_idx)
+ struct zpdesc *zpdesc, int *obj_idx)
{
unsigned int offset;
int index = *obj_idx;
unsigned long handle = 0;
- void *addr = kmap_local_page(page);
+ void *addr = kmap_local_zpdesc(zpdesc);
- offset = get_first_obj_offset(page);
+ offset = get_first_obj_offset(zpdesc_page(zpdesc));
offset += class->size * index;
while (offset < PAGE_SIZE) {
- if (obj_allocated(page, addr + offset, &handle))
+ if (obj_allocated(zpdesc, addr + offset, &handle))
break;
offset += class->size;
@@ -1623,7 +1623,7 @@ static void migrate_zspage(struct zs_pool *pool, struct zspage *src_zspage,
struct size_class *class = pool->size_class[src_zspage->class];
while (1) {
- handle = find_alloced_obj(class, s_page, &obj_idx);
+ handle = find_alloced_obj(class, page_zpdesc(s_page), &obj_idx);
if (!handle) {
s_page = get_next_page(s_page);
if (!s_page)
@@ -1856,7 +1856,7 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
for (addr = s_addr + offset; addr < s_addr + PAGE_SIZE;
addr += class->size) {
- if (obj_allocated(page, addr, &handle)) {
+ if (obj_allocated(page_zpdesc(page), addr, &handle)) {
old_obj = handle_to_obj(handle);
obj_to_location(old_obj, &dummy, &obj_idx);
--
2.45.2
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v8 08/21] mm/zsmalloc: convert init_zspage() to use zpdesc
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
` (6 preceding siblings ...)
2024-12-05 17:49 ` [PATCH v8 07/21] mm/zsmalloc: convert obj_allocated() and related helpers " Matthew Wilcox (Oracle)
@ 2024-12-05 17:49 ` Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 09/21] mm/zsmalloc: convert obj_to_page() and zs_free() " Matthew Wilcox (Oracle)
` (14 subsequent siblings)
22 siblings, 0 replies; 33+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-12-05 17:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky; +Cc: Hyeonggon Yoo, linux-mm, Alex Shi
From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Replace get_first/next_page func series and kmap_atomic to new helper,
no functional change.
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Signed-off-by: Alex Shi <alexs@kernel.org>
---
mm/zsmalloc.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 894a3131ea2f..faaebc1514ed 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -943,16 +943,16 @@ static void init_zspage(struct size_class *class, struct zspage *zspage)
{
unsigned int freeobj = 1;
unsigned long off = 0;
- struct page *page = get_first_page(zspage);
+ struct zpdesc *zpdesc = get_first_zpdesc(zspage);
- while (page) {
- struct page *next_page;
+ while (zpdesc) {
+ struct zpdesc *next_zpdesc;
struct link_free *link;
void *vaddr;
- set_first_obj_offset(page, off);
+ set_first_obj_offset(zpdesc_page(zpdesc), off);
- vaddr = kmap_local_page(page);
+ vaddr = kmap_local_zpdesc(zpdesc);
link = (struct link_free *)vaddr + off / sizeof(*link);
while ((off += class->size) < PAGE_SIZE) {
@@ -965,8 +965,8 @@ static void init_zspage(struct size_class *class, struct zspage *zspage)
* page, which must point to the first object on the next
* page (if present)
*/
- next_page = get_next_page(page);
- if (next_page) {
+ next_zpdesc = get_next_zpdesc(zpdesc);
+ if (next_zpdesc) {
link->next = freeobj++ << OBJ_TAG_BITS;
} else {
/*
@@ -976,7 +976,7 @@ static void init_zspage(struct size_class *class, struct zspage *zspage)
link->next = -1UL << OBJ_TAG_BITS;
}
kunmap_local(vaddr);
- page = next_page;
+ zpdesc = next_zpdesc;
off %= PAGE_SIZE;
}
--
2.45.2
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v8 09/21] mm/zsmalloc: convert obj_to_page() and zs_free() to use zpdesc
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
` (7 preceding siblings ...)
2024-12-05 17:49 ` [PATCH v8 08/21] mm/zsmalloc: convert init_zspage() " Matthew Wilcox (Oracle)
@ 2024-12-05 17:49 ` Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 10/21] mm/zsmalloc: add zpdesc_is_isolated()/zpdesc_zone() helper for zs_page_migrate() Matthew Wilcox (Oracle)
` (13 subsequent siblings)
22 siblings, 0 replies; 33+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-12-05 17:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky; +Cc: Hyeonggon Yoo, linux-mm, Alex Shi
From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Rename obj_to_page() to obj_to_zpdesc() and also convert it and
its user zs_free() to use zpdesc.
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Signed-off-by: Alex Shi <alexs@kernel.org>
---
mm/zsmalloc.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index faaebc1514ed..6145f7169c67 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -816,9 +816,9 @@ static void obj_to_location(unsigned long obj, struct zpdesc **zpdesc,
*obj_idx = (obj & OBJ_INDEX_MASK);
}
-static void obj_to_page(unsigned long obj, struct page **page)
+static void obj_to_zpdesc(unsigned long obj, struct zpdesc **zpdesc)
{
- *page = pfn_to_page(obj >> OBJ_INDEX_BITS);
+ *zpdesc = pfn_zpdesc(obj >> OBJ_INDEX_BITS);
}
/**
@@ -1481,7 +1481,7 @@ static void obj_free(int class_size, unsigned long obj)
void zs_free(struct zs_pool *pool, unsigned long handle)
{
struct zspage *zspage;
- struct page *f_page;
+ struct zpdesc *f_zpdesc;
unsigned long obj;
struct size_class *class;
int fullness;
@@ -1495,8 +1495,8 @@ void zs_free(struct zs_pool *pool, unsigned long handle)
*/
read_lock(&pool->migrate_lock);
obj = handle_to_obj(handle);
- obj_to_page(obj, &f_page);
- zspage = get_zspage(f_page);
+ obj_to_zpdesc(obj, &f_zpdesc);
+ zspage = get_zspage(zpdesc_page(f_zpdesc));
class = zspage_class(pool, zspage);
spin_lock(&class->lock);
read_unlock(&pool->migrate_lock);
--
2.45.2
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v8 10/21] mm/zsmalloc: add zpdesc_is_isolated()/zpdesc_zone() helper for zs_page_migrate()
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
` (8 preceding siblings ...)
2024-12-05 17:49 ` [PATCH v8 09/21] mm/zsmalloc: convert obj_to_page() and zs_free() " Matthew Wilcox (Oracle)
@ 2024-12-05 17:49 ` Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 11/21] mm/zsmalloc: rename reset_page to reset_zpdesc and use zpdesc in it Matthew Wilcox (Oracle)
` (12 subsequent siblings)
22 siblings, 0 replies; 33+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-12-05 17:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky; +Cc: Hyeonggon Yoo, linux-mm, Alex Shi
From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
To convert page to zpdesc in zs_page_migrate(), we added
zpdesc_is_isolated()/zpdesc_zone() helpers. No functional change.
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Signed-off-by: Alex Shi <alexs@kernel.org>
---
mm/zpdesc.h | 11 +++++++++++
mm/zsmalloc.c | 30 ++++++++++++++++--------------
2 files changed, 27 insertions(+), 14 deletions(-)
diff --git a/mm/zpdesc.h b/mm/zpdesc.h
index 0387f5771dc6..7d59bc2587bf 100644
--- a/mm/zpdesc.h
+++ b/mm/zpdesc.h
@@ -116,4 +116,15 @@ static inline void __zpdesc_set_movable(struct zpdesc *zpdesc,
{
__SetPageMovable(zpdesc_page(zpdesc), mops);
}
+
+static inline bool zpdesc_is_isolated(struct zpdesc *zpdesc)
+{
+ return PageIsolated(zpdesc_page(zpdesc));
+}
+
+static inline struct zone *zpdesc_zone(struct zpdesc *zpdesc)
+{
+ return page_zone(zpdesc_page(zpdesc));
+}
+
#endif
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 6145f7169c67..d8f2f8e5c877 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -1815,19 +1815,21 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
struct size_class *class;
struct zspage *zspage;
struct zpdesc *dummy;
+ struct zpdesc *newzpdesc = page_zpdesc(newpage);
+ struct zpdesc *zpdesc = page_zpdesc(page);
void *s_addr, *d_addr, *addr;
unsigned int offset;
unsigned long handle;
unsigned long old_obj, new_obj;
unsigned int obj_idx;
- VM_BUG_ON_PAGE(!PageIsolated(page), page);
+ VM_BUG_ON_PAGE(!zpdesc_is_isolated(zpdesc), zpdesc_page(zpdesc));
/* We're committed, tell the world that this is a Zsmalloc page. */
- __SetPageZsmalloc(newpage);
+ __SetPageZsmalloc(zpdesc_page(newzpdesc));
/* The page is locked, so this pointer must remain valid */
- zspage = get_zspage(page);
+ zspage = get_zspage(zpdesc_page(zpdesc));
pool = zspage->pool;
/*
@@ -1844,30 +1846,30 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
/* the migrate_write_lock protects zpage access via zs_map_object */
migrate_write_lock(zspage);
- offset = get_first_obj_offset(page);
- s_addr = kmap_local_page(page);
+ offset = get_first_obj_offset(zpdesc_page(zpdesc));
+ s_addr = kmap_local_zpdesc(zpdesc);
/*
* Here, any user cannot access all objects in the zspage so let's move.
*/
- d_addr = kmap_local_page(newpage);
+ d_addr = kmap_local_zpdesc(newzpdesc);
copy_page(d_addr, s_addr);
kunmap_local(d_addr);
for (addr = s_addr + offset; addr < s_addr + PAGE_SIZE;
addr += class->size) {
- if (obj_allocated(page_zpdesc(page), addr, &handle)) {
+ if (obj_allocated(zpdesc, addr, &handle)) {
old_obj = handle_to_obj(handle);
obj_to_location(old_obj, &dummy, &obj_idx);
- new_obj = (unsigned long)location_to_obj(newpage,
+ new_obj = (unsigned long)location_to_obj(zpdesc_page(newzpdesc),
obj_idx);
record_obj(handle, new_obj);
}
}
kunmap_local(s_addr);
- replace_sub_page(class, zspage, page_zpdesc(newpage), page_zpdesc(page));
+ replace_sub_page(class, zspage, newzpdesc, zpdesc);
/*
* Since we complete the data copy and set up new zspage structure,
* it's okay to release migration_lock.
@@ -1876,14 +1878,14 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
spin_unlock(&class->lock);
migrate_write_unlock(zspage);
- get_page(newpage);
- if (page_zone(newpage) != page_zone(page)) {
- dec_zone_page_state(page, NR_ZSPAGES);
- inc_zone_page_state(newpage, NR_ZSPAGES);
+ zpdesc_get(newzpdesc);
+ if (zpdesc_zone(newzpdesc) != zpdesc_zone(zpdesc)) {
+ zpdesc_dec_zone_page_state(zpdesc);
+ zpdesc_inc_zone_page_state(newzpdesc);
}
reset_page(page);
- put_page(page);
+ zpdesc_put(zpdesc);
return MIGRATEPAGE_SUCCESS;
}
--
2.45.2
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v8 11/21] mm/zsmalloc: rename reset_page to reset_zpdesc and use zpdesc in it
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
` (9 preceding siblings ...)
2024-12-05 17:49 ` [PATCH v8 10/21] mm/zsmalloc: add zpdesc_is_isolated()/zpdesc_zone() helper for zs_page_migrate() Matthew Wilcox (Oracle)
@ 2024-12-05 17:49 ` Matthew Wilcox (Oracle)
2024-12-10 14:00 ` Hyeonggon Yoo
2024-12-05 17:49 ` [PATCH v8 12/21] mm/zsmalloc: convert __free_zspage() to use zpdesc Matthew Wilcox (Oracle)
` (11 subsequent siblings)
22 siblings, 1 reply; 33+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-12-05 17:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky; +Cc: Alex Shi, linux-mm
From: Alex Shi <alexs@kernel.org>
zpdesc.zspage matches with page.private, zpdesc.next matches with
page.index. They will be reset in reset_page() which is called prior to
free base pages of a zspage.
Use zpdesc to replace page struct and rename it to reset_zpdesc(), few
page helper still left since they are used too widely.
Signed-off-by: Alex Shi <alexs@kernel.org>
---
mm/zsmalloc.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index d8f2f8e5c877..87b2b100782f 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -861,12 +861,14 @@ static inline bool obj_allocated(struct zpdesc *zpdesc, void *obj,
return true;
}
-static void reset_page(struct page *page)
+static void reset_zpdesc(struct zpdesc *zpdesc)
{
+ struct page *page = zpdesc_page(zpdesc);
+
__ClearPageMovable(page);
ClearPagePrivate(page);
- set_page_private(page, 0);
- page->index = 0;
+ zpdesc->zspage = NULL;
+ zpdesc->next = NULL;
__ClearPageZsmalloc(page);
}
@@ -905,7 +907,7 @@ static void __free_zspage(struct zs_pool *pool, struct size_class *class,
do {
VM_BUG_ON_PAGE(!PageLocked(page), page);
next = get_next_page(page);
- reset_page(page);
+ reset_zpdesc(page_zpdesc(page));
unlock_page(page);
dec_zone_page_state(page, NR_ZSPAGES);
put_page(page);
@@ -1884,7 +1886,7 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
zpdesc_inc_zone_page_state(newzpdesc);
}
- reset_page(page);
+ reset_zpdesc(zpdesc);
zpdesc_put(zpdesc);
return MIGRATEPAGE_SUCCESS;
--
2.45.2
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v8 12/21] mm/zsmalloc: convert __free_zspage() to use zpdesc
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
` (10 preceding siblings ...)
2024-12-05 17:49 ` [PATCH v8 11/21] mm/zsmalloc: rename reset_page to reset_zpdesc and use zpdesc in it Matthew Wilcox (Oracle)
@ 2024-12-05 17:49 ` Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 13/21] mm/zsmalloc: convert location_to_obj() to take zpdesc Matthew Wilcox (Oracle)
` (10 subsequent siblings)
22 siblings, 0 replies; 33+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-12-05 17:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky; +Cc: Hyeonggon Yoo, linux-mm, Alex Shi
From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Introduce zpdesc_is_locked() and convert __free_zspage() to use zpdesc.
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Signed-off-by: Alex Shi <alexs@kernel.org>
---
mm/zpdesc.h | 4 ++++
mm/zsmalloc.c | 20 ++++++++++----------
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/mm/zpdesc.h b/mm/zpdesc.h
index 7d59bc2587bf..89fe78a7f5ca 100644
--- a/mm/zpdesc.h
+++ b/mm/zpdesc.h
@@ -127,4 +127,8 @@ static inline struct zone *zpdesc_zone(struct zpdesc *zpdesc)
return page_zone(zpdesc_page(zpdesc));
}
+static inline bool zpdesc_is_locked(struct zpdesc *zpdesc)
+{
+ return folio_test_locked(zpdesc_folio(zpdesc));
+}
#endif
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 87b2b100782f..2932351f4698 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -896,23 +896,23 @@ static int trylock_zspage(struct zspage *zspage)
static void __free_zspage(struct zs_pool *pool, struct size_class *class,
struct zspage *zspage)
{
- struct page *page, *next;
+ struct zpdesc *zpdesc, *next;
assert_spin_locked(&class->lock);
VM_BUG_ON(get_zspage_inuse(zspage));
VM_BUG_ON(zspage->fullness != ZS_INUSE_RATIO_0);
- next = page = get_first_page(zspage);
+ next = zpdesc = get_first_zpdesc(zspage);
do {
- VM_BUG_ON_PAGE(!PageLocked(page), page);
- next = get_next_page(page);
- reset_zpdesc(page_zpdesc(page));
- unlock_page(page);
- dec_zone_page_state(page, NR_ZSPAGES);
- put_page(page);
- page = next;
- } while (page != NULL);
+ VM_BUG_ON_PAGE(!zpdesc_is_locked(zpdesc), zpdesc_page(zpdesc));
+ next = get_next_zpdesc(zpdesc);
+ reset_zpdesc(zpdesc);
+ zpdesc_unlock(zpdesc);
+ zpdesc_dec_zone_page_state(zpdesc);
+ zpdesc_put(zpdesc);
+ zpdesc = next;
+ } while (zpdesc != NULL);
cache_free_zspage(pool, zspage);
--
2.45.2
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v8 13/21] mm/zsmalloc: convert location_to_obj() to take zpdesc
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
` (11 preceding siblings ...)
2024-12-05 17:49 ` [PATCH v8 12/21] mm/zsmalloc: convert __free_zspage() to use zpdesc Matthew Wilcox (Oracle)
@ 2024-12-05 17:49 ` Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 14/21] mm/zsmalloc: convert migrate_zspage() to use zpdesc Matthew Wilcox (Oracle)
` (9 subsequent siblings)
22 siblings, 0 replies; 33+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-12-05 17:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky; +Cc: Hyeonggon Yoo, linux-mm, Alex Shi
From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
As all users of location_to_obj() now use zpdesc, convert
location_to_obj() to take zpdesc.
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Signed-off-by: Alex Shi <alexs@kernel.org>
---
mm/zsmalloc.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 2932351f4698..2960258c9cda 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -822,15 +822,15 @@ static void obj_to_zpdesc(unsigned long obj, struct zpdesc **zpdesc)
}
/**
- * location_to_obj - get obj value encoded from (<page>, <obj_idx>)
- * @page: page object resides in zspage
+ * location_to_obj - get obj value encoded from (<zpdesc>, <obj_idx>)
+ * @zpdesc: zpdesc object resides in zspage
* @obj_idx: object index
*/
-static unsigned long location_to_obj(struct page *page, unsigned int obj_idx)
+static unsigned long location_to_obj(struct zpdesc *zpdesc, unsigned int obj_idx)
{
unsigned long obj;
- obj = page_to_pfn(page) << OBJ_INDEX_BITS;
+ obj = zpdesc_pfn(zpdesc) << OBJ_INDEX_BITS;
obj |= obj_idx & OBJ_INDEX_MASK;
return obj;
@@ -1377,7 +1377,7 @@ static unsigned long obj_malloc(struct zs_pool *pool,
kunmap_local(vaddr);
mod_zspage_inuse(zspage, 1);
- obj = location_to_obj(zpdesc_page(m_zpdesc), obj);
+ obj = location_to_obj(m_zpdesc, obj);
record_obj(handle, obj);
return obj;
@@ -1864,8 +1864,7 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
old_obj = handle_to_obj(handle);
obj_to_location(old_obj, &dummy, &obj_idx);
- new_obj = (unsigned long)location_to_obj(zpdesc_page(newzpdesc),
- obj_idx);
+ new_obj = (unsigned long)location_to_obj(newzpdesc, obj_idx);
record_obj(handle, new_obj);
}
}
--
2.45.2
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v8 14/21] mm/zsmalloc: convert migrate_zspage() to use zpdesc
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
` (12 preceding siblings ...)
2024-12-05 17:49 ` [PATCH v8 13/21] mm/zsmalloc: convert location_to_obj() to take zpdesc Matthew Wilcox (Oracle)
@ 2024-12-05 17:49 ` Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 15/21] mm/zsmalloc: convert get_zspage() to take zpdesc Matthew Wilcox (Oracle)
` (8 subsequent siblings)
22 siblings, 0 replies; 33+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-12-05 17:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky; +Cc: Hyeonggon Yoo, linux-mm, Alex Shi
From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Use get_first_zpdesc/get_next_zpdesc to replace get_first/next_page. No
functional change.
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Signed-off-by: Alex Shi <alexs@kernel.org>
---
mm/zsmalloc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 2960258c9cda..6b05d0a47ed7 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -1621,14 +1621,14 @@ static void migrate_zspage(struct zs_pool *pool, struct zspage *src_zspage,
unsigned long used_obj, free_obj;
unsigned long handle;
int obj_idx = 0;
- struct page *s_page = get_first_page(src_zspage);
+ struct zpdesc *s_zpdesc = get_first_zpdesc(src_zspage);
struct size_class *class = pool->size_class[src_zspage->class];
while (1) {
- handle = find_alloced_obj(class, page_zpdesc(s_page), &obj_idx);
+ handle = find_alloced_obj(class, s_zpdesc, &obj_idx);
if (!handle) {
- s_page = get_next_page(s_page);
- if (!s_page)
+ s_zpdesc = get_next_zpdesc(s_zpdesc);
+ if (!s_zpdesc)
break;
obj_idx = 0;
continue;
--
2.45.2
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v8 15/21] mm/zsmalloc: convert get_zspage() to take zpdesc
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
` (13 preceding siblings ...)
2024-12-05 17:49 ` [PATCH v8 14/21] mm/zsmalloc: convert migrate_zspage() to use zpdesc Matthew Wilcox (Oracle)
@ 2024-12-05 17:49 ` Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 16/21] mm/zsmalloc: convert SetZsPageMovable and remove unused funcs Matthew Wilcox (Oracle)
` (7 subsequent siblings)
22 siblings, 0 replies; 33+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-12-05 17:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky; +Cc: Hyeonggon Yoo, linux-mm, Alex Shi
From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Now that all users except get_next_page() (which will be removed in
later patch) use zpdesc, convert get_zspage() to take zpdesc instead
of page.
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Signed-off-by: Alex Shi <alexs@kernel.org>
---
mm/zsmalloc.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 6b05d0a47ed7..bf1db07c58cf 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -775,9 +775,9 @@ static int fix_fullness_group(struct size_class *class, struct zspage *zspage)
return newfg;
}
-static struct zspage *get_zspage(struct page *page)
+static struct zspage *get_zspage(struct zpdesc *zpdesc)
{
- struct zspage *zspage = (struct zspage *)page_private(page);
+ struct zspage *zspage = zpdesc->zspage;
BUG_ON(zspage->magic != ZSPAGE_MAGIC);
return zspage;
@@ -785,7 +785,7 @@ static struct zspage *get_zspage(struct page *page)
static struct page *get_next_page(struct page *page)
{
- struct zspage *zspage = get_zspage(page);
+ struct zspage *zspage = get_zspage(page_zpdesc(page));
if (unlikely(ZsHugePage(zspage)))
return NULL;
@@ -795,7 +795,7 @@ static struct page *get_next_page(struct page *page)
static struct zpdesc *get_next_zpdesc(struct zpdesc *zpdesc)
{
- struct zspage *zspage = get_zspage(zpdesc_page(zpdesc));
+ struct zspage *zspage = get_zspage(zpdesc);
if (unlikely(ZsHugePage(zspage)))
return NULL;
@@ -845,7 +845,7 @@ static inline bool obj_allocated(struct zpdesc *zpdesc, void *obj,
unsigned long *phandle)
{
unsigned long handle;
- struct zspage *zspage = get_zspage(zpdesc_page(zpdesc));
+ struct zspage *zspage = get_zspage(zpdesc);
if (unlikely(ZsHugePage(zspage))) {
VM_BUG_ON_PAGE(!is_first_zpdesc(zpdesc), zpdesc_page(zpdesc));
@@ -1250,7 +1250,7 @@ void *zs_map_object(struct zs_pool *pool, unsigned long handle,
read_lock(&pool->migrate_lock);
obj = handle_to_obj(handle);
obj_to_location(obj, &zpdesc, &obj_idx);
- zspage = get_zspage(zpdesc_page(zpdesc));
+ zspage = get_zspage(zpdesc);
/*
* migration cannot move any zpages in this zspage. Here, class->lock
@@ -1300,7 +1300,7 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
obj = handle_to_obj(handle);
obj_to_location(obj, &zpdesc, &obj_idx);
- zspage = get_zspage(zpdesc_page(zpdesc));
+ zspage = get_zspage(zpdesc);
class = zspage_class(pool, zspage);
off = offset_in_page(class->size * obj_idx);
@@ -1464,7 +1464,7 @@ static void obj_free(int class_size, unsigned long obj)
obj_to_location(obj, &f_zpdesc, &f_objidx);
f_offset = offset_in_page(class_size * f_objidx);
- zspage = get_zspage(zpdesc_page(f_zpdesc));
+ zspage = get_zspage(f_zpdesc);
vaddr = kmap_local_zpdesc(f_zpdesc);
link = (struct link_free *)(vaddr + f_offset);
@@ -1498,7 +1498,7 @@ void zs_free(struct zs_pool *pool, unsigned long handle)
read_lock(&pool->migrate_lock);
obj = handle_to_obj(handle);
obj_to_zpdesc(obj, &f_zpdesc);
- zspage = get_zspage(zpdesc_page(f_zpdesc));
+ zspage = get_zspage(f_zpdesc);
class = zspage_class(pool, zspage);
spin_lock(&class->lock);
read_unlock(&pool->migrate_lock);
@@ -1831,7 +1831,7 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
__SetPageZsmalloc(zpdesc_page(newzpdesc));
/* The page is locked, so this pointer must remain valid */
- zspage = get_zspage(zpdesc_page(zpdesc));
+ zspage = get_zspage(zpdesc);
pool = zspage->pool;
/*
--
2.45.2
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v8 16/21] mm/zsmalloc: convert SetZsPageMovable and remove unused funcs
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
` (14 preceding siblings ...)
2024-12-05 17:49 ` [PATCH v8 15/21] mm/zsmalloc: convert get_zspage() to take zpdesc Matthew Wilcox (Oracle)
@ 2024-12-05 17:49 ` Matthew Wilcox (Oracle)
2024-12-10 14:04 ` Hyeonggon Yoo
2024-12-05 17:49 ` [PATCH v8 17/21] mm/zsmalloc: convert get/set_first_obj_offset() to take zpdesc Matthew Wilcox (Oracle)
` (6 subsequent siblings)
22 siblings, 1 reply; 33+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-12-05 17:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky; +Cc: Alex Shi, linux-mm, Hyeonggon Yoo
From: Alex Shi <alexs@kernel.org>
Convert SetZsPageMovable() to use zpdesc, and then remove unused
funcs: get_next_page()/get_first_page()/is_first_page().
Originally-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Signed-off-by: Alex Shi <alexs@kernel.org>
---
mm/zsmalloc.c | 33 +++++----------------------------
1 file changed, 5 insertions(+), 28 deletions(-)
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index bf1db07c58cf..16e3d6705563 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -470,11 +470,6 @@ static DEFINE_PER_CPU(struct mapping_area, zs_map_area) = {
.lock = INIT_LOCAL_LOCK(lock),
};
-static __maybe_unused int is_first_page(struct page *page)
-{
- return PagePrivate(page);
-}
-
static inline bool is_first_zpdesc(struct zpdesc *zpdesc)
{
return PagePrivate(zpdesc_page(zpdesc));
@@ -491,14 +486,6 @@ static inline void mod_zspage_inuse(struct zspage *zspage, int val)
zspage->inuse += val;
}
-static inline struct page *get_first_page(struct zspage *zspage)
-{
- struct page *first_page = zpdesc_page(zspage->first_zpdesc);
-
- VM_BUG_ON_PAGE(!is_first_page(first_page), first_page);
- return first_page;
-}
-
static struct zpdesc *get_first_zpdesc(struct zspage *zspage)
{
struct zpdesc *first_zpdesc = zspage->first_zpdesc;
@@ -783,16 +770,6 @@ static struct zspage *get_zspage(struct zpdesc *zpdesc)
return zspage;
}
-static struct page *get_next_page(struct page *page)
-{
- struct zspage *zspage = get_zspage(page_zpdesc(page));
-
- if (unlikely(ZsHugePage(zspage)))
- return NULL;
-
- return (struct page *)page->index;
-}
-
static struct zpdesc *get_next_zpdesc(struct zpdesc *zpdesc)
{
struct zspage *zspage = get_zspage(zpdesc);
@@ -1955,13 +1932,13 @@ static void init_deferred_free(struct zs_pool *pool)
static void SetZsPageMovable(struct zs_pool *pool, struct zspage *zspage)
{
- struct page *page = get_first_page(zspage);
+ struct zpdesc *zpdesc = get_first_zpdesc(zspage);
do {
- WARN_ON(!trylock_page(page));
- __SetPageMovable(page, &zsmalloc_mops);
- unlock_page(page);
- } while ((page = get_next_page(page)) != NULL);
+ WARN_ON(!zpdesc_trylock(zpdesc));
+ __zpdesc_set_movable(zpdesc, &zsmalloc_mops);
+ zpdesc_unlock(zpdesc);
+ } while ((zpdesc = get_next_zpdesc(zpdesc)) != NULL);
}
#else
static inline void zs_flush_migration(struct zs_pool *pool) { }
--
2.45.2
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v8 17/21] mm/zsmalloc: convert get/set_first_obj_offset() to take zpdesc
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
` (15 preceding siblings ...)
2024-12-05 17:49 ` [PATCH v8 16/21] mm/zsmalloc: convert SetZsPageMovable and remove unused funcs Matthew Wilcox (Oracle)
@ 2024-12-05 17:49 ` Matthew Wilcox (Oracle)
2024-12-10 14:10 ` Hyeonggon Yoo
2024-12-05 17:49 ` [PATCH v8 18/21] mm/zsmalloc: introduce __zpdesc_clear_movable Matthew Wilcox (Oracle)
` (5 subsequent siblings)
22 siblings, 1 reply; 33+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-12-05 17:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky; +Cc: Alex Shi, linux-mm, Hyeonggon Yoo
From: Alex Shi <alexs@kernel.org>
Now that all users of get/set_first_obj_offset() are converted
to use zpdesc, convert them to take zpdesc.
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Signed-off-by: Alex Shi <alexs@kernel.org>
---
mm/zsmalloc.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 16e3d6705563..a15bdcc25bb3 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -496,20 +496,20 @@ static struct zpdesc *get_first_zpdesc(struct zspage *zspage)
#define FIRST_OBJ_PAGE_TYPE_MASK 0xffffff
-static inline unsigned int get_first_obj_offset(struct page *page)
+static inline unsigned int get_first_obj_offset(struct zpdesc *zpdesc)
{
- VM_WARN_ON_ONCE(!PageZsmalloc(page));
- return page->page_type & FIRST_OBJ_PAGE_TYPE_MASK;
+ VM_WARN_ON_ONCE(!PageZsmalloc(zpdesc_page(zpdesc)));
+ return zpdesc->first_obj_offset & FIRST_OBJ_PAGE_TYPE_MASK;
}
-static inline void set_first_obj_offset(struct page *page, unsigned int offset)
+static inline void set_first_obj_offset(struct zpdesc *zpdesc, unsigned int offset)
{
- /* With 24 bits available, we can support offsets into 16 MiB pages. */
- BUILD_BUG_ON(PAGE_SIZE > SZ_16M);
- VM_WARN_ON_ONCE(!PageZsmalloc(page));
+ /* With 16 bit available, we can support offsets into 64 KiB pages. */
+ BUILD_BUG_ON(PAGE_SIZE > SZ_64K);
+ VM_WARN_ON_ONCE(!PageZsmalloc(zpdesc_page(zpdesc)));
VM_WARN_ON_ONCE(offset & ~FIRST_OBJ_PAGE_TYPE_MASK);
- page->page_type &= ~FIRST_OBJ_PAGE_TYPE_MASK;
- page->page_type |= offset & FIRST_OBJ_PAGE_TYPE_MASK;
+ zpdesc->first_obj_offset &= ~FIRST_OBJ_PAGE_TYPE_MASK;
+ zpdesc->first_obj_offset |= offset & FIRST_OBJ_PAGE_TYPE_MASK;
}
static inline unsigned int get_freeobj(struct zspage *zspage)
@@ -929,7 +929,7 @@ static void init_zspage(struct size_class *class, struct zspage *zspage)
struct link_free *link;
void *vaddr;
- set_first_obj_offset(zpdesc_page(zpdesc), off);
+ set_first_obj_offset(zpdesc, off);
vaddr = kmap_local_zpdesc(zpdesc);
link = (struct link_free *)vaddr + off / sizeof(*link);
@@ -1574,7 +1574,7 @@ static unsigned long find_alloced_obj(struct size_class *class,
unsigned long handle = 0;
void *addr = kmap_local_zpdesc(zpdesc);
- offset = get_first_obj_offset(zpdesc_page(zpdesc));
+ offset = get_first_obj_offset(zpdesc);
offset += class->size * index;
while (offset < PAGE_SIZE) {
@@ -1769,8 +1769,8 @@ static void replace_sub_page(struct size_class *class, struct zspage *zspage,
} while ((zpdesc = get_next_zpdesc(zpdesc)) != NULL);
create_page_chain(class, zspage, zpdescs);
- first_obj_offset = get_first_obj_offset(zpdesc_page(oldzpdesc));
- set_first_obj_offset(zpdesc_page(newzpdesc), first_obj_offset);
+ first_obj_offset = get_first_obj_offset(oldzpdesc);
+ set_first_obj_offset(newzpdesc, first_obj_offset);
if (unlikely(ZsHugePage(zspage)))
newzpdesc->handle = oldzpdesc->handle;
__zpdesc_set_movable(newzpdesc, &zsmalloc_mops);
@@ -1825,7 +1825,7 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
/* the migrate_write_lock protects zpage access via zs_map_object */
migrate_write_lock(zspage);
- offset = get_first_obj_offset(zpdesc_page(zpdesc));
+ offset = get_first_obj_offset(zpdesc);
s_addr = kmap_local_zpdesc(zpdesc);
/*
--
2.45.2
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v8 18/21] mm/zsmalloc: introduce __zpdesc_clear_movable
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
` (16 preceding siblings ...)
2024-12-05 17:49 ` [PATCH v8 17/21] mm/zsmalloc: convert get/set_first_obj_offset() to take zpdesc Matthew Wilcox (Oracle)
@ 2024-12-05 17:49 ` Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 19/21] mm/zsmalloc: introduce __zpdesc_clear/set_zsmalloc() Matthew Wilcox (Oracle)
` (4 subsequent siblings)
22 siblings, 0 replies; 33+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-12-05 17:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky; +Cc: Alex Shi, linux-mm
From: Alex Shi <alexs@kernel.org>
Add a helper __zpdesc_clear_movable() for __ClearPageMovable(), and use it
in callers to make code clear.
Signed-off-by: Alex Shi <alexs@kernel.org>
---
mm/zpdesc.h | 5 +++++
mm/zsmalloc.c | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/mm/zpdesc.h b/mm/zpdesc.h
index 89fe78a7f5ca..d468003c7ebd 100644
--- a/mm/zpdesc.h
+++ b/mm/zpdesc.h
@@ -117,6 +117,11 @@ static inline void __zpdesc_set_movable(struct zpdesc *zpdesc,
__SetPageMovable(zpdesc_page(zpdesc), mops);
}
+static inline void __zpdesc_clear_movable(struct zpdesc *zpdesc)
+{
+ __ClearPageMovable(zpdesc_page(zpdesc));
+}
+
static inline bool zpdesc_is_isolated(struct zpdesc *zpdesc)
{
return PageIsolated(zpdesc_page(zpdesc));
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index a15bdcc25bb3..bcd5f1b9f382 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -842,7 +842,7 @@ static void reset_zpdesc(struct zpdesc *zpdesc)
{
struct page *page = zpdesc_page(zpdesc);
- __ClearPageMovable(page);
+ __zpdesc_clear_movable(zpdesc);
ClearPagePrivate(page);
zpdesc->zspage = NULL;
zpdesc->next = NULL;
--
2.45.2
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v8 19/21] mm/zsmalloc: introduce __zpdesc_clear/set_zsmalloc()
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
` (17 preceding siblings ...)
2024-12-05 17:49 ` [PATCH v8 18/21] mm/zsmalloc: introduce __zpdesc_clear_movable Matthew Wilcox (Oracle)
@ 2024-12-05 17:49 ` Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 20/21] mm/zsmalloc: introduce zpdesc_clear_first() helper Matthew Wilcox (Oracle)
` (3 subsequent siblings)
22 siblings, 0 replies; 33+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-12-05 17:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky; +Cc: Alex Shi, linux-mm
From: Alex Shi <alexs@kernel.org>
Add helper __zpdesc_clear_zsmalloc() for __ClearPageZsmalloc(),
__zpdesc_set_zsmalloc() for __SetPageZsmalloc(), and use them in
callers.
Signed-off-by: Alex Shi <alexs@kernel.org>
---
mm/zpdesc.h | 10 ++++++++++
mm/zsmalloc.c | 8 ++++----
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/mm/zpdesc.h b/mm/zpdesc.h
index d468003c7ebd..a09a63be929e 100644
--- a/mm/zpdesc.h
+++ b/mm/zpdesc.h
@@ -122,6 +122,16 @@ static inline void __zpdesc_clear_movable(struct zpdesc *zpdesc)
__ClearPageMovable(zpdesc_page(zpdesc));
}
+static inline void __zpdesc_set_zsmalloc(struct zpdesc *zpdesc)
+{
+ __SetPageZsmalloc(zpdesc_page(zpdesc));
+}
+
+static inline void __zpdesc_clear_zsmalloc(struct zpdesc *zpdesc)
+{
+ __ClearPageZsmalloc(zpdesc_page(zpdesc));
+}
+
static inline bool zpdesc_is_isolated(struct zpdesc *zpdesc)
{
return PageIsolated(zpdesc_page(zpdesc));
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index bcd5f1b9f382..f9a9c2400e41 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -846,7 +846,7 @@ static void reset_zpdesc(struct zpdesc *zpdesc)
ClearPagePrivate(page);
zpdesc->zspage = NULL;
zpdesc->next = NULL;
- __ClearPageZsmalloc(page);
+ __zpdesc_clear_zsmalloc(zpdesc);
}
static int trylock_zspage(struct zspage *zspage)
@@ -1019,13 +1019,13 @@ static struct zspage *alloc_zspage(struct zs_pool *pool,
if (!zpdesc) {
while (--i >= 0) {
zpdesc_dec_zone_page_state(zpdescs[i]);
- __ClearPageZsmalloc(zpdesc_page(zpdescs[i]));
+ __zpdesc_clear_zsmalloc(zpdescs[i]);
free_zpdesc(zpdescs[i]);
}
cache_free_zspage(pool, zspage);
return NULL;
}
- __SetPageZsmalloc(zpdesc_page(zpdesc));
+ __zpdesc_set_zsmalloc(zpdesc);
zpdesc_inc_zone_page_state(zpdesc);
zpdescs[i] = zpdesc;
@@ -1805,7 +1805,7 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
VM_BUG_ON_PAGE(!zpdesc_is_isolated(zpdesc), zpdesc_page(zpdesc));
/* We're committed, tell the world that this is a Zsmalloc page. */
- __SetPageZsmalloc(zpdesc_page(newzpdesc));
+ __zpdesc_set_zsmalloc(newzpdesc);
/* The page is locked, so this pointer must remain valid */
zspage = get_zspage(zpdesc);
--
2.45.2
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v8 20/21] mm/zsmalloc: introduce zpdesc_clear_first() helper
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
` (18 preceding siblings ...)
2024-12-05 17:49 ` [PATCH v8 19/21] mm/zsmalloc: introduce __zpdesc_clear/set_zsmalloc() Matthew Wilcox (Oracle)
@ 2024-12-05 17:49 ` Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 21/21] mm/zsmalloc: update comments for page->zpdesc changes Matthew Wilcox (Oracle)
` (2 subsequent siblings)
22 siblings, 0 replies; 33+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-12-05 17:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky; +Cc: Alex Shi, linux-mm
From: Alex Shi <alexs@kernel.org>
Like the zpdesc_set_first(), introduce zpdesc_clear_first() helper for
ClearPagePrivate(), then clean up a 'struct page' usage in
reset_zpdesc().
Signed-off-by: Alex Shi <alexs@kernel.org>
---
mm/zsmalloc.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index f9a9c2400e41..c0e7c055847a 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -251,6 +251,11 @@ static inline void zpdesc_set_first(struct zpdesc *zpdesc)
SetPagePrivate(zpdesc_page(zpdesc));
}
+static inline void zpdesc_clear_first(struct zpdesc *zpdesc)
+{
+ ClearPagePrivate(zpdesc_page(zpdesc));
+}
+
static inline void zpdesc_inc_zone_page_state(struct zpdesc *zpdesc)
{
inc_zone_page_state(zpdesc_page(zpdesc), NR_ZSPAGES);
@@ -840,10 +845,8 @@ static inline bool obj_allocated(struct zpdesc *zpdesc, void *obj,
static void reset_zpdesc(struct zpdesc *zpdesc)
{
- struct page *page = zpdesc_page(zpdesc);
-
__zpdesc_clear_movable(zpdesc);
- ClearPagePrivate(page);
+ zpdesc_clear_first(zpdesc);
zpdesc->zspage = NULL;
zpdesc->next = NULL;
__zpdesc_clear_zsmalloc(zpdesc);
--
2.45.2
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v8 21/21] mm/zsmalloc: update comments for page->zpdesc changes
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
` (19 preceding siblings ...)
2024-12-05 17:49 ` [PATCH v8 20/21] mm/zsmalloc: introduce zpdesc_clear_first() helper Matthew Wilcox (Oracle)
@ 2024-12-05 17:49 ` Matthew Wilcox (Oracle)
2024-12-10 15:46 ` Hyeonggon Yoo
2024-12-09 11:59 ` [PATCH v8 0/21] Add zpdesc memory descriptor for zswap.zpool alexs
2024-12-10 5:54 ` [PATCH v8 00/21] " Sergey Senozhatsky
22 siblings, 1 reply; 33+ messages in thread
From: Matthew Wilcox (Oracle) @ 2024-12-05 17:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky; +Cc: Alex Shi, linux-mm
From: Alex Shi <alexs@kernel.org>
After the page to zpdesc conversion, there still left few comments or
function named with page not zpdesc, let's update the comments and
rename function create_page_chain() as create_zpdesc_chain().
Signed-off-by: Alex Shi <alexs@kernel.org>
---
mm/zsmalloc.c | 61 ++++++++++++++++++++++++++-------------------------
1 file changed, 31 insertions(+), 30 deletions(-)
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index c0e7c055847a..1f5ff0fdeb42 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -15,20 +15,19 @@
/*
* Following is how we use various fields and flags of underlying
- * struct page(s) to form a zspage.
+ * struct zpdesc(page) to form a zspage.
*
- * Usage of struct page fields:
- * page->private: points to zspage
- * page->index: links together all component pages of a zspage
+ * Usage of struct zpdesc fields:
+ * zpdesc->zspage: points to zspage
+ * zpdesc->next: links together all component zpdescs of a zspage
* For the huge page, this is always 0, so we use this field
* to store handle.
- * page->page_type: PGTY_zsmalloc, lower 24 bits locate the first object
- * offset in a subpage of a zspage
- *
- * Usage of struct page flags:
- * PG_private: identifies the first component page
- * PG_owner_priv_1: identifies the huge component page
+ * zpdesc->first_obj_offset: PGTY_zsmalloc, lower 24 bits locate the first
+ * object offset in a subpage of a zspage
*
+ * Usage of struct zpdesc(page) flags:
+ * PG_private: identifies the first component zpdesc
+ * PG_lock: lock all component zpdescs for a zspage free, serialize with
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -194,7 +193,10 @@ struct size_class {
*/
int size;
int objs_per_zspage;
- /* Number of PAGE_SIZE sized pages to combine to form a 'zspage' */
+ /*
+ * Number of PAGE_SIZE sized zpdescs/pages to combine to
+ * form a 'zspage'
+ */
int pages_per_zspage;
unsigned int index;
@@ -908,7 +910,7 @@ static void free_zspage(struct zs_pool *pool, struct size_class *class,
/*
* Since zs_free couldn't be sleepable, this function cannot call
- * lock_page. The page locks trylock_zspage got will be released
+ * lock_page. The zpdesc locks trylock_zspage got will be released
* by __free_zspage.
*/
if (!trylock_zspage(zspage)) {
@@ -965,7 +967,7 @@ static void init_zspage(struct size_class *class, struct zspage *zspage)
set_freeobj(zspage, 0);
}
-static void create_page_chain(struct size_class *class, struct zspage *zspage,
+static void create_zpdesc_chain(struct size_class *class, struct zspage *zspage,
struct zpdesc *zpdescs[])
{
int i;
@@ -974,9 +976,9 @@ static void create_page_chain(struct size_class *class, struct zspage *zspage,
int nr_zpdescs = class->pages_per_zspage;
/*
- * Allocate individual pages and link them together as:
- * 1. all pages are linked together using zpdesc->next
- * 2. each sub-page point to zspage using zpdesc->zspage
+ * Allocate individual zpdescs and link them together as:
+ * 1. all zpdescs are linked together using zpdesc->next
+ * 2. each sub-zpdesc point to zspage using zpdesc->zspage
*
* we set PG_private to identify the first zpdesc (i.e. no other zpdesc
* has this flag set).
@@ -1034,7 +1036,7 @@ static struct zspage *alloc_zspage(struct zs_pool *pool,
zpdescs[i] = zpdesc;
}
- create_page_chain(class, zspage, zpdescs);
+ create_zpdesc_chain(class, zspage, zpdescs);
init_zspage(class, zspage);
zspage->pool = pool;
zspage->class = class->index;
@@ -1351,7 +1353,7 @@ static unsigned long obj_malloc(struct zs_pool *pool,
/* record handle in the header of allocated chunk */
link->handle = handle | OBJ_ALLOCATED_TAG;
else
- /* record handle to page->index */
+ /* record handle to zpdesc->handle */
zspage->first_zpdesc->handle = handle | OBJ_ALLOCATED_TAG;
kunmap_local(vaddr);
@@ -1441,7 +1443,6 @@ static void obj_free(int class_size, unsigned long obj)
unsigned int f_objidx;
void *vaddr;
-
obj_to_location(obj, &f_zpdesc, &f_objidx);
f_offset = offset_in_page(class_size * f_objidx);
zspage = get_zspage(f_zpdesc);
@@ -1684,19 +1685,19 @@ static int putback_zspage(struct size_class *class, struct zspage *zspage)
#ifdef CONFIG_COMPACTION
/*
* To prevent zspage destroy during migration, zspage freeing should
- * hold locks of all pages in the zspage.
+ * hold locks of all component zpdesc in the zspage.
*/
static void lock_zspage(struct zspage *zspage)
{
struct zpdesc *curr_zpdesc, *zpdesc;
/*
- * Pages we haven't locked yet can be migrated off the list while we're
+ * Zpdesc we haven't locked yet can be migrated off the list while we're
* trying to lock them, so we need to be careful and only attempt to
- * lock each page under migrate_read_lock(). Otherwise, the page we lock
- * may no longer belong to the zspage. This means that we may wait for
- * the wrong page to unlock, so we must take a reference to the page
- * prior to waiting for it to unlock outside migrate_read_lock().
+ * lock each zpdesc under migrate_read_lock(). Otherwise, the zpdesc we
+ * lock may no longer belong to the zspage. This means that we may wait
+ * for the wrong zpdesc to unlock, so we must take a reference to the
+ * zpdesc prior to waiting for it to unlock outside migrate_read_lock().
*/
while (1) {
migrate_read_lock(zspage);
@@ -1771,7 +1772,7 @@ static void replace_sub_page(struct size_class *class, struct zspage *zspage,
idx++;
} while ((zpdesc = get_next_zpdesc(zpdesc)) != NULL);
- create_page_chain(class, zspage, zpdescs);
+ create_zpdesc_chain(class, zspage, zpdescs);
first_obj_offset = get_first_obj_offset(oldzpdesc);
set_first_obj_offset(newzpdesc, first_obj_offset);
if (unlikely(ZsHugePage(zspage)))
@@ -1782,8 +1783,8 @@ static void replace_sub_page(struct size_class *class, struct zspage *zspage,
static bool zs_page_isolate(struct page *page, isolate_mode_t mode)
{
/*
- * Page is locked so zspage couldn't be destroyed. For detail, look at
- * lock_zspage in free_zspage.
+ * Page/zpdesc is locked so zspage couldn't be destroyed. For detail,
+ * look at lock_zspage in free_zspage.
*/
VM_BUG_ON_PAGE(PageIsolated(page), page);
@@ -1810,7 +1811,7 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
/* We're committed, tell the world that this is a Zsmalloc page. */
__zpdesc_set_zsmalloc(newzpdesc);
- /* The page is locked, so this pointer must remain valid */
+ /* The zpdesc/page is locked, so this pointer must remain valid */
zspage = get_zspage(zpdesc);
pool = zspage->pool;
@@ -1883,7 +1884,7 @@ static const struct movable_operations zsmalloc_mops = {
};
/*
- * Caller should hold page_lock of all pages in the zspage
+ * Caller should hold zpdesc locks of all in the zspage
* In here, we cannot use zspage meta data.
*/
static void async_free_zspage(struct work_struct *work)
--
2.45.2
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v8 0/21] Add zpdesc memory descriptor for zswap.zpool
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
` (20 preceding siblings ...)
2024-12-05 17:49 ` [PATCH v8 21/21] mm/zsmalloc: update comments for page->zpdesc changes Matthew Wilcox (Oracle)
@ 2024-12-09 11:59 ` alexs
2024-12-10 5:54 ` [PATCH v8 00/21] " Sergey Senozhatsky
22 siblings, 0 replies; 33+ messages in thread
From: alexs @ 2024-12-09 11:59 UTC (permalink / raw)
To: willy; +Cc: linux-mm, minchan, senozhatsky
> I've updated this patchset for 6.13-rc1 because it's needed to eliminate
> page->index, which I'm hoping to do in the next merge window.
>
> - Renamed mops to movable_ops and changed its type to unsigned long
> - Separated out the documentation of the individual page flags
> - Removed memcg_data again
> - Adapted to memcpy_from_page() API
> - Add kmap_local_zpdesc() instead of zpdesc_kmap_atomic()
>
Thanks a lot for the rebasing.
All looks good to me.
Cheers
Alex
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
` (21 preceding siblings ...)
2024-12-09 11:59 ` [PATCH v8 0/21] Add zpdesc memory descriptor for zswap.zpool alexs
@ 2024-12-10 5:54 ` Sergey Senozhatsky
22 siblings, 0 replies; 33+ messages in thread
From: Sergey Senozhatsky @ 2024-12-10 5:54 UTC (permalink / raw)
To: Matthew Wilcox (Oracle); +Cc: Minchan Kim, Sergey Senozhatsky, linux-mm
On (24/12/05 17:49), Matthew Wilcox (Oracle) wrote:
> I've updated this patchset for 6.13-rc1 because it's needed to eliminate
> page->index, which I'm hoping to do in the next merge window.
No objections from me.
FWIW
Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Tested-by: Sergey Senozhatsky <senozhatsky@chromium.org> # zram workloads on x86_64
Thank you!
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v8 01/21] mm/zsmalloc: add zpdesc memory descriptor for zswap.zpool
2024-12-05 17:49 ` [PATCH v8 01/21] mm/zsmalloc: add " Matthew Wilcox (Oracle)
@ 2024-12-10 13:44 ` Hyeonggon Yoo
0 siblings, 0 replies; 33+ messages in thread
From: Hyeonggon Yoo @ 2024-12-10 13:44 UTC (permalink / raw)
To: Matthew Wilcox (Oracle)
Cc: Minchan Kim, Sergey Senozhatsky, Alex Shi, linux-mm
On Thu, Dec 05, 2024 at 05:49:38PM +0000, Matthew Wilcox (Oracle) wrote:
> From: Alex Shi <alexs@kernel.org>
>
> The 1st patch introduces new memory descriptor zpdesc and rename
> zspage.first_page to zspage.first_zpdesc, no functional change.
>
> We removed PG_owner_priv_1 since it was moved to zspage after
> commit a41ec880aa7b ("zsmalloc: move huge compressed obj from
> page to zspage").
>
> Originally-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> Signed-off-by: Alex Shi <alexs@kernel.org>
> ---
> mm/zpdesc.h | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++
> mm/zsmalloc.c | 9 ++++---
> 2 files changed, 74 insertions(+), 4 deletions(-)
> create mode 100644 mm/zpdesc.h
>
> diff --git a/mm/zpdesc.h b/mm/zpdesc.h
> new file mode 100644
> index 000000000000..9ad232774469
> --- /dev/null
> +++ b/mm/zpdesc.h
> @@ -0,0 +1,69 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* zpdesc.h: zswap.zpool memory descriptor
> + *
> + * Written by Alex Shi <alexs@kernel.org>
> + * Hyeonggon Yoo <42.hyeyoo@gmail.com>
> + */
> +#ifndef __MM_ZPDESC_H__
> +#define __MM_ZPDESC_H__
> +
> +/*
> + * struct zpdesc - Memory descriptor for zpool memory
> + * @flags: Page flags, mostly unused.
> + * @lru: Indirectly used by page migration
> + * @movable_ops: Used by page migration
> + * @next: Next zpdesc in a zspage in zsmalloc zpool
> + * @handle: For huge zspage in zsmalloc zpool
> + * @zspage: Points to the zspage this zpdesc is a part of
> + * @first_obj_offset: First object offset in zsmalloc zpool
> + * @_refcount: Indirectly used by page migration
> + * @memcg_data: Memory Control Group data.
It seems you forgot to remove the comment about memcg_data?
No strong opinion about memcg_data from me, tbh.
> + *
> + * This struct overlays struct page for now. Do not modify without a good
> + * understanding of the issues. In particular, do not expand into
> + * the overlap with memcg_data.
> + *
> + * Page flags used:
> + * * PG_private identifies the first component page
> + * * PG_locked is used by page migration code
> + */
> +struct zpdesc {
> + unsigned long flags;
> + struct list_head lru;
> + unsigned long movable_ops;
> + union {
> + struct zpdesc *next;
> + unsigned long handle;
> + };
> + struct zspage *zspage;
> + unsigned int first_obj_offset;
> + atomic_t _refcount;
> +};
> +#define ZPDESC_MATCH(pg, zp) \
> + static_assert(offsetof(struct page, pg) == offsetof(struct zpdesc, zp))
> +
> +ZPDESC_MATCH(flags, flags);
> +ZPDESC_MATCH(lru, lru);
> +ZPDESC_MATCH(mapping, movable_ops);
> +ZPDESC_MATCH(index, next);
> +ZPDESC_MATCH(index, handle);
> +ZPDESC_MATCH(private, zspage);
> +ZPDESC_MATCH(page_type, first_obj_offset);
> +ZPDESC_MATCH(_refcount, _refcount);
> +#undef ZPDESC_MATCH
> +static_assert(sizeof(struct zpdesc) <= sizeof(struct page));
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v8 06/21] mm/zsmalloc: convert create_page_chain() and its users to use zpdesc
2024-12-05 17:49 ` [PATCH v8 06/21] mm/zsmalloc: convert create_page_chain() and its users " Matthew Wilcox (Oracle)
@ 2024-12-10 13:53 ` Hyeonggon Yoo
0 siblings, 0 replies; 33+ messages in thread
From: Hyeonggon Yoo @ 2024-12-10 13:53 UTC (permalink / raw)
To: Matthew Wilcox (Oracle)
Cc: Minchan Kim, Sergey Senozhatsky, Alex Shi, linux-mm
On Thu, Dec 05, 2024 at 05:49:43PM +0000, Matthew Wilcox (Oracle) wrote:
> From: Alex Shi <alexs@kernel.org>
>
> Introduce a few helper functions for conversion to convert create_page_chain()
> to use zpdesc, then use zpdesc in replace_sub_page() too.
>
> Originally-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> Signed-off-by: Alex Shi <alexs@kernel.org>
Reviewed-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> ---
> mm/zpdesc.h | 6 +++
> mm/zsmalloc.c | 109 ++++++++++++++++++++++++++++++++------------------
> 2 files changed, 76 insertions(+), 39 deletions(-)
>
> diff --git a/mm/zpdesc.h b/mm/zpdesc.h
> index 937de815a4ac..0387f5771dc6 100644
> --- a/mm/zpdesc.h
> +++ b/mm/zpdesc.h
> @@ -110,4 +110,10 @@ static inline struct zpdesc *pfn_zpdesc(unsigned long pfn)
> {
> return page_zpdesc(pfn_to_page(pfn));
> }
> +
> +static inline void __zpdesc_set_movable(struct zpdesc *zpdesc,
> + const struct movable_operations *mops)
> +{
> + __SetPageMovable(zpdesc_page(zpdesc), mops);
> +}
> #endif
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index af8a6cd6b814..6beb7cce4c31 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -246,6 +246,35 @@ struct zs_pool {
> atomic_t compaction_in_progress;
> };
>
> +static inline void zpdesc_set_first(struct zpdesc *zpdesc)
> +{
> + SetPagePrivate(zpdesc_page(zpdesc));
> +}
> +
> +static inline void zpdesc_inc_zone_page_state(struct zpdesc *zpdesc)
> +{
> + inc_zone_page_state(zpdesc_page(zpdesc), NR_ZSPAGES);
> +}
> +
> +static inline void zpdesc_dec_zone_page_state(struct zpdesc *zpdesc)
> +{
> + dec_zone_page_state(zpdesc_page(zpdesc), NR_ZSPAGES);
> +}
> +
> +static inline struct zpdesc *alloc_zpdesc(gfp_t gfp)
> +{
> + struct page *page = alloc_page(gfp);
> +
> + return page_zpdesc(page);
> +}
> +
> +static inline void free_zpdesc(struct zpdesc *zpdesc)
> +{
> + struct page *page = zpdesc_page(zpdesc);
> +
> + __free_page(page);
> +}
> +
> struct zspage {
> struct {
> unsigned int huge:HUGE_BITS;
> @@ -955,35 +984,35 @@ static void init_zspage(struct size_class *class, struct zspage *zspage)
> }
>
> static void create_page_chain(struct size_class *class, struct zspage *zspage,
> - struct page *pages[])
> + struct zpdesc *zpdescs[])
> {
> int i;
> - struct page *page;
> - struct page *prev_page = NULL;
> - int nr_pages = class->pages_per_zspage;
> + struct zpdesc *zpdesc;
> + struct zpdesc *prev_zpdesc = NULL;
> + int nr_zpdescs = class->pages_per_zspage;
>
> /*
> * Allocate individual pages and link them together as:
> - * 1. all pages are linked together using page->index
> - * 2. each sub-page point to zspage using page->private
> + * 1. all pages are linked together using zpdesc->next
> + * 2. each sub-page point to zspage using zpdesc->zspage
> *
> - * we set PG_private to identify the first page (i.e. no other sub-page
> + * we set PG_private to identify the first zpdesc (i.e. no other zpdesc
> * has this flag set).
> */
> - for (i = 0; i < nr_pages; i++) {
> - page = pages[i];
> - set_page_private(page, (unsigned long)zspage);
> - page->index = 0;
> + for (i = 0; i < nr_zpdescs; i++) {
> + zpdesc = zpdescs[i];
> + zpdesc->zspage = zspage;
> + zpdesc->next = NULL;
> if (i == 0) {
> - zspage->first_zpdesc = page_zpdesc(page);
> - SetPagePrivate(page);
> + zspage->first_zpdesc = zpdesc;
> + zpdesc_set_first(zpdesc);
> if (unlikely(class->objs_per_zspage == 1 &&
> class->pages_per_zspage == 1))
> SetZsHugePage(zspage);
> } else {
> - prev_page->index = (unsigned long)page;
> + prev_zpdesc->next = zpdesc;
> }
> - prev_page = page;
> + prev_zpdesc = zpdesc;
> }
> }
>
> @@ -995,7 +1024,7 @@ static struct zspage *alloc_zspage(struct zs_pool *pool,
> gfp_t gfp)
> {
> int i;
> - struct page *pages[ZS_MAX_PAGES_PER_ZSPAGE];
> + struct zpdesc *zpdescs[ZS_MAX_PAGES_PER_ZSPAGE];
> struct zspage *zspage = cache_alloc_zspage(pool, gfp);
>
> if (!zspage)
> @@ -1005,25 +1034,25 @@ static struct zspage *alloc_zspage(struct zs_pool *pool,
> migrate_lock_init(zspage);
>
> for (i = 0; i < class->pages_per_zspage; i++) {
> - struct page *page;
> + struct zpdesc *zpdesc;
>
> - page = alloc_page(gfp);
> - if (!page) {
> + zpdesc = alloc_zpdesc(gfp);
> + if (!zpdesc) {
> while (--i >= 0) {
> - dec_zone_page_state(pages[i], NR_ZSPAGES);
> - __ClearPageZsmalloc(pages[i]);
> - __free_page(pages[i]);
> + zpdesc_dec_zone_page_state(zpdescs[i]);
> + __ClearPageZsmalloc(zpdesc_page(zpdescs[i]));
> + free_zpdesc(zpdescs[i]);
> }
> cache_free_zspage(pool, zspage);
> return NULL;
> }
> - __SetPageZsmalloc(page);
> + __SetPageZsmalloc(zpdesc_page(zpdesc));
>
> - inc_zone_page_state(page, NR_ZSPAGES);
> - pages[i] = page;
> + zpdesc_inc_zone_page_state(zpdesc);
> + zpdescs[i] = zpdesc;
> }
>
> - create_page_chain(class, zspage, pages);
> + create_page_chain(class, zspage, zpdescs);
> init_zspage(class, zspage);
> zspage->pool = pool;
> zspage->class = class->index;
> @@ -1744,26 +1773,28 @@ static void migrate_write_unlock(struct zspage *zspage)
> static const struct movable_operations zsmalloc_mops;
>
> static void replace_sub_page(struct size_class *class, struct zspage *zspage,
> - struct page *newpage, struct page *oldpage)
> + struct zpdesc *newzpdesc, struct zpdesc *oldzpdesc)
> {
> - struct page *page;
> - struct page *pages[ZS_MAX_PAGES_PER_ZSPAGE] = {NULL, };
> + struct zpdesc *zpdesc;
> + struct zpdesc *zpdescs[ZS_MAX_PAGES_PER_ZSPAGE] = {NULL, };
> + unsigned int first_obj_offset;
> int idx = 0;
>
> - page = get_first_page(zspage);
> + zpdesc = get_first_zpdesc(zspage);
> do {
> - if (page == oldpage)
> - pages[idx] = newpage;
> + if (zpdesc == oldzpdesc)
> + zpdescs[idx] = newzpdesc;
> else
> - pages[idx] = page;
> + zpdescs[idx] = zpdesc;
> idx++;
> - } while ((page = get_next_page(page)) != NULL);
> + } while ((zpdesc = get_next_zpdesc(zpdesc)) != NULL);
>
> - create_page_chain(class, zspage, pages);
> - set_first_obj_offset(newpage, get_first_obj_offset(oldpage));
> + create_page_chain(class, zspage, zpdescs);
> + first_obj_offset = get_first_obj_offset(zpdesc_page(oldzpdesc));
> + set_first_obj_offset(zpdesc_page(newzpdesc), first_obj_offset);
> if (unlikely(ZsHugePage(zspage)))
> - newpage->index = oldpage->index;
> - __SetPageMovable(newpage, &zsmalloc_mops);
> + newzpdesc->handle = oldzpdesc->handle;
> + __zpdesc_set_movable(newzpdesc, &zsmalloc_mops);
> }
>
> static bool zs_page_isolate(struct page *page, isolate_mode_t mode)
> @@ -1836,7 +1867,7 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
> }
> kunmap_local(s_addr);
>
> - replace_sub_page(class, zspage, newpage, page);
> + replace_sub_page(class, zspage, page_zpdesc(newpage), page_zpdesc(page));
> /*
> * Since we complete the data copy and set up new zspage structure,
> * it's okay to release migration_lock.
> --
> 2.45.2
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v8 11/21] mm/zsmalloc: rename reset_page to reset_zpdesc and use zpdesc in it
2024-12-05 17:49 ` [PATCH v8 11/21] mm/zsmalloc: rename reset_page to reset_zpdesc and use zpdesc in it Matthew Wilcox (Oracle)
@ 2024-12-10 14:00 ` Hyeonggon Yoo
0 siblings, 0 replies; 33+ messages in thread
From: Hyeonggon Yoo @ 2024-12-10 14:00 UTC (permalink / raw)
To: Matthew Wilcox (Oracle)
Cc: Minchan Kim, Sergey Senozhatsky, Alex Shi, linux-mm
On Thu, Dec 05, 2024 at 05:49:48PM +0000, Matthew Wilcox (Oracle) wrote:
> From: Alex Shi <alexs@kernel.org>
>
> zpdesc.zspage matches with page.private, zpdesc.next matches with
> page.index. They will be reset in reset_page() which is called prior to
> free base pages of a zspage.
> Use zpdesc to replace page struct and rename it to reset_zpdesc(), few
> page helper still left since they are used too widely.
>
> Signed-off-by: Alex Shi <alexs@kernel.org>
> ---
Looks good to me,
Reviewed-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> mm/zsmalloc.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index d8f2f8e5c877..87b2b100782f 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -861,12 +861,14 @@ static inline bool obj_allocated(struct zpdesc *zpdesc, void *obj,
> return true;
> }
>
> -static void reset_page(struct page *page)
> +static void reset_zpdesc(struct zpdesc *zpdesc)
> {
> + struct page *page = zpdesc_page(zpdesc);
> +
> __ClearPageMovable(page);
> ClearPagePrivate(page);
> - set_page_private(page, 0);
> - page->index = 0;
> + zpdesc->zspage = NULL;
> + zpdesc->next = NULL;
> __ClearPageZsmalloc(page);
> }
>
> @@ -905,7 +907,7 @@ static void __free_zspage(struct zs_pool *pool, struct size_class *class,
> do {
> VM_BUG_ON_PAGE(!PageLocked(page), page);
> next = get_next_page(page);
> - reset_page(page);
> + reset_zpdesc(page_zpdesc(page));
> unlock_page(page);
> dec_zone_page_state(page, NR_ZSPAGES);
> put_page(page);
> @@ -1884,7 +1886,7 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
> zpdesc_inc_zone_page_state(newzpdesc);
> }
>
> - reset_page(page);
> + reset_zpdesc(zpdesc);
> zpdesc_put(zpdesc);
>
> return MIGRATEPAGE_SUCCESS;
> --
> 2.45.2
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v8 16/21] mm/zsmalloc: convert SetZsPageMovable and remove unused funcs
2024-12-05 17:49 ` [PATCH v8 16/21] mm/zsmalloc: convert SetZsPageMovable and remove unused funcs Matthew Wilcox (Oracle)
@ 2024-12-10 14:04 ` Hyeonggon Yoo
0 siblings, 0 replies; 33+ messages in thread
From: Hyeonggon Yoo @ 2024-12-10 14:04 UTC (permalink / raw)
To: Matthew Wilcox (Oracle)
Cc: Minchan Kim, Sergey Senozhatsky, Alex Shi, linux-mm
On Thu, Dec 05, 2024 at 05:49:53PM +0000, Matthew Wilcox (Oracle) wrote:
> From: Alex Shi <alexs@kernel.org>
>
> Convert SetZsPageMovable() to use zpdesc, and then remove unused
> funcs: get_next_page()/get_first_page()/is_first_page().
>
> Originally-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> Signed-off-by: Alex Shi <alexs@kernel.org>
> ---
Looks good to me,
Hyeonggon Yoo <42.hyeyoo@gmail.com>
> mm/zsmalloc.c | 33 +++++----------------------------
> 1 file changed, 5 insertions(+), 28 deletions(-)
>
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index bf1db07c58cf..16e3d6705563 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -470,11 +470,6 @@ static DEFINE_PER_CPU(struct mapping_area, zs_map_area) = {
> .lock = INIT_LOCAL_LOCK(lock),
> };
>
> -static __maybe_unused int is_first_page(struct page *page)
> -{
> - return PagePrivate(page);
> -}
> -
> static inline bool is_first_zpdesc(struct zpdesc *zpdesc)
> {
> return PagePrivate(zpdesc_page(zpdesc));
> @@ -491,14 +486,6 @@ static inline void mod_zspage_inuse(struct zspage *zspage, int val)
> zspage->inuse += val;
> }
>
> -static inline struct page *get_first_page(struct zspage *zspage)
> -{
> - struct page *first_page = zpdesc_page(zspage->first_zpdesc);
> -
> - VM_BUG_ON_PAGE(!is_first_page(first_page), first_page);
> - return first_page;
> -}
> -
> static struct zpdesc *get_first_zpdesc(struct zspage *zspage)
> {
> struct zpdesc *first_zpdesc = zspage->first_zpdesc;
> @@ -783,16 +770,6 @@ static struct zspage *get_zspage(struct zpdesc *zpdesc)
> return zspage;
> }
>
> -static struct page *get_next_page(struct page *page)
> -{
> - struct zspage *zspage = get_zspage(page_zpdesc(page));
> -
> - if (unlikely(ZsHugePage(zspage)))
> - return NULL;
> -
> - return (struct page *)page->index;
> -}
> -
> static struct zpdesc *get_next_zpdesc(struct zpdesc *zpdesc)
> {
> struct zspage *zspage = get_zspage(zpdesc);
> @@ -1955,13 +1932,13 @@ static void init_deferred_free(struct zs_pool *pool)
>
> static void SetZsPageMovable(struct zs_pool *pool, struct zspage *zspage)
> {
> - struct page *page = get_first_page(zspage);
> + struct zpdesc *zpdesc = get_first_zpdesc(zspage);
>
> do {
> - WARN_ON(!trylock_page(page));
> - __SetPageMovable(page, &zsmalloc_mops);
> - unlock_page(page);
> - } while ((page = get_next_page(page)) != NULL);
> + WARN_ON(!zpdesc_trylock(zpdesc));
> + __zpdesc_set_movable(zpdesc, &zsmalloc_mops);
> + zpdesc_unlock(zpdesc);
> + } while ((zpdesc = get_next_zpdesc(zpdesc)) != NULL);
> }
> #else
> static inline void zs_flush_migration(struct zs_pool *pool) { }
> --
> 2.45.2
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v8 17/21] mm/zsmalloc: convert get/set_first_obj_offset() to take zpdesc
2024-12-05 17:49 ` [PATCH v8 17/21] mm/zsmalloc: convert get/set_first_obj_offset() to take zpdesc Matthew Wilcox (Oracle)
@ 2024-12-10 14:10 ` Hyeonggon Yoo
0 siblings, 0 replies; 33+ messages in thread
From: Hyeonggon Yoo @ 2024-12-10 14:10 UTC (permalink / raw)
To: Matthew Wilcox (Oracle)
Cc: Minchan Kim, Sergey Senozhatsky, Alex Shi, linux-mm
On Thu, Dec 05, 2024 at 05:49:54PM +0000, Matthew Wilcox (Oracle) wrote:
> From: Alex Shi <alexs@kernel.org>
>
> Now that all users of get/set_first_obj_offset() are converted
> to use zpdesc, convert them to take zpdesc.
>
> Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> Signed-off-by: Alex Shi <alexs@kernel.org>
> ---
> mm/zsmalloc.c | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index 16e3d6705563..a15bdcc25bb3 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -496,20 +496,20 @@ static struct zpdesc *get_first_zpdesc(struct zspage *zspage)
>
> #define FIRST_OBJ_PAGE_TYPE_MASK 0xffffff
>
> -static inline unsigned int get_first_obj_offset(struct page *page)
> +static inline unsigned int get_first_obj_offset(struct zpdesc *zpdesc)
> {
> - VM_WARN_ON_ONCE(!PageZsmalloc(page));
> - return page->page_type & FIRST_OBJ_PAGE_TYPE_MASK;
> + VM_WARN_ON_ONCE(!PageZsmalloc(zpdesc_page(zpdesc)));
> + return zpdesc->first_obj_offset & FIRST_OBJ_PAGE_TYPE_MASK;
> }
>
> -static inline void set_first_obj_offset(struct page *page, unsigned int offset)
> +static inline void set_first_obj_offset(struct zpdesc *zpdesc, unsigned int offset)
> {
> - /* With 24 bits available, we can support offsets into 16 MiB pages. */
> - BUILD_BUG_ON(PAGE_SIZE > SZ_16M);
> - VM_WARN_ON_ONCE(!PageZsmalloc(page));
> + /* With 16 bit available, we can support offsets into 64 KiB pages. */
> + BUILD_BUG_ON(PAGE_SIZE > SZ_64K);
> + VM_WARN_ON_ONCE(!PageZsmalloc(zpdesc_page(zpdesc)));
It seems to be a mistake that occurred during the rebase process from v6 to v7?
> VM_WARN_ON_ONCE(offset & ~FIRST_OBJ_PAGE_TYPE_MASK);
> - page->page_type &= ~FIRST_OBJ_PAGE_TYPE_MASK;
> - page->page_type |= offset & FIRST_OBJ_PAGE_TYPE_MASK;
> + zpdesc->first_obj_offset &= ~FIRST_OBJ_PAGE_TYPE_MASK;
> + zpdesc->first_obj_offset |= offset & FIRST_OBJ_PAGE_TYPE_MASK;
> }
>
> static inline unsigned int get_freeobj(struct zspage *zspage)
> @@ -929,7 +929,7 @@ static void init_zspage(struct size_class *class, struct zspage *zspage)
> struct link_free *link;
> void *vaddr;
>
> - set_first_obj_offset(zpdesc_page(zpdesc), off);
> + set_first_obj_offset(zpdesc, off);
>
> vaddr = kmap_local_zpdesc(zpdesc);
> link = (struct link_free *)vaddr + off / sizeof(*link);
> @@ -1574,7 +1574,7 @@ static unsigned long find_alloced_obj(struct size_class *class,
> unsigned long handle = 0;
> void *addr = kmap_local_zpdesc(zpdesc);
>
> - offset = get_first_obj_offset(zpdesc_page(zpdesc));
> + offset = get_first_obj_offset(zpdesc);
> offset += class->size * index;
>
> while (offset < PAGE_SIZE) {
> @@ -1769,8 +1769,8 @@ static void replace_sub_page(struct size_class *class, struct zspage *zspage,
> } while ((zpdesc = get_next_zpdesc(zpdesc)) != NULL);
>
> create_page_chain(class, zspage, zpdescs);
> - first_obj_offset = get_first_obj_offset(zpdesc_page(oldzpdesc));
> - set_first_obj_offset(zpdesc_page(newzpdesc), first_obj_offset);
> + first_obj_offset = get_first_obj_offset(oldzpdesc);
> + set_first_obj_offset(newzpdesc, first_obj_offset);
> if (unlikely(ZsHugePage(zspage)))
> newzpdesc->handle = oldzpdesc->handle;
> __zpdesc_set_movable(newzpdesc, &zsmalloc_mops);
> @@ -1825,7 +1825,7 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
> /* the migrate_write_lock protects zpage access via zs_map_object */
> migrate_write_lock(zspage);
>
> - offset = get_first_obj_offset(zpdesc_page(zpdesc));
> + offset = get_first_obj_offset(zpdesc);
> s_addr = kmap_local_zpdesc(zpdesc);
>
> /*
> --
> 2.45.2
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v8 21/21] mm/zsmalloc: update comments for page->zpdesc changes
2024-12-05 17:49 ` [PATCH v8 21/21] mm/zsmalloc: update comments for page->zpdesc changes Matthew Wilcox (Oracle)
@ 2024-12-10 15:46 ` Hyeonggon Yoo
2024-12-11 4:37 ` Matthew Wilcox
0 siblings, 1 reply; 33+ messages in thread
From: Hyeonggon Yoo @ 2024-12-10 15:46 UTC (permalink / raw)
To: Matthew Wilcox (Oracle)
Cc: Minchan Kim, Sergey Senozhatsky, Alex Shi, linux-mm
On Thu, Dec 05, 2024 at 05:49:58PM +0000, Matthew Wilcox (Oracle) wrote:
> From: Alex Shi <alexs@kernel.org>
>
> After the page to zpdesc conversion, there still left few comments or
> function named with page not zpdesc, let's update the comments and
> rename function create_page_chain() as create_zpdesc_chain().
Talking about updating comments and code by replacing 'page' with 'zpdesc',
I'm not sure if it makes sense to replace all instances of 'page' with 'zpdesc'.
A zpdesc is a descriptor, not a page that contains data (at least that's
what I have been thinking while writing the initial patch series).
In that context I'm still not sure if saying "a sub-zpdesc of a zspage", "lock zpdesc",
or "migrate zpdesc" makes sense because replacing 'page descriptor (aka. struct page)' with
'zpool descriptor (aka. zpdesc)' doesn't mean zsmalloc is throwing away the conecept of pages.
(...Or I might be thinking about this the wrong way)
> Signed-off-by: Alex Shi <alexs@kernel.org>
> ---
> mm/zsmalloc.c | 61 ++++++++++++++++++++++++++-------------------------
> 1 file changed, 31 insertions(+), 30 deletions(-)
>
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index c0e7c055847a..1f5ff0fdeb42 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -15,20 +15,19 @@
>
> /*
> * Following is how we use various fields and flags of underlying
> - * struct page(s) to form a zspage.
> + * struct zpdesc(page) to form a zspage.
> *
> - * Usage of struct page fields:
> - * page->private: points to zspage
> - * page->index: links together all component pages of a zspage
> + * Usage of struct zpdesc fields:
> + * zpdesc->zspage: points to zspage
> + * zpdesc->next: links together all component zpdescs of a zspage
> * For the huge page, this is always 0, so we use this field
> * to store handle.
> - * page->page_type: PGTY_zsmalloc, lower 24 bits locate the first object
> - * offset in a subpage of a zspage
> - *
> - * Usage of struct page flags:
> - * PG_private: identifies the first component page
> - * PG_owner_priv_1: identifies the huge component page
> + * zpdesc->first_obj_offset: PGTY_zsmalloc, lower 24 bits locate the first
> + * object offset in a subpage of a zspage
> *
> + * Usage of struct zpdesc(page) flags:
> + * PG_private: identifies the first component zpdesc
> + * PG_lock: lock all component zpdescs for a zspage free, serialize with
> */
I think this comment is unnecessary, as it's already documented in mm/zpdesc.h.
It can be removed in patch 01.
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> @@ -194,7 +193,10 @@ struct size_class {
> */
> int size;
> int objs_per_zspage;
> - /* Number of PAGE_SIZE sized pages to combine to form a 'zspage' */
> + /*
> + * Number of PAGE_SIZE sized zpdescs/pages to combine to
> + * form a 'zspage'
> + */
> int pages_per_zspage;
>
> unsigned int index;
> @@ -908,7 +910,7 @@ static void free_zspage(struct zs_pool *pool, struct size_class *class,
>
> /*
> * Since zs_free couldn't be sleepable, this function cannot call
> - * lock_page. The page locks trylock_zspage got will be released
> + * lock_page. The zpdesc locks trylock_zspage got will be released
> * by __free_zspage.
> */
> if (!trylock_zspage(zspage)) {
> @@ -965,7 +967,7 @@ static void init_zspage(struct size_class *class, struct zspage *zspage)
> set_freeobj(zspage, 0);
> }
>
> -static void create_page_chain(struct size_class *class, struct zspage *zspage,
> +static void create_zpdesc_chain(struct size_class *class, struct zspage *zspage,
> struct zpdesc *zpdescs[])
> {
> int i;
> @@ -974,9 +976,9 @@ static void create_page_chain(struct size_class *class, struct zspage *zspage,
> int nr_zpdescs = class->pages_per_zspage;
>
> /*
> - * Allocate individual pages and link them together as:
> - * 1. all pages are linked together using zpdesc->next
> - * 2. each sub-page point to zspage using zpdesc->zspage
> + * Allocate individual zpdescs and link them together as:
> + * 1. all zpdescs are linked together using zpdesc->next
> + * 2. each sub-zpdesc point to zspage using zpdesc->zspage
> *
> * we set PG_private to identify the first zpdesc (i.e. no other zpdesc
> * has this flag set).
> @@ -1034,7 +1036,7 @@ static struct zspage *alloc_zspage(struct zs_pool *pool,
> zpdescs[i] = zpdesc;
> }
>
> - create_page_chain(class, zspage, zpdescs);
> + create_zpdesc_chain(class, zspage, zpdescs);
> init_zspage(class, zspage);
> zspage->pool = pool;
> zspage->class = class->index;
> @@ -1351,7 +1353,7 @@ static unsigned long obj_malloc(struct zs_pool *pool,
> /* record handle in the header of allocated chunk */
> link->handle = handle | OBJ_ALLOCATED_TAG;
> else
> - /* record handle to page->index */
> + /* record handle to zpdesc->handle */
> zspage->first_zpdesc->handle = handle | OBJ_ALLOCATED_TAG;
the name of the field is already 'handle',
so we don't need a comment to explain it.
> kunmap_local(vaddr);
> @@ -1441,7 +1443,6 @@ static void obj_free(int class_size, unsigned long obj)
> unsigned int f_objidx;
> void *vaddr;
>
> -
> obj_to_location(obj, &f_zpdesc, &f_objidx);
> f_offset = offset_in_page(class_size * f_objidx);
> zspage = get_zspage(f_zpdesc);
> @@ -1684,19 +1685,19 @@ static int putback_zspage(struct size_class *class, struct zspage *zspage)
> #ifdef CONFIG_COMPACTION
> /*
> * To prevent zspage destroy during migration, zspage freeing should
> - * hold locks of all pages in the zspage.
> + * hold locks of all component zpdesc in the zspage.
> */
> static void lock_zspage(struct zspage *zspage)
> {
> struct zpdesc *curr_zpdesc, *zpdesc;
>
> /*
> - * Pages we haven't locked yet can be migrated off the list while we're
> + * Zpdesc we haven't locked yet can be migrated off the list while we're
> * trying to lock them, so we need to be careful and only attempt to
> - * lock each page under migrate_read_lock(). Otherwise, the page we lock
> - * may no longer belong to the zspage. This means that we may wait for
> - * the wrong page to unlock, so we must take a reference to the page
> - * prior to waiting for it to unlock outside migrate_read_lock().
> + * lock each zpdesc under migrate_read_lock(). Otherwise, the zpdesc we
> + * lock may no longer belong to the zspage. This means that we may wait
> + * for the wrong zpdesc to unlock, so we must take a reference to the
> + * zpdesc prior to waiting for it to unlock outside migrate_read_lock().
> */
> while (1) {
> migrate_read_lock(zspage);
> @@ -1771,7 +1772,7 @@ static void replace_sub_page(struct size_class *class, struct zspage *zspage,
> idx++;
> } while ((zpdesc = get_next_zpdesc(zpdesc)) != NULL);
>
> - create_page_chain(class, zspage, zpdescs);
> + create_zpdesc_chain(class, zspage, zpdescs);
> first_obj_offset = get_first_obj_offset(oldzpdesc);
> set_first_obj_offset(newzpdesc, first_obj_offset);
> if (unlikely(ZsHugePage(zspage)))
> @@ -1782,8 +1783,8 @@ static void replace_sub_page(struct size_class *class, struct zspage *zspage,
> static bool zs_page_isolate(struct page *page, isolate_mode_t mode)
> {
> /*
> - * Page is locked so zspage couldn't be destroyed. For detail, look at
> - * lock_zspage in free_zspage.
> + * Page/zpdesc is locked so zspage couldn't be destroyed. For detail,
> + * look at lock_zspage in free_zspage.
> */
> VM_BUG_ON_PAGE(PageIsolated(page), page);
>
> @@ -1810,7 +1811,7 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
> /* We're committed, tell the world that this is a Zsmalloc page. */
> __zpdesc_set_zsmalloc(newzpdesc);
>
> - /* The page is locked, so this pointer must remain valid */
> + /* The zpdesc/page is locked, so this pointer must remain valid */
> zspage = get_zspage(zpdesc);
> pool = zspage->pool;
>
> @@ -1883,7 +1884,7 @@ static const struct movable_operations zsmalloc_mops = {
> };
>
> /*
> - * Caller should hold page_lock of all pages in the zspage
> + * Caller should hold zpdesc locks of all in the zspage
> * In here, we cannot use zspage meta data.
> */
> static void async_free_zspage(struct work_struct *work)
> --
> 2.45.2
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v8 21/21] mm/zsmalloc: update comments for page->zpdesc changes
2024-12-10 15:46 ` Hyeonggon Yoo
@ 2024-12-11 4:37 ` Matthew Wilcox
2024-12-12 0:11 ` Hyeonggon Yoo
0 siblings, 1 reply; 33+ messages in thread
From: Matthew Wilcox @ 2024-12-11 4:37 UTC (permalink / raw)
To: Hyeonggon Yoo; +Cc: Minchan Kim, Sergey Senozhatsky, Alex Shi, linux-mm
On Wed, Dec 11, 2024 at 12:46:43AM +0900, Hyeonggon Yoo wrote:
> Talking about updating comments and code by replacing 'page' with 'zpdesc',
> I'm not sure if it makes sense to replace all instances of 'page' with 'zpdesc'.
> A zpdesc is a descriptor, not a page that contains data (at least that's
> what I have been thinking while writing the initial patch series).
Agreed.
> In that context I'm still not sure if saying "a sub-zpdesc of a zspage", "lock zpdesc",
> or "migrate zpdesc" makes sense because replacing 'page descriptor (aka. struct page)' with
> 'zpool descriptor (aka. zpdesc)' doesn't mean zsmalloc is throwing away the conecept of pages.
sub-zpdesc is a silly thing to say. subpage is equally silly. If it's
a page, call it a page.
However, locking the zpdesc does make sense -- we're locking the
descriptor.
> > /*
> > * Following is how we use various fields and flags of underlying
> > - * struct page(s) to form a zspage.
> > + * struct zpdesc(page) to form a zspage.
> > *
> > - * Usage of struct page fields:
> > - * page->private: points to zspage
> > - * page->index: links together all component pages of a zspage
> > + * Usage of struct zpdesc fields:
> > + * zpdesc->zspage: points to zspage
> > + * zpdesc->next: links together all component zpdescs of a zspage
> > * For the huge page, this is always 0, so we use this field
> > * to store handle.
> > - * page->page_type: PGTY_zsmalloc, lower 24 bits locate the first object
> > - * offset in a subpage of a zspage
> > - *
> > - * Usage of struct page flags:
> > - * PG_private: identifies the first component page
> > - * PG_owner_priv_1: identifies the huge component page
> > + * zpdesc->first_obj_offset: PGTY_zsmalloc, lower 24 bits locate the first
> > + * object offset in a subpage of a zspage
> > *
> > + * Usage of struct zpdesc(page) flags:
> > + * PG_private: identifies the first component zpdesc
> > + * PG_lock: lock all component zpdescs for a zspage free, serialize with
> > */
>
> I think this comment is unnecessary, as it's already documented in mm/zpdesc.h.
> It can be removed in patch 01.
Agreed. I did think about doing that, so if you want to do it too,
that's two votes for doing it.
> > @@ -1351,7 +1353,7 @@ static unsigned long obj_malloc(struct zs_pool *pool,
> > /* record handle in the header of allocated chunk */
> > link->handle = handle | OBJ_ALLOCATED_TAG;
> > else
> > - /* record handle to page->index */
> > + /* record handle to zpdesc->handle */
> > zspage->first_zpdesc->handle = handle | OBJ_ALLOCATED_TAG;
>
> the name of the field is already 'handle',
> so we don't need a comment to explain it.
Agreed.
Do you want to take on producing v9 or do you want me to fold in your
suggestions and send it?
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v8 21/21] mm/zsmalloc: update comments for page->zpdesc changes
2024-12-11 4:37 ` Matthew Wilcox
@ 2024-12-12 0:11 ` Hyeonggon Yoo
2024-12-13 2:25 ` Alex Shi
0 siblings, 1 reply; 33+ messages in thread
From: Hyeonggon Yoo @ 2024-12-12 0:11 UTC (permalink / raw)
To: Matthew Wilcox; +Cc: Minchan Kim, Sergey Senozhatsky, Alex Shi, linux-mm
On Wed, Dec 11, 2024 at 1:37 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Wed, Dec 11, 2024 at 12:46:43AM +0900, Hyeonggon Yoo wrote:
> > Talking about updating comments and code by replacing 'page' with 'zpdesc',
> > I'm not sure if it makes sense to replace all instances of 'page' with 'zpdesc'.
> > A zpdesc is a descriptor, not a page that contains data (at least that's
> > what I have been thinking while writing the initial patch series).
>
> Agreed.
>
> > In that context I'm still not sure if saying "a sub-zpdesc of a zspage", "lock zpdesc",
> > or "migrate zpdesc" makes sense because replacing 'page descriptor (aka. struct page)' with
> > 'zpool descriptor (aka. zpdesc)' doesn't mean zsmalloc is throwing away the conecept of pages.
>
> sub-zpdesc is a silly thing to say. subpage is equally silly. If it's
> a page, call it a page.
Agreed.
> However, locking the zpdesc does make sense -- we're locking the
> descriptor.
That makes sense.
> > > /*
> > > * Following is how we use various fields and flags of underlying
> > > - * struct page(s) to form a zspage.
> > > + * struct zpdesc(page) to form a zspage.
> > > *
> > > - * Usage of struct page fields:
> > > - * page->private: points to zspage
> > > - * page->index: links together all component pages of a zspage
> > > + * Usage of struct zpdesc fields:
> > > + * zpdesc->zspage: points to zspage
> > > + * zpdesc->next: links together all component zpdescs of a zspage
> > > * For the huge page, this is always 0, so we use this field
> > > * to store handle.
> > > - * page->page_type: PGTY_zsmalloc, lower 24 bits locate the first object
> > > - * offset in a subpage of a zspage
> > > - *
> > > - * Usage of struct page flags:
> > > - * PG_private: identifies the first component page
> > > - * PG_owner_priv_1: identifies the huge component page
> > > + * zpdesc->first_obj_offset: PGTY_zsmalloc, lower 24 bits locate the first
> > > + * object offset in a subpage of a zspage
> > > *
> > > + * Usage of struct zpdesc(page) flags:
> > > + * PG_private: identifies the first component zpdesc
> > > + * PG_lock: lock all component zpdescs for a zspage free, serialize with
> > > */
> >
> > I think this comment is unnecessary, as it's already documented in mm/zpdesc.h.
> > It can be removed in patch 01.
>
> Agreed. I did think about doing that, so if you want to do it too,
> that's two votes for doing it.
Will do in v9.
> > > @@ -1351,7 +1353,7 @@ static unsigned long obj_malloc(struct zs_pool *pool,
> > > /* record handle in the header of allocated chunk */
> > > link->handle = handle | OBJ_ALLOCATED_TAG;
> > > else
> > > - /* record handle to page->index */
> > > + /* record handle to zpdesc->handle */
> > > zspage->first_zpdesc->handle = handle | OBJ_ALLOCATED_TAG;
> >
> > the name of the field is already 'handle',
> > so we don't need a comment to explain it.
>
> Agreed.
>
> Do you want to take on producing v9 or do you want me to fold in your
> suggestions and send it?
I will send v9 with my feedback adjusted this weekend.
Thank you for rebasing and pushing this forward.
Best,
Hyeonggon
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v8 21/21] mm/zsmalloc: update comments for page->zpdesc changes
2024-12-12 0:11 ` Hyeonggon Yoo
@ 2024-12-13 2:25 ` Alex Shi
0 siblings, 0 replies; 33+ messages in thread
From: Alex Shi @ 2024-12-13 2:25 UTC (permalink / raw)
To: Hyeonggon Yoo
Cc: Matthew Wilcox, Minchan Kim, Sergey Senozhatsky, Alex Shi, linux-mm
Hyeonggon Yoo <42.hyeyoo@gmail.com> 于2024年12月12日周四 08:12写道:
>
> On Wed, Dec 11, 2024 at 1:37 PM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > On Wed, Dec 11, 2024 at 12:46:43AM +0900, Hyeonggon Yoo wrote:
> > > Talking about updating comments and code by replacing 'page' with 'zpdesc',
> > > I'm not sure if it makes sense to replace all instances of 'page' with 'zpdesc'.
> > > A zpdesc is a descriptor, not a page that contains data (at least that's
> > > what I have been thinking while writing the initial patch series).
> >
> > Agreed.
> >
> > > In that context I'm still not sure if saying "a sub-zpdesc of a zspage", "lock zpdesc",
> > > or "migrate zpdesc" makes sense because replacing 'page descriptor (aka. struct page)' with
> > > 'zpool descriptor (aka. zpdesc)' doesn't mean zsmalloc is throwing away the conecept of pages.
> >
> > sub-zpdesc is a silly thing to say. subpage is equally silly. If it's
> > a page, call it a page.
>
> Agreed.
>
> > However, locking the zpdesc does make sense -- we're locking the
> > descriptor.
>
> That makes sense.
>
> > > > /*
> > > > * Following is how we use various fields and flags of underlying
> > > > - * struct page(s) to form a zspage.
> > > > + * struct zpdesc(page) to form a zspage.
> > > > *
> > > > - * Usage of struct page fields:
> > > > - * page->private: points to zspage
> > > > - * page->index: links together all component pages of a zspage
> > > > + * Usage of struct zpdesc fields:
> > > > + * zpdesc->zspage: points to zspage
> > > > + * zpdesc->next: links together all component zpdescs of a zspage
> > > > * For the huge page, this is always 0, so we use this field
> > > > * to store handle.
> > > > - * page->page_type: PGTY_zsmalloc, lower 24 bits locate the first object
> > > > - * offset in a subpage of a zspage
> > > > - *
> > > > - * Usage of struct page flags:
> > > > - * PG_private: identifies the first component page
> > > > - * PG_owner_priv_1: identifies the huge component page
> > > > + * zpdesc->first_obj_offset: PGTY_zsmalloc, lower 24 bits locate the first
> > > > + * object offset in a subpage of a zspage
> > > > *
> > > > + * Usage of struct zpdesc(page) flags:
> > > > + * PG_private: identifies the first component zpdesc
> > > > + * PG_lock: lock all component zpdescs for a zspage free, serialize with
> > > > */
> > >
> > > I think this comment is unnecessary, as it's already documented in mm/zpdesc.h.
> > > It can be removed in patch 01.
> >
> > Agreed. I did think about doing that, so if you want to do it too,
> > that's two votes for doing it.
>
> Will do in v9.
>
> > > > @@ -1351,7 +1353,7 @@ static unsigned long obj_malloc(struct zs_pool *pool,
> > > > /* record handle in the header of allocated chunk */
> > > > link->handle = handle | OBJ_ALLOCATED_TAG;
> > > > else
> > > > - /* record handle to page->index */
> > > > + /* record handle to zpdesc->handle */
> > > > zspage->first_zpdesc->handle = handle | OBJ_ALLOCATED_TAG;
> > >
> > > the name of the field is already 'handle',
> > > so we don't need a comment to explain it.
> >
> > Agreed.
> >
> > Do you want to take on producing v9 or do you want me to fold in your
> > suggestions and send it?
>
> I will send v9 with my feedback adjusted this weekend.
> Thank you for rebasing and pushing this forward.
Very glad to see it is pushed forward. I am changing to a new career
recently, so sorry for not putting some effort into it.
Thanks a lot for you guys!
>
> Best,
> Hyeonggon
^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2024-12-13 2:25 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-12-05 17:49 [PATCH v8 00/21] Add zpdesc memory descriptor for zswap.zpool Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 01/21] mm/zsmalloc: add " Matthew Wilcox (Oracle)
2024-12-10 13:44 ` Hyeonggon Yoo
2024-12-05 17:49 ` [PATCH v8 02/21] mm/zsmalloc: use zpdesc in trylock_zspage()/lock_zspage() Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 03/21] mm/zsmalloc: convert __zs_map_object/__zs_unmap_object to use zpdesc Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 04/21] mm/zsmalloc: add and use pfn/zpdesc seeking funcs Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 05/21] mm/zsmalloc: convert obj_malloc() to use zpdesc Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 06/21] mm/zsmalloc: convert create_page_chain() and its users " Matthew Wilcox (Oracle)
2024-12-10 13:53 ` Hyeonggon Yoo
2024-12-05 17:49 ` [PATCH v8 07/21] mm/zsmalloc: convert obj_allocated() and related helpers " Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 08/21] mm/zsmalloc: convert init_zspage() " Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 09/21] mm/zsmalloc: convert obj_to_page() and zs_free() " Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 10/21] mm/zsmalloc: add zpdesc_is_isolated()/zpdesc_zone() helper for zs_page_migrate() Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 11/21] mm/zsmalloc: rename reset_page to reset_zpdesc and use zpdesc in it Matthew Wilcox (Oracle)
2024-12-10 14:00 ` Hyeonggon Yoo
2024-12-05 17:49 ` [PATCH v8 12/21] mm/zsmalloc: convert __free_zspage() to use zpdesc Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 13/21] mm/zsmalloc: convert location_to_obj() to take zpdesc Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 14/21] mm/zsmalloc: convert migrate_zspage() to use zpdesc Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 15/21] mm/zsmalloc: convert get_zspage() to take zpdesc Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 16/21] mm/zsmalloc: convert SetZsPageMovable and remove unused funcs Matthew Wilcox (Oracle)
2024-12-10 14:04 ` Hyeonggon Yoo
2024-12-05 17:49 ` [PATCH v8 17/21] mm/zsmalloc: convert get/set_first_obj_offset() to take zpdesc Matthew Wilcox (Oracle)
2024-12-10 14:10 ` Hyeonggon Yoo
2024-12-05 17:49 ` [PATCH v8 18/21] mm/zsmalloc: introduce __zpdesc_clear_movable Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 19/21] mm/zsmalloc: introduce __zpdesc_clear/set_zsmalloc() Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 20/21] mm/zsmalloc: introduce zpdesc_clear_first() helper Matthew Wilcox (Oracle)
2024-12-05 17:49 ` [PATCH v8 21/21] mm/zsmalloc: update comments for page->zpdesc changes Matthew Wilcox (Oracle)
2024-12-10 15:46 ` Hyeonggon Yoo
2024-12-11 4:37 ` Matthew Wilcox
2024-12-12 0:11 ` Hyeonggon Yoo
2024-12-13 2:25 ` Alex Shi
2024-12-09 11:59 ` [PATCH v8 0/21] Add zpdesc memory descriptor for zswap.zpool alexs
2024-12-10 5:54 ` [PATCH v8 00/21] " Sergey Senozhatsky
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox