2022-08-26 13:16:01

by Shiyang Ruan

[permalink] [raw]
Subject: [PATCH v7] mm, pmem, xfs: Introduce MF_MEM_REMOVE for unbind

This patch is inspired by Dan's "mm, dax, pmem: Introduce
dev_pagemap_failure()"[1]. With the help of dax_holder and
->notify_failure() mechanism, the pmem driver is able to ask filesystem
(or mapped device) on it to unmap all files in use and notify processes
who are using those files.

Call trace:
trigger unbind
-> unbind_store()
-> ... (skip)
-> devres_release_all()
-> kill_dax()
-> dax_holder_notify_failure(dax_dev, 0, U64_MAX, MF_MEM_PRE_REMOVE)
-> xfs_dax_notify_failure()

Introduce MF_MEM_PRE_REMOVE to let filesystem know this is a remove
event. So do not shutdown filesystem directly if something not
supported, or if failure range includes metadata area. Make sure all
files and processes are handled correctly.

==
Changes since v6:
1. Rebase on 6.0-rc2 and Darrick's patch[2].

Changes since v5:
1. Renamed MF_MEM_REMOVE to MF_MEM_PRE_REMOVE
2. hold s_umount before sync_filesystem()
3. do sync_filesystem() after SB_BORN check
4. Rebased on next-20220714

[1]:
https://lore.kernel.org/linux-mm/161604050314.1463742.14151665140035795571.stgit@dwillia2-desk3.amr.corp.intel.com/
[2]: https://lore.kernel.org/linux-xfs/Yv5wIa2crHioYeRr@magnolia/

Signed-off-by: Shiyang Ruan <[email protected]>
Reviewed-by: Darrick J. Wong <[email protected]>
---
drivers/dax/super.c | 3 ++-
fs/xfs/xfs_notify_failure.c | 15 +++++++++++++++
include/linux/mm.h | 1 +
3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index 9b5e2a5eb0ae..cf9a64563fbe 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -323,7 +323,8 @@ void kill_dax(struct dax_device *dax_dev)
return;
if (dax_dev->holder_data != NULL)
- dax_holder_notify_failure(dax_dev, 0, U64_MAX, 0);
+ dax_holder_notify_failure(dax_dev, 0, U64_MAX,
+ MF_MEM_PRE_REMOVE);
clear_bit(DAXDEV_ALIVE, &dax_dev->flags);
synchronize_srcu(&dax_srcu);
diff --git a/fs/xfs/xfs_notify_failure.c b/fs/xfs/xfs_notify_failure.c
index 65d5eb20878e..a9769f17e998 100644
--- a/fs/xfs/xfs_notify_failure.c
+++ b/fs/xfs/xfs_notify_failure.c
@@ -77,6 +77,9 @@ xfs_dax_failure_fn(
if (XFS_RMAP_NON_INODE_OWNER(rec->rm_owner) ||
(rec->rm_flags & (XFS_RMAP_ATTR_FORK | XFS_RMAP_BMBT_BLOCK))) {
+ /* Do not shutdown so early when device is to be removed */
+ if (notify->mf_flags & MF_MEM_PRE_REMOVE)
+ return 0;
notify->want_shutdown = true;
return 0;
}
@@ -182,12 +185,22 @@ xfs_dax_notify_failure(
struct xfs_mount *mp = dax_holder(dax_dev);
u64 ddev_start;
u64 ddev_end;
+ int error;
if (!(mp->m_sb.sb_flags & SB_BORN)) {
xfs_warn(mp, "filesystem is not ready for notify_failure()!");
return -EIO;
}
+ if (mf_flags & MF_MEM_PRE_REMOVE) {
+ xfs_info(mp, "device is about to be removed!");
+ down_write(&mp->m_super->s_umount);
+ error = sync_filesystem(mp->m_super);
+ up_write(&mp->m_super->s_umount);
+ if (error)
+ return error;
+ }
+
if (mp->m_rtdev_targp && mp->m_rtdev_targp->bt_daxdev == dax_dev) {
xfs_warn(mp,
"notify_failure() not supported on realtime device!");
@@ -196,6 +209,8 @@ xfs_dax_notify_failure(
if (mp->m_logdev_targp && mp->m_logdev_targp->bt_daxdev == dax_dev &&
mp->m_logdev_targp != mp->m_ddev_targp) {
+ if (mf_flags & MF_MEM_PRE_REMOVE)
+ return 0;
xfs_err(mp, "ondisk log corrupt, shutting down fs!");
xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_ONDISK);
return -EFSCORRUPTED;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 982f2607180b..2c7c132e6512 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -3176,6 +3176,7 @@ enum mf_flags {
MF_UNPOISON = 1 << 4,
MF_SW_SIMULATED = 1 << 5,
MF_NO_RETRY = 1 << 6,
+ MF_MEM_PRE_REMOVE = 1 << 7,
};
int mf_dax_kill_procs(struct address_space *mapping, pgoff_t index,
unsigned long count, int mf_flags);
--
2.37.2


2022-08-26 21:48:11

by Dan Williams

[permalink] [raw]
Subject: RE: [PATCH v7] mm, pmem, xfs: Introduce MF_MEM_REMOVE for unbind

Shiyang Ruan wrote:
> This patch is inspired by Dan's "mm, dax, pmem: Introduce
> dev_pagemap_failure()"[1]. With the help of dax_holder and
> ->notify_failure() mechanism, the pmem driver is able to ask filesystem
> (or mapped device) on it to unmap all files in use and notify processes
> who are using those files.
>
> Call trace:
> trigger unbind
> -> unbind_store()
> -> ... (skip)
> -> devres_release_all()
> -> kill_dax()
> -> dax_holder_notify_failure(dax_dev, 0, U64_MAX, MF_MEM_PRE_REMOVE)
> -> xfs_dax_notify_failure()
>
> Introduce MF_MEM_PRE_REMOVE to let filesystem know this is a remove
> event. So do not shutdown filesystem directly if something not
> supported, or if failure range includes metadata area. Make sure all
> files and processes are handled correctly.
>
> ==
> Changes since v6:
> 1. Rebase on 6.0-rc2 and Darrick's patch[2].
>
> Changes since v5:
> 1. Renamed MF_MEM_REMOVE to MF_MEM_PRE_REMOVE
> 2. hold s_umount before sync_filesystem()
> 3. do sync_filesystem() after SB_BORN check
> 4. Rebased on next-20220714
>
> [1]:
> https://lore.kernel.org/linux-mm/161604050314.1463742.14151665140035795571.stgit@dwillia2-desk3.amr.corp.intel.com/
> [2]: https://lore.kernel.org/linux-xfs/Yv5wIa2crHioYeRr@magnolia/
>
> Signed-off-by: Shiyang Ruan <[email protected]>
> Reviewed-by: Darrick J. Wong <[email protected]>
> ---
> drivers/dax/super.c | 3 ++-
> fs/xfs/xfs_notify_failure.c | 15 +++++++++++++++
> include/linux/mm.h | 1 +
> 3 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dax/super.c b/drivers/dax/super.c
> index 9b5e2a5eb0ae..cf9a64563fbe 100644
> --- a/drivers/dax/super.c
> +++ b/drivers/dax/super.c
> @@ -323,7 +323,8 @@ void kill_dax(struct dax_device *dax_dev)
> return;
> if (dax_dev->holder_data != NULL)
> - dax_holder_notify_failure(dax_dev, 0, U64_MAX, 0);
> + dax_holder_notify_failure(dax_dev, 0, U64_MAX,
> + MF_MEM_PRE_REMOVE);
> clear_bit(DAXDEV_ALIVE, &dax_dev->flags);
> synchronize_srcu(&dax_srcu);
> diff --git a/fs/xfs/xfs_notify_failure.c b/fs/xfs/xfs_notify_failure.c
> index 65d5eb20878e..a9769f17e998 100644
> --- a/fs/xfs/xfs_notify_failure.c
> +++ b/fs/xfs/xfs_notify_failure.c
> @@ -77,6 +77,9 @@ xfs_dax_failure_fn(
> if (XFS_RMAP_NON_INODE_OWNER(rec->rm_owner) ||
> (rec->rm_flags & (XFS_RMAP_ATTR_FORK | XFS_RMAP_BMBT_BLOCK))) {
> + /* Do not shutdown so early when device is to be removed */
> + if (notify->mf_flags & MF_MEM_PRE_REMOVE)
> + return 0;
> notify->want_shutdown = true;
> return 0;
> }
> @@ -182,12 +185,22 @@ xfs_dax_notify_failure(
> struct xfs_mount *mp = dax_holder(dax_dev);
> u64 ddev_start;
> u64 ddev_end;
> + int error;
> if (!(mp->m_sb.sb_flags & SB_BORN)) {

How are you testing the SB_BORN interactions? I have a fix for this
pending here:

https://lore.kernel.org/nvdimm/166153428094.2758201.7936572520826540019.stgit@dwillia2-xfh.jf.intel.com/

> xfs_warn(mp, "filesystem is not ready for notify_failure()!");
> return -EIO;
> }
> + if (mf_flags & MF_MEM_PRE_REMOVE) {

It appears this patch is corrupted here. I confirmed that b4 sees the
same when trying to apply it.

> + xfs_info(mp, "device is about to be removed!");
> + down_write(&mp->m_super->s_umount);
> + error = sync_filesystem(mp->m_super);

This syncs to make data persistent, but for DAX this also needs to get
invalidate all current DAX mappings. I do not see that in these changes.

> + up_write(&mp->m_super->s_umount);
> + if (error)
> + return error;
> + }
> +
> if (mp->m_rtdev_targp && mp->m_rtdev_targp->bt_daxdev == dax_dev) {
> xfs_warn(mp,
> "notify_failure() not supported on realtime device!");
> @@ -196,6 +209,8 @@ xfs_dax_notify_failure(
> if (mp->m_logdev_targp && mp->m_logdev_targp->bt_daxdev == dax_dev &&
> mp->m_logdev_targp != mp->m_ddev_targp) {
> + if (mf_flags & MF_MEM_PRE_REMOVE)
> + return 0;
> xfs_err(mp, "ondisk log corrupt, shutting down fs!");
> xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_ONDISK);
> return -EFSCORRUPTED;
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 982f2607180b..2c7c132e6512 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -3176,6 +3176,7 @@ enum mf_flags {
> MF_UNPOISON = 1 << 4,
> MF_SW_SIMULATED = 1 << 5,
> MF_NO_RETRY = 1 << 6,
> + MF_MEM_PRE_REMOVE = 1 << 7,
> };
> int mf_dax_kill_procs(struct address_space *mapping, pgoff_t index,
> unsigned long count, int mf_flags);
> --
> 2.37.2
>
>


2022-08-29 10:24:15

by Shiyang Ruan

[permalink] [raw]
Subject: Re: [PATCH v7] mm, pmem, xfs: Introduce MF_MEM_REMOVE for unbind



在 2022/8/27 5:35, Dan Williams 写道:
> Shiyang Ruan wrote:
>> This patch is inspired by Dan's "mm, dax, pmem: Introduce
>> dev_pagemap_failure()"[1]. With the help of dax_holder and
>> ->notify_failure() mechanism, the pmem driver is able to ask filesystem
>> (or mapped device) on it to unmap all files in use and notify processes
>> who are using those files.
>>
>> Call trace:
>> trigger unbind
>> -> unbind_store()
>> -> ... (skip)
>> -> devres_release_all()
>> -> kill_dax()
>> -> dax_holder_notify_failure(dax_dev, 0, U64_MAX, MF_MEM_PRE_REMOVE)
>> -> xfs_dax_notify_failure()
>>
>> Introduce MF_MEM_PRE_REMOVE to let filesystem know this is a remove
>> event. So do not shutdown filesystem directly if something not
>> supported, or if failure range includes metadata area. Make sure all
>> files and processes are handled correctly.
>>
>> ==
>> Changes since v6:
>> 1. Rebase on 6.0-rc2 and Darrick's patch[2].
>>
>> Changes since v5:
>> 1. Renamed MF_MEM_REMOVE to MF_MEM_PRE_REMOVE
>> 2. hold s_umount before sync_filesystem()
>> 3. do sync_filesystem() after SB_BORN check
>> 4. Rebased on next-20220714
>>
>> [1]:
>> https://lore.kernel.org/linux-mm/161604050314.1463742.14151665140035795571.stgit@dwillia2-desk3.amr.corp.intel.com/
>> [2]: https://lore.kernel.org/linux-xfs/Yv5wIa2crHioYeRr@magnolia/
>>
>> Signed-off-by: Shiyang Ruan <[email protected]>
>> Reviewed-by: Darrick J. Wong <[email protected]>
>> ---
>> drivers/dax/super.c | 3 ++-
>> fs/xfs/xfs_notify_failure.c | 15 +++++++++++++++
>> include/linux/mm.h | 1 +
>> 3 files changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dax/super.c b/drivers/dax/super.c
>> index 9b5e2a5eb0ae..cf9a64563fbe 100644
>> --- a/drivers/dax/super.c
>> +++ b/drivers/dax/super.c
>> @@ -323,7 +323,8 @@ void kill_dax(struct dax_device *dax_dev)
>> return;
>> if (dax_dev->holder_data != NULL)
>> - dax_holder_notify_failure(dax_dev, 0, U64_MAX, 0);
>> + dax_holder_notify_failure(dax_dev, 0, U64_MAX,
>> + MF_MEM_PRE_REMOVE);
>> clear_bit(DAXDEV_ALIVE, &dax_dev->flags);
>> synchronize_srcu(&dax_srcu);
>> diff --git a/fs/xfs/xfs_notify_failure.c b/fs/xfs/xfs_notify_failure.c
>> index 65d5eb20878e..a9769f17e998 100644
>> --- a/fs/xfs/xfs_notify_failure.c
>> +++ b/fs/xfs/xfs_notify_failure.c
>> @@ -77,6 +77,9 @@ xfs_dax_failure_fn(
>> if (XFS_RMAP_NON_INODE_OWNER(rec->rm_owner) ||
>> (rec->rm_flags & (XFS_RMAP_ATTR_FORK | XFS_RMAP_BMBT_BLOCK))) {
>> + /* Do not shutdown so early when device is to be removed */
>> + if (notify->mf_flags & MF_MEM_PRE_REMOVE)
>> + return 0;
>> notify->want_shutdown = true;
>> return 0;
>> }
>> @@ -182,12 +185,22 @@ xfs_dax_notify_failure(
>> struct xfs_mount *mp = dax_holder(dax_dev);
>> u64 ddev_start;
>> u64 ddev_end;
>> + int error;
>> if (!(mp->m_sb.sb_flags & SB_BORN)) {
>
> How are you testing the SB_BORN interactions? I have a fix for this
> pending here:
>
> https://lore.kernel.org/nvdimm/166153428094.2758201.7936572520826540019.stgit@dwillia2-xfh.jf.intel.com/

That was my mistake. Yes, it should be mp->m_super->s_flags.

(I remember my testcase did pass in my dev version, but now that seems
impossible. I think something was wrong when I did the test.)

>
>> xfs_warn(mp, "filesystem is not ready for notify_failure()!");
>> return -EIO;
>> }
>> + if (mf_flags & MF_MEM_PRE_REMOVE) {
>
> It appears this patch is corrupted here. I confirmed that b4 sees the
> same when trying to apply it.

Can't this patch be applied? It is based on 6.0-rc2 + Darrick's patch.
It's also ok to rebase on 6.0-rc3 + Darrick's patch.

>
>> + xfs_info(mp, "device is about to be removed!");
>> + down_write(&mp->m_super->s_umount);
>> + error = sync_filesystem(mp->m_super);
>
> This syncs to make data persistent, but for DAX this also needs to get
> invalidate all current DAX mappings. I do not see that in these changes.

I'll add it.


--
Thanks,
Ruan.

>
>> + up_write(&mp->m_super->s_umount);
>> + if (error)
>> + return error;
>> + }
>> +
>> if (mp->m_rtdev_targp && mp->m_rtdev_targp->bt_daxdev == dax_dev) {
>> xfs_warn(mp,
>> "notify_failure() not supported on realtime device!");
>> @@ -196,6 +209,8 @@ xfs_dax_notify_failure(
>> if (mp->m_logdev_targp && mp->m_logdev_targp->bt_daxdev == dax_dev &&
>> mp->m_logdev_targp != mp->m_ddev_targp) {
>> + if (mf_flags & MF_MEM_PRE_REMOVE)
>> + return 0;
>> xfs_err(mp, "ondisk log corrupt, shutting down fs!");
>> xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_ONDISK);
>> return -EFSCORRUPTED;
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index 982f2607180b..2c7c132e6512 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -3176,6 +3176,7 @@ enum mf_flags {
>> MF_UNPOISON = 1 << 4,
>> MF_SW_SIMULATED = 1 << 5,
>> MF_NO_RETRY = 1 << 6,
>> + MF_MEM_PRE_REMOVE = 1 << 7,
>> };
>> int mf_dax_kill_procs(struct address_space *mapping, pgoff_t index,
>> unsigned long count, int mf_flags);
>> --
>> 2.37.2
>>
>>
>
>

2022-08-29 15:35:56

by Darrick J. Wong

[permalink] [raw]
Subject: Re: [PATCH v7] mm, pmem, xfs: Introduce MF_MEM_REMOVE for unbind

On Mon, Aug 29, 2022 at 06:02:11PM +0800, Shiyang Ruan wrote:
>
>
> 在 2022/8/27 5:35, Dan Williams 写道:
> > Shiyang Ruan wrote:
> > > This patch is inspired by Dan's "mm, dax, pmem: Introduce
> > > dev_pagemap_failure()"[1]. With the help of dax_holder and
> > > ->notify_failure() mechanism, the pmem driver is able to ask filesystem
> > > (or mapped device) on it to unmap all files in use and notify processes
> > > who are using those files.
> > >
> > > Call trace:
> > > trigger unbind
> > > -> unbind_store()
> > > -> ... (skip)
> > > -> devres_release_all()
> > > -> kill_dax()
> > > -> dax_holder_notify_failure(dax_dev, 0, U64_MAX, MF_MEM_PRE_REMOVE)
> > > -> xfs_dax_notify_failure()
> > >
> > > Introduce MF_MEM_PRE_REMOVE to let filesystem know this is a remove
> > > event. So do not shutdown filesystem directly if something not
> > > supported, or if failure range includes metadata area. Make sure all
> > > files and processes are handled correctly.
> > >
> > > ==
> > > Changes since v6:
> > > 1. Rebase on 6.0-rc2 and Darrick's patch[2].
> > >
> > > Changes since v5:
> > > 1. Renamed MF_MEM_REMOVE to MF_MEM_PRE_REMOVE
> > > 2. hold s_umount before sync_filesystem()
> > > 3. do sync_filesystem() after SB_BORN check
> > > 4. Rebased on next-20220714
> > >
> > > [1]:
> > > https://lore.kernel.org/linux-mm/161604050314.1463742.14151665140035795571.stgit@dwillia2-desk3.amr.corp.intel.com/
> > > [2]: https://lore.kernel.org/linux-xfs/Yv5wIa2crHioYeRr@magnolia/
> > >
> > > Signed-off-by: Shiyang Ruan <[email protected]>
> > > Reviewed-by: Darrick J. Wong <[email protected]>
> > > ---
> > > drivers/dax/super.c | 3 ++-
> > > fs/xfs/xfs_notify_failure.c | 15 +++++++++++++++
> > > include/linux/mm.h | 1 +
> > > 3 files changed, 18 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/dax/super.c b/drivers/dax/super.c
> > > index 9b5e2a5eb0ae..cf9a64563fbe 100644
> > > --- a/drivers/dax/super.c
> > > +++ b/drivers/dax/super.c
> > > @@ -323,7 +323,8 @@ void kill_dax(struct dax_device *dax_dev)
> > > return;
> > > if (dax_dev->holder_data != NULL)
> > > - dax_holder_notify_failure(dax_dev, 0, U64_MAX, 0);
> > > + dax_holder_notify_failure(dax_dev, 0, U64_MAX,
> > > + MF_MEM_PRE_REMOVE);
> > > clear_bit(DAXDEV_ALIVE, &dax_dev->flags);
> > > synchronize_srcu(&dax_srcu);
> > > diff --git a/fs/xfs/xfs_notify_failure.c b/fs/xfs/xfs_notify_failure.c
> > > index 65d5eb20878e..a9769f17e998 100644
> > > --- a/fs/xfs/xfs_notify_failure.c
> > > +++ b/fs/xfs/xfs_notify_failure.c
> > > @@ -77,6 +77,9 @@ xfs_dax_failure_fn(
> > > if (XFS_RMAP_NON_INODE_OWNER(rec->rm_owner) ||
> > > (rec->rm_flags & (XFS_RMAP_ATTR_FORK | XFS_RMAP_BMBT_BLOCK))) {
> > > + /* Do not shutdown so early when device is to be removed */
> > > + if (notify->mf_flags & MF_MEM_PRE_REMOVE)
> > > + return 0;
> > > notify->want_shutdown = true;
> > > return 0;
> > > }
> > > @@ -182,12 +185,22 @@ xfs_dax_notify_failure(
> > > struct xfs_mount *mp = dax_holder(dax_dev);
> > > u64 ddev_start;
> > > u64 ddev_end;
> > > + int error;
> > > if (!(mp->m_sb.sb_flags & SB_BORN)) {
> >
> > How are you testing the SB_BORN interactions? I have a fix for this
> > pending here:
> >
> > https://lore.kernel.org/nvdimm/166153428094.2758201.7936572520826540019.stgit@dwillia2-xfh.jf.intel.com/
>
> That was my mistake. Yes, it should be mp->m_super->s_flags.
>
> (I remember my testcase did pass in my dev version, but now that seems
> impossible. I think something was wrong when I did the test.)
>
> >
> > > xfs_warn(mp, "filesystem is not ready for notify_failure()!");
> > > return -EIO;
> > > }
> > > + if (mf_flags & MF_MEM_PRE_REMOVE) {
> >
> > It appears this patch is corrupted here. I confirmed that b4 sees the
> > same when trying to apply it.
>
> Can't this patch be applied? It is based on 6.0-rc2 + Darrick's patch. It's
> also ok to rebase on 6.0-rc3 + Darrick's patch.
>
> >
> > > + xfs_info(mp, "device is about to be removed!");
> > > + down_write(&mp->m_super->s_umount);
> > > + error = sync_filesystem(mp->m_super);
> >
> > This syncs to make data persistent, but for DAX this also needs to get
> > invalidate all current DAX mappings. I do not see that in these changes.
>
> I'll add it.

Are you guys going to pick up [1]?

--D

[1] https://lore.kernel.org/linux-xfs/Yv5wIa2crHioYeRr@magnolia/

>
> --
> Thanks,
> Ruan.
>
> >
> > > + up_write(&mp->m_super->s_umount);
> > > + if (error)
> > > + return error;
> > > + }
> > > +
> > > if (mp->m_rtdev_targp && mp->m_rtdev_targp->bt_daxdev == dax_dev) {
> > > xfs_warn(mp,
> > > "notify_failure() not supported on realtime device!");
> > > @@ -196,6 +209,8 @@ xfs_dax_notify_failure(
> > > if (mp->m_logdev_targp && mp->m_logdev_targp->bt_daxdev == dax_dev &&
> > > mp->m_logdev_targp != mp->m_ddev_targp) {
> > > + if (mf_flags & MF_MEM_PRE_REMOVE)
> > > + return 0;
> > > xfs_err(mp, "ondisk log corrupt, shutting down fs!");
> > > xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_ONDISK);
> > > return -EFSCORRUPTED;
> > > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > > index 982f2607180b..2c7c132e6512 100644
> > > --- a/include/linux/mm.h
> > > +++ b/include/linux/mm.h
> > > @@ -3176,6 +3176,7 @@ enum mf_flags {
> > > MF_UNPOISON = 1 << 4,
> > > MF_SW_SIMULATED = 1 << 5,
> > > MF_NO_RETRY = 1 << 6,
> > > + MF_MEM_PRE_REMOVE = 1 << 7,
> > > };
> > > int mf_dax_kill_procs(struct address_space *mapping, pgoff_t index,
> > > unsigned long count, int mf_flags);
> > > --
> > > 2.37.2
> > >
> > >
> >
> >

2022-09-02 10:51:24

by Shiyang Ruan

[permalink] [raw]
Subject: [PATCH v8 0/3] mm, pmem, xfs: Introduce MF_MEM_REMOVE for unbind

Changes since v7:
1. Add P1 to fix calculation mistake
2. Add P2 to move drop_pagecache_sb() to super.c for xfs to use
3. P3: Add invalidate all mappings after sync.
4. P3: Set offset&len to be start&length of device when it is to be removed.
5. Rebase on 6.0-rc3 + Darrick's patch[1] + Dan's patch[2].

Changes since v6:
1. Rebase on 6.0-rc2 and Darrick's patch[1].

[1]: https://lore.kernel.org/linux-xfs/Yv5wIa2crHioYeRr@magnolia/
[2]: https://lore.kernel.org/linux-xfs/166153426798.2758201.15108211981034512993.stgit@dwillia2-xfh.jf.intel.com/

Shiyang Ruan (3):
xfs: fix the calculation of length and end
fs: move drop_pagecache_sb() for others to use
mm, pmem, xfs: Introduce MF_MEM_REMOVE for unbind

drivers/dax/super.c | 3 ++-
fs/drop_caches.c | 33 ---------------------------------
fs/super.c | 34 ++++++++++++++++++++++++++++++++++
fs/xfs/xfs_notify_failure.c | 31 +++++++++++++++++++++++++++----
include/linux/fs.h | 1 +
include/linux/mm.h | 1 +
6 files changed, 65 insertions(+), 38 deletions(-)

--
2.37.2

2022-09-02 11:13:37

by Shiyang Ruan

[permalink] [raw]
Subject: [PATCH 1/3] xfs: fix the calculation of length and end

The end should be start + length - 1. Also fix the calculation of the
length when seeking for intersection of notify range and device.

Signed-off-by: Shiyang Ruan <[email protected]>
---
fs/xfs/xfs_notify_failure.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_notify_failure.c b/fs/xfs/xfs_notify_failure.c
index c4078d0ec108..3830f908e215 100644
--- a/fs/xfs/xfs_notify_failure.c
+++ b/fs/xfs/xfs_notify_failure.c
@@ -114,7 +114,7 @@ xfs_dax_notify_ddev_failure(
int error = 0;
xfs_fsblock_t fsbno = XFS_DADDR_TO_FSB(mp, daddr);
xfs_agnumber_t agno = XFS_FSB_TO_AGNO(mp, fsbno);
- xfs_fsblock_t end_fsbno = XFS_DADDR_TO_FSB(mp, daddr + bblen);
+ xfs_fsblock_t end_fsbno = XFS_DADDR_TO_FSB(mp, daddr + bblen - 1);
xfs_agnumber_t end_agno = XFS_FSB_TO_AGNO(mp, end_fsbno);

error = xfs_trans_alloc_empty(mp, &tp);
@@ -210,7 +210,7 @@ xfs_dax_notify_failure(
ddev_end = ddev_start + bdev_nr_bytes(mp->m_ddev_targp->bt_bdev) - 1;

/* Ignore the range out of filesystem area */
- if (offset + len < ddev_start)
+ if (offset + len - 1 < ddev_start)
return -ENXIO;
if (offset > ddev_end)
return -ENXIO;
@@ -222,8 +222,8 @@ xfs_dax_notify_failure(
len -= ddev_start - offset;
offset = 0;
}
- if (offset + len > ddev_end)
- len -= ddev_end - offset;
+ if (offset + len - 1 > ddev_end)
+ len -= offset + len - 1 - ddev_end;

return xfs_dax_notify_ddev_failure(mp, BTOBB(offset), BTOBB(len),
mf_flags);
--
2.37.2

2022-09-07 10:07:13

by Shiyang Ruan

[permalink] [raw]
Subject: Re: [PATCH v8 0/3] mm, pmem, xfs: Introduce MF_MEM_REMOVE for unbind

ping

在 2022/9/2 18:35, Shiyang Ruan 写道:
> Changes since v7:
> 1. Add P1 to fix calculation mistake
> 2. Add P2 to move drop_pagecache_sb() to super.c for xfs to use
> 3. P3: Add invalidate all mappings after sync.
> 4. P3: Set offset&len to be start&length of device when it is to be removed.
> 5. Rebase on 6.0-rc3 + Darrick's patch[1] + Dan's patch[2].
>
> Changes since v6:
> 1. Rebase on 6.0-rc2 and Darrick's patch[1].
>
> [1]: https://lore.kernel.org/linux-xfs/Yv5wIa2crHioYeRr@magnolia/
> [2]: https://lore.kernel.org/linux-xfs/166153426798.2758201.15108211981034512993.stgit@dwillia2-xfh.jf.intel.com/
>
> Shiyang Ruan (3):
> xfs: fix the calculation of length and end
> fs: move drop_pagecache_sb() for others to use
> mm, pmem, xfs: Introduce MF_MEM_REMOVE for unbind
>
> drivers/dax/super.c | 3 ++-
> fs/drop_caches.c | 33 ---------------------------------
> fs/super.c | 34 ++++++++++++++++++++++++++++++++++
> fs/xfs/xfs_notify_failure.c | 31 +++++++++++++++++++++++++++----
> include/linux/fs.h | 1 +
> include/linux/mm.h | 1 +
> 6 files changed, 65 insertions(+), 38 deletions(-)
>

2022-09-14 18:23:44

by Darrick J. Wong

[permalink] [raw]
Subject: Re: [PATCH 1/3] xfs: fix the calculation of length and end

On Fri, Sep 02, 2022 at 10:35:59AM +0000, Shiyang Ruan wrote:
> The end should be start + length - 1. Also fix the calculation of the
> length when seeking for intersection of notify range and device.
>
> Signed-off-by: Shiyang Ruan <[email protected]>

Looks correct to me,
Reviewed-by: Darrick J. Wong <[email protected]>

--D

> ---
> fs/xfs/xfs_notify_failure.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/xfs/xfs_notify_failure.c b/fs/xfs/xfs_notify_failure.c
> index c4078d0ec108..3830f908e215 100644
> --- a/fs/xfs/xfs_notify_failure.c
> +++ b/fs/xfs/xfs_notify_failure.c
> @@ -114,7 +114,7 @@ xfs_dax_notify_ddev_failure(
> int error = 0;
> xfs_fsblock_t fsbno = XFS_DADDR_TO_FSB(mp, daddr);
> xfs_agnumber_t agno = XFS_FSB_TO_AGNO(mp, fsbno);
> - xfs_fsblock_t end_fsbno = XFS_DADDR_TO_FSB(mp, daddr + bblen);
> + xfs_fsblock_t end_fsbno = XFS_DADDR_TO_FSB(mp, daddr + bblen - 1);
> xfs_agnumber_t end_agno = XFS_FSB_TO_AGNO(mp, end_fsbno);
>
> error = xfs_trans_alloc_empty(mp, &tp);
> @@ -210,7 +210,7 @@ xfs_dax_notify_failure(
> ddev_end = ddev_start + bdev_nr_bytes(mp->m_ddev_targp->bt_bdev) - 1;
>
> /* Ignore the range out of filesystem area */
> - if (offset + len < ddev_start)
> + if (offset + len - 1 < ddev_start)
> return -ENXIO;
> if (offset > ddev_end)
> return -ENXIO;
> @@ -222,8 +222,8 @@ xfs_dax_notify_failure(
> len -= ddev_start - offset;
> offset = 0;
> }
> - if (offset + len > ddev_end)
> - len -= ddev_end - offset;
> + if (offset + len - 1 > ddev_end)
> + len -= offset + len - 1 - ddev_end;
>
> return xfs_dax_notify_ddev_failure(mp, BTOBB(offset), BTOBB(len),
> mf_flags);
> --
> 2.37.2
>

2022-09-14 18:45:14

by Darrick J. Wong

[permalink] [raw]
Subject: Re: [PATCH v8 0/3] mm, pmem, xfs: Introduce MF_MEM_REMOVE for unbind

On Wed, Sep 07, 2022 at 05:46:00PM +0800, Shiyang Ruan wrote:
> ping
>
> 在 2022/9/2 18:35, Shiyang Ruan 写道:
> > Changes since v7:
> > 1. Add P1 to fix calculation mistake
> > 2. Add P2 to move drop_pagecache_sb() to super.c for xfs to use
> > 3. P3: Add invalidate all mappings after sync.
> > 4. P3: Set offset&len to be start&length of device when it is to be removed.
> > 5. Rebase on 6.0-rc3 + Darrick's patch[1] + Dan's patch[2].
> >
> > Changes since v6:
> > 1. Rebase on 6.0-rc2 and Darrick's patch[1].
> >
> > [1]: https://lore.kernel.org/linux-xfs/Yv5wIa2crHioYeRr@magnolia/
> > [2]: https://lore.kernel.org/linux-xfs/166153426798.2758201.15108211981034512993.stgit@dwillia2-xfh.jf.intel.com/

Just out of curiosity, is it your (or djbw's) intent to send all these
as bugfixes for 6.0 via akpm like all the other dax fixen?

--D

> >
> > Shiyang Ruan (3):
> > xfs: fix the calculation of length and end
> > fs: move drop_pagecache_sb() for others to use
> > mm, pmem, xfs: Introduce MF_MEM_REMOVE for unbind
> >
> > drivers/dax/super.c | 3 ++-
> > fs/drop_caches.c | 33 ---------------------------------
> > fs/super.c | 34 ++++++++++++++++++++++++++++++++++
> > fs/xfs/xfs_notify_failure.c | 31 +++++++++++++++++++++++++++----
> > include/linux/fs.h | 1 +
> > include/linux/mm.h | 1 +
> > 6 files changed, 65 insertions(+), 38 deletions(-)
> >

2022-09-14 19:13:11

by Darrick J. Wong

[permalink] [raw]
Subject: Re: [PATCH v8 0/3] mm, pmem, xfs: Introduce MF_MEM_REMOVE for unbind

On Wed, Sep 14, 2022 at 11:09:23AM -0700, Darrick J. Wong wrote:
> On Wed, Sep 07, 2022 at 05:46:00PM +0800, Shiyang Ruan wrote:
> > ping
> >
> > 在 2022/9/2 18:35, Shiyang Ruan 写道:
> > > Changes since v7:
> > > 1. Add P1 to fix calculation mistake
> > > 2. Add P2 to move drop_pagecache_sb() to super.c for xfs to use
> > > 3. P3: Add invalidate all mappings after sync.
> > > 4. P3: Set offset&len to be start&length of device when it is to be removed.
> > > 5. Rebase on 6.0-rc3 + Darrick's patch[1] + Dan's patch[2].
> > >
> > > Changes since v6:
> > > 1. Rebase on 6.0-rc2 and Darrick's patch[1].
> > >
> > > [1]: https://lore.kernel.org/linux-xfs/Yv5wIa2crHioYeRr@magnolia/
> > > [2]: https://lore.kernel.org/linux-xfs/166153426798.2758201.15108211981034512993.stgit@dwillia2-xfh.jf.intel.com/
>
> Just out of curiosity, is it your (or djbw's) intent to send all these
> as bugfixes for 6.0 via akpm like all the other dax fixen?

Aha, this is 6.1 stuff, please ignore this question.

--D

> --D
>
> > >
> > > Shiyang Ruan (3):
> > > xfs: fix the calculation of length and end
> > > fs: move drop_pagecache_sb() for others to use
> > > mm, pmem, xfs: Introduce MF_MEM_REMOVE for unbind
> > >
> > > drivers/dax/super.c | 3 ++-
> > > fs/drop_caches.c | 33 ---------------------------------
> > > fs/super.c | 34 ++++++++++++++++++++++++++++++++++
> > > fs/xfs/xfs_notify_failure.c | 31 +++++++++++++++++++++++++++----
> > > include/linux/fs.h | 1 +
> > > include/linux/mm.h | 1 +
> > > 6 files changed, 65 insertions(+), 38 deletions(-)
> > >

2022-09-15 03:12:34

by Shiyang Ruan

[permalink] [raw]
Subject: Re: [PATCH v8 0/3] mm, pmem, xfs: Introduce MF_MEM_REMOVE for unbind



在 2022/9/15 2:15, Darrick J. Wong 写道:
> On Wed, Sep 14, 2022 at 11:09:23AM -0700, Darrick J. Wong wrote:
>> On Wed, Sep 07, 2022 at 05:46:00PM +0800, Shiyang Ruan wrote:
>>> ping
>>>
>>> 在 2022/9/2 18:35, Shiyang Ruan 写道:
>>>> Changes since v7:
>>>> 1. Add P1 to fix calculation mistake
>>>> 2. Add P2 to move drop_pagecache_sb() to super.c for xfs to use
>>>> 3. P3: Add invalidate all mappings after sync.
>>>> 4. P3: Set offset&len to be start&length of device when it is to be removed.
>>>> 5. Rebase on 6.0-rc3 + Darrick's patch[1] + Dan's patch[2].
>>>>
>>>> Changes since v6:
>>>> 1. Rebase on 6.0-rc2 and Darrick's patch[1].
>>>>
>>>> [1]: https://lore.kernel.org/linux-xfs/Yv5wIa2crHioYeRr@magnolia/
>>>> [2]: https://lore.kernel.org/linux-xfs/166153426798.2758201.15108211981034512993.stgit@dwillia2-xfh.jf.intel.com/
>>
>> Just out of curiosity, is it your (or djbw's) intent to send all these
>> as bugfixes for 6.0 via akpm like all the other dax fixen?
>
> Aha, this is 6.1 stuff, please ignore this question.

Actually I hope these patches can be merged ASAP. (But it seems a bit
late for 6.0 now.)

And do you know which/whose branch has picked up your patch[1]? I
cannot find it.


--
Thanks,
Ruan.

>
> --D
>
>> --D
>>
>>>>
>>>> Shiyang Ruan (3):
>>>> xfs: fix the calculation of length and end
>>>> fs: move drop_pagecache_sb() for others to use
>>>> mm, pmem, xfs: Introduce MF_MEM_REMOVE for unbind
>>>>
>>>> drivers/dax/super.c | 3 ++-
>>>> fs/drop_caches.c | 33 ---------------------------------
>>>> fs/super.c | 34 ++++++++++++++++++++++++++++++++++
>>>> fs/xfs/xfs_notify_failure.c | 31 +++++++++++++++++++++++++++----
>>>> include/linux/fs.h | 1 +
>>>> include/linux/mm.h | 1 +
>>>> 6 files changed, 65 insertions(+), 38 deletions(-)
>>>>

2022-09-16 15:49:12

by Darrick J. Wong

[permalink] [raw]
Subject: Re: [PATCH v8 0/3] mm, pmem, xfs: Introduce MF_MEM_REMOVE for unbind

On Thu, Sep 15, 2022 at 10:56:09AM +0800, Shiyang Ruan wrote:
>
>
> 在 2022/9/15 2:15, Darrick J. Wong 写道:
> > On Wed, Sep 14, 2022 at 11:09:23AM -0700, Darrick J. Wong wrote:
> > > On Wed, Sep 07, 2022 at 05:46:00PM +0800, Shiyang Ruan wrote:
> > > > ping
> > > >
> > > > 在 2022/9/2 18:35, Shiyang Ruan 写道:
> > > > > Changes since v7:
> > > > > 1. Add P1 to fix calculation mistake
> > > > > 2. Add P2 to move drop_pagecache_sb() to super.c for xfs to use
> > > > > 3. P3: Add invalidate all mappings after sync.
> > > > > 4. P3: Set offset&len to be start&length of device when it is to be removed.
> > > > > 5. Rebase on 6.0-rc3 + Darrick's patch[1] + Dan's patch[2].
> > > > >
> > > > > Changes since v6:
> > > > > 1. Rebase on 6.0-rc2 and Darrick's patch[1].
> > > > >
> > > > > [1]: https://lore.kernel.org/linux-xfs/Yv5wIa2crHioYeRr@magnolia/
> > > > > [2]: https://lore.kernel.org/linux-xfs/166153426798.2758201.15108211981034512993.stgit@dwillia2-xfh.jf.intel.com/
> > >
> > > Just out of curiosity, is it your (or djbw's) intent to send all these
> > > as bugfixes for 6.0 via akpm like all the other dax fixen?
> >
> > Aha, this is 6.1 stuff, please ignore this question.
>
> Actually I hope these patches can be merged ASAP. (But it seems a bit late
> for 6.0 now.)
>
> And do you know which/whose branch has picked up your patch[1]? I cannot
> find it.

It's not upstream, though the maintainer (Dave currently) reviewed it.
I don't know if he hasn't had time to put together a fixes branch or if
he's simply punting all the queued up stuff to 6.1.

(Dave?)

--D

>
> --
> Thanks,
> Ruan.
>
> >
> > --D
> >
> > > --D
> > >
> > > > >
> > > > > Shiyang Ruan (3):
> > > > > xfs: fix the calculation of length and end
> > > > > fs: move drop_pagecache_sb() for others to use
> > > > > mm, pmem, xfs: Introduce MF_MEM_REMOVE for unbind
> > > > >
> > > > > drivers/dax/super.c | 3 ++-
> > > > > fs/drop_caches.c | 33 ---------------------------------
> > > > > fs/super.c | 34 ++++++++++++++++++++++++++++++++++
> > > > > fs/xfs/xfs_notify_failure.c | 31 +++++++++++++++++++++++++++----
> > > > > include/linux/fs.h | 1 +
> > > > > include/linux/mm.h | 1 +
> > > > > 6 files changed, 65 insertions(+), 38 deletions(-)
> > > > >