Re: [PATCH] xfs: fail dax mount if reflink is enabled on a partition

Yang, Xiao/杨 晓
2022-09-15T10:14:55Z

On 2022/9/15 0:28, Darrick J. Wong wrote:
> On Wed, Sep 14, 2022 at 08:34:26AM -0400, Brian Foster wrote:
>> On Wed, Sep 14, 2022 at 05:38:02PM +0800, Yang, Xiao/杨 晓 wrote:
>>> On 2022/9/14 14:44, Yang, Xiao/杨 晓 wrote:
>>>> On 2022/9/9 21:01, Brian Foster wrote:
>>>>> Yes.. I don't recall all the internals of the tools and test, but IIRC
>>>>> it relied on discard to perform zeroing between checkpoints or some such
>>>>> and avoid spurious failures. The purpose of running on dm-thin was
>>>>> merely to provide reliable discard zeroing behavior on the target device
>>>>> and thus to allow the test to run reliably.
>>>> Hi Brian,
>>>>
>>>> As far as I know, generic/470 was original designed to verify
>>>> mmap(MAP_SYNC) on the dm-log-writes device enabling DAX. Due to the
>>>> reason, we need to ensure that all underlying devices under
>>>> dm-log-writes device support DAX. However dm-thin device never supports
>>>> DAX so
>>>> running generic/470 with dm-thin device always returns "not run".
>>>>
>>>> Please see the difference between old and new logic:
>>>>
>>>> old logic new logic
>>>> ---------------------------------------------------------------
>>>> log-writes device(DAX) log-writes device(DAX)
>>>> | |
>>>> PMEM0(DAX) + PMEM1(DAX) Thin device(non-DAX) + PMEM1(DAX)
>>>> |
>>>> PMEM0(DAX)
>>>> ---------------------------------------------------------------
>>>>
>>>> We think dm-thin device is not a good solution for generic/470, is there
>>>> any other solution to support both discard zero and DAX?
>>>
>>> Hi Brian,
>>>
>>> I have sent a patch[1] to revert your fix because I think it's not good for
>>> generic/470 to use thin volume as my revert patch[1] describes:
>>> [1] https://lore.kernel.org/fstests/20220914090625.32207-1-yangx.jy@fujitsu.com/T/#u
>>>
>>
>> I think the history here is that generic/482 was changed over first in
>> commit 65cc9a235919 ("generic/482: use thin volume as data device"), and
>> then sometime later we realized generic/455,457,470 had the same general
>> flaw and were switched over. The dm/dax compatibility thing was probably
>> just an oversight, but I am a little curious about that because it should
>
> It's not an oversight -- it used to work (albeit with EXPERIMENTAL
> tags), and now we've broken it on fsdax as the pmem/blockdev divorce
> progresses.
Hi

Do you mean that the following patch set changed the test result of
generic/470 with thin-volume? (pass => not run/failure)
https://lore.kernel.org/linux-xfs/20211129102203.2243509-1-hch@lst.de/

>
>> have been obvious that the change caused the test to no longer run. Did
>> something change after that to trigger that change in behavior?
>>
>>> With the revert, generic/470 can always run successfully on my environment
>>> so I wonder how to reproduce the out-of-order replay issue on XFS v5
>>> filesystem?
>>>
>>
>> I don't quite recall the characteristics of the failures beyond that we
>> were seeing spurious test failures with generic/482 that were due to
>> essentially putting the fs/log back in time in a way that wasn't quite
>> accurate due to the clearing by the logwrites tool not taking place. If
>> you wanted to reproduce in order to revisit that, perhaps start with
>> generic/482 and let it run in a loop for a while and see if it
>> eventually triggers a failure/corruption..?
>>
>>> PS: I want to reproduce the issue and try to find a better solution to fix
>>> it.
>>>
>>
>> It's been a while since I looked at any of this tooling to semi-grok how
>> it works.
>
> I /think/ this was the crux of the problem, back in 2019?
> https://lore.kernel.org/fstests/20190227061529.GF16436@dastard/

Agreed.

>
>> Perhaps it could learn to rely on something more explicit like
>> zero range (instead of discard?) or fall back to manual zeroing?
>
> AFAICT src/log-writes/ actually /can/ do zeroing, but (a) it probably
> ought to be adapted to call BLKZEROOUT and (b) in the worst case it
> writes zeroes to the entire device, which is/can be slow.
>
> For a (crass) example, one of my cloudy test VMs uses 34GB partitions,
> and for cost optimization purposes we're only "paying" for the cheapest
> tier. Weirdly that maps to an upper limit of 6500 write iops and
> 48MB/s(!) but that would take about 20 minutes to zero the entire
> device if the dm-thin hack wasn't in place. Frustratingly, it doesn't
> support discard or write-zeroes.

Do you mean that discard zero(BLKDISCARD) is faster than both fill
zero(BLKZEROOUT) and write zero on user space?

Best Regards,
Xiao Yang
>
>> If the
>> eventual solution is simple and low enough overhead, it might make some
>> sense to replace the dmthin hack across the set of tests mentioned
>> above.
>
> That said, for a *pmem* test you'd expect it to be faster than that...
>
> --D
>
>> Brian
>>
>>> Best Regards,
>>> Xiao Yang
>>>
>>>>
>>>> BTW, only log-writes, stripe and linear support DAX for now.
>>>
>>