On Wed, Aug 03, 2022 at 06:47:24AM +0000, ruansy.fnst@xxxxxxxxxxx wrote: > > > 在 2022/7/29 12:54, Darrick J. Wong 写道: > > On Fri, Jul 29, 2022 at 03:55:24AM +0000, ruansy.fnst@xxxxxxxxxxx wrote: > >> > >> > >> 在 2022/7/22 0:16, Darrick J. Wong 写道: > >>> On Thu, Jul 21, 2022 at 02:06:10PM +0000, ruansy.fnst@xxxxxxxxxxx wrote: > >>>> 在 2022/7/1 8:31, Darrick J. Wong 写道: > >>>>> On Thu, Jun 09, 2022 at 10:34:35PM +0800, Shiyang Ruan wrote: > >>>>>> Failure notification is not supported on partitions. So, when we mount > >>>>>> a reflink enabled xfs on a partition with dax option, let it fail with > >>>>>> -EINVAL code. > >>>>>> > >>>>>> Signed-off-by: Shiyang Ruan <ruansy.fnst@xxxxxxxxxxx> > >>>>> > >>>>> Looks good to me, though I think this patch applies to ... wherever all > >>>>> those rmap+reflink+dax patches went. I think that's akpm's tree, right? > >>>>> > >>>>> Ideally this would go in through there to keep the pieces together, but > >>>>> I don't mind tossing this in at the end of the 5.20 merge window if akpm > >>>>> is unwilling. > >>>> > >>>> BTW, since these patches (dax&reflink&rmap + THIS + pmem-unbind) are > >>>> waiting to be merged, is it time to think about "removing the > >>>> experimental tag" again? :) > >>> > >>> It's probably time to take up that question again. > >>> > >>> Yesterday I tried running generic/470 (aka the MAP_SYNC test) and it > >>> didn't succeed because it sets up dmlogwrites atop dmthinp atop pmem, > >>> and at least one of those dm layers no longer allows fsdax pass-through, > >>> so XFS silently turned mount -o dax into -o dax=never. :( > >> > >> Hi Darrick, > >> > >> I tried generic/470 but it didn't run: > >> [not run] Cannot use thin-pool devices on DAX capable block devices. > >> > >> Did you modify the _require_dm_target() in common/rc? I added thin-pool > >> to not to check dax capability: > >> > >> case $target in > >> stripe|linear|log-writes|thin-pool) # add thin-pool here > >> ;; > >> > >> then the case finally ran and it silently turned off dax as you said. > >> > >> Are the steps for reproduction correct? If so, I will continue to > >> investigate this problem. > > > > Ah, yes, I did add thin-pool to that case statement. Sorry I forgot to > > mention that. I suspect that the removal of dm support for pmem is > > going to force us to completely redesign this test. I can't really > > think of how, though, since there's no good way that I know of to gain a > > point-in-time snapshot of a pmem device. > > Hi Darrick, > > > removal of dm support for pmem > I think here we are saying about xfstest who removed the support, not > kernel? > > I found some xfstests commits: > fc7b3903894a6213c765d64df91847f4460336a2 # common/rc: add the restriction. > fc5870da485aec0f9196a0f2bed32f73f6b2c664 # generic/470: use thin-pool > > So, this case was never able to run since the second commit? (I didn't > notice the not run case. I thought it was expected to be not run.) > > And according to the first commit, the restriction was added because > some of dm devices don't support dax. So my understanding is: we should > redesign the case to make the it work, and firstly, we should add dax > support for dm devices in kernel. dm devices used to have fsdax support; I think Christoph is actively removing (or already has removed) all that support. > In addition, is there any other testcase has the same problem? so that > we can deal with them together. The last I checked, there aren't any that require MAP_SYNC or pmem aside from g/470 and the three poison notification tests that you sent a few days ago. --D > > -- > Thanks, > Ruan > > > > > > --D > > > >> > >> -- > >> Thanks, > >> Ruan. > >> > >> > >> > >>> > >>> I'm not sure how to fix that... > >>> > >>> --D > >>> > >>>> > >>>> -- > >>>> Thanks, > >>>> Ruan. > >>>> > >>>>> > >>>>> Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx> > >>>>> > >>>>> --D > >>>>> > >>>>>> --- > >>>>>> fs/xfs/xfs_super.c | 6 ++++-- > >>>>>> 1 file changed, 4 insertions(+), 2 deletions(-) > >>>>>> > >>>>>> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > >>>>>> index 8495ef076ffc..a3c221841fa6 100644 > >>>>>> --- a/fs/xfs/xfs_super.c > >>>>>> +++ b/fs/xfs/xfs_super.c > >>>>>> @@ -348,8 +348,10 @@ xfs_setup_dax_always( > >>>>>> goto disable_dax; > >>>>>> } > >>>>>> > >>>>>> - if (xfs_has_reflink(mp)) { > >>>>>> - xfs_alert(mp, "DAX and reflink cannot be used together!"); > >>>>>> + if (xfs_has_reflink(mp) && > >>>>>> + bdev_is_partition(mp->m_ddev_targp->bt_bdev)) { > >>>>>> + xfs_alert(mp, > >>>>>> + "DAX and reflink cannot work with multi-partitions!"); > >>>>>> return -EINVAL; > >>>>>> } > >>>>>> > >>>>>> -- > >>>>>> 2.36.1 > >>>>>> > >>>>>> > >>>>>>