On 27/06/2024 19:18, Johannes Weiner wrote:
Hi Usama,
On Thu, Jun 27, 2024 at 11:55:29AM +0100, Usama Arif wrote:
Approximately 10-20% of pages to be swapped out are zero pages [1].
Rather than reading/writing these pages to flash resulting
in increased I/O and flash wear, a bitmap can be used to mark these
pages as zero at write time, and the pages can be filled at
read time if the bit corresponding to the page is set.
With this patch, NVMe writes in Meta server fleet decreased
by almost 10% with conventional swap setup (zswap disabled).
[1] https://lore.kernel.org/all/20171018104832epcms5p1b2232e2236258de3d03d1344dde9fce0@epcms5p1/
Signed-off-by: Usama Arif <usamaarif642@xxxxxxxxx>
Reviewed-by: Chengming Zhou <chengming.zhou@xxxxxxxxx>
Reviewed-by: Yosry Ahmed <yosryahmed@xxxxxxxxxx>
Reviewed-by: Nhat Pham <nphamcs@xxxxxxxxx>
Cc: David Hildenbrand <david@xxxxxxxxxx>
Cc: "Huang, Ying" <ying.huang@xxxxxxxxx>
Cc: Hugh Dickins <hughd@xxxxxxxxxx>
Cc: Johannes Weiner <hannes@xxxxxxxxxxx>
Cc: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
Cc: Shakeel Butt <shakeel.butt@xxxxxxxxx>
Cc: Usama Arif <usamaarif642@xxxxxxxxx>
Cc: Andi Kleen <ak@xxxxxxxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
This looks great to me, and the numbers speak for themselves. A few
minor comments below:
---
include/linux/swap.h | 1 +
mm/page_io.c | 113 ++++++++++++++++++++++++++++++++++++++++++-
mm/swapfile.c | 20 ++++++++
3 files changed, 133 insertions(+), 1 deletion(-)
diff --git a/include/linux/swap.h b/include/linux/swap.h
index 3df75d62a835..ed03d421febd 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -299,6 +299,7 @@ struct swap_info_struct {
signed char type; /* strange name for an index */
unsigned int max; /* extent of the swap_map */
unsigned char *swap_map; /* vmalloc'ed array of usage counts */
+ unsigned long *zeromap; /* vmalloc'ed bitmap to track zero pages */
struct swap_cluster_info *cluster_info; /* cluster info. Only for SSD */
struct swap_cluster_list free_clusters; /* free clusters list */
unsigned int lowest_bit; /* index of first free in swap_map */
diff --git a/mm/page_io.c b/mm/page_io.c
index 6c1c1828bb88..480b8f221d90 100644
--- a/mm/page_io.c
+++ b/mm/page_io.c
@@ -172,6 +172,88 @@ int generic_swapfile_activate(struct swap_info_struct *sis,
goto out;
}
It might be good to have a short comment that gives 1) an overview,
that we're using a bitmap to avoid doing IO for zero-filled pages and
2) the locking, that the bits are protected by the locked swapcache
folio and atomic updates are used to protect against RMW corruption
due to other zero swap entries seeing concurrent updates.
Thanks! have addressed the comments and will include them in next
revision. Just a couple of things.
Will add the overview in swap_writepage when the check is made if the
folio is zero filled and zeromap bits are set, instead of at this point.
+static bool is_folio_page_zero_filled(struct folio *folio, int i)
+{
+ unsigned long *data;
+ unsigned int pos, last_pos = PAGE_SIZE / sizeof(*data) - 1;
+ bool ret = false;
+
+ data = kmap_local_folio(folio, i * PAGE_SIZE);
+ if (data[last_pos])
+ goto out;
+static void folio_zero_fill(struct folio *folio)
+{
+ unsigned int i;
+
+ for (i = 0; i < folio_nr_pages(folio); i++)
+ clear_highpage(folio_page(folio, i));
+}
Should this be in highmem.h next to the other folio_zero_* functions?
Thanks for pointing to highmem.h. It already has folio_zero_range, which
should do the same thing, so I think I can just do
folio_zero_range(folio, 0, folio_size(folio)) and this function shouldnt
be needed.
