| Message ID | 20240531032938.2712870-1-willy@infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/memory-failure: Stop setting the folio error flag | expand |
On 2024/5/31 11:29, Matthew Wilcox (Oracle) wrote: > Nobody checks the error flag any more, so setting it accomplishes > nothing. Remove the obsolete parts of this comment; it hasn't > been true since errseq_t was used to track writeback errors in 2017. > > Cc: Miaohe Lin <linmiaohe@huawei.com> > Cc: linux-mm@kvack.org > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> Acked-by: Miaohe Lin <linmiaohe@huawei.com> Thanks. . > --- > mm/memory-failure.c | 29 ----------------------------- > 1 file changed, 29 deletions(-) > > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index ac030061eda0..78fdf5ee8421 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -1112,7 +1112,6 @@ static int me_pagecache_dirty(struct page_state *ps, struct page *p) > struct folio *folio = page_folio(p); > struct address_space *mapping = folio_mapping(folio); > > - SetPageError(p); > /* TBD: print more information about the file. */ > if (mapping) { > /* > @@ -1120,34 +1119,6 @@ static int me_pagecache_dirty(struct page_state *ps, struct page *p) > * who check the mapping. > * This way the application knows that something went > * wrong with its dirty file data. > - * > - * There's one open issue: > - * > - * The EIO will be only reported on the next IO > - * operation and then cleared through the IO map. > - * Normally Linux has two mechanisms to pass IO error > - * first through the AS_EIO flag in the address space > - * and then through the PageError flag in the page. > - * Since we drop pages on memory failure handling the > - * only mechanism open to use is through AS_AIO. > - * > - * This has the disadvantage that it gets cleared on > - * the first operation that returns an error, while > - * the PageError bit is more sticky and only cleared > - * when the page is reread or dropped. If an > - * application assumes it will always get error on > - * fsync, but does other operations on the fd before > - * and the page is dropped between then the error > - * will not be properly reported. > - * > - * This can already happen even without hwpoisoned > - * pages: first on metadata IO errors (which only > - * report through AS_EIO) or when the page is dropped > - * at the wrong time. > - * > - * So right now we assume that the application DTRT on > - * the first EIO, but we're not worse than other parts > - * of the kernel. > */ > mapping_set_error(mapping, -EIO); > } >
On Fri, May 31, 2024 at 04:29:25AM +0100, Matthew Wilcox (Oracle) wrote: > Nobody checks the error flag any more, so setting it accomplishes > nothing. Remove the obsolete parts of this comment; it hasn't > been true since errseq_t was used to track writeback errors in 2017. > > Cc: Miaohe Lin <linmiaohe@huawei.com> > Cc: linux-mm@kvack.org > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> Reviewed-by: Oscar Salvador <osalvador@suse.de>
diff --git a/mm/memory-failure.c b/mm/memory-failure.c index ac030061eda0..78fdf5ee8421 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -1112,7 +1112,6 @@ static int me_pagecache_dirty(struct page_state *ps, struct page *p) struct folio *folio = page_folio(p); struct address_space *mapping = folio_mapping(folio); - SetPageError(p); /* TBD: print more information about the file. */ if (mapping) { /* @@ -1120,34 +1119,6 @@ static int me_pagecache_dirty(struct page_state *ps, struct page *p) * who check the mapping. * This way the application knows that something went * wrong with its dirty file data. - * - * There's one open issue: - * - * The EIO will be only reported on the next IO - * operation and then cleared through the IO map. - * Normally Linux has two mechanisms to pass IO error - * first through the AS_EIO flag in the address space - * and then through the PageError flag in the page. - * Since we drop pages on memory failure handling the - * only mechanism open to use is through AS_AIO. - * - * This has the disadvantage that it gets cleared on - * the first operation that returns an error, while - * the PageError bit is more sticky and only cleared - * when the page is reread or dropped. If an - * application assumes it will always get error on - * fsync, but does other operations on the fd before - * and the page is dropped between then the error - * will not be properly reported. - * - * This can already happen even without hwpoisoned - * pages: first on metadata IO errors (which only - * report through AS_EIO) or when the page is dropped - * at the wrong time. - * - * So right now we assume that the application DTRT on - * the first EIO, but we're not worse than other parts - * of the kernel. */ mapping_set_error(mapping, -EIO); }
Nobody checks the error flag any more, so setting it accomplishes nothing. Remove the obsolete parts of this comment; it hasn't been true since errseq_t was used to track writeback errors in 2017. Cc: Miaohe Lin <linmiaohe@huawei.com> Cc: linux-mm@kvack.org Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- mm/memory-failure.c | 29 ----------------------------- 1 file changed, 29 deletions(-)