From: jlayton@redhat.com
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Al Viro <viro@ZenIV.linux.org.uk>, Jan Kara <jack@suse.cz>,
tytso@mit.edu, axboe@kernel.dk, mawilcox@microsoft.com,
ross.zwisler@linux.intel.com, corbet@lwn.net,
Chris Mason <clm@fb.com>, Josef Bacik <jbacik@fb.com>,
David Sterba <dsterba@suse.com>,
Carlos Maiolino <cmaiolino@redhat.com>,
Eryu Guan <eguan@redhat.com>, David Howells <dhowells@redhat.com>,
Christoph Hellwig <hch@infradead.org>,
linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org,
linux-btrfs@vger.kernel.org, linux-block@vger.kernel.org
Subject: Re: [PATCH v7 21/22] xfs: minimal conversion to errseq_t writeback error reporting
Date: Mon, 26 Jun 2017 13:58:32 -0400 [thread overview]
Message-ID: <1498499912.10049.2.camel@redhat.com> (raw)
In-Reply-To: <20170626152241.GC4733@birch.djwong.org>
On Mon, 2017-06-26 at 08:22 -0700, Darrick J. Wong wrote:
> On Fri, Jun 16, 2017 at 03:34:26PM -0400, Jeff Layton wrote:
> > Just check and advance the data errseq_t in struct file before
> > before returning from fsync on normal files. Internal filemap_*
> > callers are left as-is.
> >
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > fs/xfs/xfs_file.c | 15 +++++++++++----
> > 1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> > index 5fb5a0958a14..bc3b1575e8db 100644
> > --- a/fs/xfs/xfs_file.c
> > +++ b/fs/xfs/xfs_file.c
> > @@ -134,7 +134,7 @@ xfs_file_fsync(
> > struct inode *inode = file->f_mapping-
> > >host;
> > struct xfs_inode *ip = XFS_I(inode);
> > struct xfs_mount *mp = ip->i_mount;
> > - int error = 0;
> > + int error = 0, err2;
> > int log_flushed = 0;
> > xfs_lsn_t lsn = 0;
> >
> > @@ -142,10 +142,12 @@ xfs_file_fsync(
> >
> > error = filemap_write_and_wait_range(inode->i_mapping,
> > start, end);
> > if (error)
> > - return error;
> > + goto out;
> >
> > - if (XFS_FORCED_SHUTDOWN(mp))
> > - return -EIO;
> > + if (XFS_FORCED_SHUTDOWN(mp)) {
> > + error = -EIO;
> > + goto out;
> > + }
> >
> > xfs_iflags_clear(ip, XFS_ITRUNCATED);
> >
> > @@ -197,6 +199,11 @@ xfs_file_fsync(
> > mp->m_logdev_targp == mp->m_ddev_targp)
> > xfs_blkdev_issue_flush(mp->m_ddev_targp);
> >
> > +out:
> > + err2 = filemap_report_wb_err(file);
>
> Could we have a comment here to remind anyone reading the code a year
> from now that filemap_report_wb_err has side effects? Pre-coffee me
> was
> wondering why we'd bother calling filemap_report_wb_err in the
> XFS_FORCED_SHUTDOWN case, then remembered that it touches data
> structures.
>
> The first sentence of the commit message (really, the word 'advance')
> added as a comment was adequate to remind me of the side effects.
>
> Once that's added,
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
>
> --D
>
Yeah, definitely. I'm working on a respin of the series now to
incorporate HCH's suggestion too. I'll add that in as well.
Maybe I should rename that function to file_check_and_advance_wb_err()
? It would be good to make it clear that it does advance the errseq_t
cursor.
> > + if (!error)
> > + error = err2;
> > +
> > return error;
> > }
> >
> > --
> > 2.13.0
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-
> > xfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-
> btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: jlayton@redhat.com
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Al Viro <viro@ZenIV.linux.org.uk>, Jan Kara <jack@suse.cz>,
tytso@mit.edu, axboe@kernel.dk, mawilcox@microsoft.com,
ross.zwisler@linux.intel.com, corbet@lwn.net,
Chris Mason <clm@fb.com>, Josef Bacik <jbacik@fb.com>,
David Sterba <dsterba@suse.com>,
Carlos Maiolino <cmaiolino@redhat.com>,
Eryu Guan <eguan@redhat.com>, David Howells <dhowells@redhat.com>,
Christoph Hellwig <hch@infradead.org>,
linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org,
linux-btrfs@vger.kernel.org, linux-block@vger.kernel.org
Subject: Re: [PATCH v7 21/22] xfs: minimal conversion to errseq_t writeback error reporting
Date: Mon, 26 Jun 2017 13:58:32 -0400 [thread overview]
Message-ID: <1498499912.10049.2.camel@redhat.com> (raw)
In-Reply-To: <20170626152241.GC4733@birch.djwong.org>
On Mon, 2017-06-26 at 08:22 -0700, Darrick J. Wong wrote:
> On Fri, Jun 16, 2017 at 03:34:26PM -0400, Jeff Layton wrote:
> > Just check and advance the data errseq_t in struct file before
> > before returning from fsync on normal files. Internal filemap_*
> > callers are left as-is.
> >
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > fs/xfs/xfs_file.c | 15 +++++++++++----
> > 1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> > index 5fb5a0958a14..bc3b1575e8db 100644
> > --- a/fs/xfs/xfs_file.c
> > +++ b/fs/xfs/xfs_file.c
> > @@ -134,7 +134,7 @@ xfs_file_fsync(
> > struct inode *inode = file->f_mapping-
> > >host;
> > struct xfs_inode *ip = XFS_I(inode);
> > struct xfs_mount *mp = ip->i_mount;
> > - int error = 0;
> > + int error = 0, err2;
> > int log_flushed = 0;
> > xfs_lsn_t lsn = 0;
> >
> > @@ -142,10 +142,12 @@ xfs_file_fsync(
> >
> > error = filemap_write_and_wait_range(inode->i_mapping,
> > start, end);
> > if (error)
> > - return error;
> > + goto out;
> >
> > - if (XFS_FORCED_SHUTDOWN(mp))
> > - return -EIO;
> > + if (XFS_FORCED_SHUTDOWN(mp)) {
> > + error = -EIO;
> > + goto out;
> > + }
> >
> > xfs_iflags_clear(ip, XFS_ITRUNCATED);
> >
> > @@ -197,6 +199,11 @@ xfs_file_fsync(
> > mp->m_logdev_targp == mp->m_ddev_targp)
> > xfs_blkdev_issue_flush(mp->m_ddev_targp);
> >
> > +out:
> > + err2 = filemap_report_wb_err(file);
>
> Could we have a comment here to remind anyone reading the code a year
> from now that filemap_report_wb_err has side effects? Pre-coffee me
> was
> wondering why we'd bother calling filemap_report_wb_err in the
> XFS_FORCED_SHUTDOWN case, then remembered that it touches data
> structures.
>
> The first sentence of the commit message (really, the word 'advance')
> added as a comment was adequate to remind me of the side effects.
>
> Once that's added,
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
>
> --D
>
Yeah, definitely. I'm working on a respin of the series now to
incorporate HCH's suggestion too. I'll add that in as well.
Maybe I should rename that function to file_check_and_advance_wb_err()
? It would be good to make it clear that it does advance the errseq_t
cursor.
> > + if (!error)
> > + error = err2;
> > +
> > return error;
> > }
> >
> > --
> > 2.13.0
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-
> > xfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-
> btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
WARNING: multiple messages have this Message-ID (diff)
From: jlayton@redhat.com
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Al Viro <viro@ZenIV.linux.org.uk>, Jan Kara <jack@suse.cz>,
tytso@mit.edu, axboe@kernel.dk, mawilcox@microsoft.com,
ross.zwisler@linux.intel.com, corbet@lwn.net,
Chris Mason <clm@fb.com>, Josef Bacik <jbacik@fb.com>,
David Sterba <dsterba@suse.com>,
Carlos Maiolino <cmaiolino@redhat.com>,
Eryu Guan <eguan@redhat.com>, David Howells <dhowells@redhat.com>,
Christoph Hellwig <hch@infradead.org>,
linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org,
linux-btrfs@vger.kernel.org, linux-block@vger.kernel.org
Subject: Re: [PATCH v7 21/22] xfs: minimal conversion to errseq_t writeback error reporting
Date: Mon, 26 Jun 2017 13:58:32 -0400 [thread overview]
Message-ID: <1498499912.10049.2.camel@redhat.com> (raw)
In-Reply-To: <20170626152241.GC4733@birch.djwong.org>
On Mon, 2017-06-26 at 08:22 -0700, Darrick J. Wong wrote:
> On Fri, Jun 16, 2017 at 03:34:26PM -0400, Jeff Layton wrote:
> > Just check and advance the data errseq_t in struct file before
> > before returning from fsync on normal files. Internal filemap_*
> > callers are left as-is.
> >
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > fs/xfs/xfs_file.c | 15 +++++++++++----
> > 1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> > index 5fb5a0958a14..bc3b1575e8db 100644
> > --- a/fs/xfs/xfs_file.c
> > +++ b/fs/xfs/xfs_file.c
> > @@ -134,7 +134,7 @@ xfs_file_fsync(
> > struct inode *inode = file->f_mapping-
> > >host;
> > struct xfs_inode *ip = XFS_I(inode);
> > struct xfs_mount *mp = ip->i_mount;
> > - int error = 0;
> > + int error = 0, err2;
> > int log_flushed = 0;
> > xfs_lsn_t lsn = 0;
> >
> > @@ -142,10 +142,12 @@ xfs_file_fsync(
> >
> > error = filemap_write_and_wait_range(inode->i_mapping,
> > start, end);
> > if (error)
> > - return error;
> > + goto out;
> >
> > - if (XFS_FORCED_SHUTDOWN(mp))
> > - return -EIO;
> > + if (XFS_FORCED_SHUTDOWN(mp)) {
> > + error = -EIO;
> > + goto out;
> > + }
> >
> > xfs_iflags_clear(ip, XFS_ITRUNCATED);
> >
> > @@ -197,6 +199,11 @@ xfs_file_fsync(
> > mp->m_logdev_targp == mp->m_ddev_targp)
> > xfs_blkdev_issue_flush(mp->m_ddev_targp);
> >
> > +out:
> > + err2 = filemap_report_wb_err(file);
>
> Could we have a comment here to remind anyone reading the code a year
> from now that filemap_report_wb_err has side effects? Pre-coffee me
> was
> wondering why we'd bother calling filemap_report_wb_err in the
> XFS_FORCED_SHUTDOWN case, then remembered that it touches data
> structures.
>
> The first sentence of the commit message (really, the word 'advance')
> added as a comment was adequate to remind me of the side effects.
>
> Once that's added,
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
>
> --D
>
Yeah, definitely. I'm working on a respin of the series now to
incorporate HCH's suggestion too. I'll add that in as well.
Maybe I should rename that function to file_check_and_advance_wb_err()
? It would be good to make it clear that it does advance the errseq_t
cursor.
> > + if (!error)
> > + error = err2;
> > +
> > return error;
> > }
> >
> > --
> > 2.13.0
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-
> > xfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-
> btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2017-06-26 17:58 UTC|newest]
Thread overview: 108+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-16 19:34 [PATCH v7 00/22] fs: enhanced writeback error reporting with errseq_t (pile #1) Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-16 19:34 ` [PATCH v7 01/22] fs: remove call_fsync helper function Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-20 12:31 ` Christoph Hellwig
2017-06-20 12:31 ` Christoph Hellwig
2017-06-20 15:33 ` Jan Kara
2017-06-20 15:33 ` Jan Kara
2017-06-26 8:05 ` Carlos Maiolino
2017-06-26 8:05 ` Carlos Maiolino
2017-06-16 19:34 ` [PATCH v7 02/22] buffer: use mapping_set_error instead of setting the flag Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-16 19:34 ` [PATCH v7 03/22] fs: check for writeback errors after syncing out buffers in generic_file_fsync Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-16 19:34 ` [PATCH v7 04/22] buffer: set errors in mapping at the time that the error occurs Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-26 8:19 ` Carlos Maiolino
2017-06-26 8:19 ` Carlos Maiolino
2017-06-16 19:34 ` [PATCH v7 05/22] jbd2: don't clear and reset errors after waiting on writeback Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-20 15:32 ` Jan Kara
2017-06-20 15:32 ` Jan Kara
2017-06-26 8:23 ` Carlos Maiolino
2017-06-26 8:23 ` Carlos Maiolino
2017-06-16 19:34 ` [PATCH v7 06/22] mm: clear AS_EIO/AS_ENOSPC when writeback initiation fails Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-16 19:34 ` [PATCH v7 07/22] mm: don't TestClearPageError in __filemap_fdatawait_range Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-16 19:34 ` [PATCH v7 08/22] mm: clean up error handling in write_one_page Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-16 19:34 ` [PATCH v7 09/22] fs: always sync metadata in __generic_file_fsync Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-16 19:34 ` [PATCH v7 10/22] lib: add errseq_t type and infrastructure for handling it Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-16 19:34 ` [PATCH v7 11/22] fs: new infrastructure for writeback error handling and reporting Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-20 12:34 ` Christoph Hellwig
2017-06-20 12:34 ` Christoph Hellwig
2017-06-20 12:56 ` Jeff Layton
2017-06-20 12:56 ` Jeff Layton
2017-06-20 12:56 ` Jeff Layton
2017-06-20 12:56 ` Jeff Layton
2017-06-16 19:34 ` [PATCH v7 12/22] mm: tracepoints for writeback error events Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-16 19:34 ` [PATCH v7 13/22] mm: set both AS_EIO/AS_ENOSPC and errseq_t in mapping_set_error Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-16 19:34 ` [PATCH v7 14/22] Documentation: flesh out the section in vfs.txt on storing and reporting writeback errors Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-16 19:34 ` [PATCH v7 15/22] dax: set errors in mapping when writeback fails Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-17 12:39 ` Jeff Layton
2017-06-17 12:39 ` Jeff Layton
2017-06-17 12:39 ` Jeff Layton
2017-06-17 12:39 ` Jeff Layton
2017-06-19 17:48 ` Ross Zwisler
2017-06-19 17:48 ` Ross Zwisler
2017-06-16 19:34 ` [PATCH v7 16/22] block: convert to errseq_t based writeback error tracking Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-20 12:35 ` Christoph Hellwig
2017-06-20 12:35 ` Christoph Hellwig
2017-06-20 17:44 ` Jeff Layton
2017-06-20 17:44 ` Jeff Layton
2017-06-20 17:44 ` Jeff Layton
2017-06-20 17:44 ` Jeff Layton
2017-06-24 11:59 ` Christoph Hellwig
2017-06-24 11:59 ` Christoph Hellwig
2017-06-24 13:16 ` Jeff Layton
2017-06-24 13:16 ` Jeff Layton
2017-06-24 13:16 ` Jeff Layton
2017-06-24 13:16 ` Jeff Layton
2017-06-26 14:34 ` Jeff Layton
2017-06-26 14:34 ` Jeff Layton
2017-06-26 14:34 ` Jeff Layton
2017-06-26 14:34 ` Jeff Layton
2017-06-27 15:20 ` Christoph Hellwig
2017-06-27 15:20 ` Christoph Hellwig
2017-06-16 19:34 ` [PATCH v7 17/22] ext4: use errseq_t based error handling for reporting data writeback errors Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-16 19:34 ` [PATCH v7 18/22] fs: add f_md_wb_err field to struct file for tracking metadata errors Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-16 19:34 ` [PATCH v7 19/22] ext4: add more robust reporting of metadata writeback errors Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-16 19:34 ` [PATCH v7 20/22] ext2: convert to errseq_t based writeback error tracking Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-16 19:34 ` [PATCH v7 21/22] xfs: minimal conversion to errseq_t writeback error reporting Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-26 13:40 ` Carlos Maiolino
2017-06-26 13:40 ` Carlos Maiolino
2017-06-26 15:22 ` Darrick J. Wong
2017-06-26 15:22 ` Darrick J. Wong
2017-06-26 17:58 ` jlayton [this message]
2017-06-26 17:58 ` jlayton
2017-06-26 17:58 ` jlayton
2017-06-26 17:58 ` jlayton
2017-06-26 18:10 ` Darrick J. Wong
2017-06-26 18:10 ` Darrick J. Wong
2017-06-16 19:34 ` [PATCH v7 22/22] btrfs: minimal conversion to errseq_t writeback error reporting on fsync Jeff Layton
2017-06-16 19:34 ` Jeff Layton
2017-06-19 16:23 ` [PATCH v7 00/22] fs: enhanced writeback error reporting with errseq_t (pile #1) Jeff Layton
2017-06-19 16:23 ` Jeff Layton
2017-06-19 16:23 ` Jeff Layton
2017-06-19 16:23 ` Jeff Layton
2017-06-19 23:25 ` Stephen Rothwell
2017-06-19 23:25 ` Stephen Rothwell
2017-06-20 10:16 ` Jeff Layton
2017-06-20 10:16 ` Jeff Layton
2017-06-20 10:16 ` Jeff Layton
2017-06-20 10:16 ` Jeff Layton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1498499912.10049.2.camel@redhat.com \
--to=jlayton@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=axboe@kernel.dk \
--cc=clm@fb.com \
--cc=cmaiolino@redhat.com \
--cc=corbet@lwn.net \
--cc=darrick.wong@oracle.com \
--cc=dhowells@redhat.com \
--cc=dsterba@suse.com \
--cc=eguan@redhat.com \
--cc=hch@infradead.org \
--cc=jack@suse.cz \
--cc=jbacik@fb.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-xfs@vger.kernel.org \
--cc=mawilcox@microsoft.com \
--cc=ross.zwisler@linux.intel.com \
--cc=tytso@mit.edu \
--cc=viro@ZenIV.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.