* [PATCH] Btrfs: fix outstanding_extents accounting in DIO
@ 2015-03-17 15:04 Josef Bacik
2015-03-17 15:37 ` Filipe David Manana
0 siblings, 1 reply; 2+ messages in thread
From: Josef Bacik @ 2015-03-17 15:04 UTC (permalink / raw)
To: linux-btrfs
We are keeping track of how many extents we need to reserve properly based on
the amount we want to write, but we were still incrementing outstanding_extents
if we wrote less than what we requested. We need to fix this logic to only do
this if we request less than MAX_EXTENT_SIZE or if we write less than
MAX_EXTENT_SIZE when we request an amount larger than MAX_EXTENT_SIZE. This
fixes the problem Filipe reported with generic/300. Thanks,
Reported-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
fs/btrfs/inode.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 156d0f5..f6d2c56 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -7387,7 +7387,24 @@ unlock:
if (start + len > i_size_read(inode))
i_size_write(inode, start + len);
- if (len < orig_len) {
+ /*
+ * direct_io can send down chunks > BTRFS_MAX_EXTENT_SIZE, so we
+ * don't want to jack up outstanding_extents if we're just
+ * allocating the largest extent we can for a range that we've
+ * already reserved the approriate number of outstanding_extents
+ * for.
+ *
+ * So if orig_len <= BTRFS_MAX_EXTENT_SIZE and our allocated len
+ * is less than orig_len then we know we're going to end up with
+ * more extents than we reserved.
+ *
+ * If orig_len > BTRFS_MAX_EXTENT_SIZE but we weren't able to
+ * allocate a BTRFS_MAX_EXTENT_SIZE extent then we know we have
+ * to add another outstanding extent.
+ */
+ if (len < orig_len &&
+ (orig_len <= BTRFS_MAX_EXTENT_SIZE ||
+ len < BTRFS_MAX_EXTENT_SIZE)) {
spin_lock(&BTRFS_I(inode)->lock);
BTRFS_I(inode)->outstanding_extents++;
spin_unlock(&BTRFS_I(inode)->lock);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] Btrfs: fix outstanding_extents accounting in DIO
2015-03-17 15:04 [PATCH] Btrfs: fix outstanding_extents accounting in DIO Josef Bacik
@ 2015-03-17 15:37 ` Filipe David Manana
0 siblings, 0 replies; 2+ messages in thread
From: Filipe David Manana @ 2015-03-17 15:37 UTC (permalink / raw)
To: Josef Bacik; +Cc: linux-btrfs
On Tue, Mar 17, 2015 at 3:04 PM, Josef Bacik <jbacik@fb.com> wrote:
> We are keeping track of how many extents we need to reserve properly based on
> the amount we want to write, but we were still incrementing outstanding_extents
> if we wrote less than what we requested. We need to fix this logic to only do
> this if we request less than MAX_EXTENT_SIZE or if we write less than
> MAX_EXTENT_SIZE when we request an amount larger than MAX_EXTENT_SIZE. This
> fixes the problem Filipe reported with generic/300. Thanks,
>
> Reported-by: Filipe Manana <fdmanana@suse.com>
> Signed-off-by: Josef Bacik <jbacik@fb.com>
I hate to be the bringer of bad news, but this still causes the
warnings for the modified fstests/generic/300 everytime I run it.
I have all your recent patches dealing with fixing the accounting of
outstanding_extents applied:
$ git log --author='Josef Bacik' --pretty=oneline
4bfbd94312a4dc464f1827ca03faccb8da8f2aaf Btrfs: account merges properly
e0037ac670a4e462166f3ec28e691f8595fca607 Btrfs: fix merge delalloc logic
f46e5f2aebc726ed0dad860c3769a9bf63b0ea81 Btrfs: fix
outstanding_extents accounting in DIO
53f6e7d09b796371074ada19e452b7566682fe2f Btrfs: account for the
correct number of extents for delalloc reservations
b8c3ce1cfa5a5f7f6fe71b8f6ac4f601e7ccdf1f Btrfs: remove extra
run_delayed_refs in update_cowonly_root
(...)
Also, with the new fix for the merges of buffered extents ("Btrfs:
account merges properly") I get the assertion triggered again:
[13382.732589] BTRFS: assertion failed:
BTRFS_I(inode)->outstanding_extents >= num_extents, file:
fs/btrfs/extent-tree.c, line: 4983
[13382.735759] ------------[ cut here ]------------
[13382.736031] kernel BUG at fs/btrfs/ctree.h:4013!
[13382.736031] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
(...)
[13382.736031] [<ffffffffa03be666>] drop_outstanding_extent+0x3d/0x6d [btrfs]
For this fio job (more stressful then the previous simple fio job):
[global]
ioengine=sync
direct=0
bssplit=130M/100
fallocate=none
filename=foobar
[job1]
numjobs=1
size=4G
rw=randwrite
time_based
runtime=60
> ---
> fs/btrfs/inode.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 156d0f5..f6d2c56 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -7387,7 +7387,24 @@ unlock:
> if (start + len > i_size_read(inode))
> i_size_write(inode, start + len);
>
> - if (len < orig_len) {
> + /*
> + * direct_io can send down chunks > BTRFS_MAX_EXTENT_SIZE, so we
> + * don't want to jack up outstanding_extents if we're just
> + * allocating the largest extent we can for a range that we've
> + * already reserved the approriate number of outstanding_extents
> + * for.
> + *
> + * So if orig_len <= BTRFS_MAX_EXTENT_SIZE and our allocated len
> + * is less than orig_len then we know we're going to end up with
> + * more extents than we reserved.
> + *
> + * If orig_len > BTRFS_MAX_EXTENT_SIZE but we weren't able to
> + * allocate a BTRFS_MAX_EXTENT_SIZE extent then we know we have
> + * to add another outstanding extent.
> + */
> + if (len < orig_len &&
> + (orig_len <= BTRFS_MAX_EXTENT_SIZE ||
> + len < BTRFS_MAX_EXTENT_SIZE)) {
> spin_lock(&BTRFS_I(inode)->lock);
> BTRFS_I(inode)->outstanding_extents++;
> spin_unlock(&BTRFS_I(inode)->lock);
> --
> 1.8.3.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] 2+ messages in thread
end of thread, other threads:[~2015-03-17 15:37 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-17 15:04 [PATCH] Btrfs: fix outstanding_extents accounting in DIO Josef Bacik
2015-03-17 15:37 ` Filipe David Manana
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.