* [PATCH 2/3] btrfs: delete no_used argument in btrfs_copy_from_user
2016-01-06 11:00 [PATCH 1/3] btrfs: Continue write in case of can_not_nocow Zhao Lei
@ 2016-01-06 11:00 ` Zhao Lei
2016-01-06 11:00 ` [PATCH 3/3] btrfs: merge functions for wait snapshot creation Zhao Lei
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Zhao Lei @ 2016-01-06 11:00 UTC (permalink / raw)
To: linux-btrfs; +Cc: Zhao Lei
size_t write_bytes is not necessary for btrfs_copy_from_user(),
delete it.
Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
fs/btrfs/file.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 11fd981..e4f287c 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -406,8 +406,7 @@ int btrfs_run_defrag_inodes(struct btrfs_fs_info *fs_info)
/* simple helper to fault in pages and copy. This should go away
* and be replaced with calls into generic code.
*/
-static noinline int btrfs_copy_from_user(loff_t pos, int num_pages,
- size_t write_bytes,
+static noinline int btrfs_copy_from_user(loff_t pos, size_t write_bytes,
struct page **prepared_pages,
struct iov_iter *i)
{
@@ -1575,8 +1574,7 @@ again:
ret = 0;
}
- copied = btrfs_copy_from_user(pos, num_pages,
- write_bytes, pages, i);
+ copied = btrfs_copy_from_user(pos, write_bytes, pages, i);
/*
* if we have trouble faulting in the pages, fall
--
1.8.5.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/3] btrfs: merge functions for wait snapshot creation
2016-01-06 11:00 [PATCH 1/3] btrfs: Continue write in case of can_not_nocow Zhao Lei
2016-01-06 11:00 ` [PATCH 2/3] btrfs: delete no_used argument in btrfs_copy_from_user Zhao Lei
@ 2016-01-06 11:00 ` Zhao Lei
2016-01-20 20:48 ` [PATCH 1/3] btrfs: Continue write in case of can_not_nocow Filipe Manana
2016-02-26 5:41 ` Roman Mamedov
3 siblings, 0 replies; 7+ messages in thread
From: Zhao Lei @ 2016-01-06 11:00 UTC (permalink / raw)
To: linux-btrfs; +Cc: Zhao Lei
wait_for_snapshot_creation() is in same group with oher two:
btrfs_start_write_no_snapshoting()
btrfs_end_write_no_snapshoting()
Rename wait_for_snapshot_creation() and move it into same place
with other two.
Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
fs/btrfs/ctree.h | 1 +
fs/btrfs/extent-tree.c | 20 ++++++++++++++++++++
fs/btrfs/inode.c | 22 +---------------------
3 files changed, 22 insertions(+), 21 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 0912f89..5d7a2c4d 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3566,6 +3566,7 @@ int btrfs_delayed_refs_qgroup_accounting(struct btrfs_trans_handle *trans,
int __get_raid_index(u64 flags);
int btrfs_start_write_no_snapshoting(struct btrfs_root *root);
void btrfs_end_write_no_snapshoting(struct btrfs_root *root);
+void btrfs_wait_for_snapshot_creation(struct btrfs_root *root);
void check_system_chunk(struct btrfs_trans_handle *trans,
struct btrfs_root *root,
const u64 type);
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index dc3763a..e83ffa8 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -10704,3 +10704,23 @@ int btrfs_start_write_no_snapshoting(struct btrfs_root *root)
}
return 1;
}
+
+static int wait_snapshoting_atomic_t(atomic_t *a)
+{
+ schedule();
+ return 0;
+}
+
+void btrfs_wait_for_snapshot_creation(struct btrfs_root *root)
+{
+ while (true) {
+ int ret;
+
+ ret = btrfs_start_write_no_snapshoting(root);
+ if (ret)
+ break;
+ wait_on_atomic_t(&root->will_be_snapshoted,
+ wait_snapshoting_atomic_t,
+ TASK_UNINTERRUPTIBLE);
+ }
+}
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 994490d..21d7cd1 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -4897,26 +4897,6 @@ next:
return err;
}
-static int wait_snapshoting_atomic_t(atomic_t *a)
-{
- schedule();
- return 0;
-}
-
-static void wait_for_snapshot_creation(struct btrfs_root *root)
-{
- while (true) {
- int ret;
-
- ret = btrfs_start_write_no_snapshoting(root);
- if (ret)
- break;
- wait_on_atomic_t(&root->will_be_snapshoted,
- wait_snapshoting_atomic_t,
- TASK_UNINTERRUPTIBLE);
- }
-}
-
static int btrfs_setsize(struct inode *inode, struct iattr *attr)
{
struct btrfs_root *root = BTRFS_I(inode)->root;
@@ -4948,7 +4928,7 @@ static int btrfs_setsize(struct inode *inode, struct iattr *attr)
* truncation, it must capture all writes that happened before
* this truncation.
*/
- wait_for_snapshot_creation(root);
+ btrfs_wait_for_snapshot_creation(root);
ret = btrfs_cont_expand(inode, oldsize, newsize);
if (ret) {
btrfs_end_write_no_snapshoting(root);
--
1.8.5.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 1/3] btrfs: Continue write in case of can_not_nocow
2016-01-06 11:00 [PATCH 1/3] btrfs: Continue write in case of can_not_nocow Zhao Lei
2016-01-06 11:00 ` [PATCH 2/3] btrfs: delete no_used argument in btrfs_copy_from_user Zhao Lei
2016-01-06 11:00 ` [PATCH 3/3] btrfs: merge functions for wait snapshot creation Zhao Lei
@ 2016-01-20 20:48 ` Filipe Manana
2016-01-21 3:39 ` Zhao Lei
2016-02-26 5:41 ` Roman Mamedov
3 siblings, 1 reply; 7+ messages in thread
From: Filipe Manana @ 2016-01-20 20:48 UTC (permalink / raw)
To: Zhao Lei; +Cc: linux-btrfs@vger.kernel.org
On Wed, Jan 6, 2016 at 11:00 AM, Zhao Lei <zhaolei@cn.fujitsu.com> wrote:
> btrfs failed in xfstests btrfs/080 with -o nodatacow.
>
> Can be reproduced by following script:
> DEV=/dev/vdg
> MNT=/mnt/tmp
>
> umount $DEV &>/dev/null
> mkfs.btrfs -f $DEV
> mount -o nodatacow $DEV $MNT
>
> dd if=/dev/zero of=$MNT/test bs=1 count=2048 &
> btrfs subvolume snapshot -r $MNT $MNT/test_snap &
> wait
> --
> We can see dd failed on NO_SPACE.
>
> Reason:
> __btrfs_buffered_write should run cow write when no_cow impossible,
> and current code is designed with above logic.
> But check_can_nocow() have 2 type of return value(0 and <0) on
> can_not_no_cow, and current code only continue write on first case,
> the second case happened in doing subvolume.
>
> Fix:
> Continue write when check_can_nocow() return 0 and <0.
>
> Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Could you please submit a test case for xfstests as well?
Thanks.
> ---
> fs/btrfs/file.c | 37 +++++++++++++++++--------------------
> 1 file changed, 17 insertions(+), 20 deletions(-)
>
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index 977e715..11fd981 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -1516,27 +1516,24 @@ static noinline ssize_t __btrfs_buffered_write(struct file *file,
>
> reserve_bytes = num_pages << PAGE_CACHE_SHIFT;
>
> - if (BTRFS_I(inode)->flags & (BTRFS_INODE_NODATACOW |
> - BTRFS_INODE_PREALLOC)) {
> - ret = check_can_nocow(inode, pos, &write_bytes);
> - if (ret < 0)
> - break;
> - if (ret > 0) {
> - /*
> - * For nodata cow case, no need to reserve
> - * data space.
> - */
> - only_release_metadata = true;
> - /*
> - * our prealloc extent may be smaller than
> - * write_bytes, so scale down.
> - */
> - num_pages = DIV_ROUND_UP(write_bytes + offset,
> - PAGE_CACHE_SIZE);
> - reserve_bytes = num_pages << PAGE_CACHE_SHIFT;
> - goto reserve_metadata;
> - }
> + if ((BTRFS_I(inode)->flags & (BTRFS_INODE_NODATACOW |
> + BTRFS_INODE_PREALLOC)) &&
> + check_can_nocow(inode, pos, &write_bytes) > 0) {
> + /*
> + * For nodata cow case, no need to reserve
> + * data space.
> + */
> + only_release_metadata = true;
> + /*
> + * our prealloc extent may be smaller than
> + * write_bytes, so scale down.
> + */
> + num_pages = DIV_ROUND_UP(write_bytes + offset,
> + PAGE_CACHE_SIZE);
> + reserve_bytes = num_pages << PAGE_CACHE_SHIFT;
> + goto reserve_metadata;
> }
> +
> ret = btrfs_check_data_free_space(inode, pos, write_bytes);
> if (ret < 0)
> break;
> --
> 1.8.5.1
>
>
>
> --
> 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
--
Filipe David Manana,
"Reasonable men adapt themselves to the world.
Unreasonable men adapt the world to themselves.
That's why all progress depends on unreasonable men."
^ permalink raw reply [flat|nested] 7+ messages in thread* RE: [PATCH 1/3] btrfs: Continue write in case of can_not_nocow
2016-01-20 20:48 ` [PATCH 1/3] btrfs: Continue write in case of can_not_nocow Filipe Manana
@ 2016-01-21 3:39 ` Zhao Lei
0 siblings, 0 replies; 7+ messages in thread
From: Zhao Lei @ 2016-01-21 3:39 UTC (permalink / raw)
To: fdmanana; +Cc: linux-btrfs
Hi, Filipe Manana
> -----Original Message-----
> From: Filipe Manana [mailto:fdmanana@gmail.com]
> Sent: Thursday, January 21, 2016 4:49 AM
> To: Zhao Lei <zhaolei@cn.fujitsu.com>
> Cc: linux-btrfs@vger.kernel.org
> Subject: Re: [PATCH 1/3] btrfs: Continue write in case of can_not_nocow
>
> On Wed, Jan 6, 2016 at 11:00 AM, Zhao Lei <zhaolei@cn.fujitsu.com> wrote:
> > btrfs failed in xfstests btrfs/080 with -o nodatacow.
> >
> > Can be reproduced by following script:
> > DEV=/dev/vdg
> > MNT=/mnt/tmp
> >
> > umount $DEV &>/dev/null
> > mkfs.btrfs -f $DEV
> > mount -o nodatacow $DEV $MNT
> >
> > dd if=/dev/zero of=$MNT/test bs=1 count=2048 &
> > btrfs subvolume snapshot -r $MNT $MNT/test_snap &
> > wait
> > --
> > We can see dd failed on NO_SPACE.
> >
> > Reason:
> > __btrfs_buffered_write should run cow write when no_cow impossible,
> > and current code is designed with above logic.
> > But check_can_nocow() have 2 type of return value(0 and <0) on
> > can_not_no_cow, and current code only continue write on first case,
> > the second case happened in doing subvolume.
> >
> > Fix:
> > Continue write when check_can_nocow() return 0 and <0.
> >
> > Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
> Reviewed-by: Filipe Manana <fdmanana@suse.com>
>
Thanks for review.
> Could you please submit a test case for xfstests as well?
btrfs/080 with -o nodatacow already can trigger this bug.
Thanks
Zhaolei
> Thanks.
>
> > ---
> > fs/btrfs/file.c | 37 +++++++++++++++++--------------------
> > 1 file changed, 17 insertions(+), 20 deletions(-)
> >
> > diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index 977e715..11fd981
> > 100644
> > --- a/fs/btrfs/file.c
> > +++ b/fs/btrfs/file.c
> > @@ -1516,27 +1516,24 @@ static noinline ssize_t
> > __btrfs_buffered_write(struct file *file,
> >
> > reserve_bytes = num_pages << PAGE_CACHE_SHIFT;
> >
> > - if (BTRFS_I(inode)->flags & (BTRFS_INODE_NODATACOW |
> > -
> BTRFS_INODE_PREALLOC)) {
> > - ret = check_can_nocow(inode, pos,
> &write_bytes);
> > - if (ret < 0)
> > - break;
> > - if (ret > 0) {
> > - /*
> > - * For nodata cow case, no need to
> reserve
> > - * data space.
> > - */
> > - only_release_metadata = true;
> > - /*
> > - * our prealloc extent may be smaller
> than
> > - * write_bytes, so scale down.
> > - */
> > - num_pages =
> DIV_ROUND_UP(write_bytes + offset,
> > -
> PAGE_CACHE_SIZE);
> > - reserve_bytes = num_pages <<
> PAGE_CACHE_SHIFT;
> > - goto reserve_metadata;
> > - }
> > + if ((BTRFS_I(inode)->flags & (BTRFS_INODE_NODATACOW
> |
> > +
> BTRFS_INODE_PREALLOC)) &&
> > + check_can_nocow(inode, pos, &write_bytes) > 0) {
> > + /*
> > + * For nodata cow case, no need to reserve
> > + * data space.
> > + */
> > + only_release_metadata = true;
> > + /*
> > + * our prealloc extent may be smaller than
> > + * write_bytes, so scale down.
> > + */
> > + num_pages = DIV_ROUND_UP(write_bytes +
> offset,
> > +
> PAGE_CACHE_SIZE);
> > + reserve_bytes = num_pages <<
> PAGE_CACHE_SHIFT;
> > + goto reserve_metadata;
> > }
> > +
> > ret = btrfs_check_data_free_space(inode, pos,
> write_bytes);
> > if (ret < 0)
> > break;
> > --
> > 1.8.5.1
> >
> >
> >
> > --
> > 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
>
>
>
> --
> Filipe David Manana,
>
> "Reasonable men adapt themselves to the world.
> Unreasonable men adapt the world to themselves.
> That's why all progress depends on unreasonable men."
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] btrfs: Continue write in case of can_not_nocow
2016-01-06 11:00 [PATCH 1/3] btrfs: Continue write in case of can_not_nocow Zhao Lei
` (2 preceding siblings ...)
2016-01-20 20:48 ` [PATCH 1/3] btrfs: Continue write in case of can_not_nocow Filipe Manana
@ 2016-02-26 5:41 ` Roman Mamedov
2016-02-26 14:57 ` Chris Mason
3 siblings, 1 reply; 7+ messages in thread
From: Roman Mamedov @ 2016-02-26 5:41 UTC (permalink / raw)
To: Zhao Lei; +Cc: linux-btrfs
[-- Attachment #1: Type: text/plain, Size: 2957 bytes --]
On Wed, 6 Jan 2016 19:00:17 +0800
Zhao Lei <zhaolei@cn.fujitsu.com> wrote:
> btrfs failed in xfstests btrfs/080 with -o nodatacow.
>
> Can be reproduced by following script:
> DEV=/dev/vdg
> MNT=/mnt/tmp
>
> umount $DEV &>/dev/null
> mkfs.btrfs -f $DEV
> mount -o nodatacow $DEV $MNT
>
> dd if=/dev/zero of=$MNT/test bs=1 count=2048 &
> btrfs subvolume snapshot -r $MNT $MNT/test_snap &
> wait
> --
> We can see dd failed on NO_SPACE.
>
> Reason:
> __btrfs_buffered_write should run cow write when no_cow impossible,
> and current code is designed with above logic.
> But check_can_nocow() have 2 type of return value(0 and <0) on
> can_not_no_cow, and current code only continue write on first case,
> the second case happened in doing subvolume.
>
> Fix:
> Continue write when check_can_nocow() return 0 and <0.
>
> Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
Guys please don't forget about this patch. It solves real problem for people,
http://www.spinics.net/lists/linux-btrfs/msg51276.html
http://www.spinics.net/lists/linux-btrfs/msg51819.html
but it's not in 4.4.1, not in 4.4.2... and now not in 4.4.3
> ---
> fs/btrfs/file.c | 37 +++++++++++++++++--------------------
> 1 file changed, 17 insertions(+), 20 deletions(-)
>
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index 977e715..11fd981 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -1516,27 +1516,24 @@ static noinline ssize_t __btrfs_buffered_write(struct file *file,
>
> reserve_bytes = num_pages << PAGE_CACHE_SHIFT;
>
> - if (BTRFS_I(inode)->flags & (BTRFS_INODE_NODATACOW |
> - BTRFS_INODE_PREALLOC)) {
> - ret = check_can_nocow(inode, pos, &write_bytes);
> - if (ret < 0)
> - break;
> - if (ret > 0) {
> - /*
> - * For nodata cow case, no need to reserve
> - * data space.
> - */
> - only_release_metadata = true;
> - /*
> - * our prealloc extent may be smaller than
> - * write_bytes, so scale down.
> - */
> - num_pages = DIV_ROUND_UP(write_bytes + offset,
> - PAGE_CACHE_SIZE);
> - reserve_bytes = num_pages << PAGE_CACHE_SHIFT;
> - goto reserve_metadata;
> - }
> + if ((BTRFS_I(inode)->flags & (BTRFS_INODE_NODATACOW |
> + BTRFS_INODE_PREALLOC)) &&
> + check_can_nocow(inode, pos, &write_bytes) > 0) {
> + /*
> + * For nodata cow case, no need to reserve
> + * data space.
> + */
> + only_release_metadata = true;
> + /*
> + * our prealloc extent may be smaller than
> + * write_bytes, so scale down.
> + */
> + num_pages = DIV_ROUND_UP(write_bytes + offset,
> + PAGE_CACHE_SIZE);
> + reserve_bytes = num_pages << PAGE_CACHE_SHIFT;
> + goto reserve_metadata;
> }
> +
> ret = btrfs_check_data_free_space(inode, pos, write_bytes);
> if (ret < 0)
> break;
--
With respect,
Roman
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/3] btrfs: Continue write in case of can_not_nocow
2016-02-26 5:41 ` Roman Mamedov
@ 2016-02-26 14:57 ` Chris Mason
0 siblings, 0 replies; 7+ messages in thread
From: Chris Mason @ 2016-02-26 14:57 UTC (permalink / raw)
To: Roman Mamedov; +Cc: Zhao Lei, linux-btrfs
On Fri, Feb 26, 2016 at 10:41:31AM +0500, Roman Mamedov wrote:
> On Wed, 6 Jan 2016 19:00:17 +0800
> Zhao Lei <zhaolei@cn.fujitsu.com> wrote:
>
> > btrfs failed in xfstests btrfs/080 with -o nodatacow.
> >
> > Can be reproduced by following script:
> > DEV=/dev/vdg
> > MNT=/mnt/tmp
> >
> > umount $DEV &>/dev/null
> > mkfs.btrfs -f $DEV
> > mount -o nodatacow $DEV $MNT
> >
> > dd if=/dev/zero of=$MNT/test bs=1 count=2048 &
> > btrfs subvolume snapshot -r $MNT $MNT/test_snap &
> > wait
> > --
> > We can see dd failed on NO_SPACE.
> >
> > Reason:
> > __btrfs_buffered_write should run cow write when no_cow impossible,
> > and current code is designed with above logic.
> > But check_can_nocow() have 2 type of return value(0 and <0) on
> > can_not_no_cow, and current code only continue write on first case,
> > the second case happened in doing subvolume.
> >
> > Fix:
> > Continue write when check_can_nocow() return 0 and <0.
> >
> > Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
>
> Guys please don't forget about this patch. It solves real problem for people,
> http://www.spinics.net/lists/linux-btrfs/msg51276.html
> http://www.spinics.net/lists/linux-btrfs/msg51819.html
> but it's not in 4.4.1, not in 4.4.2... and now not in 4.4.3
Dave already has it queued for the next merge window. Thanks!
-chris
^ permalink raw reply [flat|nested] 7+ messages in thread