On Wed, Aug 19, 2020 at 1:15 PM Gao Xiang hsiangkao@redhat.com wrote:
Hi Andrew,
On Wed, Aug 19, 2020 at 01:05:06PM -0700, Andrew Morton wrote:
On Thu, 20 Aug 2020 03:56:13 +0800 Gao Xiang hsiangkao@redhat.com wrote:
SWP_FS doesn't mean the device is file-backed swap device, which just means each writeback request should go through fs by DIO. Or it'll just use extents added by .swap_activate(), but it also works as file-backed swap device.
This is very hard to understand :(
Thanks for your reply...
The related logic is in __swap_writepage() and setup_swap_extents(), and also see e.g generic_swapfile_activate() or iomap_swapfile_activate()...
I think just NFS falls into this case, so you may rephrase it to:
SWP_FS is only used for swap files over NFS. So, !SWP_FS means non NFS swap, it could be either file backed or device backed.
Does this look more understandable?
I will also talk with "Huang, Ying" in person if no response here.
So in order to achieve the goal of the original patch, SWP_BLKDEV should be used instead.
FS corruption can be observed with SSD device + XFS + fragmented swapfile due to CONFIG_THP_SWAP=y.
Fixes: f0eea189e8e9 ("mm, THP, swap: Don't allocate huge cluster for file backed swap device") Fixes: 38d8b4e6bdc8 ("mm, THP, swap: delay splitting THP during swap out")
Why do you think it has taken three years to discover this?
I'm not sure if the Redhat BZ is available for public, it can be reproduced since rhel 8 https://bugzilla.redhat.com/show_bug.cgi?id=1855474
It seems hard to believe, but I think just because rare user uses the SSD device + THP + file-backed swap device combination... maybe I'm wrong here, but my test shows as it is.
Thanks, Gao Xiang