Skip to content

Commit

Permalink
mm: shmem: don't truncate page if memory failure happens
Browse files Browse the repository at this point in the history
The current behavior of memory failure is to truncate the page cache
regardless of dirty or clean.  If the page is dirty the later access will
get the obsolete data from disk without any notification to the users.
This may cause silent data loss.  It is even worse for shmem since shmem
is in-memory filesystem, truncating page cache means discarding data
blocks.  The later read would return all zero.

The right approach is to keep the corrupted page in page cache, any later
access would return error for syscalls or SIGBUS for page fault, until the
file is truncated, hole punched or removed.  The regular storage backed
filesystems would be more complicated so this patch is focused on shmem.
This also unblock the support for soft offlining shmem THP.

[[email protected]: fix uninitialized variable use in me_pagecache_clean()]
  Link: https://lkml.kernel.org/r/[email protected]
[Fix invalid pointer dereference in shmem_read_mapping_page_gfp() with a
 slight different implementation from what Ajay Garg <[email protected]>
 and Muchun Song <[email protected]> proposed and reworked the
 error handling of shmem_write_begin() suggested by Linus]
  Link: https://lore.kernel.org/linux-mm/[email protected]/
Link: https://lkml.kernel.org/r/[email protected]
Link: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Yang Shi <[email protected]>
Signed-off-by: Arnd Bergmann <[email protected]>
Cc: Hugh Dickins <[email protected]>
Cc: Kirill A. Shutemov <[email protected]>
Cc: Matthew Wilcox <[email protected]>
Cc: Naoya Horiguchi <[email protected]>
Cc: Oscar Salvador <[email protected]>
Cc: Peter Xu <[email protected]>
Cc: Ajay Garg <[email protected]>
Cc: Muchun Song <[email protected]>
Cc: Andy Lavr <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Mark Brown <[email protected]>
  • Loading branch information
yang-shi authored and broonie committed Dec 10, 2021
1 parent 4ca3db5 commit a817494
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 9 deletions.
14 changes: 11 additions & 3 deletions mm/memory-failure.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
#include <linux/ratelimit.h>
#include <linux/page-isolation.h>
#include <linux/pagewalk.h>
#include <linux/shmem_fs.h>
#include "internal.h"
#include "ras/ras_event.h"

Expand Down Expand Up @@ -867,6 +868,7 @@ static int me_pagecache_clean(struct page_state *ps, struct page *p)
{
int ret;
struct address_space *mapping;
bool extra_pins;

delete_from_lru_cache(p);

Expand Down Expand Up @@ -895,18 +897,24 @@ static int me_pagecache_clean(struct page_state *ps, struct page *p)
goto out;
}

/*
* The shmem page is kept in page cache instead of truncating
* so is expected to have an extra refcount after error-handling.
*/
extra_pins = shmem_mapping(mapping);

/*
* Truncation is a bit tricky. Enable it per file system for now.
*
* Open: to take i_rwsem or not for this? Right now we don't.
*/
ret = truncate_error_page(p, page_to_pfn(p), mapping);
if (has_extra_refcount(ps, p, extra_pins))
ret = MF_FAILED;

out:
unlock_page(p);

if (has_extra_refcount(ps, p, false))
ret = MF_FAILED;

return ret;
}

Expand Down
51 changes: 45 additions & 6 deletions mm/shmem.c
Original file line number Diff line number Diff line change
Expand Up @@ -2462,6 +2462,7 @@ shmem_write_begin(struct file *file, struct address_space *mapping,
struct inode *inode = mapping->host;
struct shmem_inode_info *info = SHMEM_I(inode);
pgoff_t index = pos >> PAGE_SHIFT;
int ret = 0;

/* i_rwsem is held by caller */
if (unlikely(info->seals & (F_SEAL_GROW |
Expand All @@ -2472,7 +2473,19 @@ shmem_write_begin(struct file *file, struct address_space *mapping,
return -EPERM;
}

return shmem_getpage(inode, index, pagep, SGP_WRITE);
ret = shmem_getpage(inode, index, pagep, SGP_WRITE);

if (ret)
return ret;

if (PageHWPoison(*pagep)) {
unlock_page(*pagep);
put_page(*pagep);
*pagep = NULL;
return -EIO;
}

return 0;
}

static int
Expand Down Expand Up @@ -2559,6 +2572,12 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
if (sgp == SGP_CACHE)
set_page_dirty(page);
unlock_page(page);

if (PageHWPoison(page)) {
put_page(page);
error = -EIO;
break;
}
}

/*
Expand Down Expand Up @@ -3098,14 +3117,22 @@ static const char *shmem_get_link(struct dentry *dentry,
page = find_get_page(inode->i_mapping, 0);
if (!page)
return ERR_PTR(-ECHILD);
if (!PageUptodate(page)) {
if (PageHWPoison(page) ||
!PageUptodate(page)) {
put_page(page);
return ERR_PTR(-ECHILD);
}
} else {
error = shmem_getpage(inode, 0, &page, SGP_READ);
if (error)
return ERR_PTR(error);
if (!page)
return ERR_PTR(-ECHILD);
if (PageHWPoison(page)) {
unlock_page(page);
put_page(page);
return ERR_PTR(-ECHILD);
}
unlock_page(page);
}
set_delayed_call(done, shmem_put_link, page);
Expand Down Expand Up @@ -3756,6 +3783,13 @@ static void shmem_destroy_inodecache(void)
kmem_cache_destroy(shmem_inode_cachep);
}

/* Keep the page in page cache instead of truncating it */
static int shmem_error_remove_page(struct address_space *mapping,
struct page *page)
{
return 0;
}

const struct address_space_operations shmem_aops = {
.writepage = shmem_writepage,
.set_page_dirty = __set_page_dirty_no_writeback,
Expand All @@ -3766,7 +3800,7 @@ const struct address_space_operations shmem_aops = {
#ifdef CONFIG_MIGRATION
.migratepage = migrate_page,
#endif
.error_remove_page = generic_error_remove_page,
.error_remove_page = shmem_error_remove_page,
};
EXPORT_SYMBOL(shmem_aops);

Expand Down Expand Up @@ -4174,9 +4208,14 @@ struct page *shmem_read_mapping_page_gfp(struct address_space *mapping,
error = shmem_getpage_gfp(inode, index, &page, SGP_CACHE,
gfp, NULL, NULL, NULL);
if (error)
page = ERR_PTR(error);
else
unlock_page(page);
return ERR_PTR(error);

unlock_page(page);
if (PageHWPoison(page)) {
put_page(page);
return ERR_PTR(-EIO);
}

return page;
#else
/*
Expand Down
5 changes: 5 additions & 0 deletions mm/userfaultfd.c
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,11 @@ static int mcontinue_atomic_pte(struct mm_struct *dst_mm,
goto out;
}

if (PageHWPoison(page)) {
ret = -EIO;
goto out_release;
}

ret = mfill_atomic_install_pte(dst_mm, dst_pmd, dst_vma, dst_addr,
page, false, wp_copy);
if (ret)
Expand Down

0 comments on commit a817494

Please sign in to comment.