From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C2FC4C433E1 for ; Fri, 21 Aug 2020 00:22:02 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 5D6FB2076E for ; Fri, 21 Aug 2020 00:22:02 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="K/iwexCs" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5D6FB2076E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id C07826B002F; Thu, 20 Aug 2020 20:22:01 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BDEB76B0030; Thu, 20 Aug 2020 20:22:01 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AF3B96B0031; Thu, 20 Aug 2020 20:22:01 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0084.hostedemail.com [216.40.44.84]) by kanga.kvack.org (Postfix) with ESMTP id 995806B002F for ; Thu, 20 Aug 2020 20:22:01 -0400 (EDT) Received: from smtpin23.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 5025D180AD811 for ; Fri, 21 Aug 2020 00:22:01 +0000 (UTC) X-FDA: 77172673242.23.sheet39_2d019a827034 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin23.hostedemail.com (Postfix) with ESMTP id 2216037604 for ; Fri, 21 Aug 2020 00:22:01 +0000 (UTC) X-HE-Tag: sheet39_2d019a827034 X-Filterd-Recvd-Size: 6951 Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by imf41.hostedemail.com (Postfix) with ESMTP for ; Fri, 21 Aug 2020 00:22:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1597969319; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=3aBHsIrR7D1rmQusl3vZJUEll0MhNbsTlUTcbOAur3g=; b=K/iwexCse09iSj0Ii0nnBpPgYOsmb+c14NbJ6D986HBd3hdPFYQ9Xm27cqc9my1OBBE5WZ 6Bi1rnoy570BT1fQIIx5lwdA+ZfHrsQykzgWFHniQ79fqMaKEaxyxoOabzffAxWlpJIn4Z NcRwBlHh3b6+bKa7nw1vwzJWFshYHyg= Received: from mail-pf1-f199.google.com (mail-pf1-f199.google.com [209.85.210.199]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-486-O-YnIU17N7O6SX4Z_2N1gw-1; Thu, 20 Aug 2020 20:21:57 -0400 X-MC-Unique: O-YnIU17N7O6SX4Z_2N1gw-1 Received: by mail-pf1-f199.google.com with SMTP id b142so233497pfb.9 for ; Thu, 20 Aug 2020 17:21:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=3aBHsIrR7D1rmQusl3vZJUEll0MhNbsTlUTcbOAur3g=; b=Dfq2kBTmA72FCK2rftMTzSxe2pdo8uU9FM49L6GR9flBhuthla20IVHkFsN3Dk/wT3 k4ijPyUb5B4VDDPUgR57Nh948+TdWLjUgUaeFjCXBHlgghxYuopok1s26c/RogD3dhoy jfgGZWYvuLBrCdhF3JJMNht8EMv4IPnOsc9f2f0bLeQ6F6YO3ccW+M+THDXoZTingbK0 8qWPyvJSuBPF1X38NXszM56kWzLCud/2T/uQnUjHhPOO9vDiBCmN9AVmYeofGjxS+k5u 39Q8H473UFgA5XsArs1BsNBMVD2/v4QqaWI9WTQO+bGOu8zOUveDTIFZcC8SszEqnztY T2PA== X-Gm-Message-State: AOAM530541YB2HIaLuxil72F3DO69adXnoqw8Xw9ujk7KKk+DruGl3rp m5ZNgbe4/AaiDWufNYXBRwmDD5wg+KOiXzPegnmDe2nIKrIqlfcX749gPt8GqqYtnV2jqhULPTA a8HTBAHA4tuI= X-Received: by 2002:a62:20e:: with SMTP id 14mr305550pfc.207.1597969316732; Thu, 20 Aug 2020 17:21:56 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwKnJ5d5JB7dEtC9MO6PnJWSvjC7EM//ku1FdHc84FzCEHPg33i7BQaa39t+IuifAAfBOurJg== X-Received: by 2002:a62:20e:: with SMTP id 14mr305520pfc.207.1597969316393; Thu, 20 Aug 2020 17:21:56 -0700 (PDT) Received: from xiangao.remote.csb ([209.132.188.80]) by smtp.gmail.com with ESMTPSA id y135sm248586pfg.148.2020.08.20.17.21.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 20 Aug 2020 17:21:55 -0700 (PDT) Date: Fri, 21 Aug 2020 08:21:45 +0800 From: Gao Xiang To: Dave Chinner Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Carlos Maiolino , Eric Sandeen , "Huang, Ying" , Yang Shi , Rafael Aquini , stable Subject: Re: [PATCH v2] mm, THP, swap: fix allocating cluster for swapfile by mistake Message-ID: <20200821002145.GA28298@xiangao.remote.csb> References: <20200820045323.7809-1-hsiangkao@redhat.com> <20200820233446.GB7728@dread.disaster.area> MIME-Version: 1.0 In-Reply-To: <20200820233446.GB7728@dread.disaster.area> User-Agent: Mutt/1.10.1 (2018-07-13) Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hsiangkao@redhat.com X-Mimecast-Spam-Score: 0.004 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Rspamd-Queue-Id: 2216037604 X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam03 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Hi Dave, On Fri, Aug 21, 2020 at 09:34:46AM +1000, Dave Chinner wrote: > On Thu, Aug 20, 2020 at 12:53:23PM +0800, Gao Xiang wrote: > > SWP_FS is used to make swap_{read,write}page() go through > > the filesystem, and it's only used for swap files over > > NFS. So, !SWP_FS means non NFS for now, it could be either > > file backed or device backed. Something similar goes with > > legacy SWP_FILE. > > > > 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. > > > > I reproduced the issue with the following details: > > > > Environment: > > QEMU + upstream kernel + buildroot + NVMe (2 GB) > > > > Kernel config: > > CONFIG_BLK_DEV_NVME=y > > CONFIG_THP_SWAP=y > > Ok, so at it's core this is a swap file extent versus THP swap > cluster alignment issue? I think yes. > > > diff --git a/mm/swapfile.c b/mm/swapfile.c > > index 6c26916e95fd..2937daf3ca02 100644 > > --- a/mm/swapfile.c > > +++ b/mm/swapfile.c > > @@ -1074,7 +1074,7 @@ int get_swap_pages(int n_goal, swp_entry_t swp_entries[], int entry_size) > > goto nextsi; > > } > > if (size == SWAPFILE_CLUSTER) { > > - if (!(si->flags & SWP_FS)) > > + if (si->flags & SWP_BLKDEV) > > n_ret = swap_alloc_cluster(si, swp_entries); > > } else > > n_ret = scan_swap_map_slots(si, SWAP_HAS_CACHE, > > IOWs, if you don't make this change, does the corruption problem go > away if you align swap extents in iomap_swapfile_add_extent() to > (SWAPFILE_CLUSTER * PAGE_SIZE) instead of just PAGE_SIZE? > > I.e. if the swapfile extents are aligned correctly to huge page swap > cluster size and alignment, does the swap clustering optimisations > for swapping THP pages work correctly? And, if so, is there any > performance benefit we get from enabling proper THP swap clustering > on swapfiles? > Yeah, I once think about some similiar thing as well. My thought for now is - First, SWAP THP doesn't claim to support such swapfile for now. And the original author tried to explicitly avoid the whole thing in f0eea189e8e9 ("mm, THP, swap: Don't allocate huge cluster for file backed swap device") So such thing would be considered as some new feature and need more testing at least. But for now I think we just need a quick fix to fix the commit f0eea189e8e9 to avoid regression and for backport use. - It is hard for users to control swapfile in SWAPFILE_CLUSTER * PAGE_SIZE extents, especially users' disk are fragmented or have some on-disk metadata limitation or something. It's very hard for users to utilize this and arrange their swapfile physical addr alignment and fragments for now. So my point is, if it's considered in the future (supporting SWAP THP swapfile), it needs more carefully consideration and decision (e.g. stability, performance, simplicity, etc). For now, it's just a exist regression which fixes the original fix, and finish the original author claim. Thanks, Gao Xiang > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com >