Hi everyone,
I'm running currently a patched linux 3.8.0 kernel (SCST iscsi target
patches and STEC enhanceIO driver support)
I created a btrfs raid10 filesystem with mkfs.btrfs -m raid10 -d
raid10 /dev/sdf /dev/sdg /dev/sdh /dev/sdi
On this volume I added a sparse file and shared it over iscsi
There are several bugs at error path of create_snapshot() when the
transaction commitment failed.
- access the freed transaction handler. At the end of the
transaction commitment, the transaction handler was freed, so we
should not access it after the transaction commitment.
- we were not
If the async transaction commitment failed, we need close the
current transaction handler, or the current transaction will be
blocked to commit because of this orphan handler.
We fix the problem by doing sync transaction commitment, that is
to invoke btrfs_commit_transaction().
Signed-off-by:
On Mon, Mar 04, 2013 at 05:44:29PM +0800, Miao Xie wrote:
There are several bugs at error path of create_snapshot() when the
transaction commitment failed.
- access the freed transaction handler. At the end of the
transaction commitment, the transaction handler was freed, so we
should not
Same problem occurs on linux kernel 3.9.0-rc1
I'll try if I can reproduce it without the use of the enhanceIO driver
from STEC.
this is my scst config file:
linux-testsan:~ # cat /etc/scst.conf
HANDLER vdisk_fileio {
DEVICE disk01 {
filename /btrfs/lun0.img
Some more maybe usefull information. After reboot, the btrfs raid10
filesystem is unmountable.
I'm now running the test without enhanceIO from IO.
linux-testsan:~ #btrfs device scan --all-devices
[ 881.520686] device fsid 21b0cd45-019d-4e83-bf2f-053eaaf8b380 devid
2 transid 10 /dev/sdd
[
On Mon, Mar 04, 2013 at 07:06:05AM -0700, Joeri Vanthienen wrote:
Some more maybe usefull information. After reboot, the btrfs raid10
filesystem is unmountable.
I'm now running the test without enhanceIO from IO.
linux-testsan:~ #btrfs device scan --all-devices
[ 881.520686] device fsid
Hi Chris,
Same problem without enhanceIO.
Wiped all the disks, created new raid10 fs with 4 sas disks.
Sparse file mapped over iscsi with SCST and iometer at the other side
running some workload.
After some seconds, minutes several CPU stuck messages and the same
messages as posted before in
Patch 1-5 can make balance process bail out gracefully after we get into
readonly by aborting transaction.
Patch 6 addresses a hang when we have two or more trans handle aborted
and committed themselves.
Liu Bo (6):
Btrfs: check for NULL pointer in updating reloc roots
Btrfs: build up error
Add a check for NULL pointer to avoid invalid reference.
Signed-off-by: Liu Bo bo.li@oracle.com
---
fs/btrfs/relocation.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 300e09a..d9be73b 100644
---
We've missed the 'free blocks' part on ENOMEM error.
Signed-off-by: Liu Bo bo.li@oracle.com
---
fs/btrfs/relocation.c |9 ++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 8608afb..8cae3d7 100644
---
We can bail out from here gracefully instead of a cold BUG_ON.
Signed-off-by: Liu Bo bo.li@oracle.com
---
fs/btrfs/relocation.c | 10 +-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 8cae3d7..c45c833 100644
---
Btrfs balance can easily hit BUG_ON in these places, but we want
to it bail out gracefully after we force the whole filesystem to
readonly. So we use btrfs_std_error hook in place of BUG_ON.
Signed-off-by: Liu Bo bo.li@oracle.com
---
fs/btrfs/relocation.c |6 +-
fs/btrfs/volumes.c
Only let one trans handle to wait for other handles, otherwise we
will get ABBA issues.
Signed-off-by: Liu Bo bo.li@oracle.com
---
fs/btrfs/transaction.c |7 +++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index
We first use btrfs_std_error hook to replace with BUG_ON, and we
also need to cleanup what is left, including reloc roots rbtree
and reloc roots list.
Here we use a helper function to cleanup both rbtree and list, and
since this function can also be used in the balance recover path,
we also make
Just ran the following command sequence and got lots of WARNINGs.
The issue is reproducible.
The box was running the cmason/for-linus that made it into Linux 3.9 RC1.
#!/bin/sh
mkfs.btrfs -f /dev/sdl /dev/sdk -m raid1 -d raid1 -l 16384
mount /dev/sdl /mnt
dd if=/dev/urandom of=/mnt/urandom.1GB
Commit 5ac00add added a testnset mutex and code that disallows
running administrative tasks in parallel. It is prevented that
the device add/delete/balance/replace/resize operations are
started in parallel. By mistake, the defragmentation operation
was included in the check for mutually
(dropping cc:s just to -btrfs)
Geert and James both sent this one in, sorry guys.
FWIW, dhowells now packages cross gcc and binutils in fedora
specifically for cross kernel builds.
# yum list cross-{binutils,gcc}-common
cross-binutils-common.noarch 2.23.51.0.3-1.fc18 fedora
On Mon, Mar 04, 2013 at 10:24:39AM -0700, Stefan Behrens wrote:
Just ran the following command sequence and got lots of WARNINGs.
The issue is reproducible.
The box was running the cmason/for-linus that made it into Linux 3.9 RC1.
#!/bin/sh
mkfs.btrfs -f /dev/sdl /dev/sdk -m raid1 -d raid1
close fd if open, and free allocated memory in buf
Signed-off-by: Eric Sandeen sand...@redhat.com
---
convert.c |8 ++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/convert.c b/convert.c
index 4a75895..76a1076 100644
--- a/convert.c
+++ b/convert.c
@@ -2455,7 +2455,7
stops an fd leak that Coverity found.
Signed-off-by: Eric Sandeen sand...@redhat.com
---
convert.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/convert.c b/convert.c
index 2b3f42f..4a75895 100644
--- a/convert.c
+++ b/convert.c
@@ -2277,7 +2277,8 @@ err:
int
consolidate error handling to ensure that peer_fd
is closed on error paths. Add a couple comments
to the error handling after the thread is complete.
Note that scrub_progress_cycle returns negative
errnos on any error.
Signed-off-by: Eric Sandeen sand...@redhat.com
---
cmds-scrub.c | 48
The two sigint handlers issue ioctls to clean up, but if
they fail, noone would know. I'm not sure there is
any other error handling to be done at this point, but a
notification seems wise.
Signed-off-by: Eric Sandeen sand...@redhat.com
---
cmds-replace.c |5 -
cmds-scrub.c |6
stops an fd leak that Coverity found.
Signed-off-by: Eric Sandeen sand...@redhat.com
---
cmds-subvolume.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index 461eed9..a13a58d 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@
Today wrong cmdlines give odd results:
# ./btrfs-vol /dev/sdb1
Unable to open device (null)
# ./btrfs-vol -a /dev/sdb1
usage: btrfs-vol [options] mount_point ...
Make it a bit more informative:
# ./btrfs-vol /dev/sdb1
No command specified
If we discover that a passed-in fd is not a mountpoint,
we determine whether it is a device, and issue another
open() against the device's mount point if it is mounted.
If we do so, ensure this 2nd fd gets closed before we return
so that it does not leak, by consolidating error returns.
This gets the coverity issue count down to 33. Before Zach started this
process, we were over 150, IIRC. So it's almost to the point where the
scans will be manageable going forward.
Not a lot of real bugfixes here, but a bit better error handling in
places. I sent out 2 dumb patches
It seems highly unlikely that posix_fadvise could fail,
and even if it does, it was only advisory. Still, if
it does, we could issue a notice to the user.
Signed-off-by: Eric Sandeen sand...@redhat.com
---
disk-io.c |6 --
volumes.c |3 ++-
2 files changed, 6 insertions(+), 3
cmd_subvol_create() currently returns without freeing resources
in almost every error case. Switch to a goto arrangement
so all cleanup can be done in one place.
Signed-off-by: Eric Sandeen sand...@redhat.com
---
cmds-subvolume.c | 29 -
1 files changed, 16
cmd_snapshot() currently returns without freeing resources
in almost every error case. Switch to a goto arrangement
so all cleanup can be done in one place.
Signed-off-by: Eric Sandeen sand...@redhat.com
---
cmds-subvolume.c | 41 -
1 files changed, 24
Just whitespace fixes, and magical return value removal.
Signed-off-by: Eric Sandeen sand...@redhat.com
---
cmds-subvolume.c | 18 +-
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index 96f7cbd..bee 100644
---
Just whitespace fixes, and magical return value removal.
Signed-off-by: Eric Sandeen sand...@redhat.com
---
cmds-subvolume.c | 30 +++---
1 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index a13a58d..a4d88a1 100644
Because it's better than a segfault if it's called improperly,
and it makes static checkers happier.
Signed-off-by: Eric Sandeen sand...@redhat.com
---
utils.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/utils.c b/utils.c
index 54d577c..a38ac70 100644
--- a/utils.c
Don't return w/ metadump still allocated
Signed-off-by: Eric Sandeen sand...@redhat.com
---
btrfs-image.c |9 +
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/btrfs-image.c b/btrfs-image.c
index a54e6c9..5b0af28 100644
--- a/btrfs-image.c
+++ b/btrfs-image.c
@@
+ ret = ioctl(cancel_fd, BTRFS_IOC_SCRUB_CANCEL, NULL);
+ if (ret 0)
+ perror(Scrub cancel failed\n);
Probably don't want the extra newline.
- z
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
- ret = pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, old);
+ ret = -pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, old);
if (ret)
- return ERR_PTR(-ret);
+ goto out;
Am I the only one who finds ret = -pthread_*() pretty odd? :) (ret = -0
on
Not a lot of real bugfixes here, but a bit better error handling in
places. I sent out 2 dumb patches (elsewhere) on Friday, though,
so feel free to take a close skeptical look at these ;)
Minus those two little direct replies I had, these seem fine :). Thanks
for keeping this work going.
Am Mittwoch, 27. Februar 2013 schrieb Ahmet Inan:
Yeah we have a lot of
ptr = kmalloc();
BUG_ON(ptr);
everywhere. I'll fix this one up but I really need to sit down and go
through all of them and make sure we do the right thing in all these
places. Thanks,
But what would be
On 3/4/13 4:00 PM, Zach Brown wrote:
-ret = pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, old);
+ret = -pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, old);
if (ret)
-return ERR_PTR(-ret);
+goto out;
Am I the only one who finds ret =
On 3/4/13 3:57 PM, Zach Brown wrote:
+ret = ioctl(cancel_fd, BTRFS_IOC_SCRUB_CANCEL, NULL);
+if (ret 0)
+perror(Scrub cancel failed\n);
Probably don't want the extra newline.
- z
Doh, thanks.
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs
The two sigint handlers issue ioctls to clean up, but if
they fail, noone would know. I'm not sure there is
any other error handling to be done at this point, but a
notification seems wise.
Signed-off-by: Eric Sandeen sand...@redhat.com
---
cmds-replace.c |5 -
cmds-scrub.c |6
consolidate error handling to ensure that peer_fd
is closed on error paths. Add a couple comments
to the error handling after the thread is complete.
Note that scrub_progress_cycle returns negative
errnos on any error.
Signed-off-by: Eric Sandeen sand...@redhat.com
---
V2: collect positive /
If pthread_mutex_lock() fails it returns the error in ret,
and does not set errno.
Signed-off-by: Eric Sandeen sand...@redhat.com
---
diff --git a/cmds-scrub.c b/cmds-scrub.c
index f73b3c6..8129601 100644
--- a/cmds-scrub.c
+++ b/cmds-scrub.c
@@ -763,7 +763,7 @@ static int
+ perr = pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, old);
+ if (perr)
+ goto out;
+out:
+ if (peer_fd != -1)
+ close(peer_fd);
+ if (perr)
+ ret = -perr;
+ return ERR_PTR(ret);
}
Great, that's a lot clearer. Thanks.
Device scanning waits on the uuid_mutex, which can result in a very long
wait if dev delete is shrinking the device.
Signed-off-by: Carey Underwood cwi...@cwillu.com
---
fs/btrfs/volumes.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/fs/btrfs/volumes.c
I must admit, it is quite convoluted :-)
Please tell me if I understand this. A file system tree (containing
the inodes, the extents of all the inodes, etc.) is itself laid out in
the leaf extents of another big tree, which is the root tree. This is
why you say that inode and other such metadata
On 2013/03/04 1:47, Brendan Hide wrote:
On 2013/02/14 09:53 AM, Tsutomu Itoh wrote:
+if (ret 0) {
+fprintf(stderr, error checking %s status: %s\n, file,
+strerror(-ret));
+exit(1);
+}
...
+/* check if the device is busy */
+
On Mon, Mar 4, 2013 at 7:57 PM, Aastha Mehta aasth...@gmail.com wrote:
I must admit, it is quite convoluted :-)
Please tell me if I understand this. A file system tree (containing
the inodes, the extents of all the inodes, etc.) is itself laid out in
the leaf extents of another big tree,
Okay, that makes lot more sense to me now.
Thank you very much.
Regards,
Aastha.
On 5 March 2013 02:51, Josef Bacik jo...@toxicpanda.com wrote:
On Mon, Mar 4, 2013 at 7:57 PM, Aastha Mehta aasth...@gmail.com wrote:
I must admit, it is quite convoluted :-)
Please tell me if I understand
On Mon, 4 Mar 2013 18:54:02 +0800, Liu Bo wrote:
On Mon, Mar 04, 2013 at 05:44:29PM +0800, Miao Xie wrote:
There are several bugs at error path of create_snapshot() when the
transaction commitment failed.
- access the freed transaction handler. At the end of the
transaction commitment, the
On Mon, Mar 04, 2013 at 06:28:38PM +0100, Stefan Behrens wrote:
Commit 5ac00add added a testnset mutex and code that disallows
running administrative tasks in parallel. It is prevented that
the device add/delete/balance/replace/resize operations are
started in parallel. By mistake, the
51 matches
Mail list logo
