lkml.org 
[lkml]   [2022]   [Sep]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 3/3] mm, pmem, xfs: Introduce MF_MEM_REMOVE for unbind
    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 <ruansy.fnst@fujitsu.com>
    > ---
    > 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
    david@fromorbit.com

    \
     
     \ /
      Last update: 2022-09-20 04:46    [W:4.525 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site