* [RFC PATCH 0/2] xfs: deprecate barrier/nobarrier @ 2016-11-30 22:54 Dave Chinner 2016-11-30 22:54 ` [PATCH 1/2] xfs: Always flush caches when integrity is required Dave Chinner 2016-11-30 22:54 ` [PATCH 2/2] xfs: deprecate barrier/nobarrier mount option Dave Chinner 0 siblings, 2 replies; 11+ messages in thread From: Dave Chinner @ 2016-11-30 22:54 UTC (permalink / raw) To: linux-xfs Hi folks, In response to the recent conversation here: https://www.spinics.net/lists/linux-xfs/msg02501.html I've come to the conclusion that barrier/nobarrier mount options no longer make sense to maintain. There is no performance penalty for the vast majority of users to enabling barriersand it's been the default behaviour for a long time. And given the misunderstanding that users have over what barriers are and when it is safe to turn them off, it seems to me like the best solution is to simply remove the option altogther. Comments, thoughts, flames all welcome. -Dave. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] xfs: Always flush caches when integrity is required 2016-11-30 22:54 [RFC PATCH 0/2] xfs: deprecate barrier/nobarrier Dave Chinner @ 2016-11-30 22:54 ` Dave Chinner 2016-12-01 12:47 ` Brian Foster 2016-12-05 16:22 ` Christoph Hellwig 2016-11-30 22:54 ` [PATCH 2/2] xfs: deprecate barrier/nobarrier mount option Dave Chinner 1 sibling, 2 replies; 11+ messages in thread From: Dave Chinner @ 2016-11-30 22:54 UTC (permalink / raw) To: linux-xfs From: Dave Chinner <dchinner@redhat.com> There is no reason anymore for not issuing device integrity operations when teh filesystem requires ordering or data integrity guarantees. We should always issue cache flushes and FUA writes where necessary and let the underlying storage optimise them as necessary for correct integrity operation. Signed-Off-By: Dave Chinner <dchinner@redhat.com> --- fs/xfs/xfs_buf.c | 3 +-- fs/xfs/xfs_file.c | 29 ++++++++++++----------------- fs/xfs/xfs_log.c | 36 +++++++++++++++--------------------- 3 files changed, 28 insertions(+), 40 deletions(-) diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c index a2f0648743db..1264908ef8f2 100644 --- a/fs/xfs/xfs_buf.c +++ b/fs/xfs/xfs_buf.c @@ -1738,8 +1738,7 @@ xfs_free_buftarg( percpu_counter_destroy(&btp->bt_io_count); list_lru_destroy(&btp->bt_lru); - if (mp->m_flags & XFS_MOUNT_BARRIER) - xfs_blkdev_issue_flush(btp); + xfs_blkdev_issue_flush(btp); kmem_free(btp); } diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index f5effa68e037..2951c483b24b 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -149,19 +149,16 @@ xfs_file_fsync( xfs_iflags_clear(ip, XFS_ITRUNCATED); - if (mp->m_flags & XFS_MOUNT_BARRIER) { - /* - * If we have an RT and/or log subvolume we need to make sure - * to flush the write cache the device used for file data - * first. This is to ensure newly written file data make - * it to disk before logging the new inode size in case of - * an extending write. - */ - if (XFS_IS_REALTIME_INODE(ip)) - xfs_blkdev_issue_flush(mp->m_rtdev_targp); - else if (mp->m_logdev_targp != mp->m_ddev_targp) - xfs_blkdev_issue_flush(mp->m_ddev_targp); - } + /* + * If we have an RT and/or log subvolume we need to make sure to flush + * the write cache the device used for file data first. This is to + * ensure newly written file data make it to disk before logging the new + * inode size in case of an extending write. + */ + if (XFS_IS_REALTIME_INODE(ip)) + xfs_blkdev_issue_flush(mp->m_rtdev_targp); + else if (mp->m_logdev_targp != mp->m_ddev_targp) + xfs_blkdev_issue_flush(mp->m_ddev_targp); /* * All metadata updates are logged, which means that we just have to @@ -196,10 +193,8 @@ xfs_file_fsync( * an already allocated file and thus do not have any metadata to * commit. */ - if ((mp->m_flags & XFS_MOUNT_BARRIER) && - mp->m_logdev_targp == mp->m_ddev_targp && - !XFS_IS_REALTIME_INODE(ip) && - !log_flushed) + if (!log_flushed && !XFS_IS_REALTIME_INODE(ip) && + mp->m_logdev_targp == mp->m_ddev_targp) xfs_blkdev_issue_flush(mp->m_ddev_targp); return error; diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c index 3ebe444eb60f..573d0841851d 100644 --- a/fs/xfs/xfs_log.c +++ b/fs/xfs/xfs_log.c @@ -1863,25 +1863,21 @@ xlog_sync( bp->b_io_length = BTOBB(count); bp->b_fspriv = iclog; bp->b_flags &= ~(XBF_FUA | XBF_FLUSH); - bp->b_flags |= (XBF_ASYNC | XBF_SYNCIO | XBF_WRITE); + bp->b_flags |= (XBF_ASYNC | XBF_SYNCIO | XBF_WRITE | XBF_FUA); - if (log->l_mp->m_flags & XFS_MOUNT_BARRIER) { - bp->b_flags |= XBF_FUA; - - /* - * Flush the data device before flushing the log to make - * sure all meta data written back from the AIL actually made - * it to disk before stamping the new log tail LSN into the - * log buffer. For an external log we need to issue the - * flush explicitly, and unfortunately synchronously here; - * for an internal log we can simply use the block layer - * state machine for preflushes. - */ - if (log->l_mp->m_logdev_targp != log->l_mp->m_ddev_targp) - xfs_blkdev_issue_flush(log->l_mp->m_ddev_targp); - else - bp->b_flags |= XBF_FLUSH; - } + /* + * Flush the data device before flushing the log to make + * sure all meta data written back from the AIL actually made + * it to disk before stamping the new log tail LSN into the + * log buffer. For an external log we need to issue the + * flush explicitly, and unfortunately synchronously here; + * for an internal log we can simply use the block layer + * state machine for preflushes. + */ + if (log->l_mp->m_logdev_targp != log->l_mp->m_ddev_targp) + xfs_blkdev_issue_flush(log->l_mp->m_ddev_targp); + else + bp->b_flags |= XBF_FLUSH; ASSERT(XFS_BUF_ADDR(bp) <= log->l_logBBsize-1); ASSERT(XFS_BUF_ADDR(bp) + BTOBB(count) <= log->l_logBBsize); @@ -1907,9 +1903,7 @@ xlog_sync( (char *)&iclog->ic_header + count, split); bp->b_fspriv = iclog; bp->b_flags &= ~(XBF_FUA | XBF_FLUSH); - bp->b_flags |= (XBF_ASYNC | XBF_SYNCIO | XBF_WRITE); - if (log->l_mp->m_flags & XFS_MOUNT_BARRIER) - bp->b_flags |= XBF_FUA; + bp->b_flags |= (XBF_ASYNC | XBF_SYNCIO | XBF_WRITE | XBF_FUA); ASSERT(XFS_BUF_ADDR(bp) <= log->l_logBBsize-1); ASSERT(XFS_BUF_ADDR(bp) + BTOBB(count) <= log->l_logBBsize); -- 2.10.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] xfs: Always flush caches when integrity is required 2016-11-30 22:54 ` [PATCH 1/2] xfs: Always flush caches when integrity is required Dave Chinner @ 2016-12-01 12:47 ` Brian Foster 2016-12-05 16:22 ` Christoph Hellwig 1 sibling, 0 replies; 11+ messages in thread From: Brian Foster @ 2016-12-01 12:47 UTC (permalink / raw) To: Dave Chinner; +Cc: linux-xfs On Thu, Dec 01, 2016 at 09:54:43AM +1100, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > There is no reason anymore for not issuing device integrity > operations when teh filesystem requires ordering or data integrity > guarantees. We should always issue cache flushes and FUA writes > where necessary and let the underlying storage optimise them as > necessary for correct integrity operation. > > Signed-Off-By: Dave Chinner <dchinner@redhat.com> > --- Seems reasonable to me. Just a few nits below.. > fs/xfs/xfs_buf.c | 3 +-- > fs/xfs/xfs_file.c | 29 ++++++++++++----------------- > fs/xfs/xfs_log.c | 36 +++++++++++++++--------------------- > 3 files changed, 28 insertions(+), 40 deletions(-) > > diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c > index a2f0648743db..1264908ef8f2 100644 > --- a/fs/xfs/xfs_buf.c > +++ b/fs/xfs/xfs_buf.c > @@ -1738,8 +1738,7 @@ xfs_free_buftarg( > percpu_counter_destroy(&btp->bt_io_count); > list_lru_destroy(&btp->bt_lru); > > - if (mp->m_flags & XFS_MOUNT_BARRIER) > - xfs_blkdev_issue_flush(btp); > + xfs_blkdev_issue_flush(btp); > > kmem_free(btp); > } > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c > index f5effa68e037..2951c483b24b 100644 > --- a/fs/xfs/xfs_file.c > +++ b/fs/xfs/xfs_file.c > @@ -149,19 +149,16 @@ xfs_file_fsync( > > xfs_iflags_clear(ip, XFS_ITRUNCATED); > > - if (mp->m_flags & XFS_MOUNT_BARRIER) { > - /* > - * If we have an RT and/or log subvolume we need to make sure > - * to flush the write cache the device used for file data > - * first. This is to ensure newly written file data make > - * it to disk before logging the new inode size in case of > - * an extending write. > - */ > - if (XFS_IS_REALTIME_INODE(ip)) > - xfs_blkdev_issue_flush(mp->m_rtdev_targp); > - else if (mp->m_logdev_targp != mp->m_ddev_targp) > - xfs_blkdev_issue_flush(mp->m_ddev_targp); > - } > + /* > + * If we have an RT and/or log subvolume we need to make sure to flush > + * the write cache the device used for file data first. This is to > + * ensure newly written file data make it to disk before logging the new > + * inode size in case of an extending write. > + */ > + if (XFS_IS_REALTIME_INODE(ip)) > + xfs_blkdev_issue_flush(mp->m_rtdev_targp); > + else if (mp->m_logdev_targp != mp->m_ddev_targp) > + xfs_blkdev_issue_flush(mp->m_ddev_targp); > > /* > * All metadata updates are logged, which means that we just have to > @@ -196,10 +193,8 @@ xfs_file_fsync( > * an already allocated file and thus do not have any metadata to > * commit. > */ > - if ((mp->m_flags & XFS_MOUNT_BARRIER) && > - mp->m_logdev_targp == mp->m_ddev_targp && > - !XFS_IS_REALTIME_INODE(ip) && > - !log_flushed) > + if (!log_flushed && !XFS_IS_REALTIME_INODE(ip) && > + mp->m_logdev_targp == mp->m_ddev_targp) > xfs_blkdev_issue_flush(mp->m_ddev_targp); > > return error; > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c > index 3ebe444eb60f..573d0841851d 100644 > --- a/fs/xfs/xfs_log.c > +++ b/fs/xfs/xfs_log.c > @@ -1863,25 +1863,21 @@ xlog_sync( > bp->b_io_length = BTOBB(count); > bp->b_fspriv = iclog; > bp->b_flags &= ~(XBF_FUA | XBF_FLUSH); > - bp->b_flags |= (XBF_ASYNC | XBF_SYNCIO | XBF_WRITE); > + bp->b_flags |= (XBF_ASYNC | XBF_SYNCIO | XBF_WRITE | XBF_FUA); There's no need to clear XBF_FUA on the line above any longer. > > - if (log->l_mp->m_flags & XFS_MOUNT_BARRIER) { > - bp->b_flags |= XBF_FUA; > - > - /* > - * Flush the data device before flushing the log to make > - * sure all meta data written back from the AIL actually made > - * it to disk before stamping the new log tail LSN into the > - * log buffer. For an external log we need to issue the > - * flush explicitly, and unfortunately synchronously here; > - * for an internal log we can simply use the block layer > - * state machine for preflushes. > - */ > - if (log->l_mp->m_logdev_targp != log->l_mp->m_ddev_targp) > - xfs_blkdev_issue_flush(log->l_mp->m_ddev_targp); > - else > - bp->b_flags |= XBF_FLUSH; > - } > + /* > + * Flush the data device before flushing the log to make > + * sure all meta data written back from the AIL actually made > + * it to disk before stamping the new log tail LSN into the > + * log buffer. For an external log we need to issue the > + * flush explicitly, and unfortunately synchronously here; > + * for an internal log we can simply use the block layer > + * state machine for preflushes. > + */ Comment can be rewrapped. > + if (log->l_mp->m_logdev_targp != log->l_mp->m_ddev_targp) > + xfs_blkdev_issue_flush(log->l_mp->m_ddev_targp); > + else > + bp->b_flags |= XBF_FLUSH; > > ASSERT(XFS_BUF_ADDR(bp) <= log->l_logBBsize-1); > ASSERT(XFS_BUF_ADDR(bp) + BTOBB(count) <= log->l_logBBsize); > @@ -1907,9 +1903,7 @@ xlog_sync( > (char *)&iclog->ic_header + count, split); > bp->b_fspriv = iclog; > bp->b_flags &= ~(XBF_FUA | XBF_FLUSH); No need to clear XBF_FUA here as well. Brian > - bp->b_flags |= (XBF_ASYNC | XBF_SYNCIO | XBF_WRITE); > - if (log->l_mp->m_flags & XFS_MOUNT_BARRIER) > - bp->b_flags |= XBF_FUA; > + bp->b_flags |= (XBF_ASYNC | XBF_SYNCIO | XBF_WRITE | XBF_FUA); > > ASSERT(XFS_BUF_ADDR(bp) <= log->l_logBBsize-1); > ASSERT(XFS_BUF_ADDR(bp) + BTOBB(count) <= log->l_logBBsize); > -- > 2.10.2 > > -- > 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] xfs: Always flush caches when integrity is required 2016-11-30 22:54 ` [PATCH 1/2] xfs: Always flush caches when integrity is required Dave Chinner 2016-12-01 12:47 ` Brian Foster @ 2016-12-05 16:22 ` Christoph Hellwig 1 sibling, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2016-12-05 16:22 UTC (permalink / raw) To: Dave Chinner; +Cc: linux-xfs On Thu, Dec 01, 2016 at 09:54:43AM +1100, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > There is no reason anymore for not issuing device integrity > operations when teh filesystem requires ordering or data integrity > guarantees. We should always issue cache flushes and FUA writes > where necessary and let the underlying storage optimise them as > necessary for correct integrity operation. > > Signed-Off-By: Dave Chinner <dchinner@redhat.com> Looks fine, Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] xfs: deprecate barrier/nobarrier mount option 2016-11-30 22:54 [RFC PATCH 0/2] xfs: deprecate barrier/nobarrier Dave Chinner 2016-11-30 22:54 ` [PATCH 1/2] xfs: Always flush caches when integrity is required Dave Chinner @ 2016-11-30 22:54 ` Dave Chinner 2016-12-01 9:54 ` Jan Kara ` (2 more replies) 1 sibling, 3 replies; 11+ messages in thread From: Dave Chinner @ 2016-11-30 22:54 UTC (permalink / raw) To: linux-xfs From: Dave Chinner <dchinner@redhat.com> We always perform integrity operations now, so these mount options don't do anything. Deprecate them and mark them for removal in in a year. Signed-Off-By: Dave Chinner <dchinner@redhat.com> --- Documentation/filesystems/xfs.txt | 12 ++++-------- fs/xfs/xfs_super.c | 25 ++++++++++++++++--------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/Documentation/filesystems/xfs.txt b/Documentation/filesystems/xfs.txt index c2d44e6e117b..68a057c8fccf 100644 --- a/Documentation/filesystems/xfs.txt +++ b/Documentation/filesystems/xfs.txt @@ -51,13 +51,6 @@ default behaviour. CRC enabled filesystems always use the attr2 format, and so will reject the noattr2 mount option if it is set. - barrier (*) - nobarrier - Enables/disables the use of block layer write barriers for - writes into the journal and for data integrity operations. - This allows for drive level write caching to be enabled, for - devices that support write barriers. - discard nodiscard (*) Enable/disable the issuing of commands to let the block @@ -228,7 +221,10 @@ default behaviour. Deprecated Mount Options ======================== -None at present. + Name Removal Schedule + ---- ---------------- + barrier December 2017 + nobarrier December 2017 Removed Mount Options diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c index 563d1d146b8c..eecbaac08eba 100644 --- a/fs/xfs/xfs_super.c +++ b/fs/xfs/xfs_super.c @@ -104,9 +104,6 @@ static const match_table_t tokens = { {Opt_sysvgroups,"sysvgroups"}, /* group-ID from current process */ {Opt_allocsize, "allocsize=%s"},/* preferred allocation size */ {Opt_norecovery,"norecovery"}, /* don't run XFS recovery */ - {Opt_barrier, "barrier"}, /* use writer barriers for log write and - * unwritten extent conversion */ - {Opt_nobarrier, "nobarrier"}, /* .. disable */ {Opt_inode64, "inode64"}, /* inodes can be allocated anywhere */ {Opt_inode32, "inode32"}, /* inode allocation limited to * XFS_MAXINUMBER_32 */ @@ -134,6 +131,12 @@ static const match_table_t tokens = { {Opt_nodiscard, "nodiscard"}, /* Do not discard unused blocks */ {Opt_dax, "dax"}, /* Enable direct access to bdev pages */ + + /* Deprecated mount options scheduled for removal */ + {Opt_barrier, "barrier"}, /* use writer barriers for log write and + * unwritten extent conversion */ + {Opt_nobarrier, "nobarrier"}, /* .. disable */ + {Opt_err, NULL}, }; @@ -301,12 +304,6 @@ xfs_parseargs( case Opt_nouuid: mp->m_flags |= XFS_MOUNT_NOUUID; break; - case Opt_barrier: - mp->m_flags |= XFS_MOUNT_BARRIER; - break; - case Opt_nobarrier: - mp->m_flags &= ~XFS_MOUNT_BARRIER; - break; case Opt_ikeep: mp->m_flags |= XFS_MOUNT_IKEEP; break; @@ -374,6 +371,14 @@ xfs_parseargs( mp->m_flags |= XFS_MOUNT_DAX; break; #endif + case Opt_barrier: + xfs_warn(mp, "%s option is deprecated, ignoring.", p); + mp->m_flags |= XFS_MOUNT_BARRIER; + break; + case Opt_nobarrier: + xfs_warn(mp, "%s option is deprecated, ignoring.", p); + mp->m_flags &= ~XFS_MOUNT_BARRIER; + break; default: xfs_warn(mp, "unknown mount option [%s].", p); return -EINVAL; @@ -1238,9 +1243,11 @@ xfs_fs_remount( token = match_token(p, tokens, args); switch (token) { case Opt_barrier: + xfs_warn(mp, "%s option is deprecated, ignoring.", p); mp->m_flags |= XFS_MOUNT_BARRIER; break; case Opt_nobarrier: + xfs_warn(mp, "%s option is deprecated, ignoring.", p); mp->m_flags &= ~XFS_MOUNT_BARRIER; break; case Opt_inode64: -- 2.10.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] xfs: deprecate barrier/nobarrier mount option 2016-11-30 22:54 ` [PATCH 2/2] xfs: deprecate barrier/nobarrier mount option Dave Chinner @ 2016-12-01 9:54 ` Jan Kara 2016-12-01 9:59 ` Christoph Hellwig 2016-12-01 12:47 ` Brian Foster 2016-12-05 16:23 ` Christoph Hellwig 2 siblings, 1 reply; 11+ messages in thread From: Jan Kara @ 2016-12-01 9:54 UTC (permalink / raw) To: Dave Chinner; +Cc: linux-xfs On Thu 01-12-16 09:54:44, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > We always perform integrity operations now, so these mount options > don't do anything. Deprecate them and mark them for removal in > in a year. > > Signed-Off-By: Dave Chinner <dchinner@redhat.com> We use 'nobarrier' mount option to simulate disks with battery-backed caches in some of our IO performance testing. I don't say we cannot live with this mount option but it was convenient... Honza > --- > Documentation/filesystems/xfs.txt | 12 ++++-------- > fs/xfs/xfs_super.c | 25 ++++++++++++++++--------- > 2 files changed, 20 insertions(+), 17 deletions(-) > > diff --git a/Documentation/filesystems/xfs.txt b/Documentation/filesystems/xfs.txt > index c2d44e6e117b..68a057c8fccf 100644 > --- a/Documentation/filesystems/xfs.txt > +++ b/Documentation/filesystems/xfs.txt > @@ -51,13 +51,6 @@ default behaviour. > CRC enabled filesystems always use the attr2 format, and so > will reject the noattr2 mount option if it is set. > > - barrier (*) > - nobarrier > - Enables/disables the use of block layer write barriers for > - writes into the journal and for data integrity operations. > - This allows for drive level write caching to be enabled, for > - devices that support write barriers. > - > discard > nodiscard (*) > Enable/disable the issuing of commands to let the block > @@ -228,7 +221,10 @@ default behaviour. > Deprecated Mount Options > ======================== > > -None at present. > + Name Removal Schedule > + ---- ---------------- > + barrier December 2017 > + nobarrier December 2017 > > > Removed Mount Options > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > index 563d1d146b8c..eecbaac08eba 100644 > --- a/fs/xfs/xfs_super.c > +++ b/fs/xfs/xfs_super.c > @@ -104,9 +104,6 @@ static const match_table_t tokens = { > {Opt_sysvgroups,"sysvgroups"}, /* group-ID from current process */ > {Opt_allocsize, "allocsize=%s"},/* preferred allocation size */ > {Opt_norecovery,"norecovery"}, /* don't run XFS recovery */ > - {Opt_barrier, "barrier"}, /* use writer barriers for log write and > - * unwritten extent conversion */ > - {Opt_nobarrier, "nobarrier"}, /* .. disable */ > {Opt_inode64, "inode64"}, /* inodes can be allocated anywhere */ > {Opt_inode32, "inode32"}, /* inode allocation limited to > * XFS_MAXINUMBER_32 */ > @@ -134,6 +131,12 @@ static const match_table_t tokens = { > {Opt_nodiscard, "nodiscard"}, /* Do not discard unused blocks */ > > {Opt_dax, "dax"}, /* Enable direct access to bdev pages */ > + > + /* Deprecated mount options scheduled for removal */ > + {Opt_barrier, "barrier"}, /* use writer barriers for log write and > + * unwritten extent conversion */ > + {Opt_nobarrier, "nobarrier"}, /* .. disable */ > + > {Opt_err, NULL}, > }; > > @@ -301,12 +304,6 @@ xfs_parseargs( > case Opt_nouuid: > mp->m_flags |= XFS_MOUNT_NOUUID; > break; > - case Opt_barrier: > - mp->m_flags |= XFS_MOUNT_BARRIER; > - break; > - case Opt_nobarrier: > - mp->m_flags &= ~XFS_MOUNT_BARRIER; > - break; > case Opt_ikeep: > mp->m_flags |= XFS_MOUNT_IKEEP; > break; > @@ -374,6 +371,14 @@ xfs_parseargs( > mp->m_flags |= XFS_MOUNT_DAX; > break; > #endif > + case Opt_barrier: > + xfs_warn(mp, "%s option is deprecated, ignoring.", p); > + mp->m_flags |= XFS_MOUNT_BARRIER; > + break; > + case Opt_nobarrier: > + xfs_warn(mp, "%s option is deprecated, ignoring.", p); > + mp->m_flags &= ~XFS_MOUNT_BARRIER; > + break; > default: > xfs_warn(mp, "unknown mount option [%s].", p); > return -EINVAL; > @@ -1238,9 +1243,11 @@ xfs_fs_remount( > token = match_token(p, tokens, args); > switch (token) { > case Opt_barrier: > + xfs_warn(mp, "%s option is deprecated, ignoring.", p); > mp->m_flags |= XFS_MOUNT_BARRIER; > break; > case Opt_nobarrier: > + xfs_warn(mp, "%s option is deprecated, ignoring.", p); > mp->m_flags &= ~XFS_MOUNT_BARRIER; > break; > case Opt_inode64: > -- > 2.10.2 > > -- > 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 -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] xfs: deprecate barrier/nobarrier mount option 2016-12-01 9:54 ` Jan Kara @ 2016-12-01 9:59 ` Christoph Hellwig 0 siblings, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2016-12-01 9:59 UTC (permalink / raw) To: Jan Kara; +Cc: Dave Chinner, linux-xfs On Thu, Dec 01, 2016 at 10:54:39AM +0100, Jan Kara wrote: > On Thu 01-12-16 09:54:44, Dave Chinner wrote: > > From: Dave Chinner <dchinner@redhat.com> > > > > We always perform integrity operations now, so these mount options > > don't do anything. Deprecate them and mark them for removal in > > in a year. > > > > Signed-Off-By: Dave Chinner <dchinner@redhat.com> > > We use 'nobarrier' mount option to simulate disks with battery-backed > caches in some of our IO performance testing. I don't say we cannot live > with this mount option but it was convenient... FYI, for scsi (and thus ATA as well) you can override the cache type using the cache_type sysfs file, which is a much better interface for this than anything file system related. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] xfs: deprecate barrier/nobarrier mount option 2016-11-30 22:54 ` [PATCH 2/2] xfs: deprecate barrier/nobarrier mount option Dave Chinner 2016-12-01 9:54 ` Jan Kara @ 2016-12-01 12:47 ` Brian Foster 2016-12-01 20:20 ` Dave Chinner 2016-12-05 16:23 ` Christoph Hellwig 2 siblings, 1 reply; 11+ messages in thread From: Brian Foster @ 2016-12-01 12:47 UTC (permalink / raw) To: Dave Chinner; +Cc: linux-xfs On Thu, Dec 01, 2016 at 09:54:44AM +1100, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > We always perform integrity operations now, so these mount options > don't do anything. Deprecate them and mark them for removal in > in a year. > > Signed-Off-By: Dave Chinner <dchinner@redhat.com> > --- > Documentation/filesystems/xfs.txt | 12 ++++-------- > fs/xfs/xfs_super.c | 25 ++++++++++++++++--------- > 2 files changed, 20 insertions(+), 17 deletions(-) > ... > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > index 563d1d146b8c..eecbaac08eba 100644 > --- a/fs/xfs/xfs_super.c > +++ b/fs/xfs/xfs_super.c ... > @@ -374,6 +371,14 @@ xfs_parseargs( > mp->m_flags |= XFS_MOUNT_DAX; > break; > #endif > + case Opt_barrier: > + xfs_warn(mp, "%s option is deprecated, ignoring.", p); > + mp->m_flags |= XFS_MOUNT_BARRIER; > + break; > + case Opt_nobarrier: > + xfs_warn(mp, "%s option is deprecated, ignoring.", p); > + mp->m_flags &= ~XFS_MOUNT_BARRIER; > + break; So basically XFS_MOUNT_BARRIER exists solely for the purpose of showargs. Should we just kill it too and do something deterministic on showargs (i.e., always show 'barrier' or just drop it entirely), or does precedent suggest otherwise? That aside, this all seems Ok to me. Brian > default: > xfs_warn(mp, "unknown mount option [%s].", p); > return -EINVAL; > @@ -1238,9 +1243,11 @@ xfs_fs_remount( > token = match_token(p, tokens, args); > switch (token) { > case Opt_barrier: > + xfs_warn(mp, "%s option is deprecated, ignoring.", p); > mp->m_flags |= XFS_MOUNT_BARRIER; > break; > case Opt_nobarrier: > + xfs_warn(mp, "%s option is deprecated, ignoring.", p); > mp->m_flags &= ~XFS_MOUNT_BARRIER; > break; > case Opt_inode64: > -- > 2.10.2 > > -- > 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] xfs: deprecate barrier/nobarrier mount option 2016-12-01 12:47 ` Brian Foster @ 2016-12-01 20:20 ` Dave Chinner 2016-12-01 21:31 ` Brian Foster 0 siblings, 1 reply; 11+ messages in thread From: Dave Chinner @ 2016-12-01 20:20 UTC (permalink / raw) To: Brian Foster; +Cc: linux-xfs On Thu, Dec 01, 2016 at 07:47:46AM -0500, Brian Foster wrote: > On Thu, Dec 01, 2016 at 09:54:44AM +1100, Dave Chinner wrote: > > From: Dave Chinner <dchinner@redhat.com> > > > > We always perform integrity operations now, so these mount options > > don't do anything. Deprecate them and mark them for removal in > > in a year. > > > > Signed-Off-By: Dave Chinner <dchinner@redhat.com> > > --- > > Documentation/filesystems/xfs.txt | 12 ++++-------- > > fs/xfs/xfs_super.c | 25 ++++++++++++++++--------- > > 2 files changed, 20 insertions(+), 17 deletions(-) > > > ... > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > > index 563d1d146b8c..eecbaac08eba 100644 > > --- a/fs/xfs/xfs_super.c > > +++ b/fs/xfs/xfs_super.c > ... > > @@ -374,6 +371,14 @@ xfs_parseargs( > > mp->m_flags |= XFS_MOUNT_DAX; > > break; > > #endif > > + case Opt_barrier: > > + xfs_warn(mp, "%s option is deprecated, ignoring.", p); > > + mp->m_flags |= XFS_MOUNT_BARRIER; > > + break; > > + case Opt_nobarrier: > > + xfs_warn(mp, "%s option is deprecated, ignoring.", p); > > + mp->m_flags &= ~XFS_MOUNT_BARRIER; > > + break; > > So basically XFS_MOUNT_BARRIER exists solely for the purpose of > showargs. Should we just kill it too and do something deterministic on > showargs (i.e., always show 'barrier' or just drop it entirely), or does > precedent suggest otherwise? Well, they are deprecated and ignored, but we can't remove them straight away so I thought we should still report them appropriately in showargs. I don't really care either way - I can kill it completely if you think that's better. Cheers, Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] xfs: deprecate barrier/nobarrier mount option 2016-12-01 20:20 ` Dave Chinner @ 2016-12-01 21:31 ` Brian Foster 0 siblings, 0 replies; 11+ messages in thread From: Brian Foster @ 2016-12-01 21:31 UTC (permalink / raw) To: Dave Chinner; +Cc: linux-xfs On Fri, Dec 02, 2016 at 07:20:06AM +1100, Dave Chinner wrote: > On Thu, Dec 01, 2016 at 07:47:46AM -0500, Brian Foster wrote: > > On Thu, Dec 01, 2016 at 09:54:44AM +1100, Dave Chinner wrote: > > > From: Dave Chinner <dchinner@redhat.com> > > > > > > We always perform integrity operations now, so these mount options > > > don't do anything. Deprecate them and mark them for removal in > > > in a year. > > > > > > Signed-Off-By: Dave Chinner <dchinner@redhat.com> > > > --- > > > Documentation/filesystems/xfs.txt | 12 ++++-------- > > > fs/xfs/xfs_super.c | 25 ++++++++++++++++--------- > > > 2 files changed, 20 insertions(+), 17 deletions(-) > > > > > ... > > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > > > index 563d1d146b8c..eecbaac08eba 100644 > > > --- a/fs/xfs/xfs_super.c > > > +++ b/fs/xfs/xfs_super.c > > ... > > > @@ -374,6 +371,14 @@ xfs_parseargs( > > > mp->m_flags |= XFS_MOUNT_DAX; > > > break; > > > #endif > > > + case Opt_barrier: > > > + xfs_warn(mp, "%s option is deprecated, ignoring.", p); > > > + mp->m_flags |= XFS_MOUNT_BARRIER; > > > + break; > > > + case Opt_nobarrier: > > > + xfs_warn(mp, "%s option is deprecated, ignoring.", p); > > > + mp->m_flags &= ~XFS_MOUNT_BARRIER; > > > + break; > > > > So basically XFS_MOUNT_BARRIER exists solely for the purpose of > > showargs. Should we just kill it too and do something deterministic on > > showargs (i.e., always show 'barrier' or just drop it entirely), or does > > precedent suggest otherwise? > > Well, they are deprecated and ignored, but we can't remove them > straight away so I thought we should still report them appropriately > in showargs. > > I don't really care either way - I can kill it completely if > you think that's better. > Ok. No major preference.. my first instinct was just to see it all killed off to explicitly show that the mount option has no effect. Either way is fine, thanks. Brian > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] xfs: deprecate barrier/nobarrier mount option 2016-11-30 22:54 ` [PATCH 2/2] xfs: deprecate barrier/nobarrier mount option Dave Chinner 2016-12-01 9:54 ` Jan Kara 2016-12-01 12:47 ` Brian Foster @ 2016-12-05 16:23 ` Christoph Hellwig 2 siblings, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2016-12-05 16:23 UTC (permalink / raw) To: Dave Chinner; +Cc: linux-xfs On Thu, Dec 01, 2016 at 09:54:44AM +1100, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > We always perform integrity operations now, so these mount options > don't do anything. Deprecate them and mark them for removal in > in a year. This might be a bit of a too aggressive schedule, but we'll see.. Looks fine otherwise: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-12-05 16:23 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-11-30 22:54 [RFC PATCH 0/2] xfs: deprecate barrier/nobarrier Dave Chinner 2016-11-30 22:54 ` [PATCH 1/2] xfs: Always flush caches when integrity is required Dave Chinner 2016-12-01 12:47 ` Brian Foster 2016-12-05 16:22 ` Christoph Hellwig 2016-11-30 22:54 ` [PATCH 2/2] xfs: deprecate barrier/nobarrier mount option Dave Chinner 2016-12-01 9:54 ` Jan Kara 2016-12-01 9:59 ` Christoph Hellwig 2016-12-01 12:47 ` Brian Foster 2016-12-01 20:20 ` Dave Chinner 2016-12-01 21:31 ` Brian Foster 2016-12-05 16:23 ` Christoph Hellwig
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).