Skip to content

Commit c5a1858

Browse files
yang-shisfrothwell
authored andcommitted
mm: shmem: don't truncate page if memory failure happens
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. [arnd@arndb.de: fix uninitialized variable use in me_pagecache_clean()] Link: https://lkml.kernel.org/r/20211022064748.4173718-1-arnd@kernel.org [Fix invalid pointer dereference in shmem_read_mapping_page_gfp() with a slight different implementation from what Ajay Garg <ajaygargnsit@gmail.com> and Muchun Song <songmuchun@bytedance.com> proposed and reworked the error handling of shmem_write_begin() suggested by Linus] Link: https://lore.kernel.org/linux-mm/20211111084617.6746-1-ajaygargnsit@gmail.com/ Link: https://lkml.kernel.org/r/20211020210755.23964-6-shy828301@gmail.com Link: https://lkml.kernel.org/r/20211116193247.21102-1-shy828301@gmail.com Signed-off-by: Yang Shi <shy828301@gmail.com> Signed-off-by: Arnd Bergmann <arnd@arndb.de> Cc: Hugh Dickins <hughd@google.com> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> Cc: Matthew Wilcox <willy@infradead.org> Cc: Naoya Horiguchi <naoya.horiguchi@nec.com> Cc: Oscar Salvador <osalvador@suse.de> Cc: Peter Xu <peterx@redhat.com> Cc: Ajay Garg <ajaygargnsit@gmail.com> Cc: Muchun Song <songmuchun@bytedance.com> Cc: Andy Lavr <andy.lavr@gmail.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
1 parent 969662c commit c5a1858

File tree

3 files changed

+61
-9
lines changed

3 files changed

+61
-9
lines changed

mm/memory-failure.c

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
#include <linux/ratelimit.h>
5959
#include <linux/page-isolation.h>
6060
#include <linux/pagewalk.h>
61+
#include <linux/shmem_fs.h>
6162
#include "internal.h"
6263
#include "ras/ras_event.h"
6364

@@ -867,6 +868,7 @@ static int me_pagecache_clean(struct page_state *ps, struct page *p)
867868
{
868869
int ret;
869870
struct address_space *mapping;
871+
bool extra_pins;
870872

871873
delete_from_lru_cache(p);
872874

@@ -895,18 +897,24 @@ static int me_pagecache_clean(struct page_state *ps, struct page *p)
895897
goto out;
896898
}
897899

900+
/*
901+
* The shmem page is kept in page cache instead of truncating
902+
* so is expected to have an extra refcount after error-handling.
903+
*/
904+
extra_pins = shmem_mapping(mapping);
905+
898906
/*
899907
* Truncation is a bit tricky. Enable it per file system for now.
900908
*
901909
* Open: to take i_rwsem or not for this? Right now we don't.
902910
*/
903911
ret = truncate_error_page(p, page_to_pfn(p), mapping);
912+
if (has_extra_refcount(ps, p, extra_pins))
913+
ret = MF_FAILED;
914+
904915
out:
905916
unlock_page(p);
906917

907-
if (has_extra_refcount(ps, p, false))
908-
ret = MF_FAILED;
909-
910918
return ret;
911919
}
912920

mm/shmem.c

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2462,6 +2462,7 @@ shmem_write_begin(struct file *file, struct address_space *mapping,
24622462
struct inode *inode = mapping->host;
24632463
struct shmem_inode_info *info = SHMEM_I(inode);
24642464
pgoff_t index = pos >> PAGE_SHIFT;
2465+
int ret = 0;
24652466

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

2475-
return shmem_getpage(inode, index, pagep, SGP_WRITE);
2476+
ret = shmem_getpage(inode, index, pagep, SGP_WRITE);
2477+
2478+
if (ret)
2479+
return ret;
2480+
2481+
if (PageHWPoison(*pagep)) {
2482+
unlock_page(*pagep);
2483+
put_page(*pagep);
2484+
*pagep = NULL;
2485+
return -EIO;
2486+
}
2487+
2488+
return 0;
24762489
}
24772490

24782491
static int
@@ -2559,6 +2572,12 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
25592572
if (sgp == SGP_CACHE)
25602573
set_page_dirty(page);
25612574
unlock_page(page);
2575+
2576+
if (PageHWPoison(page)) {
2577+
put_page(page);
2578+
error = -EIO;
2579+
break;
2580+
}
25622581
}
25632582

25642583
/*
@@ -3098,14 +3117,22 @@ static const char *shmem_get_link(struct dentry *dentry,
30983117
page = find_get_page(inode->i_mapping, 0);
30993118
if (!page)
31003119
return ERR_PTR(-ECHILD);
3101-
if (!PageUptodate(page)) {
3120+
if (PageHWPoison(page) ||
3121+
!PageUptodate(page)) {
31023122
put_page(page);
31033123
return ERR_PTR(-ECHILD);
31043124
}
31053125
} else {
31063126
error = shmem_getpage(inode, 0, &page, SGP_READ);
31073127
if (error)
31083128
return ERR_PTR(error);
3129+
if (!page)
3130+
return ERR_PTR(-ECHILD);
3131+
if (PageHWPoison(page)) {
3132+
unlock_page(page);
3133+
put_page(page);
3134+
return ERR_PTR(-ECHILD);
3135+
}
31093136
unlock_page(page);
31103137
}
31113138
set_delayed_call(done, shmem_put_link, page);
@@ -3756,6 +3783,13 @@ static void shmem_destroy_inodecache(void)
37563783
kmem_cache_destroy(shmem_inode_cachep);
37573784
}
37583785

3786+
/* Keep the page in page cache instead of truncating it */
3787+
static int shmem_error_remove_page(struct address_space *mapping,
3788+
struct page *page)
3789+
{
3790+
return 0;
3791+
}
3792+
37593793
const struct address_space_operations shmem_aops = {
37603794
.writepage = shmem_writepage,
37613795
.set_page_dirty = __set_page_dirty_no_writeback,
@@ -3766,7 +3800,7 @@ const struct address_space_operations shmem_aops = {
37663800
#ifdef CONFIG_MIGRATION
37673801
.migratepage = migrate_page,
37683802
#endif
3769-
.error_remove_page = generic_error_remove_page,
3803+
.error_remove_page = shmem_error_remove_page,
37703804
};
37713805
EXPORT_SYMBOL(shmem_aops);
37723806

@@ -4174,9 +4208,14 @@ struct page *shmem_read_mapping_page_gfp(struct address_space *mapping,
41744208
error = shmem_getpage_gfp(inode, index, &page, SGP_CACHE,
41754209
gfp, NULL, NULL, NULL);
41764210
if (error)
4177-
page = ERR_PTR(error);
4178-
else
4179-
unlock_page(page);
4211+
return ERR_PTR(error);
4212+
4213+
unlock_page(page);
4214+
if (PageHWPoison(page)) {
4215+
put_page(page);
4216+
return ERR_PTR(-EIO);
4217+
}
4218+
41804219
return page;
41814220
#else
41824221
/*

mm/userfaultfd.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,11 @@ static int mcontinue_atomic_pte(struct mm_struct *dst_mm,
232232
goto out;
233233
}
234234

235+
if (PageHWPoison(page)) {
236+
ret = -EIO;
237+
goto out_release;
238+
}
239+
235240
ret = mfill_atomic_install_pte(dst_mm, dst_pmd, dst_vma, dst_addr,
236241
page, false, wp_copy);
237242
if (ret)

0 commit comments

Comments
 (0)