Re: [Jfs-discussion] [PATCH v3 11/20] cifs: set mapping error when page writeback fails in writepage or launder_pages

2017-04-24 Thread Jeff Layton
On Mon, 2017-04-24 at 08:27 -0700, Christoph Hellwig wrote:
> On Mon, Apr 24, 2017 at 09:22:50AM -0400, Jeff Layton wrote:
> > Signed-off-by: Jeff Layton 
> > ---
> >  fs/cifs/file.c | 10 ++
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> > index 21d404535739..4b696a23b0b1 100644
> > --- a/fs/cifs/file.c
> > +++ b/fs/cifs/file.c
> > @@ -2234,14 +2234,16 @@ cifs_writepage_locked(struct page *page, struct 
> > writeback_control *wbc)
> > set_page_writeback(page);
> >  retry_write:
> > rc = cifs_partialpagewrite(page, 0, PAGE_SIZE);
> > +   if (rc == -EAGAIN && wbc->sync_mode == WB_SYNC_ALL) {
> > goto retry_write;
> > +   } else if (rc == -EAGAIN) {
> > redirty_page_for_writepage(wbc, page);
> > +   } else if (rc != 0) {
> > SetPageError(page);
> > +   mapping_set_error(page->mapping, rc);
> > +   } else {
> > SetPageUptodate(page);
> > +   }
> 
> Hmmm.  I might be a little too nitpicky, but I hate having the same
> partial condition duplicated if possible.  Why not:
> 
>   if (rc == -EAGAIN) {
>   if (wbc->sync_mode == WB_SYNC_ALL)
>   goto retry_write;
>   redirty_page_for_writepage(wbc, page);
>   } else if (rc) {
>   SetPageError(page);
>   mapping_set_error(page->mapping, rc);
>   } else {
>   SetPageUptodate(page);
>   }
> 
> Otherwise this looks fine to me:
> 
> Reviewed-by: Christoph Hellwig 

No, you're absolutely right. I merged that change into the patch.

Thanks for the review so far!
-- 
Jeff Layton 

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Jfs-discussion mailing list
Jfs-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/jfs-discussion


[Jfs-discussion] [PATCH v3 11/20] cifs: set mapping error when page writeback fails in writepage or launder_pages

2017-04-24 Thread Jeff Layton
Signed-off-by: Jeff Layton 
---
 fs/cifs/file.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 21d404535739..4b696a23b0b1 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2234,14 +2234,16 @@ cifs_writepage_locked(struct page *page, struct 
writeback_control *wbc)
set_page_writeback(page);
 retry_write:
rc = cifs_partialpagewrite(page, 0, PAGE_SIZE);
-   if (rc == -EAGAIN && wbc->sync_mode == WB_SYNC_ALL)
+   if (rc == -EAGAIN && wbc->sync_mode == WB_SYNC_ALL) {
goto retry_write;
-   else if (rc == -EAGAIN)
+   } else if (rc == -EAGAIN) {
redirty_page_for_writepage(wbc, page);
-   else if (rc != 0)
+   } else if (rc != 0) {
SetPageError(page);
-   else
+   mapping_set_error(page->mapping, rc);
+   } else {
SetPageUptodate(page);
+   }
end_page_writeback(page);
put_page(page);
free_xid(xid);
-- 
2.9.3


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Jfs-discussion mailing list
Jfs-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/jfs-discussion