On 2023/11/21 17:42, Johan.Erlandsson@xxxxxxxx wrote:
>>>>> Hi
>>>>> Sharing 3 changes for zram regarding swap cache handling. Please have a look.
>>>>>
>>>>> Subject: [PATCH 1/3] zram, swap cache missing page tree offset
>>>>> Subject: [PATCH 2/3] zram, swap cache entries are pointer to struct page
>>>>> Subject: [PATCH 3/3] zram, exclude shadow entries from swap cache lookup
>>>>
>>>> Thank you for the patches.
>>>>
>>>> > /* this already exists in maple_tree.h add to defs.h ? */
>>>>
>>>> Is it ok to add '#include maple_tree.h' ?
>>>
>>> Yes, that should work perfectly for 'xa_is_value'.
>>
>> Thanks for the reply.
>>
>> I'd like to squash the patches into a patch and add our signed-off-by
>> tags. Please let me know if there is any trouble with the attached patch.
>
> All looks good, verified.
>
>>
>> One more thing, do you have any error message without the patch? I'd
>> like to add it to the commit log, if possible.
>
> No, I haven't seen any error messages. Short description what happens:
>
> - lookup_swap_cache() return NULL since do_xarray() call return FALSE
> - in try_zram_decompress(), since 'entry' is NULL, page is filled with 00
> if (!entry || (flags & ZRAM_FLAG_SAME_BIT)) {
>
> So without the patch, pages in swap cache will be seen to be a 'zero' page.
Thanks for the description, added.
Linabo, I updated the attached patch.
Thanks,
KazuFrom d3ec42c71bd6cea02e1e674d3243b803c233cbb9 Mon Sep 17 00:00:00 2001
From: Johan Erlandsson <johan.erlandsson@xxxxxxxx>
Date: Fri, 20 Oct 2023 19:10:52 +0200
Subject: [PATCH] zram: Fixes for lookup_swap_cache()
Fix the following three issues:
(1) swap cache missing page tree offset
The radix or xarray start at an offset inside struct address_space.
(2) swap cache entries are pointer to struct page
The entries in radix, xarray (swap cache) are address to struct page.
(3) exclude shadow entries from swap cache lookup
radix or xarray can contain shadow entries from previous page
entries. These should be ignored when looking for a page pointer.
Without the patch,
- lookup_swap_cache() returns NULL since do_xarray() call returns FALSE,
- in try_zram_decompress(), since 'entry' is NULL, page is filled with 0,
if (!entry || (flags & ZRAM_FLAG_SAME_BIT)) {
and pages in swap cache will be seen to be a 'zero' page.
Signed-off-by: Johan Erlandsson <johan.erlandsson@xxxxxxxx>
Signed-off-by: Kazuhito Hagio <k-hagio-ab@xxxxxxx>
---
diskdump.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/diskdump.c b/diskdump.c
index 0fe46f4644d0..5886d90403f0 100644
--- a/diskdump.c
+++ b/diskdump.c
@@ -27,6 +27,7 @@
#include "diskdump.h"
#include "xen_dom0.h"
#include "vmcore.h"
+#include "maple_tree.h"
#define BITMAP_SECT_LEN 4096
@@ -2871,11 +2872,16 @@ out:
return zram_buf;
}
+static inline bool radix_tree_exceptional_entry(ulong entry)
+{
+ return entry & RADIX_TREE_EXCEPTIONAL_ENTRY;
+}
+
static unsigned char *
lookup_swap_cache(ulonglong pte_val, unsigned char *zram_buf)
{
ulonglong swp_offset;
- ulong swp_type, swp_space, page;
+ ulong swp_type, swp_space;
struct list_pair lp;
physaddr_t paddr;
static int is_xarray = -1;
@@ -2901,10 +2907,13 @@ lookup_swap_cache(ulonglong pte_val, unsigned char *zram_buf)
swp_space += (swp_offset >> SWAP_ADDRESS_SPACE_SHIFT) * SIZE(address_space);
lp.index = swp_offset;
- if ((is_xarray ? do_xarray : do_radix_tree)(swp_space, RADIX_TREE_SEARCH, &lp)) {
- readmem((ulong)lp.value, KVADDR, &page, sizeof(void *),
- "swap_cache page", FAULT_ON_ERROR);
- if (!is_page_ptr(page, &paddr)) {
+ if ((is_xarray ? do_xarray : do_radix_tree)
+ (swp_space+OFFSET(address_space_page_tree), RADIX_TREE_SEARCH, &lp)) {
+ if ((is_xarray ? xa_is_value : radix_tree_exceptional_entry)((ulong)lp.value)) {
+ /* ignore shadow values */
+ return NULL;
+ }
+ if (!is_page_ptr((ulong)lp.value, &paddr)) {
error(WARNING, "radix page: %lx: not a page pointer\n", lp.value);
return NULL;
}
--
2.31.1
--
Crash-utility mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
