| Message ID | 20230526075552.363524-3-mcgrof@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add support for blocksize > PAGE_SIZE | expand |
On Fri, May 26, 2023 at 12:55:46AM -0700, Luis Chamberlain wrote: > The PageHWPoison() call can be converted over to the respective folio > call is_folio_hwpoison(). This introduces no functional changes. Yes, it very much does! > @@ -4548,7 +4548,7 @@ struct page *shmem_read_mapping_page_gfp(struct address_space *mapping, > return &folio->page; > > page = folio_file_page(folio, index); > - if (PageHWPoison(page)) { > + if (is_folio_hwpoison(folio)) { > folio_put(folio); Imagine you have an order-9 folio and one of the pages in it gets HWPoison. Before, you can read the other 511 pages in the folio. After your patch, you can't read any of them. You've effectively increased the blast radius of any hwerror, and I don't think that's an acceptable change.
On Fri, May 26, 2023 at 03:32:54PM +0100, Matthew Wilcox wrote: > On Fri, May 26, 2023 at 12:55:46AM -0700, Luis Chamberlain wrote: > > The PageHWPoison() call can be converted over to the respective folio > > call is_folio_hwpoison(). This introduces no functional changes. > > Yes, it very much does! > > > @@ -4548,7 +4548,7 @@ struct page *shmem_read_mapping_page_gfp(struct address_space *mapping, > > return &folio->page; > > > > page = folio_file_page(folio, index); > > - if (PageHWPoison(page)) { > > + if (is_folio_hwpoison(folio)) { > > folio_put(folio); > > Imagine you have an order-9 folio and one of the pages in it gets > HWPoison. Before, you can read the other 511 pages in the folio. But before we didn't use high order folios for reads on tmpfs? But I get the idea. > After your patch, you can't read any of them. You've effectively > increased the blast radius of any hwerror, and I don't think that's an > acceptable change. I see, thanks! Will fix if we move forward with this. Luis
On Fri, May 26, 2023 at 10:41:00AM -0700, Luis Chamberlain wrote: > On Fri, May 26, 2023 at 03:32:54PM +0100, Matthew Wilcox wrote: > > On Fri, May 26, 2023 at 12:55:46AM -0700, Luis Chamberlain wrote: > > > The PageHWPoison() call can be converted over to the respective folio > > > call is_folio_hwpoison(). This introduces no functional changes. > > > > Yes, it very much does! > > > > > @@ -4548,7 +4548,7 @@ struct page *shmem_read_mapping_page_gfp(struct address_space *mapping, > > > return &folio->page; > > > > > > page = folio_file_page(folio, index); > > > - if (PageHWPoison(page)) { > > > + if (is_folio_hwpoison(folio)) { > > > folio_put(folio); > > > > Imagine you have an order-9 folio and one of the pages in it gets > > HWPoison. Before, you can read the other 511 pages in the folio. > > But before we didn't use high order folios for reads on tmpfs? Sure we did! If SHMEM_HUGE_ALWAYS is set, we can see reads of THPs (order-9 folios) in this path.
diff --git a/mm/shmem.c b/mm/shmem.c index 351803415ad2..a947f2678a39 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -3360,7 +3360,7 @@ static const char *shmem_get_link(struct dentry *dentry, folio = filemap_get_folio(inode->i_mapping, 0); if (IS_ERR(folio)) return ERR_PTR(-ECHILD); - if (PageHWPoison(folio_page(folio, 0)) || + if (is_folio_hwpoison(folio) || !folio_test_uptodate(folio)) { folio_put(folio); return ERR_PTR(-ECHILD); @@ -3371,7 +3371,7 @@ static const char *shmem_get_link(struct dentry *dentry, return ERR_PTR(error); if (!folio) return ERR_PTR(-ECHILD); - if (PageHWPoison(folio_page(folio, 0))) { + if (is_folio_hwpoison(folio)) { folio_unlock(folio); folio_put(folio); return ERR_PTR(-ECHILD); @@ -4548,7 +4548,7 @@ struct page *shmem_read_mapping_page_gfp(struct address_space *mapping, return &folio->page; page = folio_file_page(folio, index); - if (PageHWPoison(page)) { + if (is_folio_hwpoison(folio)) { folio_put(folio); return ERR_PTR(-EIO); }
The PageHWPoison() call can be converted over to the respective folio call is_folio_hwpoison(). This introduces no functional changes. Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> --- mm/shmem.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)