ACK/Cmnt: [SRU][B][F] s390: dbginfo.sh triggers kernel panic, reading from /sys/kernel/mm/page_idle/bitmap (LP: 1904884)

Stefan Bader stefan.bader at canonical.com
Wed Nov 25 08:40:21 UTC 2020


On 24.11.20 17:38, frank.heimes at canonical.com wrote:
> BugLink: https://bugs.launchpad.net/bugs/1904884
> 
> SRU Justification:
> 
> [Impact]
> 
> * While executing dbginfo.sh (a script to collect runtime, configuration, and trace information on s390x) the systems hangs.
> 
> * This is because 'idle page tracking' users can pass random pfn, that might be mapped to
> an offline page - and attempts to access offline pages lead to the hang.
> 
> * It needs to be avoided that such pages are accessed.
> 
> * The upstream commit modifies 'page_idle_get_page()' to use 'pfn_to_online_page()' instead of a
> 'pfn_valid()' and 'pfn_to_page()' combination, so that the pfn mapped to an offline page is skipped.
> 
> [Fix]
> 
> * 92fb1db26eef "mm/page_idle.c: skip offline pages"
> 
> [Test Case]
> 
> * IBM Z or LinuxONE hardware with Ubuntu Server 18.04 (GA kernel, 4.15) installed.
> 
> * Execute a test application that tries to access offline pages.
> 
> * Or execute dbginfo.sh with having some offline (idle) pages in the system.
> 
> [Regression Potential]
> 
> * There is a certain regression risk, especially for bionic, since the structure in the kernel 4.15 is a bit different compared to kernel 5.4 (and newer).
> 
> * However, for newer kernels the modification is pretty save, since it's upstream accepted since kernel 5.8 and with that already inluded in hirsute and groovy.
> 
> * And the patch is fine (and cherry picks cleanly) for focal as well.
> 
> * For bionic there is a slightly conflicting context, since the struct 'zone' was replaced by 'pg_data_t *pgdat' (by another commit: 92fb1db26eef),
>   but that change (or any change to the struct zone) would not be necessary to fix the uninitialized struct page access.
> 
> * Hence the upstream commit/patch needs to be adjusted/backported to bionic 4.15, largely by replacing line 'pg_data_t *pgdat;' with 'struct zone *zone;' (or actually leaving this line).
> 
> * But this needs to be carefully considered, since the handling of idle pages could be harmful, in the end it could make things even worse, means break even more.
> 
> [Other]
> 
> * The patch got upstream accepted with kernel v5.8, hence it's already is in groovy and hirsute.
> 
> * The upstream commit cherry picks cleanly to focal, but for bionic a backport is needed.
> 
> * Hence this kernel SRU request is for focal (cherry-pick) and bionic (backport).
> 
> SeongJae Park (1):
>   mm/page_idle.c: skip offline pages
> 
>  mm/page_idle.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
From what I can see there is no difference in the way the page frame number is
converted into a page. So it seems valid to switch this to the test for online
pages. That should be obvious if being wrong as well. So with testing results,

Acked-by: Stefan Bader <stefan.bader at canonical.com>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20201125/4125b264/attachment-0001.sig>


More information about the kernel-team mailing list