[PATCH v3] btrfs: not a disk error if the bio_add_page fails
bio_add_page() can fail for logical reasons as from the bio_add_page()
comments:- 'This will only fail if either
bio->bi_vcnt == bio->bi_max_vecs or it's a cloned bio.'
Here as we have just allocated the bio, so both of those failures can't
occur. So drop the check for fail. Which would also drop write error
statistics.
Signed-off-by: Anand Jain
---
v2->v3: Drop check for bio_add_page return value. Thanks Liubo.
v1->v2: Add missing bio_put(). Thanks Filipe.
fs/btrfs/scrub.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index ecfe3118d9dd..5030898247db 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -4600,7 +4600,6 @@ static int write_page_nocow(struct scrub_ctx *sctx,
{
struct bio *bio;
struct btrfs_device *dev;
- int ret;
dev = sctx->wr_tgtdev;
if (!dev)
@@ -4615,17 +4614,15 @@ static int write_page_nocow(struct scrub_ctx *sctx,
bio->bi_iter.bi_sector = physical_for_dev_replace >> 9;
bio_set_dev(bio, dev->bdev);
bio->bi_opf = REQ_OP_WRITE | REQ_SYNC;
- ret = bio_add_page(bio, page, PAGE_SIZE, 0);
- if (ret != PAGE_SIZE) {
-leave_with_eio:
+ /* bio_add_page won't fail here */
+ bio_add_page(bio, page, PAGE_SIZE, 0);
+
+ if (btrfsic_submit_bio_wait(bio)) {
bio_put(bio);
btrfs_dev_stat_inc_and_print(dev, BTRFS_DEV_STAT_WRITE_ERRS);
return -EIO;
}
- if (btrfsic_submit_bio_wait(bio))
- goto leave_with_eio;
-
bio_put(bio);
return 0;
}
--
2.15.0
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] btrfs: not a disk error if the bio_add_page fails
On 01/05/2018 02:50 AM, Liu Bo wrote:
On Thu, Jan 04, 2018 at 09:47:56AM -0800, Liu Bo wrote:
On Wed, Jan 03, 2018 at 09:52:43PM +0800, Anand Jain wrote:
bio_add_page() can fail for logical reasons as from the bio_add_page()
comments:- 'This will only fail if either
bio->bi_vcnt == bio->bi_max_vecs or it's a cloned bio.' Don't inc the
write error statistics for this. And set -EINVAL instead of -EIO.
It's correct to skip increasing counter, but why -EINVAL?
(its caller only cares about if it's non-zero.)
Well...there is no chance that bio_add_page() could return
non-PAGE_SIZE as this bio has been just created, we can remove the
error handling after bio_add_page().
Ah. right, bio_add_page() can't fail in this thread, its not
a bio clone, and its the first call to bio_add_page(). Will
remove error handling part.
Thanks, Anand
thanks,
-liubo
thanks,
-liubo
Signed-off-by: Anand Jain
---
v1->v2: Add missing bio_put(). Thanks Filipe.
fs/btrfs/scrub.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index ecfe3118d9dd..57ac39a3f046 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -4617,15 +4617,16 @@ static int write_page_nocow(struct scrub_ctx *sctx,
bio->bi_opf = REQ_OP_WRITE | REQ_SYNC;
ret = bio_add_page(bio, page, PAGE_SIZE, 0);
if (ret != PAGE_SIZE) {
-leave_with_eio:
+ bio_put(bio);
+ return -EINVAL;
+ }
+
+ if (btrfsic_submit_bio_wait(bio)) {
bio_put(bio);
btrfs_dev_stat_inc_and_print(dev, BTRFS_DEV_STAT_WRITE_ERRS);
return -EIO;
}
- if (btrfsic_submit_bio_wait(bio))
- goto leave_with_eio;
-
bio_put(bio);
return 0;
}
--
2.15.0
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@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 majord...@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 majord...@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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] btrfs: not a disk error if the bio_add_page fails
On Thu, Jan 04, 2018 at 09:47:56AM -0800, Liu Bo wrote:
> On Wed, Jan 03, 2018 at 09:52:43PM +0800, Anand Jain wrote:
> > bio_add_page() can fail for logical reasons as from the bio_add_page()
> > comments:- 'This will only fail if either
> > bio->bi_vcnt == bio->bi_max_vecs or it's a cloned bio.' Don't inc the
> > write error statistics for this. And set -EINVAL instead of -EIO.
> >
>
> It's correct to skip increasing counter, but why -EINVAL?
> (its caller only cares about if it's non-zero.)
Well...there is no chance that bio_add_page() could return
non-PAGE_SIZE as this bio has been just created, we can remove the
error handling after bio_add_page().
thanks,
-liubo
>
> thanks,
>
> -liubo
> > Signed-off-by: Anand Jain
> > ---
> > v1->v2: Add missing bio_put(). Thanks Filipe.
> >
> > fs/btrfs/scrub.c | 9 +
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> > index ecfe3118d9dd..57ac39a3f046 100644
> > --- a/fs/btrfs/scrub.c
> > +++ b/fs/btrfs/scrub.c
> > @@ -4617,15 +4617,16 @@ static int write_page_nocow(struct scrub_ctx *sctx,
> > bio->bi_opf = REQ_OP_WRITE | REQ_SYNC;
> > ret = bio_add_page(bio, page, PAGE_SIZE, 0);
> > if (ret != PAGE_SIZE) {
> > -leave_with_eio:
> > + bio_put(bio);
> > + return -EINVAL;
> > + }
> > +
> > + if (btrfsic_submit_bio_wait(bio)) {
> > bio_put(bio);
> > btrfs_dev_stat_inc_and_print(dev, BTRFS_DEV_STAT_WRITE_ERRS);
> > return -EIO;
> > }
> >
> > - if (btrfsic_submit_bio_wait(bio))
> > - goto leave_with_eio;
> > -
> > bio_put(bio);
> > return 0;
> > }
> > --
> > 2.15.0
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> > the body of a message to majord...@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 majord...@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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] btrfs: not a disk error if the bio_add_page fails
On Wed, Jan 03, 2018 at 09:52:43PM +0800, Anand Jain wrote:
> bio_add_page() can fail for logical reasons as from the bio_add_page()
> comments:- 'This will only fail if either
> bio->bi_vcnt == bio->bi_max_vecs or it's a cloned bio.' Don't inc the
> write error statistics for this. And set -EINVAL instead of -EIO.
>
It's correct to skip increasing counter, but why -EINVAL?
(its caller only cares about if it's non-zero.)
thanks,
-liubo
> Signed-off-by: Anand Jain
> ---
> v1->v2: Add missing bio_put(). Thanks Filipe.
>
> fs/btrfs/scrub.c | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index ecfe3118d9dd..57ac39a3f046 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -4617,15 +4617,16 @@ static int write_page_nocow(struct scrub_ctx *sctx,
> bio->bi_opf = REQ_OP_WRITE | REQ_SYNC;
> ret = bio_add_page(bio, page, PAGE_SIZE, 0);
> if (ret != PAGE_SIZE) {
> -leave_with_eio:
> + bio_put(bio);
> + return -EINVAL;
> + }
> +
> + if (btrfsic_submit_bio_wait(bio)) {
> bio_put(bio);
> btrfs_dev_stat_inc_and_print(dev, BTRFS_DEV_STAT_WRITE_ERRS);
> return -EIO;
> }
>
> - if (btrfsic_submit_bio_wait(bio))
> - goto leave_with_eio;
> -
> bio_put(bio);
> return 0;
> }
> --
> 2.15.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fstests: btrfs/158: reproduce a scrub bug on raid6 corruption
On Thu, Jan 04, 2018 at 05:05:34PM +0800, Eryu Guan wrote:
> On Tue, Jan 02, 2018 at 01:35:00PM -0700, Liu Bo wrote:
> > This is to reproduce a bug of scrub, with which scrub is unable to
> > repair raid6 corruption as expected.
> >
> > The kernel side fixes are
> > Btrfs: make raid6 rebuild retry more
> > Btrfs: fix scrub to repair raid6 corruption
> >
> > Signed-off-by: Liu Bo
>
> Looks fine overall, I tested it with 4.15-rc6 kernel and test failed as
> expected, re-tested successfully after applying the patches mentioned in
> commit log.
>
Thanks for testing it.
> Just some really minor issues below, and I can fix them on commit if
> what I suggest looks sane to you.
>
They all look good to me, thanks a lot for the comments.
> > ---
> > tests/btrfs/158 | 114
> >
> > tests/btrfs/158.out | 10 +
> > tests/btrfs/group | 1 +
> > 3 files changed, 125 insertions(+)
> > create mode 100755 tests/btrfs/158
> > create mode 100644 tests/btrfs/158.out
> >
> > diff --git a/tests/btrfs/158 b/tests/btrfs/158
> > new file mode 100755
> > index 000..43afc2d
> > --- /dev/null
> > +++ b/tests/btrfs/158
> > @@ -0,0 +1,114 @@
> > +#! /bin/bash
> > +# FS QA Test 158
> > +#
> > +# The test case is check if scrub is able fix raid6 data corruption,
> > +# ie. if there is data corruption on two disks in the same horizontal
> > +# stripe, e.g. due to bitrot.
> > +#
> > +# The kernel fixes are
> > +# Btrfs: make raid6 rebuild retry more
> > +# Btrfs: fix scrub to repair raid6 corruption
> > +#
> > +#---
> > +# Copyright (c) 2017 Oracle. All Rights Reserved.
> 2018?
oops, it was 2017 when I made the test.
> > +#
> > +# This program is free software; you can redistribute it and/or
> > +# modify it under the terms of the GNU General Public License as
> > +# published by the Free Software Foundation.
> > +#
> > +# This program is distributed in the hope that it would be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program; if not, write the Free Software Foundation,
> > +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> > +#---
> > +#
> > +
> > +seq=`basename $0`
> > +seqres=$RESULT_DIR/$seq
> > +echo "QA output created by $seq"
> > +
> > +here=`pwd`
> > +tmp=/tmp/$$
> > +status=1 # failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > + cd /
> > + rm -f $tmp.*
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +
> > +# remove previous $seqres.full before test
> > +rm -f $seqres.full
> > +
> > +# real QA test starts here
> > +
> > +# Modify as appropriate.
> > +_supported_fs btrfs
> > +_supported_os Linux
> > +_require_scratch_dev_pool 4
> > +_require_btrfs_command inspect-internal dump-tree
> > +
> > +get_physical_stripe0()
> > +{
> > + $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
> > + grep " DATA\|RAID6" -A 10 | \
> > + $AWK_PROG '($1 ~ /stripe/ && $3 ~ /devid/ && $2 ~ /0/) { print $6 }'
> > +}
> > +
> > +get_physical_stripe1()
> > +{
> > + $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
> > + grep " DATA\|RAID6" -A 10 | \
> > + $AWK_PROG '($1 ~ /stripe/ && $3 ~ /devid/ && $2 ~ /1/) { print $6 }'
> > +}
> > +
> > +_scratch_dev_pool_get 4
> > +# step 1: create a raid6 btrfs and create a 4K file
> > +echo "step 1..mkfs.btrfs" >>$seqres.full
> > +
> > +mkfs_opts="-d raid6 -b 1G"
> > +_scratch_pool_mkfs $mkfs_opts >>$seqres.full 2>&1
> > +
> > +# -o nospace_cache makes sure data is written to the start position of the
> > data
> > +# chunk
> > +_scratch_mount -o nospace_cache
> > +
> > +# [0,64K) is written to stripe 0 and [64K, 128K) is written to stripe 1
> > +$XFS_IO_PROG -f -d -c "pwrite -S 0xaa 0 128K" -c "fsync" \
> > + "$SCRATCH_MNT/foobar" | _filter_xfs_io
> > +
> > +_scratch_unmount
> > +
> > +stripe_0=`get_physical_stripe0`
> > +stripe_1=`get_physical_stripe1`
> > +dev4=`echo $SCRATCH_DEV_POOL | awk '{print $4}'`
> > +dev3=`echo $SCRATCH_DEV_POOL | awk '{print $3}'`
> > +
> > +# step 2: corrupt the 1st and 2nd stripe (stripe 0 and 1)
> > +echo "step 2..simulate bitrot at offset $stripe_0 of device_4($dev4)
> > and offset $stripe_1 of device_3($dev3)" >>$seqres.full
> > +
> > +$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $stripe_0 64K" $dev4 | _filter_xfs_io
> > +$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $stripe_1 64K" $dev3 | _filter_xfs_io
> > +
> > +# step 3: read foobar to repair the bitrot
>
> Comment meant to be "scrub to repair the b
Business Possibility
Hello there, My name is Peter Deng a South African citizen and a friend to Mrs Mugabe sister . I got your contact through Korean business online directory. I represent the interest of Mrs Mugabe who wishes to move a total amount of $19 million into a safe account owns by a trusted business man who is capable of accommodating such volume of funds with absolute trust & confidentiality. We are willing to give 5-10 percent of the total money for this efforts . A non disclosure non circumvent agreement will be signed before the eventual transfer of funds. It is important for to know that the fund owner is going through a period of political turmoil and she is in a precarious position, so this transaction has to be highly secretive & very confidential. Kindly respond back to me if this is what you can handle . Regards, Peter. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH bpf-next v2 0/4] Separate error injection table from kprobes
On Tue, Dec 26, 2017 at 04:46:28PM +0900, Masami Hiramatsu wrote: > Hi Josef and Alexei, > > Here are the 2nd version of patches to moving error injection > table from kprobes. In this series I did a small fixes and > add function-based fault injection. > > Here is the previous version: > > https://lkml.org/lkml/2017/12/22/554 > > There are 2 main reasons why I separate it from kprobes. > > - kprobes users can modify execution path not only at >error-injection whitelist functions but also other >functions. I don't like to suggest user that such >limitation is from kprobes itself. > > - This error injection information is also useful for >ftrace (function-hook) and livepatch. It should not >be limited by CONFIG_KPROBES. > > So I introduced CONFIG_FUNCTION_ERROR_INJECTION for this feature. > Also CONFIG_FAIL_FUNCTION is added, which provides function-based > error injection interface via debugfs following fault-injection > framework. See [4/4]. > > Any thoughts? Sorry Masami, I've been on vacation for the last two weeks. This approach is fine by me, if we want to allow other mechanisms other than bpf to use this functionality then hooray. I'll do a proper review when you post v3, just wanted to let you know I wasn't ignoring you. Thanks, Josef -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] Cleanups for later btrfs_alloc_chunk() rework
On 3.01.2018 09:13, Qu Wenruo wrote: > Just small cleanups for incoming btrfs_alloc_chunk() rework, which is > designed to allow btrfs_alloc_chunk() to be able to alloc meta chunk, > even current meta chunks are already full. > > The cleanups are quite small, most of them are just removing unnecessary > parameters, and make some function static. > > Qu Wenruo (5): > btrfs-progs: Use bool parameter to determine if we're allocating data > extent > btrfs-progs: volumes: Make find_free_dev_extent_start static > btrfs-progs: volumes: Remove unnecessary trans parameter > btrfs-progs: volumes: Remove unnecessary parameters when allocating > device extent > btrfs-progs: Remove unnecessary parameter for btrfs_add_block_group > For the whole series: Reviewed-by: Nikolay Borisov > cmds-check.c | 4 ++-- > convert/main.c | 4 +--- > ctree.h| 7 +++ > extent-tree.c | 31 --- > mkfs/main.c| 14 -- > volumes.c | 35 ++- > 6 files changed, 40 insertions(+), 55 deletions(-) > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 12/19] ocfs2: convert to new i_version API
On Fri, 2017-12-22 at 07:05 -0500, Jeff Layton wrote:
> From: Jeff Layton
>
> Signed-off-by: Jeff Layton
> Reviewed-by: Jan Kara
> ---
> fs/ocfs2/dir.c | 15 ---
> fs/ocfs2/inode.c| 3 ++-
> fs/ocfs2/namei.c| 3 ++-
> fs/ocfs2/quota_global.c | 3 ++-
> 4 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> index febe6312ceff..32f9c72dff17 100644
> --- a/fs/ocfs2/dir.c
> +++ b/fs/ocfs2/dir.c
> @@ -42,6 +42,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -1174,7 +1175,7 @@ static int __ocfs2_delete_entry(handle_t *handle,
> struct inode *dir,
> le16_add_cpu(&pde->rec_len,
> le16_to_cpu(de->rec_len));
> de->inode = 0;
> - dir->i_version++;
> + inode_inc_iversion(dir);
> ocfs2_journal_dirty(handle, bh);
> goto bail;
> }
> @@ -1729,7 +1730,7 @@ int __ocfs2_add_entry(handle_t *handle,
> if (ocfs2_dir_indexed(dir))
> ocfs2_recalc_free_list(dir, handle, lookup);
>
> - dir->i_version++;
> + inode_inc_iversion(dir);
> ocfs2_journal_dirty(handle, insert_bh);
> retval = 0;
> goto bail;
> @@ -1775,7 +1776,7 @@ static int ocfs2_dir_foreach_blk_id(struct inode *inode,
>* readdir(2), then we might be pointing to an invalid
>* dirent right now. Scan from the start of the block
>* to make sure. */
> - if (*f_version != inode->i_version) {
> + if (inode_cmp_iversion(inode, *f_version)) {
> for (i = 0; i < i_size_read(inode) && i < offset; ) {
> de = (struct ocfs2_dir_entry *)
> (data->id_data + i);
> @@ -1791,7 +1792,7 @@ static int ocfs2_dir_foreach_blk_id(struct inode *inode,
> i += le16_to_cpu(de->rec_len);
> }
> ctx->pos = offset = i;
> - *f_version = inode->i_version;
> + *f_version = inode_query_iversion(inode);
> }
>
> de = (struct ocfs2_dir_entry *) (data->id_data + ctx->pos);
> @@ -1869,7 +1870,7 @@ static int ocfs2_dir_foreach_blk_el(struct inode *inode,
>* readdir(2), then we might be pointing to an invalid
>* dirent right now. Scan from the start of the block
>* to make sure. */
> - if (*f_version != inode->i_version) {
> + if (inode_cmp_iversion(inode, *f_version)) {
> for (i = 0; i < sb->s_blocksize && i < offset; ) {
> de = (struct ocfs2_dir_entry *) (bh->b_data +
> i);
> /* It's too expensive to do a full
> @@ -1886,7 +1887,7 @@ static int ocfs2_dir_foreach_blk_el(struct inode *inode,
> offset = i;
> ctx->pos = (ctx->pos & ~(sb->s_blocksize - 1))
> | offset;
> - *f_version = inode->i_version;
> + *f_version = inode_query_iversion(inode);
> }
>
> while (ctx->pos < i_size_read(inode)
> @@ -1940,7 +1941,7 @@ static int ocfs2_dir_foreach_blk(struct inode *inode,
> u64 *f_version,
> */
> int ocfs2_dir_foreach(struct inode *inode, struct dir_context *ctx)
> {
> - u64 version = inode->i_version;
> + u64 version = inode_query_iversion(inode);
> ocfs2_dir_foreach_blk(inode, &version, ctx, true);
> return 0;
> }
> diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
> index 1a1e0078ab38..d51b80edd972 100644
> --- a/fs/ocfs2/inode.c
> +++ b/fs/ocfs2/inode.c
> @@ -28,6 +28,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -302,7 +303,7 @@ void ocfs2_populate_inode(struct inode *inode, struct
> ocfs2_dinode *fe,
> OCFS2_I(inode)->ip_attr = le32_to_cpu(fe->i_attr);
> OCFS2_I(inode)->ip_dyn_features = le16_to_cpu(fe->i_dyn_features);
>
> - inode->i_version = 1;
> + inode_set_iversion(inode, 1);
> inode->i_generation = le32_to_cpu(fe->i_generation);
> inode->i_rdev = huge_decode_dev(le64_to_cpu(fe->id1.dev1.i_rdev));
> inode->i_mode = le16_to_cpu(fe->i_mode);
> diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
> index 3b0a10d9b36f..c801eddc4bf3 100644
> --- a/fs/ocfs2/namei.c
> +++ b/fs/ocfs2/namei.c
> @@ -41,6 +41,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -1520,7 +1521,7 @@ static int ocfs2_rename(struct inode *old_dir,
> mlog_errno(status);
> goto bail;
> }
> -
Re: [PATCH 1/2] btrfs: remove check for BTRFS_FS_STATE_ERROR which we just set
On 4.01.2018 12:01, Anand Jain wrote:
> __btrfs_handle_fs_error() sets BTRFS_FS_STATE_ERROR, and calls
> btrfs_handle_error() so no need to check if the BTRFS_FS_STATE_ERROR
> is set in btrfs_handle_error(). And there is no other user of
> btrfs_handle_error() as well.
>
> Signed-off-by: Anand Jain
Reviewed-by: Nikolay Borisov
> ---
> fs/btrfs/super.c | 26 --
> 1 file changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 4c49bb5632a2..60766f8ca434 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -113,20 +113,18 @@ static void btrfs_handle_error(struct btrfs_fs_info
> *fs_info)
> if (sb_rdonly(sb))
> return;
>
> - if (test_bit(BTRFS_FS_STATE_ERROR, &fs_info->fs_state)) {
> - sb->s_flags |= SB_RDONLY;
> - btrfs_info(fs_info, "forced readonly");
> - /*
> - * Note that a running device replace operation is not
> - * canceled here although there is no way to update
> - * the progress. It would add the risk of a deadlock,
> - * therefore the canceling is omitted. The only penalty
> - * is that some I/O remains active until the procedure
> - * completes. The next time when the filesystem is
> - * mounted writeable again, the device replace
> - * operation continues.
> - */
> - }
> + sb->s_flags |= SB_RDONLY;
> + btrfs_info(fs_info, "forced readonly");
> + /*
> + * Note that a running device replace operation is not
> + * canceled here although there is no way to update
> + * the progress. It would add the risk of a deadlock,
> + * therefore the canceling is omitted. The only penalty
> + * is that some I/O remains active until the procedure
> + * completes. The next time when the filesystem is
> + * mounted writeable again, the device replace
> + * operation continues.
> + */
> }
>
> /*
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] btrfs: remove check for BTRFS_FS_STATE_ERROR which we just set
__btrfs_handle_fs_error() sets BTRFS_FS_STATE_ERROR, and calls
btrfs_handle_error() so no need to check if the BTRFS_FS_STATE_ERROR
is set in btrfs_handle_error(). And there is no other user of
btrfs_handle_error() as well.
Signed-off-by: Anand Jain
---
fs/btrfs/super.c | 26 --
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 4c49bb5632a2..60766f8ca434 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -113,20 +113,18 @@ static void btrfs_handle_error(struct btrfs_fs_info
*fs_info)
if (sb_rdonly(sb))
return;
- if (test_bit(BTRFS_FS_STATE_ERROR, &fs_info->fs_state)) {
- sb->s_flags |= SB_RDONLY;
- btrfs_info(fs_info, "forced readonly");
- /*
-* Note that a running device replace operation is not
-* canceled here although there is no way to update
-* the progress. It would add the risk of a deadlock,
-* therefore the canceling is omitted. The only penalty
-* is that some I/O remains active until the procedure
-* completes. The next time when the filesystem is
-* mounted writeable again, the device replace
-* operation continues.
-*/
- }
+ sb->s_flags |= SB_RDONLY;
+ btrfs_info(fs_info, "forced readonly");
+ /*
+* Note that a running device replace operation is not
+* canceled here although there is no way to update
+* the progress. It would add the risk of a deadlock,
+* therefore the canceling is omitted. The only penalty
+* is that some I/O remains active until the procedure
+* completes. The next time when the filesystem is
+* mounted writeable again, the device replace
+* operation continues.
+*/
}
/*
--
2.15.0
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] btrfS: collapse btrfs_handle_error() into __btrfs_handle_fs_error()
On 4.01.2018 12:01, Anand Jain wrote:
> There is no other consumer for btrfs_handle_error() other than
> __btrfs_handle_fs_error(), further this function quite small.
> Merge it into its parent.
>
> Signed-off-by: Anand Jain
Reviewed-by: Nikolay Borisov
> ---
> fs/btrfs/super.c | 43 +++
> 1 file changed, 19 insertions(+), 24 deletions(-)
>
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 60766f8ca434..0216a5dd0f4b 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -105,28 +105,6 @@ const char *btrfs_decode_error(int errno)
> return errstr;
> }
>
> -/* btrfs handle error by forcing the filesystem readonly */
> -static void btrfs_handle_error(struct btrfs_fs_info *fs_info)
> -{
> - struct super_block *sb = fs_info->sb;
> -
> - if (sb_rdonly(sb))
> - return;
> -
> - sb->s_flags |= SB_RDONLY;
> - btrfs_info(fs_info, "forced readonly");
> - /*
> - * Note that a running device replace operation is not
> - * canceled here although there is no way to update
> - * the progress. It would add the risk of a deadlock,
> - * therefore the canceling is omitted. The only penalty
> - * is that some I/O remains active until the procedure
> - * completes. The next time when the filesystem is
> - * mounted writeable again, the device replace
> - * operation continues.
> - */
> -}
> -
> /*
> * __btrfs_handle_fs_error decodes expected errors from the caller and
> * invokes the approciate error response.
> @@ -173,8 +151,25 @@ void __btrfs_handle_fs_error(struct btrfs_fs_info
> *fs_info, const char *function
> set_bit(BTRFS_FS_STATE_ERROR, &fs_info->fs_state);
>
> /* Don't go through full error handling during mount */
> - if (sb->s_flags & SB_BORN)
> - btrfs_handle_error(fs_info);
> + if (!(sb->s_flags & SB_BORN))
> + return;
> +
> + if (sb_rdonly(sb))
> + return;
> +
> + /* btrfs handle error by forcing the filesystem readonly */
> + sb->s_flags |= SB_RDONLY;
> + btrfs_info(fs_info, "forced readonly");
> + /*
> + * Note that a running device replace operation is not
> + * canceled here although there is no way to update
> + * the progress. It would add the risk of a deadlock,
> + * therefore the canceling is omitted. The only penalty
> + * is that some I/O remains active until the procedure
> + * completes. The next time when the filesystem is
> + * mounted writeable again, the device replace
> + * operation continues.
> + */
> }
>
> #ifdef CONFIG_PRINTK
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] btrfS: collapse btrfs_handle_error() into __btrfs_handle_fs_error()
There is no other consumer for btrfs_handle_error() other than
__btrfs_handle_fs_error(), further this function quite small.
Merge it into its parent.
Signed-off-by: Anand Jain
---
fs/btrfs/super.c | 43 +++
1 file changed, 19 insertions(+), 24 deletions(-)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 60766f8ca434..0216a5dd0f4b 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -105,28 +105,6 @@ const char *btrfs_decode_error(int errno)
return errstr;
}
-/* btrfs handle error by forcing the filesystem readonly */
-static void btrfs_handle_error(struct btrfs_fs_info *fs_info)
-{
- struct super_block *sb = fs_info->sb;
-
- if (sb_rdonly(sb))
- return;
-
- sb->s_flags |= SB_RDONLY;
- btrfs_info(fs_info, "forced readonly");
- /*
-* Note that a running device replace operation is not
-* canceled here although there is no way to update
-* the progress. It would add the risk of a deadlock,
-* therefore the canceling is omitted. The only penalty
-* is that some I/O remains active until the procedure
-* completes. The next time when the filesystem is
-* mounted writeable again, the device replace
-* operation continues.
-*/
-}
-
/*
* __btrfs_handle_fs_error decodes expected errors from the caller and
* invokes the approciate error response.
@@ -173,8 +151,25 @@ void __btrfs_handle_fs_error(struct btrfs_fs_info
*fs_info, const char *function
set_bit(BTRFS_FS_STATE_ERROR, &fs_info->fs_state);
/* Don't go through full error handling during mount */
- if (sb->s_flags & SB_BORN)
- btrfs_handle_error(fs_info);
+ if (!(sb->s_flags & SB_BORN))
+ return;
+
+ if (sb_rdonly(sb))
+ return;
+
+ /* btrfs handle error by forcing the filesystem readonly */
+ sb->s_flags |= SB_RDONLY;
+ btrfs_info(fs_info, "forced readonly");
+ /*
+* Note that a running device replace operation is not
+* canceled here although there is no way to update
+* the progress. It would add the risk of a deadlock,
+* therefore the canceling is omitted. The only penalty
+* is that some I/O remains active until the procedure
+* completes. The next time when the filesystem is
+* mounted writeable again, the device replace
+* operation continues.
+*/
}
#ifdef CONFIG_PRINTK
--
2.15.0
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs: Remove unused btrfs_start_transaction_lflush function
On 2.01.2018 18:17, David Sterba wrote:
> On Fri, Dec 15, 2017 at 12:06:18PM +0200, Nikolay Borisov wrote:
>> Commit 0e8c36a9fd81 ("Btrfs: fix lots of orphan inodes when the space
>> is not enough") changed the way transaction reservation is made in
>> btrfs_evict_node and as a result this function became unused. This has
>> been the status quo for 5 years in which time no one noticed, so I'd
>> say it's safe to assume it's unlikely it will ever be used again.
>>
>> Signed-off-by: Nikolay Borisov
>
> https://marc.info/?l=linux-btrfs&m=142117508730660&w=2
> https://marc.info/?l=linux-kernel&m=142119399504907&w=2
>
> "Transcaction API, removing the func does not make sense without
> removing BTRFS_RESERVE_FLUSH_LIMIT at the same time."
I can't quite understand what you are saying. There are couple of more
places where BTRFS_RESERVE_FLUSH_LIMIT is used outside of the
transaction api (btrfs_delalloc_reserve_metadata, btrfs_evict_inode and
reserve_metadata_space). Do you mean those function should also remove
the usage of FLUSH_LIMIT?
>
>> ---
>> fs/btrfs/transaction.c | 10 +-
>> fs/btrfs/transaction.h | 3 ---
>> 2 files changed, 1 insertion(+), 12 deletions(-)
>>
>> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
>> index 5a8c2649af2f..b1793688c716 100644
>> --- a/fs/btrfs/transaction.c
>> +++ b/fs/btrfs/transaction.c
>> @@ -542,7 +542,7 @@ start_transaction(struct btrfs_root *root, unsigned int
>> num_items,
>> * and then we deadlock with somebody doing a freeze.
>> *
>> * If we are ATTACH, it means we just want to catch the current
>> - * transaction and commit it, so we needn't do sb_start_intwrite().
>> + * transaction and commit it, so we needn't do sb_start_intwrite().
>
> Unrelated change.
>
>> */
>> if (type & __TRANS_FREEZABLE)
>> sb_start_intwrite(fs_info->sb);
>> @@ -658,14 +658,6 @@ struct btrfs_trans_handle
>> *btrfs_start_transaction_fallback_global_rsv(
>> return trans;
>> }
>>
>> -struct btrfs_trans_handle *btrfs_start_transaction_lflush(
>> -struct btrfs_root *root,
>> -unsigned int num_items)
>> -{
>> -return start_transaction(root, num_items, TRANS_START,
>> - BTRFS_RESERVE_FLUSH_LIMIT, true);
>> -}
>> -
>> struct btrfs_trans_handle *btrfs_join_transaction(struct btrfs_root *root)
>> {
>> return start_transaction(root, 0, TRANS_JOIN, BTRFS_RESERVE_NO_FLUSH,
>> diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
>> index c55e44560103..9b25b0ba92ee 100644
>> --- a/fs/btrfs/transaction.h
>> +++ b/fs/btrfs/transaction.h
>> @@ -187,9 +187,6 @@ struct btrfs_trans_handle
>> *btrfs_start_transaction_fallback_global_rsv(
>> struct btrfs_root *root,
>> unsigned int num_items,
>> int min_factor);
>> -struct btrfs_trans_handle *btrfs_start_transaction_lflush(
>> -struct btrfs_root *root,
>> -unsigned int num_items);
>> struct btrfs_trans_handle *btrfs_join_transaction(struct btrfs_root *root);
>> struct btrfs_trans_handle *btrfs_join_transaction_nolock(struct btrfs_root
>> *root);
>> struct btrfs_trans_handle *btrfs_attach_transaction(struct btrfs_root
>> *root);
>> --
>> 2.7.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majord...@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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 06/10] writeback: introduce super_operations->write_metadata
On Thu 04-01-18 12:32:07, Dave Chinner wrote: > On Wed, Jan 03, 2018 at 02:59:21PM +0100, Jan Kara wrote: > > On Wed 03-01-18 13:32:19, Dave Chinner wrote: > > > I think we could probably block ->write_metadata if necessary via a > > > completion/wakeup style notification when a specific LSN is reached > > > by the log tail, but realistically if there's any amount of data > > > needing to be written it'll throttle data writes because the IO > > > pipeline is being kept full by background metadata writes > > > > So the problem I'm concerned about is a corner case. Consider a situation > > when you have no dirty data, only dirty metadata but enough of them to > > trigger background writeback. How should metadata writeback behave for XFS > > in this case? Who should be responsible that wb_writeback() just does not > > loop invoking ->write_metadata() as fast as CPU allows until xfsaild makes > > enough progress? > > > > Thinking about this today, I think this looping prevention belongs to > > wb_writeback(). > > Well, backgroudn data writeback can block in two ways. One is during > IO submission when the request queue is full, the other is when all > dirty inodes have had some work done on them and have all been moved > to b_more_io - wb_writeback waits for the __I_SYNC bit to be cleared > on the last(?) inode on that list, hence backing off before > submitting more IO. > > IOws, there's a "during writeback" blocking mechanism as well as a > "between cycles" block mechanism. > > > Sadly we don't have much info to decide how long to sleep > > before trying more writeback so we'd have to just sleep for > > if we found no writeback happened in the last writeback > > round before going through the whole writeback loop again. > > Right - I don't think we can provide a generic "between cycles" > blocking mechanism for XFS, but I'm pretty sure we can emulate a > "during writeback" blocking mechanism to avoid busy looping inside > the XFS code. > > e.g. if we get a writeback call that asks for 5% to be written, > and we already have a metadata writeback target of 5% in place, > that means we should block for a while. That would emulate request > queue blocking and prevent busy looping in this case If you can do this in XFS then fine, it saves some mess in the generic code. > > And > > ->write_metadata() for XFS would need to always return 0 (as in "no progress > > made") to make sure this busyloop avoidance logic in wb_writeback() > > triggers. ext4 and btrfs would return number of bytes written from > > ->write_metadata (or just 1 would be enough to indicate some progress in > > metadata writeback was made and busyloop avoidance is not needed). > > Well, if we block for a little while, we can indicate that progress > has been made and this whole mess would go away, right? Right. So let's just ignore the problem for the sake of Josef's patch set. Once the patches land and when XFS starts using the infrastructure, we will make sure this is handled properly. Honza -- Jan Kara SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fstests: btrfs/158: reproduce a scrub bug on raid6 corruption
On Tue, Jan 02, 2018 at 01:35:00PM -0700, Liu Bo wrote:
> This is to reproduce a bug of scrub, with which scrub is unable to
> repair raid6 corruption as expected.
>
> The kernel side fixes are
> Btrfs: make raid6 rebuild retry more
> Btrfs: fix scrub to repair raid6 corruption
>
> Signed-off-by: Liu Bo
Looks fine overall, I tested it with 4.15-rc6 kernel and test failed as
expected, re-tested successfully after applying the patches mentioned in
commit log.
Just some really minor issues below, and I can fix them on commit if
what I suggest looks sane to you.
> ---
> tests/btrfs/158 | 114
>
> tests/btrfs/158.out | 10 +
> tests/btrfs/group | 1 +
> 3 files changed, 125 insertions(+)
> create mode 100755 tests/btrfs/158
> create mode 100644 tests/btrfs/158.out
>
> diff --git a/tests/btrfs/158 b/tests/btrfs/158
> new file mode 100755
> index 000..43afc2d
> --- /dev/null
> +++ b/tests/btrfs/158
> @@ -0,0 +1,114 @@
> +#! /bin/bash
> +# FS QA Test 158
> +#
> +# The test case is check if scrub is able fix raid6 data corruption,
> +# ie. if there is data corruption on two disks in the same horizontal
> +# stripe, e.g. due to bitrot.
> +#
> +# The kernel fixes are
> +#Btrfs: make raid6 rebuild retry more
> +#Btrfs: fix scrub to repair raid6 corruption
> +#
> +#---
> +# Copyright (c) 2017 Oracle. All Rights Reserved.
2018?
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> +#---
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> + cd /
> + rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs btrfs
> +_supported_os Linux
> +_require_scratch_dev_pool 4
> +_require_btrfs_command inspect-internal dump-tree
> +
> +get_physical_stripe0()
> +{
> + $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
> + grep " DATA\|RAID6" -A 10 | \
> + $AWK_PROG '($1 ~ /stripe/ && $3 ~ /devid/ && $2 ~ /0/) { print $6 }'
> +}
> +
> +get_physical_stripe1()
> +{
> + $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
> + grep " DATA\|RAID6" -A 10 | \
> + $AWK_PROG '($1 ~ /stripe/ && $3 ~ /devid/ && $2 ~ /1/) { print $6 }'
> +}
> +
> +_scratch_dev_pool_get 4
> +# step 1: create a raid6 btrfs and create a 4K file
> +echo "step 1..mkfs.btrfs" >>$seqres.full
> +
> +mkfs_opts="-d raid6 -b 1G"
> +_scratch_pool_mkfs $mkfs_opts >>$seqres.full 2>&1
> +
> +# -o nospace_cache makes sure data is written to the start position of the
> data
> +# chunk
> +_scratch_mount -o nospace_cache
> +
> +# [0,64K) is written to stripe 0 and [64K, 128K) is written to stripe 1
> +$XFS_IO_PROG -f -d -c "pwrite -S 0xaa 0 128K" -c "fsync" \
> + "$SCRATCH_MNT/foobar" | _filter_xfs_io
> +
> +_scratch_unmount
> +
> +stripe_0=`get_physical_stripe0`
> +stripe_1=`get_physical_stripe1`
> +dev4=`echo $SCRATCH_DEV_POOL | awk '{print $4}'`
> +dev3=`echo $SCRATCH_DEV_POOL | awk '{print $3}'`
> +
> +# step 2: corrupt the 1st and 2nd stripe (stripe 0 and 1)
> +echo "step 2..simulate bitrot at offset $stripe_0 of device_4($dev4) and
> offset $stripe_1 of device_3($dev3)" >>$seqres.full
> +
> +$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $stripe_0 64K" $dev4 | _filter_xfs_io
> +$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $stripe_1 64K" $dev3 | _filter_xfs_io
> +
> +# step 3: read foobar to repair the bitrot
Comment meant to be "scrub to repair the bitrot"?
> +echo "step 3..repair the bitrot" >> $seqres.full
> +_scratch_mount -o nospace_cache
> +
> +btrfs scrub start -B $SCRATCH_MNT >> $seqres.full 2>&1
$BTRFS_UTIL_PROG ...
> +
> +od -x $SCRATCH_MNT/foobar
> +
> +_scratch_dev_pool_put
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/158.out b/tests/btrfs/158.out
> new file mode 100644
> index 000..1f5ad3f
> --- /dev/null
> +++ b/tests/btrfs/158
