[RFC v2 2/8] shmem: convert to use is_folio_hwpoison()

Luis Chamberlain posted 8 patches 1 year, 1 month ago
[RFC v2 2/8] shmem: convert to use is_folio_hwpoison()
Posted by Luis Chamberlain 1 year, 1 month ago
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(-)

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);
 	}
-- 
2.39.2
Re: [RFC v2 2/8] shmem: convert to use is_folio_hwpoison()
Posted by Matthew Wilcox 1 year ago
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.
Re: [RFC v2 2/8] shmem: convert to use is_folio_hwpoison()
Posted by Luis Chamberlain 1 year ago
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
Re: [RFC v2 2/8] shmem: convert to use is_folio_hwpoison()
Posted by Matthew Wilcox 1 year ago
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.