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() # was pmem driver ->remove() in v1
-> 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.
[1]: https://lore.kernel.org/linux-mm/161604050314.1463742.14151665140035795571.stgit@dwillia2-desk3.amr.corp.intel.com/
Signed-off-by: Shiyang Ruan <[email protected]>
---
drivers/dax/super.c | 3 ++-
fs/xfs/xfs_notify_failure.c | 23 +++++++++++++++++++++++
include/linux/mm.h | 1 +
3 files changed, 26 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 3830f908e215..5e04ba7fa403 100644
--- a/fs/xfs/xfs_notify_failure.c
+++ b/fs/xfs/xfs_notify_failure.c
@@ -22,6 +22,7 @@
#include <linux/mm.h>
#include <linux/dax.h>
+#include <linux/fs.h>
struct xfs_failure_info {
xfs_agblock_t startblock;
@@ -77,6 +78,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))) {
+ /* The device is about to be removed. Not a really failure. */
+ if (notify->mf_flags & MF_MEM_PRE_REMOVE)
+ return 0;
notify->want_shutdown = true;
return 0;
}
@@ -182,12 +186,23 @@ xfs_dax_notify_failure(
struct xfs_mount *mp = dax_holder(dax_dev);
u64 ddev_start;
u64 ddev_end;
+ int error;
if (!(mp->m_super->s_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);
+ drop_pagecache_sb(mp->m_super, NULL);
+ 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_debug(mp,
"notify_failure() not supported on realtime device!");
@@ -196,6 +211,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;
@@ -209,6 +226,12 @@ xfs_dax_notify_failure(
ddev_start = mp->m_ddev_targp->bt_dax_part_off;
ddev_end = ddev_start + bdev_nr_bytes(mp->m_ddev_targp->bt_bdev) - 1;
+ /* Notify failure on the whole device */
+ if (offset == 0 && len == U64_MAX) {
+ offset = ddev_start;
+ len = bdev_nr_bytes(mp->m_ddev_targp->bt_bdev);
+ }
+
/* Ignore the range out of filesystem area */
if (offset + len - 1 < ddev_start)
return -ENXIO;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 21f8b27bd9fd..9122a1c57dd2 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -3183,6 +3183,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
On Fri, Sep 02, 2022 at 10:36:01AM +0000, 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() # was pmem driver ->remove() in v1
> -> 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.
>
> [1]: https://lore.kernel.org/linux-mm/161604050314.1463742.14151665140035795571.stgit@dwillia2-desk3.amr.corp.intel.com/
>
> Signed-off-by: Shiyang Ruan <[email protected]>
> ---
> drivers/dax/super.c | 3 ++-
> fs/xfs/xfs_notify_failure.c | 23 +++++++++++++++++++++++
> include/linux/mm.h | 1 +
> 3 files changed, 26 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 3830f908e215..5e04ba7fa403 100644
> --- a/fs/xfs/xfs_notify_failure.c
> +++ b/fs/xfs/xfs_notify_failure.c
> @@ -22,6 +22,7 @@
>
> #include <linux/mm.h>
> #include <linux/dax.h>
> +#include <linux/fs.h>
>
> struct xfs_failure_info {
> xfs_agblock_t startblock;
> @@ -77,6 +78,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))) {
> + /* The device is about to be removed. Not a really failure. */
> + if (notify->mf_flags & MF_MEM_PRE_REMOVE)
> + return 0;
> notify->want_shutdown = true;
> return 0;
> }
> @@ -182,12 +186,23 @@ xfs_dax_notify_failure(
> struct xfs_mount *mp = dax_holder(dax_dev);
> u64 ddev_start;
> u64 ddev_end;
> + int error;
>
> if (!(mp->m_super->s_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);
> + drop_pagecache_sb(mp->m_super, NULL);
> + 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_debug(mp,
> "notify_failure() not supported on realtime device!");
> @@ -196,6 +211,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;
> @@ -209,6 +226,12 @@ xfs_dax_notify_failure(
> ddev_start = mp->m_ddev_targp->bt_dax_part_off;
> ddev_end = ddev_start + bdev_nr_bytes(mp->m_ddev_targp->bt_bdev) - 1;
>
> + /* Notify failure on the whole device */
> + if (offset == 0 && len == U64_MAX) {
> + offset = ddev_start;
> + len = bdev_nr_bytes(mp->m_ddev_targp->bt_bdev);
> + }
I wonder, won't the trimming code below take care of this?
The rest of the patch looks ok to me.
--D
> +
> /* Ignore the range out of filesystem area */
> if (offset + len - 1 < ddev_start)
> return -ENXIO;
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 21f8b27bd9fd..9122a1c57dd2 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -3183,6 +3183,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/9/15 2:15, Darrick J. Wong 写道:
> On Fri, Sep 02, 2022 at 10:36:01AM +0000, 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() # was pmem driver ->remove() in v1
>> -> 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.
>>
>> [1]: https://lore.kernel.org/linux-mm/161604050314.1463742.14151665140035795571.stgit@dwillia2-desk3.amr.corp.intel.com/
>>
>> Signed-off-by: Shiyang Ruan <[email protected]>
>> ---
>> drivers/dax/super.c | 3 ++-
>> fs/xfs/xfs_notify_failure.c | 23 +++++++++++++++++++++++
>> include/linux/mm.h | 1 +
>> 3 files changed, 26 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 3830f908e215..5e04ba7fa403 100644
>> --- a/fs/xfs/xfs_notify_failure.c
>> +++ b/fs/xfs/xfs_notify_failure.c
>> @@ -22,6 +22,7 @@
>>
>> #include <linux/mm.h>
>> #include <linux/dax.h>
>> +#include <linux/fs.h>
>>
>> struct xfs_failure_info {
>> xfs_agblock_t startblock;
>> @@ -77,6 +78,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))) {
>> + /* The device is about to be removed. Not a really failure. */
>> + if (notify->mf_flags & MF_MEM_PRE_REMOVE)
>> + return 0;
>> notify->want_shutdown = true;
>> return 0;
>> }
>> @@ -182,12 +186,23 @@ xfs_dax_notify_failure(
>> struct xfs_mount *mp = dax_holder(dax_dev);
>> u64 ddev_start;
>> u64 ddev_end;
>> + int error;
>>
>> if (!(mp->m_super->s_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);
>> + drop_pagecache_sb(mp->m_super, NULL);
>> + 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_debug(mp,
>> "notify_failure() not supported on realtime device!");
>> @@ -196,6 +211,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;
>> @@ -209,6 +226,12 @@ xfs_dax_notify_failure(
>> ddev_start = mp->m_ddev_targp->bt_dax_part_off;
>> ddev_end = ddev_start + bdev_nr_bytes(mp->m_ddev_targp->bt_bdev) - 1;
>>
>> + /* Notify failure on the whole device */
>> + if (offset == 0 && len == U64_MAX) {
>> + offset = ddev_start;
>> + len = bdev_nr_bytes(mp->m_ddev_targp->bt_bdev);
>> + }
>
> I wonder, won't the trimming code below take care of this?
The len is U64_MAX, so 'offset + len - 1' will overflow. That can't be
handled correctly by the trimming code below.
--
Thanks,
Ruan.
>
> The rest of the patch looks ok to me.
>
> --D
>
>> +
>> /* Ignore the range out of filesystem area */
>> if (offset + len - 1 < ddev_start)
>> return -ENXIO;
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index 21f8b27bd9fd..9122a1c57dd2 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -3183,6 +3183,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
>>
On Fri, Sep 02, 2022 at 10:36:01AM +0000, 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() # was pmem driver ->remove() in v1
> -> 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.
>
> [1]: https://lore.kernel.org/linux-mm/161604050314.1463742.14151665140035795571.stgit@dwillia2-desk3.amr.corp.intel.com/
>
> Signed-off-by: Shiyang Ruan <[email protected]>
> ---
> drivers/dax/super.c | 3 ++-
> fs/xfs/xfs_notify_failure.c | 23 +++++++++++++++++++++++
> include/linux/mm.h | 1 +
> 3 files changed, 26 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 3830f908e215..5e04ba7fa403 100644
> --- a/fs/xfs/xfs_notify_failure.c
> +++ b/fs/xfs/xfs_notify_failure.c
> @@ -22,6 +22,7 @@
>
> #include <linux/mm.h>
> #include <linux/dax.h>
> +#include <linux/fs.h>
>
> struct xfs_failure_info {
> xfs_agblock_t startblock;
> @@ -77,6 +78,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))) {
> + /* The device is about to be removed. Not a really failure. */
> + if (notify->mf_flags & MF_MEM_PRE_REMOVE)
> + return 0;
> notify->want_shutdown = true;
> return 0;
> }
> @@ -182,12 +186,23 @@ xfs_dax_notify_failure(
> struct xfs_mount *mp = dax_holder(dax_dev);
> u64 ddev_start;
> u64 ddev_end;
> + int error;
>
> if (!(mp->m_super->s_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);
> + drop_pagecache_sb(mp->m_super, NULL);
> + up_write(&mp->m_super->s_umount);
> + if (error)
> + return error;
If the device is about to go away unexpectedly, shouldn't this shut
down the filesystem after syncing it here? If the filesystem has
been shut down, then everything will fail before removal finally
triggers, and the act of unmounting the filesystem post device
removal will clean up the page cache and all the other caches.
IOWs, I don't understand why the page cache is considered special
here (as opposed to, say, the inode or dentry caches), nor why we
aren't shutting down the filesystem directly after syncing it to
disk to ensure that we don't end up with applications losing data as
a result of racing with the removal....
Cheers,
Dave.
--
Dave Chinner
[email protected]
On Tue, Sep 20, 2022 at 12:45:19PM +1000, Dave Chinner wrote:
> On Fri, Sep 02, 2022 at 10:36:01AM +0000, 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() # was pmem driver ->remove() in v1
> > -> 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.
> >
> > [1]: https://lore.kernel.org/linux-mm/161604050314.1463742.14151665140035795571.stgit@dwillia2-desk3.amr.corp.intel.com/
> >
> > Signed-off-by: Shiyang Ruan <[email protected]>
> > ---
> > drivers/dax/super.c | 3 ++-
> > fs/xfs/xfs_notify_failure.c | 23 +++++++++++++++++++++++
> > include/linux/mm.h | 1 +
> > 3 files changed, 26 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 3830f908e215..5e04ba7fa403 100644
> > --- a/fs/xfs/xfs_notify_failure.c
> > +++ b/fs/xfs/xfs_notify_failure.c
> > @@ -22,6 +22,7 @@
> >
> > #include <linux/mm.h>
> > #include <linux/dax.h>
> > +#include <linux/fs.h>
> >
> > struct xfs_failure_info {
> > xfs_agblock_t startblock;
> > @@ -77,6 +78,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))) {
> > + /* The device is about to be removed. Not a really failure. */
> > + if (notify->mf_flags & MF_MEM_PRE_REMOVE)
> > + return 0;
> > notify->want_shutdown = true;
> > return 0;
> > }
> > @@ -182,12 +186,23 @@ xfs_dax_notify_failure(
> > struct xfs_mount *mp = dax_holder(dax_dev);
> > u64 ddev_start;
> > u64 ddev_end;
> > + int error;
> >
> > if (!(mp->m_super->s_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);
> > + drop_pagecache_sb(mp->m_super, NULL);
> > + up_write(&mp->m_super->s_umount);
> > + if (error)
> > + return error;
>
> If the device is about to go away unexpectedly, shouldn't this shut
> down the filesystem after syncing it here? If the filesystem has
> been shut down, then everything will fail before removal finally
> triggers, and the act of unmounting the filesystem post device
> removal will clean up the page cache and all the other caches.
IIRC they want to kill all the processes with MAP_SYNC mappings sooner
than whenever the admin gets around to unmounting the filesystem, which
is why PRE_REMOVE will then go walk the rmapbt to find processes to
shoot down. I'm not sure, though, if drop_pagecache_sb only touches
DRAM page cache or if it'll shoot down fsdax mappings too?
> IOWs, I don't understand why the page cache is considered special
> here (as opposed to, say, the inode or dentry caches), nor why we
> aren't shutting down the filesystem directly after syncing it to
> disk to ensure that we don't end up with applications losing data as
> a result of racing with the removal....
But yeah, we might as well shut down the fs at the end of PRE_REMOVE
handling, if the rmap walk hasn't already done that.
--D
> Cheers,
>
> Dave.
> --
> Dave Chinner
> [email protected]