On Thu, 2019-01-10 at 14:24 -0800, Pavel Shilovsky wrote: > This patch aims to address writeback code problems related to error > paths. In particular it respects EINTR and related error codes and > stores the first error occured during writeback in order to return it > to a caller. > The current semantic with respect to fsync is to return the last error code. > Signed-off-by: Pavel Shilovsky <pshilov@xxxxxxxxxxxxx> > --- > fs/cifs/cifsglob.h | 19 +++++++++++++++++++ > fs/cifs/cifssmb.c | 7 ++++--- > fs/cifs/file.c | 29 +++++++++++++++++++++++------ > fs/cifs/inode.c | 10 ++++++++++ > 4 files changed, 56 insertions(+), 9 deletions(-) > > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h > index 7709268..94dbdbe 100644 > --- a/fs/cifs/cifsglob.h > +++ b/fs/cifs/cifsglob.h > @@ -1575,6 +1575,25 @@ static inline void free_dfs_info_array(struct dfs_info3_param *param, > kfree(param); > } > > +static inline bool is_interrupt_error(int error) > +{ > + switch (error) { > + case -EINTR: > + case -ERESTARTSYS: > + case -ERESTARTNOHAND: > + case -ERESTARTNOINTR: > + return true; > + } > + return false; > +} > + > +static inline bool is_retryable_error(int error) > +{ > + if (is_interrupt_error(error) || error == -EAGAIN) > + return true; > + return false; > +} > + > #define MID_FREE 0 > #define MID_REQUEST_ALLOCATED 1 > #define MID_REQUEST_SUBMITTED 2 > diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c > index b1f49c1..6930cdb 100644 > --- a/fs/cifs/cifssmb.c > +++ b/fs/cifs/cifssmb.c > @@ -2114,7 +2114,7 @@ cifs_writev_requeue(struct cifs_writedata *wdata) > > for (j = 0; j < nr_pages; j++) { > unlock_page(wdata2->pages[j]); > - if (rc != 0 && rc != -EAGAIN) { > + if (rc != 0 && !is_retryable_error(rc)) { > SetPageError(wdata2->pages[j]); > end_page_writeback(wdata2->pages[j]); > put_page(wdata2->pages[j]); > @@ -2123,7 +2123,7 @@ cifs_writev_requeue(struct cifs_writedata *wdata) > > if (rc) { > kref_put(&wdata2->refcount, cifs_writedata_release); > - if (rc == -EAGAIN) > + if (is_retryable_error(rc)) > continue; > break; > } > @@ -2132,7 +2132,8 @@ cifs_writev_requeue(struct cifs_writedata *wdata) > i += nr_pages; > } while (i < wdata->nr_pages); > > - mapping_set_error(inode->i_mapping, rc); > + if (rc != 0 && !is_retryable_error(rc)) > + mapping_set_error(inode->i_mapping, rc); > kref_put(&wdata->refcount, cifs_writedata_release); > } > > diff --git a/fs/cifs/file.c b/fs/cifs/file.c > index c23bf9d..109b1ef 100644 > --- a/fs/cifs/file.c > +++ b/fs/cifs/file.c > @@ -732,7 +732,8 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flush) > > if (can_flush) { > rc = filemap_write_and_wait(inode->i_mapping); > - mapping_set_error(inode->i_mapping, rc); > + if (!is_interrupt_error(rc)) > + mapping_set_error(inode->i_mapping, rc); > > if (tcon->unix_ext) > rc = cifs_get_inode_info_unix(&inode, full_path, > @@ -2109,6 +2110,7 @@ static int cifs_writepages(struct address_space *mapping, > pgoff_t end, index; > struct cifs_writedata *wdata; > int rc = 0; > + int saved_rc = 0; > unsigned int xid; > > /* > @@ -2137,8 +2139,10 @@ static int cifs_writepages(struct address_space *mapping, > > rc = server->ops->wait_mtu_credits(server, cifs_sb->wsize, > &wsize, &credits); > - if (rc) > + if (rc != 0) { > + done = true; > break; > + } > > tofind = min((wsize / PAGE_SIZE) - 1, end - index) + 1; > > @@ -2146,6 +2150,7 @@ static int cifs_writepages(struct address_space *mapping, > &found_pages); > if (!wdata) { > rc = -ENOMEM; > + done = true; > add_credits_and_wake_if(server, credits, 0); > break; > } > @@ -2174,7 +2179,7 @@ static int cifs_writepages(struct address_space *mapping, > if (rc != 0) { > add_credits_and_wake_if(server, wdata->credits, 0); > for (i = 0; i < nr_pages; ++i) { > - if (rc == -EAGAIN) > + if (is_retryable_error(rc)) > redirty_page_for_writepage(wbc, > wdata->pages[i]); > else > @@ -2182,7 +2187,7 @@ static int cifs_writepages(struct address_space *mapping, > end_page_writeback(wdata->pages[i]); > put_page(wdata->pages[i]); > } > - if (rc != -EAGAIN) > + if (!is_retryable_error(rc)) > mapping_set_error(mapping, rc); > } > kref_put(&wdata->refcount, cifs_writedata_release); > @@ -2192,6 +2197,15 @@ static int cifs_writepages(struct address_space *mapping, > continue; > } > > + /* Return immediately if we received a signal during writing */ > + if (is_interrupt_error(rc)) { > + done = true; > + break; > + } > + > + if (rc != 0 && saved_rc == 0) > + saved_rc = rc; > + > wbc->nr_to_write -= nr_pages; > if (wbc->nr_to_write <= 0) > done = true; > @@ -2209,6 +2223,9 @@ static int cifs_writepages(struct address_space *mapping, > goto retry; > } > > + if (saved_rc != 0) > + rc = saved_rc; > + > if (wbc->range_cyclic || (range_whole && wbc->nr_to_write > 0)) > mapping->writeback_index = index; > > @@ -2241,8 +2258,8 @@ 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) { > - if (wbc->sync_mode == WB_SYNC_ALL) > + if (is_retryable_error(rc)) { > + if (wbc->sync_mode == WB_SYNC_ALL && rc == -EAGAIN) > goto retry_write; > redirty_page_for_writepage(wbc, page); > } else if (rc != 0) { > diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c > index 5b883a0..ba1152b 100644 > --- a/fs/cifs/inode.c > +++ b/fs/cifs/inode.c > @@ -2260,6 +2260,11 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs) > * the flush returns error? > */ > rc = filemap_write_and_wait(inode->i_mapping); > + if (is_interrupt_error(rc)) { > + rc = -ERESTARTSYS; > + goto out; > + } > + > mapping_set_error(inode->i_mapping, rc); > rc = 0; > > @@ -2403,6 +2408,11 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs) > * the flush returns error? > */ > rc = filemap_write_and_wait(inode->i_mapping); > + if (is_interrupt_error(rc)) { > + rc = -ERESTARTSYS; > + goto cifs_setattr_exit; > + } > + > mapping_set_error(inode->i_mapping, rc); > rc = 0; > Took me a minute to look over this code again, but I think this is OK. Acked-by: Jeff Layton <jlayton@xxxxxxxxxx>