| Message ID | 20240215063649.2164017-2-hch@lst.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [01/14] writeback: don't call mapping_set_error on AOP_WRITEPAGE_ACTIVATE | expand |
On Thu 15-02-24 07:36:36, Christoph Hellwig wrote: > mapping_set_error should only be called on 0 returns (which it ignores) > or a negative error code. > > writepage_cb ends up being able to call writepage_cb on the magic > AOP_WRITEPAGE_ACTIVATE return value from ->writepage which means > success but the caller needs to unlock the page. Ignore that and > just call mapping_set_error on negative errors. > > (no fixes tag as this goes back more than 20 years over various renames > and refactors so I've given up chasing down the original introduction) > > Signed-off-by: Christoph Hellwig <hch@lst.de> Looks good. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > mm/page-writeback.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/mm/page-writeback.c b/mm/page-writeback.c > index 3f255534986a2f..703e83c69ffe08 100644 > --- a/mm/page-writeback.c > +++ b/mm/page-writeback.c > @@ -2535,7 +2535,9 @@ static int writepage_cb(struct folio *folio, struct writeback_control *wbc, > { > struct address_space *mapping = data; > int ret = mapping->a_ops->writepage(&folio->page, wbc); > - mapping_set_error(mapping, ret); > + > + if (ret < 0) > + mapping_set_error(mapping, ret); > return ret; > } > > -- > 2.39.2 >
On Thu, Feb 15, 2024 at 07:36:36AM +0100, Christoph Hellwig wrote: > mapping_set_error should only be called on 0 returns (which it ignores) > or a negative error code. > > writepage_cb ends up being able to call writepage_cb on the magic > AOP_WRITEPAGE_ACTIVATE return value from ->writepage which means > success but the caller needs to unlock the page. Ignore that and > just call mapping_set_error on negative errors. > > (no fixes tag as this goes back more than 20 years over various renames > and refactors so I've given up chasing down the original introduction) > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- Reviewed-by: Brian Foster <bfoster@redhat.com> > mm/page-writeback.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/mm/page-writeback.c b/mm/page-writeback.c > index 3f255534986a2f..703e83c69ffe08 100644 > --- a/mm/page-writeback.c > +++ b/mm/page-writeback.c > @@ -2535,7 +2535,9 @@ static int writepage_cb(struct folio *folio, struct writeback_control *wbc, > { > struct address_space *mapping = data; > int ret = mapping->a_ops->writepage(&folio->page, wbc); > - mapping_set_error(mapping, ret); > + > + if (ret < 0) > + mapping_set_error(mapping, ret); > return ret; > } > > -- > 2.39.2 > >
diff --git a/mm/page-writeback.c b/mm/page-writeback.c index 3f255534986a2f..703e83c69ffe08 100644 --- a/mm/page-writeback.c +++ b/mm/page-writeback.c @@ -2535,7 +2535,9 @@ static int writepage_cb(struct folio *folio, struct writeback_control *wbc, { struct address_space *mapping = data; int ret = mapping->a_ops->writepage(&folio->page, wbc); - mapping_set_error(mapping, ret); + + if (ret < 0) + mapping_set_error(mapping, ret); return ret; }
mapping_set_error should only be called on 0 returns (which it ignores) or a negative error code. writepage_cb ends up being able to call writepage_cb on the magic AOP_WRITEPAGE_ACTIVATE return value from ->writepage which means success but the caller needs to unlock the page. Ignore that and just call mapping_set_error on negative errors. (no fixes tag as this goes back more than 20 years over various renames and refactors so I've given up chasing down the original introduction) Signed-off-by: Christoph Hellwig <hch@lst.de> --- mm/page-writeback.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)