Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 24 Oct 2022 10:12:35 -0700
From:   Dan Williams <dan.j.williams@intel.com>
To:     Dave Chinner <david@fromorbit.com>,
        "Darrick J. Wong" <djwong@kernel.org>
CC:     =?utf-8?B?WWFuZywgWGlhby/mnagg5pmT?= <yangx.jy@fujitsu.com>,
        =?utf-8?B?R290b3UsIFlhc3Vub3JpL+S6lOWztiDlurfmloc=?= 
        <y-goto@fujitsu.com>, Brian Foster <bfoster@redhat.com>,
        "hch@infradead.org" <hch@infradead.org>,
        =?utf-8?B?UnVhbiwgU2hpeWFuZy/pmK4g5LiW6Ziz?= 
        <ruansy.fnst@fujitsu.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-xfs@vger.kernel.org" <linux-xfs@vger.kernel.org>,
        "nvdimm@lists.linux.dev" <nvdimm@lists.linux.dev>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
        <zwisler@kernel.org>, Jeff Moyer <jmoyer@redhat.com>,
        <dm-devel@redhat.com>, <toshi.kani@hpe.com>
Subject: Re: [PATCH] xfs: fail dax mount if reflink is enabled on a partition
Message-ID: <6356c783c1813_1d2129457@dwillia2-mobl3.amr.corp.intel.com.notmuch>
References: <a6e7f4eb-0664-bbe8-98d2-f8386b226113@fujitsu.com>
 <e3d51a6b-12e9-2a19-1280-5fd9dd64117c@fujitsu.com>
 <deb54a77-90d3-df44-1880-61cce6e3f670@fujitsu.com>
 <1444b9b5-363a-163c-0513-55d1ea951799@fujitsu.com>
 <Yzt6eWLuX/RTjmjj@magnolia>
 <f196bcab-6aa2-6313-8a7c-f8ab409621b7@fujitsu.com>
 <Yzx64zGt2kTiDYaP@magnolia>
 <6a83a56e-addc-f3c4-2357-9589a49bf582@fujitsu.com>
 <Y1NRNtToQTjs0Dbd@magnolia>
 <20221023220018.GX3600936@dread.disaster.area>
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20221023220018.GX3600936@dread.disaster.area>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?cVY5VjVsZzRiSy9yQ05IUDhzMGFhUFlhUHBZS2ZPZUF5d1hRWGV4cXpjUWlk?=
 =?utf-8?B?WEJ2V0lnZmNWSTVJQ1ZTZXZ6Z0IvT2tjQWFjS2ROeGJXZ3EzMnorWDh6bHYv?=
 =?utf-8?B?N3ZVb21IMGR5aVc3SHB0WjQ4Q0RXdEc2OFAwMjlCa2dlQ01rK0pLeHpDRFRp?=
 =?utf-8?B?TnErSHFBbFNmNWxNRjNMSUIvOHpRZnlEemhsSzM3WWtsTzZVTGVqTTNzVGNr?=
 =?utf-8?B?YngyVUhQVEhEbTYvT2ZabzFiZDk0WEhWMzkzakJVamxPb1JCcXQ5dlJHYWt6?=
 =?utf-8?B?c0pPRkVzQ0RWOXlLSncrWTJ2bWVNZVBrazgyRkpoOHdLeklxM1pWbmE0NW1V?=
 =?utf-8?B?R28yUjJYdHdNZFV2SWhzQ091b00vSzhXMjlaa21VQjJ1VnlwdEJTK25oUk1P?=
 =?utf-8?B?bWpKVHpDWU40bW93d2FxejVtTkFEckZEbDZZdlp5N0NSSk9tR0lMemZlVE1D?=
 =?utf-8?B?ZkkxQXZtZDFhd0diOFdMMENUQUZHMFU2a0lLajQzS0M2ZHpYM0FYMExacEN2?=
 =?utf-8?B?eENRYjhSSDY3V052VW5BbGFCeU8vSFZJTmFmRzk3SmFoQXdwMFdoSHR3VEVL?=
 =?utf-8?B?anVuUXB5TmdqaHhzZGkrbU9TbEtnTUEvZDNBQlpJUWRvTDF3STVVMEV6MnpV?=
 =?utf-8?B?c0gxcGR2aE5zc2oxVHZQd1dENXFGMURxS3FiZWl6SjN1L1FUd3JXWWRkL0RY?=
 =?utf-8?B?MTQ0Qm11VGNaelQ1cmlSOWgvRDN4Z1V2M1p1blFWaFZPZ1lqQWRLajdBMEZj?=
 =?utf-8?B?Z0hxT3NMd2hwUyt1VjlvQWFEUnBYaWVLZllhenUzRC9rbFhrWEhZRG5RYXVH?=
 =?utf-8?B?NTljVndjdUpmOURJN3FhNm5CZFcvV1U5clNIT2ZTYnJqVnRiU1JDREtzamhU?=
 =?utf-8?B?WVNaZ1dENkdEK0pOeUpKcUFOaWp6Ui9GQVlwMDNlMWFsL3VsNFpvRnI5UXg0?=
 =?utf-8?B?RkZ6akVKUDA4MDFEVnJaLzJCaHRFZUpncHVSamk0NnZuN3JPTkcxbGNMd0d0?=
 =?utf-8?B?bzdidnM4clRmSy81dUsxMG5lSkdORXMvMHJmdjlJMDZNUFhab0hudGJkOTJz?=
 =?utf-8?B?dFErbHUyeENMdGRPSUw3TW9jNCs1Vm8ycWJjMVVaaFkzUHJSTTBReDRzUkh0?=
 =?utf-8?B?cVZnbXVJOXJabXlybnBqRWFna3daZGRWcWFwTXRUdktWYTJiKythcFU5MTVw?=
 =?utf-8?B?Ykx4M0RCcExiL2lsVVNDYWlBZEFZcEo2MlB0STVrRVNGbS9SU3dPZDdEczg3?=
 =?utf-8?B?WlZBNmZRbHo5VFdBOEZIUkZPYk5QQk84a1VUNVptS2EvZURrTk1MVVBKTEsx?=
 =?utf-8?B?SWVJN0xWTjNMZkVYUDd5OUdhT1ZweWRoVmNCT050bUhmWTVWeXk3YjU0RG5M?=
 =?utf-8?B?L3VtVWZTTGUyYmZKdzd0TExrS2pIeDdyQ256MUk5Y1hxZkpnUk9QSnJpNnRp?=
 =?utf-8?B?UHJzU1hreXplbkYvcGxPankvL0FBM3preWtNRS83YnE5eHVkVUZ4c1FjWWtV?=
 =?utf-8?B?elZxWkJ1Skkva1p3VCtlTXNhYkRKdVZJS2E3MS9Obm80K0hkWDhaWjRTVDd4?=
 =?utf-8?B?U3RDTkdrQVVrQ1hWZkFhUmdGMXBNeWljZGVteEZrd2lKRzhZeTB4dkw4eFd4?=
 =?utf-8?B?cC9KS29vc2pLVE1BZUU1Y3BjT3VMeEpiQnFjc1kxQVJHMEYvdlN4aXZ4azVQ?=
 =?utf-8?B?TEswdXVKeFVSbjR2MGVSVStVSTNCSldZTTJTRFFLVVhETld2ZGRVWTdmMTJh?=
 =?utf-8?B?amVvdnpVOGJnMlJITS9GWjc3RG92TDV3ZlV2THJWTHgwY0hZeXVRWUdtUDNF?=
 =?utf-8?B?T21FN1MwWm1uMFk0ay9IdEhNd0Vld21TRnBYQmcxK2R4aE43WkNWMm9XeE1j?=
 =?utf-8?B?RmFnd1RrRXF2Y0lFZnM5MlN6azNVaVVKZVdlU2NtMHlFU1lYU1MyS0tRQjA3?=
 =?utf-8?B?U0JjTnRBczR6WVhRWmtEaTg3WENQUDRMbVFXK25lWEF1eE9vaUZsYlhwQVFS?=
 =?utf-8?B?dzFFazEzMDY3d2ltNUhrSHdtQXV5Vy8rK1F5NVV2NkNyTFlvOVFwaUJIdTlK?=
 =?utf-8?B?cHU0ZTRUc2JBbmswbFlnWW1TcTdLTVZLRVhJaU90MjNZTzh4WkZBcWcxRURT?=
 =?utf-8?B?WTJrL29ZZ0ZQSlY2anRaQ1pMVUVoSXdIMjBWS3dDWC9HdVM0RkZNYUdHamNG?=
 =?utf-8?B?UUE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: c6002ab9-5bde-4c3c-91df-08dab5e2f43c
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1101MB2126.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Oct 2022 17:12:39.5255
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: UljFVLLSrf42kQ93wkN2mXp/yaLzSaKXdCm4xOfzNsbLwn/brLFiN3teIXRRdRQm1jccnwt8tBAWRA+gc2vT6QYIpzjM5dj9cfBdVzbl1fY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR11MB4582
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Dave Chinner wrote:
> On Fri, Oct 21, 2022 at 07:11:02PM -0700, Darrick J. Wong wrote:
> > On Thu, Oct 20, 2022 at 10:17:45PM +0800, Yang, Xiao/杨 晓 wrote:
> > > In addition, I don't like your idea about the test change because it will
> > > make generic/470 become the special test for XFS. Do you know if we can fix
> > > the issue by changing the test in another way? blkdiscard -z can fix the
> > > issue because it does zero-fill rather than discard on the block device.
> > > However, blkdiscard -z will take a lot of time when the block device is
> > > large.
> > 
> > Well we /could/ just do that too, but that will suck if you have 2TB of
> > pmem. ;)
> > 
> > Maybe as an alternative path we could just create a very small
> > filesystem on the pmem and then blkdiscard -z it?
> > 
> > That said -- does persistent memory actually have a future?  Intel
> > scuttled the entire Optane product, cxl.mem sounds like expansion
> > chassis full of DRAM, and fsdax is horribly broken in 6.0 (weird kernel
> > asserts everywhere) and 6.1 (every time I run fstests now I see massive
> > data corruption).
> 
> Yup, I see the same thing. fsdax was a train wreck in 6.0 - broken
> on both ext4 and XFS. Now that I run a quick check on 6.1-rc1, I
> don't think that has changed at all - I still see lots of kernel
> warnings, data corruption and "XFS_IOC_CLONE_RANGE: Invalid
> argument" errors.
> 
> If I turn off reflink, then instead of data corruption I get kernel
> warnings like this from fsx and fsstress workloads:
> 
> [415478.558426] ------------[ cut here ]------------
> [415478.560548] WARNING: CPU: 12 PID: 1515260 at fs/dax.c:380 dax_insert_entry+0x2a5/0x320
> [415478.564028] Modules linked in:
> [415478.565488] CPU: 12 PID: 1515260 Comm: fsx Tainted: G        W 6.1.0-rc1-dgc+ #1615
> [415478.569221] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
> [415478.572876] RIP: 0010:dax_insert_entry+0x2a5/0x320
> [415478.574980] Code: 08 48 83 c4 30 5b 5d 41 5c 41 5d 41 5e 41 5f c3 48 8b 58 20 48 8d 53 01 e9 65 ff ff ff 48 8b 58 20 48 8d 53 01 e9 50 ff ff ff <0f> 0b e9 70 ff ff ff 31 f6 4c 89 e7 e8 da ee a7 00 eb a4 48 81 e6
> [415478.582740] RSP: 0000:ffffc90002867b70 EFLAGS: 00010002
> [415478.584730] RAX: ffffea000f0d0800 RBX: 0000000000000001 RCX: 0000000000000001
> [415478.587487] RDX: ffffea0000000000 RSI: 000000000000003a RDI: ffffea000f0d0840
> [415478.590122] RBP: 0000000000000011 R08: 0000000000000000 R09: 0000000000000000
> [415478.592380] R10: ffff888800dc9c18 R11: 0000000000000001 R12: ffffc90002867c58
> [415478.594865] R13: ffff888800dc9c18 R14: ffffc90002867e18 R15: 0000000000000000
> [415478.596983] FS:  00007fd719fa2b80(0000) GS:ffff88883ec00000(0000) knlGS:0000000000000000
> [415478.599364] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [415478.600905] CR2: 00007fd71a1ad640 CR3: 00000005cf241006 CR4: 0000000000060ee0
> [415478.602883] Call Trace:
> [415478.603598]  <TASK>
> [415478.604229]  dax_fault_iter+0x240/0x600
> [415478.605410]  dax_iomap_pte_fault+0x19c/0x3d0
> [415478.606706]  __xfs_filemap_fault+0x1dd/0x2b0
> [415478.607744]  __do_fault+0x2e/0x1d0
> [415478.608587]  __handle_mm_fault+0xcec/0x17b0
> [415478.609593]  handle_mm_fault+0xd0/0x2a0
> [415478.610517]  exc_page_fault+0x1d9/0x810
> [415478.611398]  asm_exc_page_fault+0x22/0x30
> [415478.612311] RIP: 0033:0x7fd71a04b9ba
> [415478.613168] Code: 4d 29 c1 4c 29 c2 48 3b 15 db 95 11 00 0f 87 af 00 00 00 0f 10 01 0f 10 49 f0 0f 10 51 e0 0f 10 59 d0 48 83 e9 40 48 83 ea 40 <41> 0f 29 01 41 0f 29 49 f0 41 0f 29 51 e0 41 0f 29 59 d0 49 83 e9
> [415478.617083] RSP: 002b:00007ffcf277be18 EFLAGS: 00010206
> [415478.618213] RAX: 00007fd71a1a3fc5 RBX: 0000000000000fc5 RCX: 00007fd719f5a610
> [415478.619854] RDX: 000000000000964b RSI: 00007fd719f50fd5 RDI: 00007fd71a1a3fc5
> [415478.621286] RBP: 0000000000030fc5 R08: 000000000000000e R09: 00007fd71a1ad640
> [415478.622730] R10: 0000000000000001 R11: 00007fd71a1ad64e R12: 0000000000009699
> [415478.624164] R13: 000000000000a65e R14: 00007fd71a1a3000 R15: 0000000000000001
> [415478.625600]  </TASK>
> [415478.626087] ---[ end trace 0000000000000000 ]---
> 
> Even generic/247 is generating a warning like this from xfs_io,
> which is a mmap vs DIO racer. Given that DIO doesn't exist for
> fsdax, this test turns into just a normal write() vs mmap() racer.
> 
> Given these are the same fsdax infrastructure failures that I
> reported for 6.0, it is also likely that ext4 is still throwing
> them. IOWs, whatever got broke in the 6.0 cycle wasn't fixed in the
> 6.1 cycle.
> 
> > Frankly at this point I'm tempted just to turn of fsdax support for XFS
> > for the 6.1 LTS because I don't have time to fix it.
> 
> /me shrugs
> 
> Backporting fixes (whenever they come along) is a problem for the
> LTS kernel maintainer to deal with, not the upstream maintainer.
> 
> IMO, the issue right now is that the DAX maintainers seem to have
> little interest in ensuring that the FSDAX infrastructure actually
> works correctly. If anything, they seem to want to make things
> harder for block based filesystems to use pmem devices and hence
> FSDAX. e.g. the direction of the DAX core away from block interfaces
> that filesystems need for their userspace tools to manage the
> storage.
> 
> At what point do we simply say "the experiment failed, FSDAX is
> dead" and remove it from XFS altogether?

A fair question, given the regressions made it all the way into
v6.0-final. In retrospect I made the wrong priority call to focus on dax
page reference counting these past weeks.

When I fired up the dax unit tests on v6.0-rc1 I found basic problems
with the notify failure patches that concerned me that they had never
been tested after the final version was merged [1]. Then the rest of the
development cycle was spent fixing dax reference counting [2]. That was
a longstanding wishlist item from gup and folio developers, but, as I
said, that seems the wrong priority given the lingering regressions. I
will take a look the current dax-xfstests regression backlog. That may
find a need to consider reverting the problematic commits depending on
what is still broken if the fixes are trending towards being invasive.

[1]: https://lore.kernel.org/all/166153426798.2758201.15108211981034512993.stgit@dwillia2-xfh.jf.intel.com/

[2]: https://lore.kernel.org/all/166579181584.2236710.17813547487183983273.stgit@dwillia2-xfh.jf.intel.com/