Re: Btrfs Raid5 issue.

[Chris Murphy]

On Mon, Aug 21, 2017 at 11:19 PM, Robert LeBlanc  wrote:
> Chris and Qu thanks for your help. I was able to restore the data off
> the volume. I only could not read one file that I tried to rsync (a
> MySQl bin log), but it wasn't critical as I had an off-site snapshot
> from that morning and ownclould could resync the files that were
> changed anyway. This turned out much better than the md RAID failure
> that I had a year ago. Much faster recovery thanks to snapshots.
>
> Is there anything you would like from this damaged filesystem to help
> determine what went wrong and to help make btrfs better? If I don't
> hear back from you in a day, I'll destroy it so that I can add the
> disks into the new btrfs volumes to restore redundancy.
>
> Bcache wasn't providing the performance I was hoping for, so I'm
> putting the root and roots for my LXC containers on the SSDs (btrfs
> RAID1) and the bulk stuff on the three spindle drives (btrfs RAID1).
> For some reason, it seemed that the btrfs RAID5 setup required one of
> the drives, but I thought I had data with RAID5 and metadata with 2
> copies. Was I missing something else that prevented mounting with that
> specific drive? I don't want to get into a situation where one drive
> dies and I can't get to any data.

With all three connected, what do you get for 'btrfs fi show' ?

The first email says the supers on all three drives are OK, but still
it's confusing the degraded is working. It suggests it's not finding
something on one of the drives that it needs to mount - usually that's
the first superblock or it could be the system block group is partly
corrupt or read error or something; and when degraded it makes it
possible to mount.

Anyway at least all of the data is safe now. Pretty much all you can
do to guard against data loss is backups. Any degraded state is
precarious because it requires just one more thing to go wrong and
it's all bad news from there.

Gluster is pretty easy to setup, and use either gluster native mount
on linux or smb with everything else. Stick a big drive in a raspberry
pi (or two) and even though it's only fast ethernet (haha, now slow
100bps ethernet) it will still replicate automatically as well as
failover. Plus one of those could be XFS if you wanted to hedge your
bets. Or one of the less expensive Intel NUCs will also work if you
want to stick with x86.



-- 
Chris Murphy
--
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] btrfs-progs: Make in-place exit to a common exit block

[Gu Jinxiang]

As comment pointed out by David, make in-place exit
to a common exit block of mkfs.

v1:
Add some close(fd) when error occures in mkfs.
And add close(fd) when end use it.

Signed-off-by: Gu Jinxiang 
---
 mkfs/main.c | 74 +
 1 file changed, 40 insertions(+), 34 deletions(-)

diff --git a/mkfs/main.c b/mkfs/main.c
index 2b109a5..1244dfa 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -1410,7 +1410,7 @@ int main(int argc, char **argv)
u32 sectorsize = 4096;
u32 stripesize = 4096;
int zero_end = 1;
-   int fd;
+   int fd = -1;
int ret;
int i;
int mixed = 0;
@@ -1496,12 +1496,12 @@ int main(int argc, char **argv)
error("unrecognized filesystem feature 
'%s'",
tmp);
free(orig);
-   exit(1);
+   goto error;
}
free(orig);
if (features & BTRFS_FEATURE_LIST_ALL) {
btrfs_list_all_fs_features(0);
-   exit(0);
+   goto success;
}
break;
}
@@ -1515,8 +1515,7 @@ int main(int argc, char **argv)
case 'V':
printf("mkfs.btrfs, part of %s\n",
PACKAGE_STRING);
-   exit(0);
-   break;
+   goto success;
case 'r':
source_dir = optarg;
source_dir_set = 1;
@@ -1551,7 +1550,7 @@ int main(int argc, char **argv)
 
if (source_dir_set && dev_cnt > 1) {
error("the option -r is limited to a single device");
-   exit(1);
+   goto error;
}
 
if (*fs_uuid) {
@@ -1559,11 +1558,11 @@ int main(int argc, char **argv)
 
if (uuid_parse(fs_uuid, dummy_uuid) != 0) {
error("could not parse UUID: %s", fs_uuid);
-   exit(1);
+   goto error;
}
if (!test_uuid_unique(fs_uuid)) {
error("non-unique UUID: %s", fs_uuid);
-   exit(1);
+   goto error;
}
}
 
@@ -1571,7 +1570,7 @@ int main(int argc, char **argv)
file = argv[optind++];
if (is_block_device(file) == 1)
if (test_dev_for_mkfs(file, force_overwrite))
-   exit(1);
+   goto error;
}
 
optind = saved_optind;
@@ -1606,7 +1605,7 @@ int main(int argc, char **argv)
if (metadata_profile != data_profile) {
error(
"with mixed block groups data and metadata profiles must be the same");
-   exit(1);
+   goto error;
}
}
 
@@ -1628,12 +1627,12 @@ int main(int argc, char **argv)
 
if (btrfs_check_nodesize(nodesize, sectorsize,
 features))
-   exit(1);
+   goto error;
 
if (sectorsize < sizeof(struct btrfs_super_block)) {
error("sectorsize smaller than superblock: %u < %zu",
sectorsize, sizeof(struct btrfs_super_block));
-   exit(1);
+   goto error;
}
 
/* Check device/block_count after the nodesize is determined */
@@ -1642,7 +1641,7 @@ int main(int argc, char **argv)
block_count);
error("minimum size for btrfs filesystem is %llu",
btrfs_min_dev_size(nodesize));
-   exit(1);
+   goto error;
}
for (i = saved_optind; i < saved_optind + dev_cnt; i++) {
char *path;
@@ -1652,20 +1651,20 @@ int main(int argc, char **argv)
if (ret < 0) {
error("failed to check size for %s: %s",
path, strerror(-ret));
-   exit (1);
+   goto error;
}
if (ret > 0) {
error("'%s' is too small to make a usable filesystem",
path);
error("minimum size for each btrfs device is %llu",
btrfs_min_dev_size(nodesize));
-   exit(1);
+   goto 

Re: Btrfs Raid5 issue.

[Robert LeBlanc]

Chris and Qu thanks for your help. I was able to restore the data off
the volume. I only could not read one file that I tried to rsync (a
MySQl bin log), but it wasn't critical as I had an off-site snapshot
from that morning and ownclould could resync the files that were
changed anyway. This turned out much better than the md RAID failure
that I had a year ago. Much faster recovery thanks to snapshots.

Is there anything you would like from this damaged filesystem to help
determine what went wrong and to help make btrfs better? If I don't
hear back from you in a day, I'll destroy it so that I can add the
disks into the new btrfs volumes to restore redundancy.

Bcache wasn't providing the performance I was hoping for, so I'm
putting the root and roots for my LXC containers on the SSDs (btrfs
RAID1) and the bulk stuff on the three spindle drives (btrfs RAID1).
For some reason, it seemed that the btrfs RAID5 setup required one of
the drives, but I thought I had data with RAID5 and metadata with 2
copies. Was I missing something else that prevented mounting with that
specific drive? I don't want to get into a situation where one drive
dies and I can't get to any data.

Thank you again.

Robert LeBlanc
PGP Fingerprint 79A2 9CA4 6CC4 45DD A904  C70E E654 3BB2 FA62 B9F1
--
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: btrfs corrupt message when mounting (but still mounts and no scrub errors.)

[Chris Murphy]

On Mon, Aug 21, 2017 at 10:55 PM, Rich Rauenzahn  wrote:
> I'm getting messages like this when mounting:
>
> [4.034300] BTRFS info (device sdg3): bdev /dev/sdg3 errs: wr 0, rd
> 0, flush 0, corrupt 4, gen 0
> [4.034828] BTRFS info (device sdg3): bdev /dev/sdf3 errs: wr 0, rd
> 0, flush 0, corrupt 68, gen 0


These are cumulative values stored in the file system persistently.
They should match up with 'btrfs dev stats'. You can reset it by using
-z.


>
> But it mounts fine, and scrub says it is fine:
>
> $  sudo btrfs scrub start -Bd /dev/sdf3
> scrub device /dev/sdf3 (id 1) done
> scrub started at Mon Aug 21 21:34:10 2017 and finished after 00:02:55
> total bytes scrubbed: 28.33GiB with 0 errors
>
> $ sudo btrfs scrub start -Bd /dev/sdg3
> scrub device /dev/sdg3 (id 2) done
> scrub started at Mon Aug 21 21:49:47 2017 and finished after 00:02:28
> total bytes scrubbed: 28.33GiB with 0 errors

Scrub shows errors for the particular scrub with that time stamp, so
it's not cumulative.


-- 
Chris Murphy
--
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

btrfs corrupt message when mounting (but still mounts and no scrub errors.)

[Rich Rauenzahn]

I'm getting messages like this when mounting:

[4.034300] BTRFS info (device sdg3): bdev /dev/sdg3 errs: wr 0, rd
0, flush 0, corrupt 4, gen 0
[4.034828] BTRFS info (device sdg3): bdev /dev/sdf3 errs: wr 0, rd
0, flush 0, corrupt 68, gen 0

But it mounts fine, and scrub says it is fine:

$  sudo btrfs scrub start -Bd /dev/sdf3
scrub device /dev/sdf3 (id 1) done
scrub started at Mon Aug 21 21:34:10 2017 and finished after 00:02:55
total bytes scrubbed: 28.33GiB with 0 errors

$ sudo btrfs scrub start -Bd /dev/sdg3
scrub device /dev/sdg3 (id 2) done
scrub started at Mon Aug 21 21:49:47 2017 and finished after 00:02:28
total bytes scrubbed: 28.33GiB with 0 errors

These are a pair of SSDs that I have been scrubbing at least weekly.

How do I clean them up?  And figure out what got corrupt?

(I'm also getting these but I think they aren't related?)

[2.863461] ata5.00: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
[2.863576] ata6.00: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
[2.866105] ata5.00: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
[2.866306] ata6.00: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
[   31.944514] ata5.00: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
[   31.946941] ata5.00: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
[   44.577598] ata5.00: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
[   44.582437] ata5.00: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
[   75.458058] ata5.00: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
[   75.466163] ata5.00: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
[   98.941941] ata5.00: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
[   98.949956] ata5.00: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
[  115.825455] ata6.00: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
[  115.827832] ata6.00: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
[  221.219641] ata6.00: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
[  221.225138] ata6.00: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
[  251.208339] ata6.00: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
[  251.213116] ata6.00: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
[  266.214212] ata6.00: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
[  266.217727] ata6.00: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
--
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: BTRFS error (device sda4): failed to read chunk tree: -5

[Qu Wenruo]

On 2017年08月18日 11:49, Qu Wenruo wrote:



On 2017年08月18日 11:13, Zirconium Hacker wrote:

I hope "Reply All" is the right option here.  Again, first time
interacting with a mailing list.  Google said that was what to do.


You're doing quite well, and yes, Reply All is the right option.



I have found no I/O errors in dmesg -- at least, none mentioning
'I/O', 'IO', or anything triggered by mount besides BTRFS's
complaints.

$ sudo btrfs rescue chunk -v /dev/sda4
(See https://pastebin.com/YaRHuKeT -- the output hasn't visibly
changed since I tried this around a week ago, but this output is
recent)
$ man btrfs | grep show-super -A1
btrfs-show-super
moved to btrfs inspect-internal dump-super
$ sudo btrfs inspect-internal dump-super -fa /dev/sda4
(See https://pastebin.com/DbABqXGQ)


All your superblocks are saying that chunk root locates at 131072, which 
at least matches with your system chunk array.


But the problem is, your system chunk restored in superblock says that 
your system chunk is located in the very beginning of your first device.
Which is invalid as for each device, 0~1M is reserved and no chunk 
should be allocated to that range.


Quite strange to me.

Is this image a native btrfs? Or converted from ext*?


Finally I get answer why there will be such strange chunk layout.

I reproduced it by making a btrfs with "-r" option.
Then it will create such strange chunk layout.

I'll try to fix that option, to make it follow the correct method to 
create file system.


Thanks,
Qu




$ sudo btrfs-find-root -o 5 /dev/sda4
(See 
https://zerobin.net/?496ed00aed01ab0c#Kvp+FqrF6mfqQLZvUYJ1ODWYIzGayJbdyuMXc9RTauA= 


-- Pastebin wouldn't let me paste that much)


I'm sorry that my previous comment has something wrong.
The magic number is not 5, but should be 3.

Please dump it again, and I think this time output will be much shorter.

Thanks,
Qu


I hope the way I'm organizing the output is OK.

On Thu, Aug 17, 2017 at 6:42 PM, Qu Wenruo  
wrote:



On 2017年08月18日 00:53, Chris Murphy wrote:


Readding Btrfslist, and adding Qu:


On Thu, Aug 17, 2017 at 12:48 AM, Zirconium Hacker 


wrote:


Oh, sorry, I guess the output of the command I ran wasn't clear -- it
was collecting the output of running the debug command on all 1,084
and showing that it was the same.  Here's specifically what you asked
for:

$ sudo btrfs-debug-tree -b 61809344512 /dev/sda4
btrfs-progs v4.12
bytenr mismatch, want=61809344512, have=0
Couldn't read tree root
ERROR: unable to open /dev/sda4
$ sudo btrfs-debug-tree -b 108544 /dev/sda4
btrfs-progs v4.12
bytenr mismatch, want=61809344512, have=0



This means either chunk root is corrupted, or system chunk array in
superblock is corrupted.
Bytenr mismatch is normally impossible for normal operation.

BTW are you using discard mount option? Sometimes it can cause problem.

And please also paste the following output:

# btrfs-show-super -fa /dev/sda4
# btrfs-find-root -o 5 /dev/sda4

The first is to output the full backup roots and current chunk root 
for us

to debug.
The second one will try to iterate your whole disk to find a valid 
but old

chunk root.
If we could find one (even a little old), it may make it possible to 
mount

the fs.

Thanks,
Qu


Couldn't read tree root
ERROR: unable to open /dev/sda4

I'm using GMail, and it's confusing me by trimming off quotes and
stuff, so sorry if I miss something.



OK well now we're in the bad part of Btrfs repair where the error
messages don't help. > It's one thing for it to complain about
108544 being invalid, because by now it might have been
overwritten, but to say it wants some other root that we already know
it can't read, and then fail reading that root is not helpful
information.
Maybe Qu has an idea. But it does sound like something really
catastrophic happened to blow away all of the backup root trees.

Going back to your first email, -o ro,usebackuproot failed with a
chunk tree error. I wonder if 'rescue chunk' might help.

Try 'btrfs rescue chunk -v' and see what you get.





--
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

[PATCH 1/2 RESEND] Btrfs: make plug in writing meta blocks really work

[Liu Bo]

We have started plug in btrfs_write_and_wait_marked_extents() but the
generated IOs actually go to device's schedule IO list where the work
is doing in another task, thus the started plug doesn't make any
sense.

And since we wait for IOs immediately after writing meta blocks, it's
the same case as writing log tree, doing sync submit can merge more
IOs.

Signed-off-by: Liu Bo 
---
 fs/btrfs/disk-io.c | 6 --
 fs/btrfs/transaction.c | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 080e2eb..8d097ba 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1005,8 +1005,10 @@ static blk_status_t __btree_submit_bio_done(void 
*private_data, struct bio *bio,
return ret;
 }
 
-static int check_async_write(unsigned long bio_flags)
+static int check_async_write(struct btrfs_inode *bi, unsigned long bio_flags)
 {
+   if (atomic_read(>sync_writers))
+   return 0;
if (bio_flags & EXTENT_BIO_TREE_LOG)
return 0;
 #ifdef CONFIG_X86
@@ -1022,7 +1024,7 @@ static blk_status_t btree_submit_bio_hook(void 
*private_data, struct bio *bio,
 {
struct inode *inode = private_data;
struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
-   int async = check_async_write(bio_flags);
+   int async = check_async_write(BTRFS_I(inode), bio_flags);
blk_status_t ret;
 
if (bio_op(bio) != REQ_OP_WRITE) {
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index f615d59..9c5f126 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -950,6 +950,7 @@ int btrfs_write_marked_extents(struct btrfs_fs_info 
*fs_info,
u64 start = 0;
u64 end;
 
+   atomic_inc(_I(fs_info->btree_inode)->sync_writers);
while (!find_first_extent_bit(dirty_pages, start, , ,
  mark, _state)) {
bool wait_writeback = false;
@@ -985,6 +986,7 @@ int btrfs_write_marked_extents(struct btrfs_fs_info 
*fs_info,
cond_resched();
start = end + 1;
}
+   atomic_dec(_I(fs_info->btree_inode)->sync_writers);
return werr;
 }
 
-- 
2.9.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

[PATCH 2/2] Btrfs: remove bio_flags which indicates a meta block of log-tree

[Liu Bo]

Since both committing transaction and writing log-tree are doing
plugging on metadata IO, we can unify to use %sync_writers to benefit
both cases, instead of checking bio_flags while writing meta blocks of
log-tree.

This removes the bio_flags related stuff for writing log-tree.

Signed-off-by: Liu Bo 
---
 fs/btrfs/disk-io.c   |  6 ++
 fs/btrfs/extent_io.c | 13 ++---
 fs/btrfs/extent_io.h |  1 -
 3 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 8d097ba..b0e353e 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1005,12 +1005,10 @@ static blk_status_t __btree_submit_bio_done(void 
*private_data, struct bio *bio,
return ret;
 }
 
-static int check_async_write(struct btrfs_inode *bi, unsigned long bio_flags)
+static int check_async_write(struct btrfs_inode *bi)
 {
if (atomic_read(>sync_writers))
return 0;
-   if (bio_flags & EXTENT_BIO_TREE_LOG)
-   return 0;
 #ifdef CONFIG_X86
if (static_cpu_has(X86_FEATURE_XMM4_2))
return 0;
@@ -1024,7 +1022,7 @@ static blk_status_t btree_submit_bio_hook(void 
*private_data, struct bio *bio,
 {
struct inode *inode = private_data;
struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
-   int async = check_async_write(BTRFS_I(inode), bio_flags);
+   int async = check_async_write(BTRFS_I(inode));
blk_status_t ret;
 
if (bio_op(bio) != REQ_OP_WRITE) {
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 0aff9b2..b61d68f 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -110,7 +110,6 @@ struct extent_page_data {
struct bio *bio;
struct extent_io_tree *tree;
get_extent_t *get_extent;
-   unsigned long bio_flags;
 
/* tells writepage not to lock the state bits for this range
 * it still does the unlocking
@@ -3713,7 +3712,6 @@ static noinline_for_stack int write_one_eb(struct 
extent_buffer *eb,
u64 offset = eb->start;
u32 nritems;
unsigned long i, num_pages;
-   unsigned long bio_flags = 0;
unsigned long start, end;
int write_flags = (epd->sync_io ? REQ_SYNC : 0) | REQ_META;
int ret = 0;
@@ -3721,8 +3719,6 @@ static noinline_for_stack int write_one_eb(struct 
extent_buffer *eb,
clear_bit(EXTENT_BUFFER_WRITE_ERR, >bflags);
num_pages = num_extent_pages(eb->start, eb->len);
atomic_set(>io_pages, num_pages);
-   if (btrfs_header_owner(eb) == BTRFS_TREE_LOG_OBJECTID)
-   bio_flags = EXTENT_BIO_TREE_LOG;
 
/* set btree blocks beyond nritems with 0 to avoid stale content. */
nritems = btrfs_header_nritems(eb);
@@ -3749,8 +3745,7 @@ static noinline_for_stack int write_one_eb(struct 
extent_buffer *eb,
 p, offset >> 9, PAGE_SIZE, 0, bdev,
 >bio,
 end_bio_extent_buffer_writepage,
-0, epd->bio_flags, bio_flags, false);
-   epd->bio_flags = bio_flags;
+0, 0, 0, false);
if (ret) {
set_btree_ioerr(p);
if (PageWriteback(p))
@@ -3787,7 +3782,6 @@ int btree_write_cache_pages(struct address_space *mapping,
.tree = tree,
.extent_locked = 0,
.sync_io = wbc->sync_mode == WB_SYNC_ALL,
-   .bio_flags = 0,
};
int ret = 0;
int done = 0;
@@ -4063,7 +4057,7 @@ static void flush_epd_write_bio(struct extent_page_data 
*epd)
bio_set_op_attrs(epd->bio, REQ_OP_WRITE,
 epd->sync_io ? REQ_SYNC : 0);
 
-   ret = submit_one_bio(epd->bio, 0, epd->bio_flags);
+   ret = submit_one_bio(epd->bio, 0, 0);
BUG_ON(ret < 0); /* -ENOMEM */
epd->bio = NULL;
}
@@ -4086,7 +4080,6 @@ int extent_write_full_page(struct extent_io_tree *tree, 
struct page *page,
.get_extent = get_extent,
.extent_locked = 0,
.sync_io = wbc->sync_mode == WB_SYNC_ALL,
-   .bio_flags = 0,
};
 
ret = __extent_writepage(page, wbc, );
@@ -4111,7 +4104,6 @@ int extent_write_locked_range(struct extent_io_tree 
*tree, struct inode *inode,
.get_extent = get_extent,
.extent_locked = 1,
.sync_io = mode == WB_SYNC_ALL,
-   .bio_flags = 0,
};
struct writeback_control wbc_writepages = {
.sync_mode  = mode,
@@ -4151,7 +4143,6 @@ int extent_writepages(struct extent_io_tree *tree,
.get_extent = get_extent,
.extent_locked = 0,
.sync_io = wbc->sync_mode == WB_SYNC_ALL,
-   .bio_flags = 0,

Re: [PATCH] Btrfs: make plug in writing meta blocks really work

[Liu Bo]

On Mon, Aug 21, 2017 at 03:23:30PM -0400, Josef Bacik wrote:
> On Mon, Aug 21, 2017 at 12:14:16PM -0700, Liu Bo wrote:
> > On Mon, Aug 21, 2017 at 01:48:01PM -0400, Josef Bacik wrote:
> > > On Fri, Aug 18, 2017 at 11:42:07AM -0600, Liu Bo wrote:
> > > > We have started plug in btrfs_write_and_wait_marked_extents() but the
> > > > generated IOs actually go to device's schedule IO list where the work
> > > > is doing in another task, thus the started plug doesn't make any
> > > > sense.
> > > > 
> > > > And since we wait for IOs immediately after writing meta blocks, it's
> > > > the same case as writing log tree, doing sync submit can merge more
> > > > IOs.
> > > > 
> > > 
> > > We're plugging when we do the per-device scheduled IO right?
> > 
> > Yes, we are.
> > 
> > > So we aren't
> > > really gaining anything by it being async.  Also we do a lot of work 
> > > between the
> > > time that we start writing the marked extents for the tree-log and when we
> > > actually wait for them, so we really don't want to do a synchronous write 
> > > out in
> > > that case.
> > 
> > Hmm, we've always been doing sync write for meta blocks of log
> > tree/log root tree, because of EXTENT_BIO_TREE_LOG (introduced in
> > commit de0022b9da616b95ea5b41eab32da825b0b5150f), and the commit log
> > claimed about 15% performance gaining in O_SYNC workloads (maybe we
> > need to re-evaluate it?).
> > 
> > > Instead move the sync_writers into write_and_wait_marked_extents.
> > > Thanks,
> > 
> > I'm OK with the change, but if sync write benefits both transaction
> > commit case and log tree case, we can unify them to %sync_writers
> > instead of a bio_flag.
> > 
> 
> Sigh you're right, I forgot about all of that.  Just delete the magic bio 
> flags
> stuff and then this is fine.  Thanks,
>

OK, good to know it, thanks for the comments.

thanks,
-liubo
--
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: make plug in writing meta blocks really work

[Josef Bacik]

On Mon, Aug 21, 2017 at 12:14:16PM -0700, Liu Bo wrote:
> On Mon, Aug 21, 2017 at 01:48:01PM -0400, Josef Bacik wrote:
> > On Fri, Aug 18, 2017 at 11:42:07AM -0600, Liu Bo wrote:
> > > We have started plug in btrfs_write_and_wait_marked_extents() but the
> > > generated IOs actually go to device's schedule IO list where the work
> > > is doing in another task, thus the started plug doesn't make any
> > > sense.
> > > 
> > > And since we wait for IOs immediately after writing meta blocks, it's
> > > the same case as writing log tree, doing sync submit can merge more
> > > IOs.
> > > 
> > 
> > We're plugging when we do the per-device scheduled IO right?
> 
> Yes, we are.
> 
> > So we aren't
> > really gaining anything by it being async.  Also we do a lot of work 
> > between the
> > time that we start writing the marked extents for the tree-log and when we
> > actually wait for them, so we really don't want to do a synchronous write 
> > out in
> > that case.
> 
> Hmm, we've always been doing sync write for meta blocks of log
> tree/log root tree, because of EXTENT_BIO_TREE_LOG (introduced in
> commit de0022b9da616b95ea5b41eab32da825b0b5150f), and the commit log
> claimed about 15% performance gaining in O_SYNC workloads (maybe we
> need to re-evaluate it?).
> 
> > Instead move the sync_writers into write_and_wait_marked_extents.
> > Thanks,
> 
> I'm OK with the change, but if sync write benefits both transaction
> commit case and log tree case, we can unify them to %sync_writers
> instead of a bio_flag.
> 

Sigh you're right, I forgot about all of that.  Just delete the magic bio flags
stuff and then this is fine.  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] Btrfs: make plug in writing meta blocks really work

[Liu Bo]

On Mon, Aug 21, 2017 at 01:48:01PM -0400, Josef Bacik wrote:
> On Fri, Aug 18, 2017 at 11:42:07AM -0600, Liu Bo wrote:
> > We have started plug in btrfs_write_and_wait_marked_extents() but the
> > generated IOs actually go to device's schedule IO list where the work
> > is doing in another task, thus the started plug doesn't make any
> > sense.
> > 
> > And since we wait for IOs immediately after writing meta blocks, it's
> > the same case as writing log tree, doing sync submit can merge more
> > IOs.
> > 
> 
> We're plugging when we do the per-device scheduled IO right?

Yes, we are.

> So we aren't
> really gaining anything by it being async.  Also we do a lot of work between 
> the
> time that we start writing the marked extents for the tree-log and when we
> actually wait for them, so we really don't want to do a synchronous write out 
> in
> that case.

Hmm, we've always been doing sync write for meta blocks of log
tree/log root tree, because of EXTENT_BIO_TREE_LOG (introduced in
commit de0022b9da616b95ea5b41eab32da825b0b5150f), and the commit log
claimed about 15% performance gaining in O_SYNC workloads (maybe we
need to re-evaluate it?).

> Instead move the sync_writers into write_and_wait_marked_extents.
> Thanks,

I'm OK with the change, but if sync write benefits both transaction
commit case and log tree case, we can unify them to %sync_writers
instead of a bio_flag.

thanks,
-liubo
--
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: submit superblock io with REQ_META and REQ_PRIO

[Liu Bo]

On Mon, Aug 21, 2017 at 05:42:27PM +0200, David Sterba wrote:
> The superblock is also metadata of the filesystem so the relevant IO
> should be tagged as such. We also tag it as high priority, as it's the
> last block committed for metadata from a given transaction. Any delays
> would effectively block the whole transaction, also blocking any other
> operation holding the device_list_mutex.
> 

Reviewed-by: Liu Bo 

-liubo

> Signed-off-by: David Sterba 
> ---
>  fs/btrfs/disk-io.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 90b967ae46d0..27d458640536 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3433,9 +3433,10 @@ static int write_dev_supers(struct btrfs_device 
> *device,
>*/
>   if (i == 0) {
>   ret = btrfsic_submit_bh(REQ_OP_WRITE,
> - REQ_SYNC | REQ_FUA, bh);
> + REQ_SYNC | REQ_FUA | REQ_META | REQ_PRIO, bh);
>   } else {
> - ret = btrfsic_submit_bh(REQ_OP_WRITE, REQ_SYNC, bh);
> + ret = btrfsic_submit_bh(REQ_OP_WRITE,
> + REQ_SYNC | REQ_META | REQ_PRIO, bh);
>   }
>   if (ret)
>   errors++;
> -- 
> 2.14.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: BTRFS warning (device dm-0): unhandled fiemap cache detected

[Omar Sandoval]

On Mon, Aug 21, 2017 at 11:31:34AM +0800, Qu Wenruo wrote:
> 
> 
> On 2017年08月21日 11:27, Christoph Anton Mitterer wrote:
> > On Mon, 2017-08-21 at 10:43 +0800, Qu Wenruo wrote:
> > > Harmless, it is only designed to merge fiemap output.
> > Thanks for the info :)
> > 
> > 
> > On Mon, 2017-08-21 at 10:57 +0800, Qu Wenruo wrote:
> > > Quite strange, according to upstream git log, that commit is merged
> > > between v4.12-rc7 and v4.12.
> > > Maybe I misunderstand the stable kernel release cycle.
> > 
> > Seems it was only added in 4.12.7? Maybe a typo?
> 
> I'm talking about upstream, which is managed by Linus.
> The 4.12.x is managed by maintainers of stable releases.
> 
> In Linus repo, the commit is between v4.12 and v4.12-rc7 tag.
> Maybe v4.12.x is started before v4.12 release.

It's not in 4.12.

$ git tag --contains 848c23b78fafdcd3270b06a30737f8dbd70c347f
for-4.13-part2-tag
v4.13-rc1
v4.13-rc2
v4.13-rc3
v4.13-rc4
v4.13-rc5
--
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: make plug in writing meta blocks really work

[Josef Bacik]

On Fri, Aug 18, 2017 at 11:42:07AM -0600, Liu Bo wrote:
> We have started plug in btrfs_write_and_wait_marked_extents() but the
> generated IOs actually go to device's schedule IO list where the work
> is doing in another task, thus the started plug doesn't make any
> sense.
> 
> And since we wait for IOs immediately after writing meta blocks, it's
> the same case as writing log tree, doing sync submit can merge more
> IOs.
> 

We're plugging when we do the per-device scheduled IO right?  So we aren't
really gaining anything by it being async.  Also we do a lot of work between the
time that we start writing the marked extents for the tree-log and when we
actually wait for them, so we really don't want to do a synchronous write out in
that case.  Instead move the sync_writers into write_and_wait_marked_extents.
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] btrfs-progs: mkfs: Replace number with enum

[David Sterba]

On Wed, Jun 28, 2017 at 05:59:24PM +0800, Gu Jinxiang wrote:
> For code maintainability and scalability,
> replace hardcoded constant with a meaningful enum.
> 
> Signed-off-by: Gu Jinxiang 

Sorry for late reply. Patch applied with some tweaks, the added enum
names are too generic, so I've added some prefix.
> ---
>  mkfs/common.c | 47 ---
>  mkfs/common.h | 14 +-
>  2 files changed, 37 insertions(+), 24 deletions(-)
> 
> diff --git a/mkfs/common.c b/mkfs/common.c
> index e4785c5..f34c4eb 100644
> --- a/mkfs/common.c
> +++ b/mkfs/common.c
> @@ -93,18 +93,18 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>   uuid_generate(super.dev_item.uuid);
>   uuid_generate(chunk_tree_uuid);
>  
> - cfg->blocks[0] = BTRFS_SUPER_INFO_OFFSET;
> - for (i = 1; i < 7; i++) {
> + cfg->blocks[SUPER_BLOCK] = BTRFS_SUPER_INFO_OFFSET;
> + for (i = 1; i < BLOCK_COUNT; i++) {
>   cfg->blocks[i] = BTRFS_SUPER_INFO_OFFSET + 1024 * 1024 +
>   cfg->nodesize * i;
>   }
>  
> - btrfs_set_super_bytenr(, cfg->blocks[0]);
> + btrfs_set_super_bytenr(, cfg->blocks[SUPER_BLOCK]);
>   btrfs_set_super_num_devices(, 1);
>   btrfs_set_super_magic(, BTRFS_MAGIC_PARTIAL);
>   btrfs_set_super_generation(, 1);
> - btrfs_set_super_root(, cfg->blocks[1]);
> - btrfs_set_super_chunk_root(, cfg->blocks[3]);
> + btrfs_set_super_root(, cfg->blocks[ROOT_TREE]);
> + btrfs_set_super_chunk_root(, cfg->blocks[CHUNK_TREE]);
>   btrfs_set_super_total_bytes(, num_bytes);
>   btrfs_set_super_bytes_used(, 6 * cfg->nodesize);
>   btrfs_set_super_sectorsize(, cfg->sectorsize);
> @@ -121,7 +121,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>   /* create the tree of root objects */
>   memset(buf->data, 0, cfg->nodesize);
>   buf->len = cfg->nodesize;
> - btrfs_set_header_bytenr(buf, cfg->blocks[1]);
> + btrfs_set_header_bytenr(buf, cfg->blocks[ROOT_TREE]);
>   btrfs_set_header_nritems(buf, 4);
>   btrfs_set_header_generation(buf, 1);
>   btrfs_set_header_backref_rev(buf, BTRFS_MIXED_BACKREF_REV);
> @@ -151,7 +151,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>   nritems = 0;
>  
>   itemoff = __BTRFS_LEAF_DATA_SIZE(cfg->nodesize) - sizeof(root_item);
> - btrfs_set_root_bytenr(_item, cfg->blocks[2]);
> + btrfs_set_root_bytenr(_item, cfg->blocks[EXTENT_TREE]);
>   btrfs_set_disk_key_objectid(_key, BTRFS_EXTENT_TREE_OBJECTID);
>   btrfs_set_item_key(buf, _key, nritems);
>   btrfs_set_item_offset(buf, btrfs_item_nr(nritems), itemoff);
> @@ -162,7 +162,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>   nritems++;
>  
>   itemoff = itemoff - sizeof(root_item);
> - btrfs_set_root_bytenr(_item, cfg->blocks[4]);
> + btrfs_set_root_bytenr(_item, cfg->blocks[DEV_TREE]);
>   btrfs_set_disk_key_objectid(_key, BTRFS_DEV_TREE_OBJECTID);
>   btrfs_set_item_key(buf, _key, nritems);
>   btrfs_set_item_offset(buf, btrfs_item_nr(nritems), itemoff);
> @@ -174,7 +174,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>   nritems++;
>  
>   itemoff = itemoff - sizeof(root_item);
> - btrfs_set_root_bytenr(_item, cfg->blocks[5]);
> + btrfs_set_root_bytenr(_item, cfg->blocks[FS_TREE]);
>   btrfs_set_disk_key_objectid(_key, BTRFS_FS_TREE_OBJECTID);
>   btrfs_set_item_key(buf, _key, nritems);
>   btrfs_set_item_offset(buf, btrfs_item_nr(nritems), itemoff);
> @@ -186,7 +186,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>   nritems++;
>  
>   itemoff = itemoff - sizeof(root_item);
> - btrfs_set_root_bytenr(_item, cfg->blocks[6]);
> + btrfs_set_root_bytenr(_item, cfg->blocks[CSUM_TREE]);
>   btrfs_set_disk_key_objectid(_key, BTRFS_CSUM_TREE_OBJECTID);
>   btrfs_set_item_key(buf, _key, nritems);
>   btrfs_set_item_offset(buf, btrfs_item_nr(nritems), itemoff);
> @@ -199,7 +199,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>  
>  
>   csum_tree_block_size(buf, BTRFS_CRC32_SIZE, 0);
> - ret = pwrite(fd, buf->data, cfg->nodesize, cfg->blocks[1]);
> + ret = pwrite(fd, buf->data, cfg->nodesize, cfg->blocks[ROOT_TREE]);
>   if (ret != cfg->nodesize) {
>   ret = (ret < 0 ? -errno : -EIO);
>   goto out;
> @@ -210,7 +210,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>   cfg->nodesize - sizeof(struct btrfs_header));
>   nritems = 0;
>   itemoff = __BTRFS_LEAF_DATA_SIZE(cfg->nodesize);
> - for (i = 1; i < 7; i++) {
> + for (i = 1; i < BLOCK_COUNT; i++) {
>   item_size = sizeof(struct btrfs_extent_item);
>   if (!skinny_metadata)
>   item_size += sizeof(struct btrfs_tree_block_info);
> @@ -267,11 +267,11 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>   

Re: [PATCH] Btrfs: do not async submit for nodatacsum inodes

[Josef Bacik]

On Fri, Aug 18, 2017 at 11:54:02AM -0600, Liu Bo wrote:
> While we submit direct writes, if the inode is flagged with nodatasum,
> there's no benefit to submit asynchronously, because
> 
> a) we don't have to calculate checksum across processors,
> 
> b) and direct IO has started a plug, but async submit makes us queue
> IO on each device's scheduled IO list instead of DIO's plug list, so
> that IOs get much less merges in general.
> 
> Lets use sync submit for nodatasum inodes.
> 
> Signed-off-by: Liu Bo 

Reviewed-by: Josef Bacik 

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 00/10] Unused parameter cleanup

[Josef Bacik]

On Mon, Aug 21, 2017 at 12:43:40PM +0300, Nikolay Borisov wrote:
> Hello, 
> 
> Here is a series that I've been sitting on for a while. It removes unused 
> parameter in various functions, no functional changes. Patch 09/10 reworks
> some error handling to eliminate an if branch in __btrfs_alloc_chunk, but it's
> still a minor refactoring. 
> 
> Nikolay Borisov (10):
>   btrfs: Remove chunk_objectid parameter of btrfs_alloc_dev_extent
>   btrfs: remove superfluous chunk_tree argument from
> btrfs_alloc_dev_extent
>   btrfs: Remove unused variable
>   btrfs: Remove unused parameters from various functions
>   btrfs: Remove unused arguments from btrfs_changed_cb_t
>   btrfs: Remove unused parameter from check_direct_IO
>   btrfs: Remove unused parameter from extent_clear_unlock_delalloc
>   btrfs: remove unused parameter in cow_file_range
>   btrfs: Rework error handling of add_extent_mapping in
> __btrfs_alloc_chunk
>   btrfs: Remove redundant argument of __link_block_group
> 

You can add

Reviewed-by: Josef Bacik 

to the whole series.  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 v2] btrfs: submit superblock io with REQ_META and REQ_PRIO

[Josef Bacik]

On Mon, Aug 21, 2017 at 05:42:27PM +0200, David Sterba wrote:
> The superblock is also metadata of the filesystem so the relevant IO
> should be tagged as such. We also tag it as high priority, as it's the
> last block committed for metadata from a given transaction. Any delays
> would effectively block the whole transaction, also blocking any other
> operation holding the device_list_mutex.
> 
> Signed-off-by: David Sterba 

Reviewed-by: Josef Bacik 

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 2/2] btrfs-progs: delete not-used parameter fd

[David Sterba]

On Fri, Aug 18, 2017 at 01:32:46AM -0700, Gu Jinxiang wrote:
> Parameter fd is not used in function make_image and
> traverse_directory of mkfs.
> Delete it.
> 
> Signed-off-by: Gu Jinxiang 

Apparently the parameter has never been used. Applied, thanks.
--
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 1/2] btrfs-progs: add necessary close(fd) in mkfs

[David Sterba]

On Fri, Aug 18, 2017 at 01:32:45AM -0700, Gu Jinxiang wrote:
> Add some close(fd) when error occures in mkfs.
> And add close(fd) when end use it.

Can you please rework it so all the in-place exists are gotos to a
common exit block that does the close(fd) cleanup? Thanks.
--
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-progs: Allow inspect dump-tree to show specified tree block even some tree roots are corrupted

[David Sterba]

On Fri, Aug 18, 2017 at 06:36:30PM +0900, Qu Wenruo wrote:
> For btrfs inspect-internal dump-tree, if we use "-b" parameter to show
> specified tree block, then we don't really need extra tree roots.
> 
> Only chunk root is needed to build up the whole chunk mapping so we can
> read tree blocks.
> 
> This patch will add __OPEN_CTREE_RETURN_CHUNK_ROOT flag when show
> speicifed tree block.
> So even root tree is corrupted, we can still use inspect-internal
> dump-tree to do some debugging.

Sounds useful.

> Reported-by: Zirconium Hacker 
> Signed-off-by: Qu Wenruo 

Applied, thanks.
--
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-progs: Doc: Fix asciidoc grammar of btrfs-rescue

[David Sterba]

On Mon, Aug 21, 2017 at 03:57:04PM +0900, Qu Wenruo wrote:
> Code block of kernel backtrace lacks leading change line, causing the
> following man page result:
> --
>One can determine whether zero-log is needed according to the
>kernel backtrace:
> 
>? replay_one_dir_item+0xb5/0xb5 [btrfs]
>? walk_log_tree+0x9c/0x19d [btrfs]
>? btrfs_read_fs_root_no_radix+0x169/0x1a1 [btrfs]
>? btrfs_recover_log_trees+0x195/0x29c [btrfs]
>? replay_one_dir_item+0xb5/0xb5 [btrfs]
>? btree_read_extent_buffer_pages+0x76/0xbc [btrfs]
>? open_ctree+0xff6/0x132c [btrfs]
> 
>+ If the errors are like above, then zero-log should be used to clear
>the log and the filesystem may be mounted normally again. The keywords
> --
> 
> Not only "+" is rendered as is, but also wrong indent.
> 
> Fix it by adding change line before code block.
> 
> Signed-off-by: Qu Wenruo 

Applied, thanks.
--
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 broken memory barrier

[David Sterba]

On Tue, Aug 01, 2017 at 06:25:56PM +0300, Nikolay Borisov wrote:
> Commit 38851cc19adb ("Btrfs: implement unlocked dio write") implemented
> unlocked dio write, allowing multiple dio writers to write to non-overlapping,
> and non-eof-extending regions. In doing so it also introduced a broken memory
> barrier. It is broken due to 2 things:
> 
> 1. Memory barriers _MUST_ always be paired, this is clearly not the case here
> 
> 2. Checkpatch actually produces a warning if a memory barrier is introduced 
> that
> doesn't have a comment explaining how it's being paired.

Specifically for inode::i_dio_count that's wrapped inside
inode_dio_begin, there is no explicit barrier semantics attached, so
removing is fine as the atomic is used in common the waiter/wakeup pattern.

Reviewed-by: David Sterba 
--
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: Btrfs Raid5 issue.

[Chris Murphy]

On Mon, Aug 21, 2017 at 10:31 AM, Robert LeBlanc  wrote:
> Qu,
>
> Sorry, I'm not on the list (I was for a few years about three years ago).
>
> I looked at the backup roots like you mentioned.
>
> # ./btrfs inspect dump-super -f /dev/bcache0
> superblock: bytenr=65536, device=/dev/bcache0
> -
> csum_type   0 (crc32c)
> csum_size   4
> csum0x45302c8f [match]
> bytenr  65536
> flags   0x1
> ( WRITTEN )
> magic   _BHRfS_M [match]
> fsidfef29f0a-dc4c-4cc4-b524-914e6630803c
> label   kvm-btrfs
> generation  1620386
> root5310022877184
> sys_array_size  161
> chunk_root_generation   1620164
> root_level  1
> chunk_root  4725030256640
> chunk_root_level1
> log_root2876047507456
> log_root_transid0
> log_root_level  0
> total_bytes 8998588280832
> bytes_used  3625869234176
> sectorsize  4096
> nodesize16384
> leafsize (deprecated)   16384
> stripesize  4096
> root_dir6
> num_devices 3
> compat_flags0x0
> compat_ro_flags 0x0
> incompat_flags  0x1e1
> ( MIXED_BACKREF |
>   BIG_METADATA |
>   EXTENDED_IREF |
>   RAID56 |
>   SKINNY_METADATA )
> cache_generation1620386
> uuid_tree_generation42
> dev_item.uuid   cb56a9b7-8d67-4ae8-8cb0-076b0b93f9c4
> dev_item.fsid   fef29f0a-dc4c-4cc4-b524-914e6630803c [match]
> dev_item.type   0
> dev_item.total_bytes2998998654976
> dev_item.bytes_used 2295693574144
> dev_item.io_align   4096
> dev_item.io_width   4096
> dev_item.sector_size4096
> dev_item.devid  2
> dev_item.dev_group  0
> dev_item.seek_speed 0
> dev_item.bandwidth  0
> dev_item.generation 0
> sys_chunk_array[2048]:
> item 0 key (FIRST_CHUNK_TREE CHUNK_ITEM 4725030256640)
> length 67108864 owner 2 stripe_len 65536 type
> SYSTEM|RAID5
> io_align 65536 io_width 65536 sector_size 4096
> num_stripes 3 sub_stripes 1
> stripe 0 devid 1 offset 2185232384
> dev_uuid e273c794-b231-4d86-9a38-53a6d2fa8643
> stripe 1 devid 3 offset 1195075698688
> dev_uuid 120d6a05-b0bc-46c8-a87e-ca4fe5008d09
> stripe 2 devid 2 offset 41340108800
> dev_uuid cb56a9b7-8d67-4ae8-8cb0-076b0b93f9c4
> backup_roots[4]:
> backup 0:
> backup_tree_root:   5309879451648   gen: 1620384
> level: 1
> backup_chunk_root:  4725030256640   gen: 1620164
> level: 1
> backup_extent_root: 5309910958080   gen: 1620385
> level: 2
> backup_fs_root: 3658468147200   gen: 1618016
> level: 1
> backup_dev_root:5309904224256   gen: 1620384
> level: 1
> backup_csum_root:   5309910532096   gen: 1620385
> level: 3
> backup_total_bytes: 8998588280832
> backup_bytes_used:  3625871646720
> backup_num_devices: 3
>
> backup 1:
> backup_tree_root:   5309780492288   gen: 1620385
> level: 1
> backup_chunk_root:  4725030256640   gen: 1620164
> level: 1
> backup_extent_root: 5309659037696   gen: 1620385
> level: 2
> backup_fs_root: 0   gen: 0  level: 0
> backup_dev_root:5309872275456   gen: 1620385
> level: 1
> backup_csum_root:   5309674536960   gen: 1620385
> level: 3
> backup_total_bytes: 8998588280832
> backup_bytes_used:  3625869234176
> backup_num_devices: 3


Well that's strange. A backup entry with a null fs root.



> I noticed on that page that there is a 'nologreplay' mount option so I
> tried it with degraded and it requires ro, but the volume mounted and
> I can "see" things on the volume.

Degraded suggests it's not finding one of the three devices.


> So with this nologreplay option, if I do a btrfs send of the subvolume
> that I'm interested in (I don't think it was being written to at the
> time of failure), would it copy (send) over the corruption as well.

Anything that results in EIO will get included in the send, and by
default receive fails. You can use verbose messaging on the receive
side, and use -E option to permit the errors. But file system specific
problems aren't going to 

Re: Btrfs Raid5 issue.

[Robert LeBlanc]

Qu,

Sorry, I'm not on the list (I was for a few years about three years ago).

I looked at the backup roots like you mentioned.

# ./btrfs inspect dump-super -f /dev/bcache0
superblock: bytenr=65536, device=/dev/bcache0
-
csum_type   0 (crc32c)
csum_size   4
csum0x45302c8f [match]
bytenr  65536
flags   0x1
( WRITTEN )
magic   _BHRfS_M [match]
fsidfef29f0a-dc4c-4cc4-b524-914e6630803c
label   kvm-btrfs
generation  1620386
root5310022877184
sys_array_size  161
chunk_root_generation   1620164
root_level  1
chunk_root  4725030256640
chunk_root_level1
log_root2876047507456
log_root_transid0
log_root_level  0
total_bytes 8998588280832
bytes_used  3625869234176
sectorsize  4096
nodesize16384
leafsize (deprecated)   16384
stripesize  4096
root_dir6
num_devices 3
compat_flags0x0
compat_ro_flags 0x0
incompat_flags  0x1e1
( MIXED_BACKREF |
  BIG_METADATA |
  EXTENDED_IREF |
  RAID56 |
  SKINNY_METADATA )
cache_generation1620386
uuid_tree_generation42
dev_item.uuid   cb56a9b7-8d67-4ae8-8cb0-076b0b93f9c4
dev_item.fsid   fef29f0a-dc4c-4cc4-b524-914e6630803c [match]
dev_item.type   0
dev_item.total_bytes2998998654976
dev_item.bytes_used 2295693574144
dev_item.io_align   4096
dev_item.io_width   4096
dev_item.sector_size4096
dev_item.devid  2
dev_item.dev_group  0
dev_item.seek_speed 0
dev_item.bandwidth  0
dev_item.generation 0
sys_chunk_array[2048]:
item 0 key (FIRST_CHUNK_TREE CHUNK_ITEM 4725030256640)
length 67108864 owner 2 stripe_len 65536 type
SYSTEM|RAID5
io_align 65536 io_width 65536 sector_size 4096
num_stripes 3 sub_stripes 1
stripe 0 devid 1 offset 2185232384
dev_uuid e273c794-b231-4d86-9a38-53a6d2fa8643
stripe 1 devid 3 offset 1195075698688
dev_uuid 120d6a05-b0bc-46c8-a87e-ca4fe5008d09
stripe 2 devid 2 offset 41340108800
dev_uuid cb56a9b7-8d67-4ae8-8cb0-076b0b93f9c4
backup_roots[4]:
backup 0:
backup_tree_root:   5309879451648   gen: 1620384level: 1
backup_chunk_root:  4725030256640   gen: 1620164level: 1
backup_extent_root: 5309910958080   gen: 1620385level: 2
backup_fs_root: 3658468147200   gen: 1618016level: 1
backup_dev_root:5309904224256   gen: 1620384level: 1
backup_csum_root:   5309910532096   gen: 1620385level: 3
backup_total_bytes: 8998588280832
backup_bytes_used:  3625871646720
backup_num_devices: 3

backup 1:
backup_tree_root:   5309780492288   gen: 1620385level: 1
backup_chunk_root:  4725030256640   gen: 1620164level: 1
backup_extent_root: 5309659037696   gen: 1620385level: 2
backup_fs_root: 0   gen: 0  level: 0
backup_dev_root:5309872275456   gen: 1620385level: 1
backup_csum_root:   5309674536960   gen: 1620385level: 3
backup_total_bytes: 8998588280832
backup_bytes_used:  3625869234176
backup_num_devices: 3

backup 2:
backup_tree_root:   5310022877184   gen: 1620386level: 1
backup_chunk_root:  4725030256640   gen: 1620164level: 1
backup_extent_root: 2876048949248   gen: 1620387level: 2
backup_fs_root: 3658468147200   gen: 1618016level: 1
backup_dev_root:5309872275456   gen: 1620385level: 1
backup_csum_root:   5310042259456   gen: 1620386level: 3
backup_total_bytes: 8998588280832
backup_bytes_used:  3625869250560
backup_num_devices: 3

backup 3:
backup_tree_root:   5309771448320   gen: 1620383level: 1
backup_chunk_root:  4725030256640   gen: 1620164level: 1
backup_extent_root: 5309779804160   gen: 1620384level: 2
backup_fs_root: 3658468147200   gen: 1618016level: 1
backup_dev_root:5309848158208   

Re: [PATCH 1/2] btrfs: Remove chunk_objectid parameter of btrfs_alloc_dev_extent

[David Sterba]

On Fri, Aug 18, 2017 at 05:58:22PM +0300, Nikolay Borisov wrote:
> THe function is always called with chunk_objectid set to
> BTRFS_FIRST_CHUNK_TREE_OBJECTID. Let's collapse the parameter in the function
> itself. No functional changes
> 
> Signed-off-by: Nikolay Borisov 

Reviewed-by: David Sterba 
--
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: remove superfluous chunk_tree argument from btrfs_alloc_dev_extent

[David Sterba]

On Fri, Aug 18, 2017 at 05:58:23PM +0300, Nikolay Borisov wrote:
> Currently this function is always called with the object id of the root key of
> the chunk_tree, which is always BTRFS_CHUNK_TREE_OBJECTID. So let's subsume it
> straight into the function itself. No functional change.
> 
> Signed-off-by: Nikolay Borisov 

Reviewed-by: David Sterba 
--
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 unnecessary skip_sum

[David Sterba]

On Fri, Aug 18, 2017 at 11:58:04AM -0600, Liu Bo wrote:
> Now that we can get inode from dip->inode, it's not necessary to pass
> skip_sum from callers.

Already there, https://patchwork.kernel.org/patch/9879029/
--
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: pass fs_info to btrfs_del_root instead of tree_root

[David Sterba]

On Thu, Aug 17, 2017 at 10:25:11AM -0400, Jeff Mahoney wrote:
> btrfs_del_roots always uses the tree_root.  Let's pass fs_info instead.
> 
> Signed-off-by: Jeff Mahoney 

Reviewed-by: David Sterba 
--
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 v2] btrfs: submit superblock io with REQ_META and REQ_PRIO

[David Sterba]

The superblock is also metadata of the filesystem so the relevant IO
should be tagged as such. We also tag it as high priority, as it's the
last block committed for metadata from a given transaction. Any delays
would effectively block the whole transaction, also blocking any other
operation holding the device_list_mutex.

Signed-off-by: David Sterba 
---
 fs/btrfs/disk-io.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 90b967ae46d0..27d458640536 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3433,9 +3433,10 @@ static int write_dev_supers(struct btrfs_device *device,
 */
if (i == 0) {
ret = btrfsic_submit_bh(REQ_OP_WRITE,
-   REQ_SYNC | REQ_FUA, bh);
+   REQ_SYNC | REQ_FUA | REQ_META | REQ_PRIO, bh);
} else {
-   ret = btrfsic_submit_bh(REQ_OP_WRITE, REQ_SYNC, bh);
+   ret = btrfsic_submit_bh(REQ_OP_WRITE,
+   REQ_SYNC | REQ_META | REQ_PRIO, bh);
}
if (ret)
errors++;
-- 
2.14.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 v3 0/7] add sanity check for extent inline ref type

[David Sterba]

On Fri, Aug 18, 2017 at 03:15:17PM -0600, Liu Bo wrote:
> An invalid extent inline ref type could be read from a btrfs image and
> it ends up with a panic[1], this set is to deal with the insane value
> gracefully in patch 1-2 and clean up BUG() in the code in patch 3-6.
> 
> Patch 7 adds one more check to see if the ref is a valid shared one.
> 
> I'm not sure in the real world what may result in this corruption, but
> I've seen several reports on the ML about __btrfs_free_extent saying
> something was missing (or simply wrong), while testing this set with
> btrfs-corrupt-block, I found that switching ref type could end up that
> situation as well, eg. a data extent's ref type
> (BTRFS_EXTENT_DATA_REF_KEY) is switched to (BTRFS_TREE_BLOCK_REF_KEY).
> Hopefully this can give people more sights next time when that
> happens.
> 
> [1]:https://www.spinics.net/lists/linux-btrfs/msg65646.html
> 
> v3:
> - btrfs_inline_ref_types -> btrfs_inline_ref_type
> - convert WARN(1) to btrfs_err so that we know which btrfs has that error.
> 
> v2:
> - add enum type and return BTRFS_REF_TYPE_INVALID instead of -EINVAL.
> - remove one more BUG_ON which is in __add_tree_block.
> - add validation check for shared refs.
> - improve btrfs_print_leaf to show which refs has something wrong.
> 
> Liu Bo (7):
>   Btrfs: add a helper to retrive extent inline ref type
>   Btrfs: convert to use btrfs_get_extent_inline_ref_type
>   Btrfs: remove BUG() in btrfs_extent_inline_ref_size
>   Btrfs: remove BUG() in print_extent_item
>   Btrfs: remove BUG() in add_data_reference
>   Btrfs: remove BUG_ON in __add_tree_block
>   Btrfs: add one more sanity check for shared ref type

Added to 4.14 queue, some trivial fixups were needed due to other patches.
--
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 7/7] Btrfs: add one more sanity check for shared ref type

[David Sterba]

On Fri, Aug 18, 2017 at 03:15:24PM -0600, Liu Bo wrote:
> Every shared ref has a parent tree block, which can be get from
> btrfs_extent_inline_ref_offset().  And the tree block must be aligned
> to the nodesize, so we'd know this inline ref is not valid if this
> block's bytenr is not aligned to the nodesize, in which case, most
> likely the ref type has been misused.
> 
> This adds the above mentioned check and also updates
> print_extent_item() called by btrfs_print_leaf() to point out the
> invalid ref while printing the tree structure.
> 
> Signed-off-by: Liu Bo 

Reviewed-by: David Sterba 
--
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 6/7] Btrfs: remove BUG_ON in __add_tree_block

[David Sterba]

On Fri, Aug 18, 2017 at 03:15:23PM -0600, Liu Bo wrote:
> The BUG_ON() can be triggered when the caller is processing an invalid
> extent inline ref, e.g.
> 
> a shared data ref is offered instead of a extent data ref, such that
> it tries to find a non-exist tree block and then btrfs_search_slot
> returns 1 for no such item.
> 
> This replaces the BUG_ON() with a WARN() followed by calling
> btrfs_print_leaf() to show more details about what's going on and
> returning -EINVAL to upper callers.
> 
> Signed-off-by: Liu Bo 

Reviewed-by: David Sterba 
--
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 5/7] Btrfs: remove BUG() in add_data_reference

[David Sterba]

On Fri, Aug 18, 2017 at 03:15:22PM -0600, Liu Bo wrote:
> Now that we have a helper to report invalid value of extent inline ref
> type, we need to quit gracefully instead of throwing out a kernel panic.
> 
> Signed-off-by: Liu Bo 

Reviewed-by: David Sterba 
--
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 4/7] Btrfs: remove BUG() in print_extent_item

[David Sterba]

On Fri, Aug 18, 2017 at 03:15:21PM -0600, Liu Bo wrote:
> btrfs_print_leaf() is used in btrfs_get_extent_inline_ref_type, so
> here we really want to print the invalid value of ref type instead of
> causing a kernel panic.
> 
> Signed-off-by: Liu Bo 
> ---
>  fs/btrfs/print-tree.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/print-tree.c b/fs/btrfs/print-tree.c
> index fcae61e..0e52e47 100644
> --- a/fs/btrfs/print-tree.c
> +++ b/fs/btrfs/print-tree.c
> @@ -121,7 +121,10 @@ static void print_extent_item(struct extent_buffer *eb, 
> int slot, int type)
>  offset, btrfs_shared_data_ref_count(eb, sref));
>   break;
>   default:
> - BUG();
> + btrfs_err(eb->fs_info,
> +   "extent %llu has invalid ref type %d\n",

I'll remove the trailing \n

Reviewed-by: David Sterba 
--
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 3/7] Btrfs: remove BUG() in btrfs_extent_inline_ref_size

[David Sterba]

On Fri, Aug 18, 2017 at 03:15:20PM -0600, Liu Bo wrote:
> Now that btrfs_get_extent_inline_ref_type() can report if type is a
> valid one and all callers can gracefully deal with that, we don't need
> to crash here.
> 
> Signed-off-by: Liu Bo 

Reviewed-by: David Sterba 
--
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 2/7] Btrfs: convert to use btrfs_get_extent_inline_ref_type

[David Sterba]

On Fri, Aug 18, 2017 at 03:15:19PM -0600, Liu Bo wrote:
> Since we have a helper which can do sanity check, this converts all
> btrfs_extent_inline_ref_type to it.
> 
> Signed-off-by: Liu Bo 

Reviewed-by: David Sterba 
--
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 1/7] Btrfs: add a helper to retrive extent inline ref type

[David Sterba]

On Fri, Aug 18, 2017 at 03:15:18PM -0600, Liu Bo wrote:
> An invalid value of extent inline ref type may be read from a
> malicious image which may force btrfs to crash.
> 
> This adds a helper which does sanity check for the ref type, so we can
> know if it's sane, return type if so, otherwise return an error.
> 
> Signed-off-by: Liu Bo 

Reviewed-by: David Sterba 
--
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: btrfs: Adjust 32 checks for null pointers

[Timofey Titovets]

Sorry Markus, but main problem with your patches described at that page:
https://btrfs.wiki.kernel.org/index.php/Developer%27s_FAQ#How_not_to_start

I.e. it's cool that you try to help as you can, but not that way, thanks.

2017-08-21 16:27 GMT+03:00 SF Markus Elfring :
>> That's will work,
>
> Thanks for your acknowledgement.
>
>
>> but that's don't improve anything.
>
> Do you like a small source code reduction here?
>
> Regards,
> Markus



-- 
Have a nice day,
Timofey.
--
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][v2] btrfs: make the delalloc block rsv per inode

[josef]

From: Josef Bacik 

The way we handle delalloc metadata reservations has gotten
progressively more complicated over the years.  There is so much cruft
and weirdness around keeping the reserved count and outstanding counters
consistent and handling the error cases that it's impossible to
understand.

Fix this by making the delalloc block rsv per-inode.  This way we can
calculate the actual size of the outstanding metadata reservations every
time we make a change, and then reserve the delta based on that amount.
This greatly simplifies the code everywhere, and makes the error
handling in btrfs_delalloc_reserve_metadata far less terrifying.

Signed-off-by: Josef Bacik 
---
v1->v2:
- make btrfs_block_rsv_release_bytes return the bytes released instead of using
  a pointer to store it.
- use lockdep_assert_held instead of ASSERT().

 fs/btrfs/btrfs_inode.h   |  29 ++--
 fs/btrfs/ctree.h |   5 +-
 fs/btrfs/delayed-inode.c |  46 +--
 fs/btrfs/disk-io.c   |  18 ++-
 fs/btrfs/extent-tree.c   | 320 +++
 fs/btrfs/inode.c |  18 ++-
 include/trace/events/btrfs.h |  22 ---
 7 files changed, 141 insertions(+), 317 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 05db430..f9c6887 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -36,14 +36,13 @@
 #define BTRFS_INODE_ORPHAN_META_RESERVED   1
 #define BTRFS_INODE_DUMMY  2
 #define BTRFS_INODE_IN_DEFRAG  3
-#define BTRFS_INODE_DELALLOC_META_RESERVED 4
-#define BTRFS_INODE_HAS_ORPHAN_ITEM5
-#define BTRFS_INODE_HAS_ASYNC_EXTENT   6
-#define BTRFS_INODE_NEEDS_FULL_SYNC7
-#define BTRFS_INODE_COPY_EVERYTHING8
-#define BTRFS_INODE_IN_DELALLOC_LIST   9
-#define BTRFS_INODE_READDIO_NEED_LOCK  10
-#define BTRFS_INODE_HAS_PROPS  11
+#define BTRFS_INODE_HAS_ORPHAN_ITEM4
+#define BTRFS_INODE_HAS_ASYNC_EXTENT   5
+#define BTRFS_INODE_NEEDS_FULL_SYNC6
+#define BTRFS_INODE_COPY_EVERYTHING7
+#define BTRFS_INODE_IN_DELALLOC_LIST   8
+#define BTRFS_INODE_READDIO_NEED_LOCK  9
+#define BTRFS_INODE_HAS_PROPS  10
 
 /* in memory btrfs inode */
 struct btrfs_inode {
@@ -176,7 +175,8 @@ struct btrfs_inode {
 * of extent items we've reserved metadata for.
 */
unsigned outstanding_extents;
-   unsigned reserved_extents;
+
+   struct btrfs_block_rsv block_rsv;
 
/*
 * Cached values of inode properties
@@ -278,17 +278,6 @@ static inline void btrfs_mod_outstanding_extents(struct 
btrfs_inode *inode,
  mod);
 }
 
-static inline void btrfs_mod_reserved_extents(struct btrfs_inode *inode,
- int mod)
-{
-   ASSERT(spin_is_locked(>lock));
-   inode->reserved_extents += mod;
-   if (btrfs_is_free_space_inode(inode))
-   return;
-   trace_btrfs_inode_mod_reserved_extents(inode->root, btrfs_ino(inode),
-  mod);
-}
-
 static inline int btrfs_inode_in_log(struct btrfs_inode *inode, u64 generation)
 {
int ret = 0;
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 04c3f5d..d49b045 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -763,8 +763,6 @@ struct btrfs_fs_info {
 * delayed dir index item
 */
struct btrfs_block_rsv global_block_rsv;
-   /* block reservation for delay allocation */
-   struct btrfs_block_rsv delalloc_block_rsv;
/* block reservation for metadata operations */
struct btrfs_block_rsv trans_block_rsv;
/* block reservation for chunk tree */
@@ -2744,6 +2742,9 @@ int btrfs_delalloc_reserve_space(struct inode *inode,
 void btrfs_init_block_rsv(struct btrfs_block_rsv *rsv, unsigned short type);
 struct btrfs_block_rsv *btrfs_alloc_block_rsv(struct btrfs_fs_info *fs_info,
  unsigned short type);
+void btrfs_init_metadata_block_rsv(struct btrfs_fs_info *fs_info,
+  struct btrfs_block_rsv *rsv,
+  unsigned short type);
 void btrfs_free_block_rsv(struct btrfs_fs_info *fs_info,
  struct btrfs_block_rsv *rsv);
 void __btrfs_free_block_rsv(struct btrfs_block_rsv *rsv);
diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
index 19e4ad2..5d73f79 100644
--- a/fs/btrfs/delayed-inode.c
+++ b/fs/btrfs/delayed-inode.c
@@ -581,7 +581,6 @@ static int btrfs_delayed_inode_reserve_metadata(
struct btrfs_block_rsv *dst_rsv;
u64 num_bytes;
int ret;
-   bool release = false;
 
src_rsv = trans->block_rsv;
dst_rsv = _info->delayed_block_rsv;
@@ -589,36 +588,13 @@ static int 

Re: [PATCH][v2] Btrfs: rework outstanding_extents

[Josef Bacik]

Ignore this, I git format-patch'ed the wrong patch.

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

[PATCH v2] btrfs: Use common error handling code in update_ref_path()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 21 Aug 2017 15:45:23 +0200

Add a jump target so that a bit of exception handling can be better reused
at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---

v2:
Do you find this refactoring acceptable instead?

 fs/btrfs/send.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 59fb1ed6ca20..527a9a735664 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -3697,20 +3697,20 @@ static int update_ref_path(struct send_ctx *sctx, 
struct recorded_ref *ref)
return -ENOMEM;
 
ret = get_cur_path(sctx, ref->dir, ref->dir_gen, new_path);
-   if (ret < 0) {
-   fs_path_free(new_path);
-   return ret;
-   }
+   if (ret < 0)
+   goto free_path;
+
ret = fs_path_add(new_path, ref->name, ref->name_len);
-   if (ret < 0) {
-   fs_path_free(new_path);
-   return ret;
-   }
+   if (ret < 0)
+   goto free_path;
 
fs_path_free(ref->full_path);
set_ref_path(ref, new_path);
 
return 0;
+free_path:
+   fs_path_free(new_path);
+   return ret;
 }
 
 /*
-- 
2.14.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

[PATCH][v2] Btrfs: rework outstanding_extents

[josef]

From: Josef Bacik 

Right now we do a lot of weird hoops around outstanding_extents in order
to keep the extent count consistent.  This is because we logically
transfer the outstanding_extent count from the initial reservation
through the set_delalloc_bits.  This makes it pretty difficult to get a
handle on how and when we need to mess with outstanding_extents.

Fix this by revamping the rules of how we deal with outstanding_extents.
Now instead everybody that is holding on to a delalloc extent is
required to increase the outstanding extents count for itself.  This
means we'll have something like this

btrfs_dealloc_reserve_metadata  - outstanding_extents = 1
 btrfs_set_delalloc - outstanding_extents = 2
btrfs_release_delalloc_extents  - outstanding_extents = 1

for an initial file write.  Now take the append write where we extend an
existing delalloc range but still under the maximum extent size

btrfs_delalloc_reserve_metadata - outstanding_extents = 2
  btrfs_set_delalloc
btrfs_set_bit_hook  - outstanding_extents = 3
btrfs_merge_bit_hook- outstanding_extents = 2
btrfs_release_delalloc_extents  - outstanding_extnets = 1

In order to make the ordered extent transition we of course must now
make ordered extents carry their own outstanding_extent reservation, so
for cow_file_range we end up with

btrfs_add_ordered_extent- outstanding_extents = 2
clear_extent_bit- outstanding_extents = 1
btrfs_remove_ordered_extent - outstanding_extents = 0

This makes all manipulations of outstanding_extents much more explicit.
Every successful call to btrfs_reserve_delalloc_metadata _must_ now be
combined with btrfs_release_delalloc_extents, even in the error case, as
that is the only function that actually modifies the
outstanding_extents counter.

The drawback to this is now we are much more likely to have transient
cases where outstanding_extents is much larger than it actually should
be.  This could happen before as we manipulated the delalloc bits, but
now it happens basically at every write.  This may put more pressure on
the ENOSPC flushing code, but I think making this code simpler is worth
the cost.  I have another change coming to mitigate this side-effect
somewhat.

I also added trace points for the counter manipulation.  These were used
by a bpf script I wrote to help track down leak issues.

Signed-off-by: Josef Bacik 
---
v1->v2:
- make btrfs_block_rsv_release_bytes return the bytes released instead of using
  a pointer to store it.
- use lockdep_assert_held instead of ASSERT().

 fs/btrfs/btrfs_inode.h   |  22 +++
 fs/btrfs/ctree.h |   2 +
 fs/btrfs/extent-tree.c   | 141 ---
 fs/btrfs/file.c  |  22 +++
 fs/btrfs/inode-map.c |   3 +-
 fs/btrfs/inode.c | 114 +++---
 fs/btrfs/ioctl.c |   2 +
 fs/btrfs/ordered-data.c  |  21 ++-
 fs/btrfs/relocation.c|   3 +
 fs/btrfs/tests/inode-tests.c |  18 ++
 include/trace/events/btrfs.h |  44 ++
 11 files changed, 236 insertions(+), 156 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index eccadb5..05db430 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -267,6 +267,28 @@ static inline bool btrfs_is_free_space_inode(struct 
btrfs_inode *inode)
return false;
 }
 
+static inline void btrfs_mod_outstanding_extents(struct btrfs_inode *inode,
+int mod)
+{
+   ASSERT(spin_is_locked(>lock));
+   inode->outstanding_extents += mod;
+   if (btrfs_is_free_space_inode(inode))
+   return;
+   trace_btrfs_inode_mod_outstanding_extents(inode->root, btrfs_ino(inode),
+ mod);
+}
+
+static inline void btrfs_mod_reserved_extents(struct btrfs_inode *inode,
+ int mod)
+{
+   ASSERT(spin_is_locked(>lock));
+   inode->reserved_extents += mod;
+   if (btrfs_is_free_space_inode(inode))
+   return;
+   trace_btrfs_inode_mod_reserved_extents(inode->root, btrfs_ino(inode),
+  mod);
+}
+
 static inline int btrfs_inode_in_log(struct btrfs_inode *inode, u64 generation)
 {
int ret = 0;
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 02edcdd..04c3f5d 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2735,6 +2735,8 @@ int btrfs_subvolume_reserve_metadata(struct btrfs_root 
*root,
 u64 *qgroup_reserved, bool use_global_rsv);
 void btrfs_subvolume_release_metadata(struct btrfs_fs_info *fs_info,
  struct btrfs_block_rsv *rsv);
+void btrfs_delalloc_release_extents(struct btrfs_inode *inode, u64 num_bytes);
+
 int btrfs_delalloc_reserve_metadata(struct btrfs_inode 

Re: [PATCH 4/5] btrfs: Use common error handling code in update_ref_path()

[Dan Carpenter]

On Mon, Aug 21, 2017 at 09:08:04AM -0400, Jeff Mahoney wrote:
> On 8/21/17 8:41 AM, SF Markus Elfring wrote:
> > From: Markus Elfring 
> > Date: Mon, 21 Aug 2017 13:34:29 +0200
> > 
> > Add a jump target so that a bit of exception handling can be better reused
> > in this function.
> > 
> > This issue was detected by using the Coccinelle software.
> 
> Adding a jump label in the middle of a conditional for "common" error
> handling makes the code more difficult to understand.
> 

I have said that a bunch of times.  It's like bashing my face into the
keyboard for all the good it does.  On the other hand, some people
accept these oddly placed labels...  No one else writes code like this
so far as I know.

regards,
dan carpenter

--
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: btrfs: Delete an unnecessary variable initialisation in tree_mod_log_eb_copy()

[SF Markus Elfring]

> Don't needed, and you did miss several similar places (L573 & L895) in
> that file with explicit initialisation.

Would you like to adjust any remaining places in such source files?

Regards,
Markus
--
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: make the delalloc block rsv per inode

[Josef Bacik]

On Mon, Aug 21, 2017 at 03:48:51PM +0300, Nikolay Borisov wrote:
> 
> 
> On 18.08.2017 00:23, jo...@toxicpanda.com wrote:
> > From: Josef Bacik 
> > 
> > The way we handle delalloc metadata reservations has gotten
> > progressively more complicated over the years.  There is so much cruft
> > and weirdness around keeping the reserved count and outstanding counters
> > consistent and handling the error cases that it's impossible to
> > understand.
> > 
> > Fix this by making the delalloc block rsv per-inode.  This way we can
> > calculate the actual size of the outstanding metadata reservations every
> > time we make a change, and then reserve the delta based on that amount.
> > This greatly simplifies the code everywhere, and makes the error
> > handling in btrfs_delalloc_reserve_metadata far less terrifying.
> > 
> > Signed-off-by: Josef Bacik 
> > ---
> >  fs/btrfs/btrfs_inode.h   |  29 ++--
> >  fs/btrfs/ctree.h |   5 +-
> >  fs/btrfs/delayed-inode.c |  46 +-
> >  fs/btrfs/disk-io.c   |  18 ++-
> >  fs/btrfs/extent-tree.c   | 330 
> > +++
> >  fs/btrfs/inode.c |  18 ++-
> >  include/trace/events/btrfs.h |  22 ---
> >  7 files changed, 147 insertions(+), 321 deletions(-)
> > 
> > diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> > index 2894ad6..71a4ded 100644
> > --- a/fs/btrfs/btrfs_inode.h
> > +++ b/fs/btrfs/btrfs_inode.h
> > @@ -36,14 +36,13 @@
> >  #define BTRFS_INODE_ORPHAN_META_RESERVED   1
> >  #define BTRFS_INODE_DUMMY  2
> >  #define BTRFS_INODE_IN_DEFRAG  3
> > -#define BTRFS_INODE_DELALLOC_META_RESERVED 4
> > -#define BTRFS_INODE_HAS_ORPHAN_ITEM5
> > -#define BTRFS_INODE_HAS_ASYNC_EXTENT   6
> > -#define BTRFS_INODE_NEEDS_FULL_SYNC7
> > -#define BTRFS_INODE_COPY_EVERYTHING8
> > -#define BTRFS_INODE_IN_DELALLOC_LIST   9
> > -#define BTRFS_INODE_READDIO_NEED_LOCK  10
> > -#define BTRFS_INODE_HAS_PROPS  11
> > +#define BTRFS_INODE_HAS_ORPHAN_ITEM4
> > +#define BTRFS_INODE_HAS_ASYNC_EXTENT   5
> > +#define BTRFS_INODE_NEEDS_FULL_SYNC6
> > +#define BTRFS_INODE_COPY_EVERYTHING7
> > +#define BTRFS_INODE_IN_DELALLOC_LIST   8
> > +#define BTRFS_INODE_READDIO_NEED_LOCK  9
> > +#define BTRFS_INODE_HAS_PROPS  10
> >  
> >  /* in memory btrfs inode */
> >  struct btrfs_inode {
> > @@ -176,7 +175,8 @@ struct btrfs_inode {
> >  * of extent items we've reserved metadata for.
> >  */
> > unsigned outstanding_extents;
> > -   unsigned reserved_extents;
> > +
> > +   struct btrfs_block_rsv block_rsv;
> >  
> > /*
> >  * Cached values if inode properties
> > @@ -278,17 +278,6 @@ static inline void 
> > btrfs_mod_outstanding_extents(struct btrfs_inode *inode,
> >   mod);
> >  }
> >  
> > -static inline void btrfs_mod_reserved_extents(struct btrfs_inode *inode,
> > - int mod)
> > -{
> > -   ASSERT(spin_is_locked(>lock));
> > -   inode->reserved_extents += mod;
> > -   if (btrfs_is_free_space_inode(inode))
> > -   return;
> > -   trace_btrfs_inode_mod_reserved_extents(inode->root, btrfs_ino(inode),
> > -  mod);
> > -}
> > -
> >  static inline int btrfs_inode_in_log(struct btrfs_inode *inode, u64 
> > generation)
> >  {
> > int ret = 0;
> > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> > index b43114d..a4ee115 100644
> > --- a/fs/btrfs/ctree.h
> > +++ b/fs/btrfs/ctree.h
> > @@ -763,8 +763,6 @@ struct btrfs_fs_info {
> >  * delayed dir index item
> >  */
> > struct btrfs_block_rsv global_block_rsv;
> > -   /* block reservation for delay allocation */
> > -   struct btrfs_block_rsv delalloc_block_rsv;
> > /* block reservation for metadata operations */
> > struct btrfs_block_rsv trans_block_rsv;
> > /* block reservation for chunk tree */
> > @@ -2744,6 +2742,9 @@ int btrfs_delalloc_reserve_space(struct inode *inode,
> >  void btrfs_init_block_rsv(struct btrfs_block_rsv *rsv, unsigned short 
> > type);
> >  struct btrfs_block_rsv *btrfs_alloc_block_rsv(struct btrfs_fs_info 
> > *fs_info,
> >   unsigned short type);
> > +void btrfs_init_metadata_block_rsv(struct btrfs_fs_info *fs_info,
> > +  struct btrfs_block_rsv *rsv,
> > +  unsigned short type);
> >  void btrfs_free_block_rsv(struct btrfs_fs_info *fs_info,
> >   struct btrfs_block_rsv *rsv);
> >  void __btrfs_free_block_rsv(struct btrfs_block_rsv *rsv);
> > diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> > index 19e4ad2..5d73f79 100644
> > --- a/fs/btrfs/delayed-inode.c
> > +++ 

Re: btrfs: Adjust 32 checks for null pointers

[SF Markus Elfring]

> That's will work,

Thanks for your acknowledgement.


> but that's don't improve anything.

Do you like a small source code reduction here?

Regards,
Markus
--
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] BTRFS: Fine-tuning for five function implementations

[David Sterba]

On Mon, Aug 21, 2017 at 02:35:56PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Mon, 21 Aug 2017 14:30:12 +0200
> 
> A few update suggestions were taken into account
> from static source code analysis.

All patches make a nice gallery of anti-patterns.  None of them will get
applied.
--
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/5] btrfs: Use common error handling code in __btrfs_free_extent()

[Dan Carpenter]

On Mon, Aug 21, 2017 at 09:07:47AM -0400, Jeff Mahoney wrote:
> On 8/21/17 8:38 AM, SF Markus Elfring wrote:
> > From: Markus Elfring 
> > Date: Mon, 21 Aug 2017 10:03:00 +0200
> > 
> > Add a jump target so that a bit of exception handling can be better reused
> > at the end of this function.
> > 
> > This issue was detected by using the Coccinelle software.
> 
> btrfs_abort_transaction dumps __FILE__:__LINE__ in the log so this patch
> makes the code more difficult to debug.
> 

I was just reviewing this and I missed that issue.  These patches are
just exhausting...

regards,
dan carpenter

--
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 3/5] btrfs: Use common error handling code in btrfs_update_root()

[Jeff Mahoney]

On 8/21/17 8:40 AM, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Mon, 21 Aug 2017 13:10:15 +0200
> 
> Add a jump target so that a bit of exception handling can be better reused
> at the end of this function.
> 
> This issue was detected by using the Coccinelle software.

btrfs_abort_transaction dumps __FILE__:__LINE__ in the log so this patch
makes the code more difficult to debug.

-Jeff

> Signed-off-by: Markus Elfring 
> ---
>  fs/btrfs/root-tree.c | 27 +++
>  1 file changed, 11 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
> index 5b488af6f25e..bc497ba9d9d1 100644
> --- a/fs/btrfs/root-tree.c
> +++ b/fs/btrfs/root-tree.c
> @@ -145,10 +145,8 @@ int btrfs_update_root(struct btrfs_trans_handle *trans, 
> struct btrfs_root
>   return -ENOMEM;
>  
>   ret = btrfs_search_slot(trans, root, key, path, 0, 1);
> - if (ret < 0) {
> - btrfs_abort_transaction(trans, ret);
> - goto out;
> - }
> + if (ret < 0)
> + goto abort_transaction;
>  
>   if (ret != 0) {
>   btrfs_print_leaf(path->nodes[0]);
> @@ -171,23 +169,17 @@ int btrfs_update_root(struct btrfs_trans_handle *trans, 
> struct btrfs_root
>   btrfs_release_path(path);
>   ret = btrfs_search_slot(trans, root, key, path,
>   -1, 1);
> - if (ret < 0) {
> - btrfs_abort_transaction(trans, ret);
> - goto out;
> - }
> + if (ret < 0)
> + goto abort_transaction;
>  
>   ret = btrfs_del_item(trans, root, path);
> - if (ret < 0) {
> - btrfs_abort_transaction(trans, ret);
> - goto out;
> - }
> + if (ret < 0)
> + goto abort_transaction;
>   btrfs_release_path(path);
>   ret = btrfs_insert_empty_item(trans, root, path,
>   key, sizeof(*item));
> - if (ret < 0) {
> - btrfs_abort_transaction(trans, ret);
> - goto out;
> - }
> + if (ret < 0)
> + goto abort_transaction;
>   l = path->nodes[0];
>   slot = path->slots[0];
>   ptr = btrfs_item_ptr_offset(l, slot);
> @@ -204,6 +196,9 @@ int btrfs_update_root(struct btrfs_trans_handle *trans, 
> struct btrfs_root
>  out:
>   btrfs_free_path(path);
>   return ret;
> +abort_transaction:
> + btrfs_abort_transaction(trans, ret);
> + goto out;
>  }
>  
>  int btrfs_insert_root(struct btrfs_trans_handle *trans, struct btrfs_root 
> *root,
> 


-- 
Jeff Mahoney
SUSE Labs



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 4/5] btrfs: Use common error handling code in update_ref_path()

[Jeff Mahoney]

On 8/21/17 8:41 AM, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Mon, 21 Aug 2017 13:34:29 +0200
> 
> Add a jump target so that a bit of exception handling can be better reused
> in this function.
> 
> This issue was detected by using the Coccinelle software.

Adding a jump label in the middle of a conditional for "common" error
handling makes the code more difficult to understand.

-Jeff

> Signed-off-by: Markus Elfring 
> ---
>  fs/btrfs/send.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index 59fb1ed6ca20..a96edc91a101 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -3697,12 +3697,12 @@ static int update_ref_path(struct send_ctx *sctx, 
> struct recorded_ref *ref)
>   return -ENOMEM;
>  
>   ret = get_cur_path(sctx, ref->dir, ref->dir_gen, new_path);
> - if (ret < 0) {
> - fs_path_free(new_path);
> - return ret;
> - }
> + if (ret < 0)
> + goto free_path;
> +
>   ret = fs_path_add(new_path, ref->name, ref->name_len);
>   if (ret < 0) {
> +free_path:
>   fs_path_free(new_path);
>   return ret;
>   }
> 


-- 
Jeff Mahoney
SUSE Labs



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 2/5] btrfs: Use common error handling code in __btrfs_free_extent()

[Jeff Mahoney]

On 8/21/17 8:38 AM, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Mon, 21 Aug 2017 10:03:00 +0200
> 
> Add a jump target so that a bit of exception handling can be better reused
> at the end of this function.
> 
> This issue was detected by using the Coccinelle software.

btrfs_abort_transaction dumps __FILE__:__LINE__ in the log so this patch
makes the code more difficult to debug.

-Jeff


> Signed-off-by: Markus Elfring 
> ---
>  fs/btrfs/extent-tree.c | 69 
> --
>  1 file changed, 27 insertions(+), 42 deletions(-)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 116c5615d6c2..c6b7aca88491 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -6913,10 +6913,9 @@ static int __btrfs_free_extent(struct 
> btrfs_trans_handle *trans,
>   ret = remove_extent_backref(trans, info, path, NULL,
>   refs_to_drop,
>   is_data, _ref);
> - if (ret) {
> - btrfs_abort_transaction(trans, ret);
> - goto out;
> - }
> + if (ret)
> + goto abort_transaction;
> +
>   btrfs_release_path(path);
>   path->leave_spinning = 1;
>  
> @@ -6962,10 +6961,9 @@ static int __btrfs_free_extent(struct 
> btrfs_trans_handle *trans,
>   if (ret > 0)
>   btrfs_print_leaf(path->nodes[0]);
>   }
> - if (ret < 0) {
> - btrfs_abort_transaction(trans, ret);
> - goto out;
> - }
> + if (ret < 0)
> + goto abort_transaction;
> +
>   extent_slot = path->slots[0];
>   }
>   } else if (WARN_ON(ret == -ENOENT)) {
> @@ -6974,11 +6972,9 @@ static int __btrfs_free_extent(struct 
> btrfs_trans_handle *trans,
>   "unable to find ref byte nr %llu parent %llu root %llu  
> owner %llu offset %llu",
>   bytenr, parent, root_objectid, owner_objectid,
>   owner_offset);
> - btrfs_abort_transaction(trans, ret);
> - goto out;
> + goto abort_transaction;
>   } else {
> - btrfs_abort_transaction(trans, ret);
> - goto out;
> + goto abort_transaction;
>   }
>  
>   leaf = path->nodes[0];
> @@ -6988,10 +6984,8 @@ static int __btrfs_free_extent(struct 
> btrfs_trans_handle *trans,
>   BUG_ON(found_extent || extent_slot != path->slots[0]);
>   ret = convert_extent_item_v0(trans, info, path, owner_objectid,
>0);
> - if (ret < 0) {
> - btrfs_abort_transaction(trans, ret);
> - goto out;
> - }
> + if (ret < 0)
> + goto abort_transaction;
>  
>   btrfs_release_path(path);
>   path->leave_spinning = 1;
> @@ -7008,10 +7002,8 @@ static int __btrfs_free_extent(struct 
> btrfs_trans_handle *trans,
>   ret, bytenr);
>   btrfs_print_leaf(path->nodes[0]);
>   }
> - if (ret < 0) {
> - btrfs_abort_transaction(trans, ret);
> - goto out;
> - }
> + if (ret < 0)
> + goto abort_transaction;
>  
>   extent_slot = path->slots[0];
>   leaf = path->nodes[0];
> @@ -7035,8 +7027,7 @@ static int __btrfs_free_extent(struct 
> btrfs_trans_handle *trans,
> "trying to drop %d refs but we only have %Lu for 
> bytenr %Lu",
> refs_to_drop, refs, bytenr);
>   ret = -EINVAL;
> - btrfs_abort_transaction(trans, ret);
> - goto out;
> + goto abort_transaction;
>   }
>   refs -= refs_to_drop;
>  
> @@ -7057,10 +7048,8 @@ static int __btrfs_free_extent(struct 
> btrfs_trans_handle *trans,
>   ret = remove_extent_backref(trans, info, path,
>   iref, refs_to_drop,
>   is_data, _ref);
> - if (ret) {
> - btrfs_abort_transaction(trans, ret);
> - goto out;
> - }
> + if (ret)
> + goto abort_transaction;
>   }
>   } else {
>   if (found_extent) {
> @@ -7078,37 +7067,33 @@ static int __btrfs_free_extent(struct 
> 

Re: [PATCH 5/5] btrfs: Use common error handling code in btrfs_mark_extent_written()

[Jeff Mahoney]

On 8/21/17 8:42 AM, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Mon, 21 Aug 2017 14:15:23 +0200
> 
> Add jump targets so that a bit of exception handling can be better reused
> at the end of this function.
> 
> This issue was detected by using the Coccinelle software.

btrfs_abort_transaction dumps __FILE__:__LINE__ in the log so this patch
makes the code more difficult to debug.

-Jeff

> Signed-off-by: Markus Elfring 
> ---
>  fs/btrfs/file.c | 62 
> ++---
>  1 file changed, 24 insertions(+), 38 deletions(-)
> 
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index 74fd7756cff3..675683051cbc 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -1122,25 +1122,17 @@ int btrfs_mark_extent_written(struct 
> btrfs_trans_handle *trans,
>  
>   leaf = path->nodes[0];
>   btrfs_item_key_to_cpu(leaf, , path->slots[0]);
> - if (key.objectid != ino ||
> - key.type != BTRFS_EXTENT_DATA_KEY) {
> - ret = -EINVAL;
> - btrfs_abort_transaction(trans, ret);
> - goto out;
> - }
> + if (key.objectid != ino || key.type != BTRFS_EXTENT_DATA_KEY)
> + goto e_inval;
> +
>   fi = btrfs_item_ptr(leaf, path->slots[0],
>   struct btrfs_file_extent_item);
> - if (btrfs_file_extent_type(leaf, fi) != BTRFS_FILE_EXTENT_PREALLOC) {
> - ret = -EINVAL;
> - btrfs_abort_transaction(trans, ret);
> - goto out;
> - }
> + if (btrfs_file_extent_type(leaf, fi) != BTRFS_FILE_EXTENT_PREALLOC)
> + goto e_inval;
> +
>   extent_end = key.offset + btrfs_file_extent_num_bytes(leaf, fi);
> - if (key.offset > start || extent_end < end) {
> - ret = -EINVAL;
> - btrfs_abort_transaction(trans, ret);
> - goto out;
> - }
> + if (key.offset > start || extent_end < end)
> + goto e_inval;
>  
>   bytenr = btrfs_file_extent_disk_bytenr(leaf, fi);
>   num_bytes = btrfs_file_extent_disk_num_bytes(leaf, fi);
> @@ -1213,10 +1205,8 @@ int btrfs_mark_extent_written(struct 
> btrfs_trans_handle *trans,
>   btrfs_release_path(path);
>   goto again;
>   }
> - if (ret < 0) {
> - btrfs_abort_transaction(trans, ret);
> - goto out;
> - }
> + if (ret < 0)
> + goto abort_transaction;
>  
>   leaf = path->nodes[0];
>   fi = btrfs_item_ptr(leaf, path->slots[0] - 1,
> @@ -1237,18 +1227,15 @@ int btrfs_mark_extent_written(struct 
> btrfs_trans_handle *trans,
>   ret = btrfs_inc_extent_ref(trans, fs_info, bytenr, num_bytes,
>  0, root->root_key.objectid,
>  ino, orig_offset);
> - if (ret) {
> - btrfs_abort_transaction(trans, ret);
> - goto out;
> - }
> + if (ret)
> + goto abort_transaction;
>  
>   if (split == start) {
>   key.offset = start;
>   } else {
>   if (start != key.offset) {
>   ret = -EINVAL;
> - btrfs_abort_transaction(trans, ret);
> - goto out;
> + goto abort_transaction;
>   }
>   path->slots[0]--;
>   extent_end = end;
> @@ -1271,10 +1258,8 @@ int btrfs_mark_extent_written(struct 
> btrfs_trans_handle *trans,
>   ret = btrfs_free_extent(trans, fs_info, bytenr, num_bytes,
>   0, root->root_key.objectid,
>   ino, orig_offset);
> - if (ret) {
> - btrfs_abort_transaction(trans, ret);
> - goto out;
> - }
> + if (ret)
> + goto abort_transaction;
>   }
>   other_start = 0;
>   other_end = start;
> @@ -1291,10 +1276,8 @@ int btrfs_mark_extent_written(struct 
> btrfs_trans_handle *trans,
>   ret = btrfs_free_extent(trans, fs_info, bytenr, num_bytes,
>   0, root->root_key.objectid,
>   ino, orig_offset);
> - if (ret) {
> - btrfs_abort_transaction(trans, ret);
> - goto out;
> - }
> + if (ret)
> + goto abort_transaction;
>   }
>   if (del_nr == 0) {
>   fi = btrfs_item_ptr(leaf, path->slots[0],
> @@ -1314,14 +1297,17 @@ int btrfs_mark_extent_written(struct 
> btrfs_trans_handle *trans,
>   btrfs_mark_buffer_dirty(leaf);
>  
> 

Re: [PATCH 03/10] btrfs: Remove unused variable

[Timofey Titovets]

Reviewed-by: Timofey Titovets 

2017-08-21 12:43 GMT+03:00 Nikolay Borisov :
> Src was initially part of 31ff1cd25d37 ("Btrfs: Copy into the log tree in
> big batches"), however 16e7549f045d ("Btrfs: incompatible format change to 
> remove hole extents") changed parameters passed to copy_items which made the 
> src
> variable redundant.
>
> Signed-off-by: Nikolay Borisov 
> ---
>  fs/btrfs/tree-log.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index ad7f4bab640b..cb29528a8bba 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -2740,7 +2740,7 @@ static inline void btrfs_remove_log_ctx(struct 
> btrfs_root *root,
> mutex_unlock(>log_mutex);
>  }
>
> -/*
> +/*
>   * Invoked in log mutex context, or be sure there is no other task which
>   * can access the list.
>   */
> @@ -4637,7 +4637,6 @@ static int btrfs_log_inode(struct btrfs_trans_handle 
> *trans,
> struct btrfs_key min_key;
> struct btrfs_key max_key;
> struct btrfs_root *log = root->log_root;
> -   struct extent_buffer *src = NULL;
> LIST_HEAD(logged_list);
> u64 last_extent = 0;
> int err = 0;
> @@ -4880,7 +4879,6 @@ static int btrfs_log_inode(struct btrfs_trans_handle 
> *trans,
> goto next_slot;
> }
>
> -   src = path->nodes[0];
> if (ins_nr && ins_start_slot + ins_nr == path->slots[0]) {
> ins_nr++;
> goto next_slot;
> --
> 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



-- 
Have a nice day,
Timofey.
--
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 01/10] btrfs: Remove chunk_objectid parameter of btrfs_alloc_dev_extent

[Timofey Titovets]

Reviewed-by: Timofey Titovets 

2017-08-21 12:43 GMT+03:00 Nikolay Borisov :
> THe function is always called with chunk_objectid set to
> BTRFS_FIRST_CHUNK_TREE_OBJECTID. Let's collapse the parameter in the function
> itself. No functional changes
>
> Signed-off-by: Nikolay Borisov 
> ---
>  fs/btrfs/volumes.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index a37a31ba6843..63608c5f4487 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1571,8 +1571,8 @@ static int btrfs_free_dev_extent(struct 
> btrfs_trans_handle *trans,
>
>  static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
>   struct btrfs_device *device,
> - u64 chunk_tree, u64 chunk_objectid,
> - u64 chunk_offset, u64 start, u64 num_bytes)
> + u64 chunk_tree, u64 chunk_offset, u64 start,
> + u64 num_bytes)
>  {
> int ret;
> struct btrfs_path *path;
> @@ -1600,7 +1600,8 @@ static int btrfs_alloc_dev_extent(struct 
> btrfs_trans_handle *trans,
> extent = btrfs_item_ptr(leaf, path->slots[0],
> struct btrfs_dev_extent);
> btrfs_set_dev_extent_chunk_tree(leaf, extent, chunk_tree);
> -   btrfs_set_dev_extent_chunk_objectid(leaf, extent, chunk_objectid);
> +   btrfs_set_dev_extent_chunk_objectid(leaf, extent,
> +   BTRFS_FIRST_CHUNK_TREE_OBJECTID);
> btrfs_set_dev_extent_chunk_offset(leaf, extent, chunk_offset);
>
> btrfs_set_dev_extent_length(leaf, extent, num_bytes);
> @@ -4904,7 +4905,6 @@ int btrfs_finish_chunk_alloc(struct btrfs_trans_handle 
> *trans,
> break;
> ret = btrfs_alloc_dev_extent(trans, device,
>  chunk_root->root_key.objectid,
> -BTRFS_FIRST_CHUNK_TREE_OBJECTID,
>  chunk_offset, dev_offset,
>  stripe_size);
> if (ret)
> --
> 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



-- 
Have a nice day,
Timofey.
--
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: make the delalloc block rsv per inode

[Nikolay Borisov]

On 18.08.2017 00:23, jo...@toxicpanda.com wrote:
> From: Josef Bacik 
> 
> The way we handle delalloc metadata reservations has gotten
> progressively more complicated over the years.  There is so much cruft
> and weirdness around keeping the reserved count and outstanding counters
> consistent and handling the error cases that it's impossible to
> understand.
> 
> Fix this by making the delalloc block rsv per-inode.  This way we can
> calculate the actual size of the outstanding metadata reservations every
> time we make a change, and then reserve the delta based on that amount.
> This greatly simplifies the code everywhere, and makes the error
> handling in btrfs_delalloc_reserve_metadata far less terrifying.
> 
> Signed-off-by: Josef Bacik 
> ---
>  fs/btrfs/btrfs_inode.h   |  29 ++--
>  fs/btrfs/ctree.h |   5 +-
>  fs/btrfs/delayed-inode.c |  46 +-
>  fs/btrfs/disk-io.c   |  18 ++-
>  fs/btrfs/extent-tree.c   | 330 
> +++
>  fs/btrfs/inode.c |  18 ++-
>  include/trace/events/btrfs.h |  22 ---
>  7 files changed, 147 insertions(+), 321 deletions(-)
> 
> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> index 2894ad6..71a4ded 100644
> --- a/fs/btrfs/btrfs_inode.h
> +++ b/fs/btrfs/btrfs_inode.h
> @@ -36,14 +36,13 @@
>  #define BTRFS_INODE_ORPHAN_META_RESERVED 1
>  #define BTRFS_INODE_DUMMY2
>  #define BTRFS_INODE_IN_DEFRAG3
> -#define BTRFS_INODE_DELALLOC_META_RESERVED   4
> -#define BTRFS_INODE_HAS_ORPHAN_ITEM  5
> -#define BTRFS_INODE_HAS_ASYNC_EXTENT 6
> -#define BTRFS_INODE_NEEDS_FULL_SYNC  7
> -#define BTRFS_INODE_COPY_EVERYTHING  8
> -#define BTRFS_INODE_IN_DELALLOC_LIST 9
> -#define BTRFS_INODE_READDIO_NEED_LOCK10
> -#define BTRFS_INODE_HAS_PROPS11
> +#define BTRFS_INODE_HAS_ORPHAN_ITEM  4
> +#define BTRFS_INODE_HAS_ASYNC_EXTENT 5
> +#define BTRFS_INODE_NEEDS_FULL_SYNC  6
> +#define BTRFS_INODE_COPY_EVERYTHING  7
> +#define BTRFS_INODE_IN_DELALLOC_LIST 8
> +#define BTRFS_INODE_READDIO_NEED_LOCK9
> +#define BTRFS_INODE_HAS_PROPS10
>  
>  /* in memory btrfs inode */
>  struct btrfs_inode {
> @@ -176,7 +175,8 @@ struct btrfs_inode {
>* of extent items we've reserved metadata for.
>*/
>   unsigned outstanding_extents;
> - unsigned reserved_extents;
> +
> + struct btrfs_block_rsv block_rsv;
>  
>   /*
>* Cached values if inode properties
> @@ -278,17 +278,6 @@ static inline void btrfs_mod_outstanding_extents(struct 
> btrfs_inode *inode,
> mod);
>  }
>  
> -static inline void btrfs_mod_reserved_extents(struct btrfs_inode *inode,
> -   int mod)
> -{
> - ASSERT(spin_is_locked(>lock));
> - inode->reserved_extents += mod;
> - if (btrfs_is_free_space_inode(inode))
> - return;
> - trace_btrfs_inode_mod_reserved_extents(inode->root, btrfs_ino(inode),
> -mod);
> -}
> -
>  static inline int btrfs_inode_in_log(struct btrfs_inode *inode, u64 
> generation)
>  {
>   int ret = 0;
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index b43114d..a4ee115 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -763,8 +763,6 @@ struct btrfs_fs_info {
>* delayed dir index item
>*/
>   struct btrfs_block_rsv global_block_rsv;
> - /* block reservation for delay allocation */
> - struct btrfs_block_rsv delalloc_block_rsv;
>   /* block reservation for metadata operations */
>   struct btrfs_block_rsv trans_block_rsv;
>   /* block reservation for chunk tree */
> @@ -2744,6 +2742,9 @@ int btrfs_delalloc_reserve_space(struct inode *inode,
>  void btrfs_init_block_rsv(struct btrfs_block_rsv *rsv, unsigned short type);
>  struct btrfs_block_rsv *btrfs_alloc_block_rsv(struct btrfs_fs_info *fs_info,
> unsigned short type);
> +void btrfs_init_metadata_block_rsv(struct btrfs_fs_info *fs_info,
> +struct btrfs_block_rsv *rsv,
> +unsigned short type);
>  void btrfs_free_block_rsv(struct btrfs_fs_info *fs_info,
> struct btrfs_block_rsv *rsv);
>  void __btrfs_free_block_rsv(struct btrfs_block_rsv *rsv);
> diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> index 19e4ad2..5d73f79 100644
> --- a/fs/btrfs/delayed-inode.c
> +++ b/fs/btrfs/delayed-inode.c
> @@ -581,7 +581,6 @@ static int btrfs_delayed_inode_reserve_metadata(
>   struct btrfs_block_rsv *dst_rsv;
>   u64 num_bytes;
>   int ret;
> - bool release = false;
>  
>   src_rsv = trans->block_rsv;
>   dst_rsv = _info->delayed_block_rsv;

[PATCH 5/5] btrfs: Use common error handling code in btrfs_mark_extent_written()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 21 Aug 2017 14:15:23 +0200

Add jump targets so that a bit of exception handling can be better reused
at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 fs/btrfs/file.c | 62 ++---
 1 file changed, 24 insertions(+), 38 deletions(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 74fd7756cff3..675683051cbc 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1122,25 +1122,17 @@ int btrfs_mark_extent_written(struct btrfs_trans_handle 
*trans,
 
leaf = path->nodes[0];
btrfs_item_key_to_cpu(leaf, , path->slots[0]);
-   if (key.objectid != ino ||
-   key.type != BTRFS_EXTENT_DATA_KEY) {
-   ret = -EINVAL;
-   btrfs_abort_transaction(trans, ret);
-   goto out;
-   }
+   if (key.objectid != ino || key.type != BTRFS_EXTENT_DATA_KEY)
+   goto e_inval;
+
fi = btrfs_item_ptr(leaf, path->slots[0],
struct btrfs_file_extent_item);
-   if (btrfs_file_extent_type(leaf, fi) != BTRFS_FILE_EXTENT_PREALLOC) {
-   ret = -EINVAL;
-   btrfs_abort_transaction(trans, ret);
-   goto out;
-   }
+   if (btrfs_file_extent_type(leaf, fi) != BTRFS_FILE_EXTENT_PREALLOC)
+   goto e_inval;
+
extent_end = key.offset + btrfs_file_extent_num_bytes(leaf, fi);
-   if (key.offset > start || extent_end < end) {
-   ret = -EINVAL;
-   btrfs_abort_transaction(trans, ret);
-   goto out;
-   }
+   if (key.offset > start || extent_end < end)
+   goto e_inval;
 
bytenr = btrfs_file_extent_disk_bytenr(leaf, fi);
num_bytes = btrfs_file_extent_disk_num_bytes(leaf, fi);
@@ -1213,10 +1205,8 @@ int btrfs_mark_extent_written(struct btrfs_trans_handle 
*trans,
btrfs_release_path(path);
goto again;
}
-   if (ret < 0) {
-   btrfs_abort_transaction(trans, ret);
-   goto out;
-   }
+   if (ret < 0)
+   goto abort_transaction;
 
leaf = path->nodes[0];
fi = btrfs_item_ptr(leaf, path->slots[0] - 1,
@@ -1237,18 +1227,15 @@ int btrfs_mark_extent_written(struct btrfs_trans_handle 
*trans,
ret = btrfs_inc_extent_ref(trans, fs_info, bytenr, num_bytes,
   0, root->root_key.objectid,
   ino, orig_offset);
-   if (ret) {
-   btrfs_abort_transaction(trans, ret);
-   goto out;
-   }
+   if (ret)
+   goto abort_transaction;
 
if (split == start) {
key.offset = start;
} else {
if (start != key.offset) {
ret = -EINVAL;
-   btrfs_abort_transaction(trans, ret);
-   goto out;
+   goto abort_transaction;
}
path->slots[0]--;
extent_end = end;
@@ -1271,10 +1258,8 @@ int btrfs_mark_extent_written(struct btrfs_trans_handle 
*trans,
ret = btrfs_free_extent(trans, fs_info, bytenr, num_bytes,
0, root->root_key.objectid,
ino, orig_offset);
-   if (ret) {
-   btrfs_abort_transaction(trans, ret);
-   goto out;
-   }
+   if (ret)
+   goto abort_transaction;
}
other_start = 0;
other_end = start;
@@ -1291,10 +1276,8 @@ int btrfs_mark_extent_written(struct btrfs_trans_handle 
*trans,
ret = btrfs_free_extent(trans, fs_info, bytenr, num_bytes,
0, root->root_key.objectid,
ino, orig_offset);
-   if (ret) {
-   btrfs_abort_transaction(trans, ret);
-   goto out;
-   }
+   if (ret)
+   goto abort_transaction;
}
if (del_nr == 0) {
fi = btrfs_item_ptr(leaf, path->slots[0],
@@ -1314,14 +1297,17 @@ int btrfs_mark_extent_written(struct btrfs_trans_handle 
*trans,
btrfs_mark_buffer_dirty(leaf);
 
ret = btrfs_del_items(trans, root, path, del_slot, del_nr);
-   if (ret < 0) {
-   btrfs_abort_transaction(trans, ret);
-   goto out;
-   }
+   if (ret < 0)

[PATCH 4/5] btrfs: Use common error handling code in update_ref_path()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 21 Aug 2017 13:34:29 +0200

Add a jump target so that a bit of exception handling can be better reused
in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 fs/btrfs/send.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 59fb1ed6ca20..a96edc91a101 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -3697,12 +3697,12 @@ static int update_ref_path(struct send_ctx *sctx, 
struct recorded_ref *ref)
return -ENOMEM;
 
ret = get_cur_path(sctx, ref->dir, ref->dir_gen, new_path);
-   if (ret < 0) {
-   fs_path_free(new_path);
-   return ret;
-   }
+   if (ret < 0)
+   goto free_path;
+
ret = fs_path_add(new_path, ref->name, ref->name_len);
if (ret < 0) {
+free_path:
fs_path_free(new_path);
return ret;
}
-- 
2.14.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

[PATCH 3/5] btrfs: Use common error handling code in btrfs_update_root()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 21 Aug 2017 13:10:15 +0200

Add a jump target so that a bit of exception handling can be better reused
at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 fs/btrfs/root-tree.c | 27 +++
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
index 5b488af6f25e..bc497ba9d9d1 100644
--- a/fs/btrfs/root-tree.c
+++ b/fs/btrfs/root-tree.c
@@ -145,10 +145,8 @@ int btrfs_update_root(struct btrfs_trans_handle *trans, 
struct btrfs_root
return -ENOMEM;
 
ret = btrfs_search_slot(trans, root, key, path, 0, 1);
-   if (ret < 0) {
-   btrfs_abort_transaction(trans, ret);
-   goto out;
-   }
+   if (ret < 0)
+   goto abort_transaction;
 
if (ret != 0) {
btrfs_print_leaf(path->nodes[0]);
@@ -171,23 +169,17 @@ int btrfs_update_root(struct btrfs_trans_handle *trans, 
struct btrfs_root
btrfs_release_path(path);
ret = btrfs_search_slot(trans, root, key, path,
-1, 1);
-   if (ret < 0) {
-   btrfs_abort_transaction(trans, ret);
-   goto out;
-   }
+   if (ret < 0)
+   goto abort_transaction;
 
ret = btrfs_del_item(trans, root, path);
-   if (ret < 0) {
-   btrfs_abort_transaction(trans, ret);
-   goto out;
-   }
+   if (ret < 0)
+   goto abort_transaction;
btrfs_release_path(path);
ret = btrfs_insert_empty_item(trans, root, path,
key, sizeof(*item));
-   if (ret < 0) {
-   btrfs_abort_transaction(trans, ret);
-   goto out;
-   }
+   if (ret < 0)
+   goto abort_transaction;
l = path->nodes[0];
slot = path->slots[0];
ptr = btrfs_item_ptr_offset(l, slot);
@@ -204,6 +196,9 @@ int btrfs_update_root(struct btrfs_trans_handle *trans, 
struct btrfs_root
 out:
btrfs_free_path(path);
return ret;
+abort_transaction:
+   btrfs_abort_transaction(trans, ret);
+   goto out;
 }
 
 int btrfs_insert_root(struct btrfs_trans_handle *trans, struct btrfs_root 
*root,
-- 
2.14.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

[PATCH 2/5] btrfs: Use common error handling code in __btrfs_free_extent()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 21 Aug 2017 10:03:00 +0200

Add a jump target so that a bit of exception handling can be better reused
at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 fs/btrfs/extent-tree.c | 69 --
 1 file changed, 27 insertions(+), 42 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 116c5615d6c2..c6b7aca88491 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -6913,10 +6913,9 @@ static int __btrfs_free_extent(struct btrfs_trans_handle 
*trans,
ret = remove_extent_backref(trans, info, path, NULL,
refs_to_drop,
is_data, _ref);
-   if (ret) {
-   btrfs_abort_transaction(trans, ret);
-   goto out;
-   }
+   if (ret)
+   goto abort_transaction;
+
btrfs_release_path(path);
path->leave_spinning = 1;
 
@@ -6962,10 +6961,9 @@ static int __btrfs_free_extent(struct btrfs_trans_handle 
*trans,
if (ret > 0)
btrfs_print_leaf(path->nodes[0]);
}
-   if (ret < 0) {
-   btrfs_abort_transaction(trans, ret);
-   goto out;
-   }
+   if (ret < 0)
+   goto abort_transaction;
+
extent_slot = path->slots[0];
}
} else if (WARN_ON(ret == -ENOENT)) {
@@ -6974,11 +6972,9 @@ static int __btrfs_free_extent(struct btrfs_trans_handle 
*trans,
"unable to find ref byte nr %llu parent %llu root %llu  
owner %llu offset %llu",
bytenr, parent, root_objectid, owner_objectid,
owner_offset);
-   btrfs_abort_transaction(trans, ret);
-   goto out;
+   goto abort_transaction;
} else {
-   btrfs_abort_transaction(trans, ret);
-   goto out;
+   goto abort_transaction;
}
 
leaf = path->nodes[0];
@@ -6988,10 +6984,8 @@ static int __btrfs_free_extent(struct btrfs_trans_handle 
*trans,
BUG_ON(found_extent || extent_slot != path->slots[0]);
ret = convert_extent_item_v0(trans, info, path, owner_objectid,
 0);
-   if (ret < 0) {
-   btrfs_abort_transaction(trans, ret);
-   goto out;
-   }
+   if (ret < 0)
+   goto abort_transaction;
 
btrfs_release_path(path);
path->leave_spinning = 1;
@@ -7008,10 +7002,8 @@ static int __btrfs_free_extent(struct btrfs_trans_handle 
*trans,
ret, bytenr);
btrfs_print_leaf(path->nodes[0]);
}
-   if (ret < 0) {
-   btrfs_abort_transaction(trans, ret);
-   goto out;
-   }
+   if (ret < 0)
+   goto abort_transaction;
 
extent_slot = path->slots[0];
leaf = path->nodes[0];
@@ -7035,8 +7027,7 @@ static int __btrfs_free_extent(struct btrfs_trans_handle 
*trans,
  "trying to drop %d refs but we only have %Lu for 
bytenr %Lu",
  refs_to_drop, refs, bytenr);
ret = -EINVAL;
-   btrfs_abort_transaction(trans, ret);
-   goto out;
+   goto abort_transaction;
}
refs -= refs_to_drop;
 
@@ -7057,10 +7048,8 @@ static int __btrfs_free_extent(struct btrfs_trans_handle 
*trans,
ret = remove_extent_backref(trans, info, path,
iref, refs_to_drop,
is_data, _ref);
-   if (ret) {
-   btrfs_abort_transaction(trans, ret);
-   goto out;
-   }
+   if (ret)
+   goto abort_transaction;
}
} else {
if (found_extent) {
@@ -7078,37 +7067,33 @@ static int __btrfs_free_extent(struct 
btrfs_trans_handle *trans,
last_ref = 1;
ret = btrfs_del_items(trans, extent_root, path, path->slots[0],
  num_to_del);
-   if (ret) {
-   

[PATCH 1/5] btrfs: Use common error handling code in tree_mod_log_eb_copy()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 21 Aug 2017 09:36:48 +0200

Add a jump target so that a bit of exception handling can be better reused
in this function.

Signed-off-by: Markus Elfring 
---
 fs/btrfs/ctree.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 6d49db7d86be..d29cf946ebf9 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -823,17 +823,13 @@ tree_mod_log_eb_copy(struct btrfs_fs_info *fs_info, 
struct extent_buffer *dst,
for (i = 0; i < nr_items; i++) {
tm_list_rem[i] = alloc_tree_mod_elem(src, i + src_offset,
MOD_LOG_KEY_REMOVE, GFP_NOFS);
-   if (!tm_list_rem[i]) {
-   ret = -ENOMEM;
-   goto free_tms;
-   }
+   if (!tm_list_rem[i])
+   goto e_nomem;
 
tm_list_add[i] = alloc_tree_mod_elem(dst, i + dst_offset,
MOD_LOG_KEY_ADD, GFP_NOFS);
-   if (!tm_list_add[i]) {
-   ret = -ENOMEM;
-   goto free_tms;
-   }
+   if (!tm_list_add[i])
+   goto e_nomem;
}
 
if (tree_mod_dont_log(fs_info, NULL))
@@ -854,6 +850,8 @@ tree_mod_log_eb_copy(struct btrfs_fs_info *fs_info, struct 
extent_buffer *dst,
 
return 0;
 
+e_nomem:
+   ret = -ENOMEM;
 free_tms:
for (i = 0; i < nr_items * 2; i++) {
if (tm_list[i] && !RB_EMPTY_NODE(_list[i]->node))
-- 
2.14.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

[PATCH 0/5] BTRFS: Fine-tuning for five function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 21 Aug 2017 14:30:12 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (5):
  Use common error handling code in tree_mod_log_eb_copy()
  Use common error handling code in __btrfs_free_extent()
  Use common error handling code in btrfs_update_root()
  Use common error handling code in update_ref_path()
  Use common error handling code in btrfs_mark_extent_written()

 fs/btrfs/ctree.c   | 14 +-
 fs/btrfs/extent-tree.c | 69 --
 fs/btrfs/file.c| 62 ++---
 fs/btrfs/root-tree.c   | 27 
 fs/btrfs/send.c|  8 +++---
 5 files changed, 72 insertions(+), 108 deletions(-)

-- 
2.14.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 06/10] btrfs: Remove unused parameter from check_direct_IO

[Timofey Titovets]

Reviewed-by: Timofey Titovets 

2017-08-21 12:43 GMT+03:00 Nikolay Borisov :
> Introduced by 5a5f79b57069 ("Btrfs: allow unaligned DIO") and never used,
>
> Signed-off-by: Nikolay Borisov 
> ---
>  fs/btrfs/inode.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 61f1ad89e97a..f1b3d6146924 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -8764,7 +8764,6 @@ static void btrfs_submit_direct(struct bio *dio_bio, 
> struct inode *inode,
>  }
>
>  static ssize_t check_direct_IO(struct btrfs_fs_info *fs_info,
> -  struct kiocb *iocb,
>const struct iov_iter *iter, loff_t offset)
>  {
> int seg;
> @@ -8811,7 +8810,7 @@ static ssize_t btrfs_direct_IO(struct kiocb *iocb, 
> struct iov_iter *iter)
> bool relock = false;
> ssize_t ret;
>
> -   if (check_direct_IO(fs_info, iocb, iter, offset))
> +   if (check_direct_IO(fs_info, iter, offset))
> return 0;
>
> inode_dio_begin(inode);
> --
> 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



-- 
Have a nice day,
Timofey.
--
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: Btrfs Raid5 issue.

[Janos Toth F.]

I lost enough Btrfs m=d=s=RAID5 filesystems in past experiments (I
didn't try using RAID5 for metadata and system chunks in the last few
years) to faulty SATA cables + hotplug enabled SATA controllers (where
a disk could disappear and reappear "as the wind blew"). Since then, I
made a habit of always disabling hotplug for all SATA disks involved
with Btrfs, even those with m=d=s=single profile (and I never desired
to built multi-devices filesystems from USB attached disks anyway but
this is good reason for me to explicitly avoid that).

I am not sure if other RAID profiles are affected in a similar way or
it's just RAID56. (Well, I mean RAID0 is obviously toast and RAID1/10
will obviously get degraded but I am not sure if it's possible to
re-sync RAID1/10 with a simple balance [possibly even without
remounting and doing manual device delete/add?] or the filesystem has
to be recreated from scratch [like RAID5].)

I think this hotplug problem is an entirely different issue from the
RAID56-scrub race-conditions (which are now considered fixed in linux
4.12) and nobody is currently working on this (if it's RAID56-only
then I don't expect it anytime soon [think years]).
--
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 4/4] btrfs: Delete an unnecessary variable initialisation in tree_mod_log_eb_copy()

[Timofey Titovets]

Don't needed, and you did miss several similar places (L573 & L895) in
that file with explicit initialisation.

Reviewed-by: Timofey Titovets 

2017-08-20 23:20 GMT+03:00 SF Markus Elfring :
> From: Markus Elfring 
> Date: Sun, 20 Aug 2017 22:02:54 +0200
>
> The variable "tm_list" will eventually be set to an appropriate pointer
> a bit later. Thus omit the explicit initialisation at the beginning.
>
> Signed-off-by: Markus Elfring 
> ---
>  fs/btrfs/ctree.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 3b49f39eaaf6..78387f5be0ce 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -801,7 +801,7 @@ tree_mod_log_eb_copy(struct btrfs_fs_info *fs_info, 
> struct extent_buffer *dst,
>  unsigned long src_offset, int nr_items)
>  {
> int ret = 0;
> -   struct tree_mod_elem **tm_list = NULL;
> +   struct tree_mod_elem **tm_list;
> struct tree_mod_elem **tm_list_add, **tm_list_rem;
> int i;
> int locked = 0;
> --
> 2.14.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



-- 
Have a nice day,
Timofey.
--
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/4] btrfs: Adjust 32 checks for null pointers

[Timofey Titovets]

That's will work, but that's don't improve anything.

Reviewed-by: Timofey Titovets 

2017-08-20 23:18 GMT+03:00 SF Markus Elfring :
> From: Markus Elfring 
> Date: Sun, 20 Aug 2017 21:36:31 +0200
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> The script “checkpatch.pl” pointed information out like the following.
>
> Comparison to NULL could be written …
>
> Thus fix the affected source code places.
>
> Signed-off-by: Markus Elfring 
> ---
>  fs/btrfs/ctree.c   |  4 ++--
>  fs/btrfs/extent-tree.c |  2 +-
>  fs/btrfs/extent_io.c   |  2 +-
>  fs/btrfs/inode.c   |  4 ++--
>  fs/btrfs/lzo.c |  4 ++--
>  fs/btrfs/qgroup.c  |  6 +++---
>  fs/btrfs/reada.c   |  4 ++--
>  fs/btrfs/scrub.c   | 12 ++--
>  fs/btrfs/transaction.c |  2 +-
>  fs/btrfs/tree-log.c|  4 ++--
>  fs/btrfs/volumes.c | 14 +++---
>  fs/btrfs/xattr.c   |  2 +-
>  fs/btrfs/zlib.c|  4 ++--
>  13 files changed, 32 insertions(+), 32 deletions(-)
>
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 6d49db7d86be..b89c101ef45e 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -2686,7 +2686,7 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, 
> struct btrfs_root *root,
>
> lowest_level = p->lowest_level;
> WARN_ON(lowest_level && ins_len > 0);
> -   WARN_ON(p->nodes[0] != NULL);
> +   WARN_ON(p->nodes[0]);
> BUG_ON(!cow && ins_len);
>
> if (ins_len < 0) {
> @@ -2965,7 +2965,7 @@ int btrfs_search_old_slot(struct btrfs_root *root, 
> const struct btrfs_key *key,
> int prev_cmp = -1;
>
> lowest_level = p->lowest_level;
> -   WARN_ON(p->nodes[0] != NULL);
> +   WARN_ON(p->nodes[0]);
>
> if (p->search_commit_root) {
> BUG_ON(time_seq);
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 116c5615d6c2..240dccf8d41c 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -9774,7 +9774,7 @@ void btrfs_put_block_group_cache(struct btrfs_fs_info 
> *info)
> block_group->iref = 0;
> block_group->inode = NULL;
> spin_unlock(_group->lock);
> -   ASSERT(block_group->io_ctl.inode == NULL);
> +   ASSERT(!block_group->io_ctl.inode);
> iput(inode);
> last = block_group->key.objectid + block_group->key.offset;
> btrfs_put_block_group(block_group);
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index ea4947c97505..7fd39a1ec7d6 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -4793,7 +4793,7 @@ struct extent_buffer *btrfs_clone_extent_buffer(struct 
> extent_buffer *src)
> unsigned long num_pages = num_extent_pages(src->start, src->len);
>
> new = __alloc_extent_buffer(src->fs_info, src->start, src->len);
> -   if (new == NULL)
> +   if (!new)
> return NULL;
>
> for (i = 0; i < num_pages; i++) {
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 4cb399854f0e..508057ec2534 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -7599,7 +7599,7 @@ bool btrfs_page_exists_in_range(struct inode *inode, 
> loff_t start, loff_t end)
>  * found idx is less than or equal to the end idx then we know that
>  * a page exists.  If no pages are found or if those pages are
>  * outside of the range then we're fine (yay!) */
> -   while (page == NULL &&
> +   while (!page &&
>radix_tree_gang_lookup_slot(root, , NULL, start_idx, 1)) 
> {
> page = radix_tree_deref_slot(pagep);
> if (unlikely(!page))
> @@ -9588,7 +9588,7 @@ int btrfs_drop_inode(struct inode *inode)
>  {
> struct btrfs_root *root = BTRFS_I(inode)->root;
>
> -   if (root == NULL)
> +   if (!root)
> return 1;
>
> /* the snap/subvol tree is on deleting */
> diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
> index d433e75d489a..29452a0fec91 100644
> --- a/fs/btrfs/lzo.c
> +++ b/fs/btrfs/lzo.c
> @@ -125,7 +125,7 @@ static int lzo_compress_pages(struct list_head *ws,
>  * the first 4 bytes
>  */
> out_page = alloc_page(GFP_NOFS | __GFP_HIGHMEM);
> -   if (out_page == NULL) {
> +   if (!out_page) {
> ret = -ENOMEM;
> goto out;
> }
> @@ -195,7 +195,7 @@ static int lzo_compress_pages(struct list_head *ws,
> }
>
> out_page = alloc_page(GFP_NOFS | 
> __GFP_HIGHMEM);
> -   if (out_page == NULL) {
> +   if (!out_page) {
> ret = -ENOMEM;
> goto out;
>

[PATCH 03/10] btrfs: Remove unused variable

[Nikolay Borisov]

Src was initially part of 31ff1cd25d37 ("Btrfs: Copy into the log tree in
big batches"), however 16e7549f045d ("Btrfs: incompatible format change to 
remove hole extents") changed parameters passed to copy_items which made the 
src 
variable redundant. 

Signed-off-by: Nikolay Borisov 
---
 fs/btrfs/tree-log.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index ad7f4bab640b..cb29528a8bba 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -2740,7 +2740,7 @@ static inline void btrfs_remove_log_ctx(struct btrfs_root 
*root,
mutex_unlock(>log_mutex);
 }
 
-/* 
+/*
  * Invoked in log mutex context, or be sure there is no other task which
  * can access the list.
  */
@@ -4637,7 +4637,6 @@ static int btrfs_log_inode(struct btrfs_trans_handle 
*trans,
struct btrfs_key min_key;
struct btrfs_key max_key;
struct btrfs_root *log = root->log_root;
-   struct extent_buffer *src = NULL;
LIST_HEAD(logged_list);
u64 last_extent = 0;
int err = 0;
@@ -4880,7 +4879,6 @@ static int btrfs_log_inode(struct btrfs_trans_handle 
*trans,
goto next_slot;
}
 
-   src = path->nodes[0];
if (ins_nr && ins_start_slot + ins_nr == path->slots[0]) {
ins_nr++;
goto next_slot;
-- 
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

[PATCH 02/10] btrfs: remove superfluous chunk_tree argument from btrfs_alloc_dev_extent

[Nikolay Borisov]

Currently this function is always called with the object id of the root key of
the chunk_tree, which is always BTRFS_CHUNK_TREE_OBJECTID. So let's subsume it
straight into the function itself. No functional change.

Signed-off-by: Nikolay Borisov 
---
 fs/btrfs/volumes.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 63608c5f4487..d024f1b07282 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1571,8 +1571,7 @@ static int btrfs_free_dev_extent(struct 
btrfs_trans_handle *trans,
 
 static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
  struct btrfs_device *device,
- u64 chunk_tree, u64 chunk_offset, u64 start,
- u64 num_bytes)
+ u64 chunk_offset, u64 start, u64 num_bytes)
 {
int ret;
struct btrfs_path *path;
@@ -1599,7 +1598,8 @@ static int btrfs_alloc_dev_extent(struct 
btrfs_trans_handle *trans,
leaf = path->nodes[0];
extent = btrfs_item_ptr(leaf, path->slots[0],
struct btrfs_dev_extent);
-   btrfs_set_dev_extent_chunk_tree(leaf, extent, chunk_tree);
+   btrfs_set_dev_extent_chunk_tree(leaf, extent,
+   BTRFS_CHUNK_TREE_OBJECTID);
btrfs_set_dev_extent_chunk_objectid(leaf, extent,
BTRFS_FIRST_CHUNK_TREE_OBJECTID);
btrfs_set_dev_extent_chunk_offset(leaf, extent, chunk_offset);
@@ -4903,10 +4903,8 @@ int btrfs_finish_chunk_alloc(struct btrfs_trans_handle 
*trans,
ret = btrfs_update_device(trans, device);
if (ret)
break;
-   ret = btrfs_alloc_dev_extent(trans, device,
-chunk_root->root_key.objectid,
-chunk_offset, dev_offset,
-stripe_size);
+   ret = btrfs_alloc_dev_extent(trans, device, chunk_offset,
+dev_offset, stripe_size);
if (ret)
break;
}
-- 
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

[PATCH 01/10] btrfs: Remove chunk_objectid parameter of btrfs_alloc_dev_extent

[Nikolay Borisov]

THe function is always called with chunk_objectid set to
BTRFS_FIRST_CHUNK_TREE_OBJECTID. Let's collapse the parameter in the function
itself. No functional changes

Signed-off-by: Nikolay Borisov 
---
 fs/btrfs/volumes.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index a37a31ba6843..63608c5f4487 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1571,8 +1571,8 @@ static int btrfs_free_dev_extent(struct 
btrfs_trans_handle *trans,
 
 static int btrfs_alloc_dev_extent(struct btrfs_trans_handle *trans,
  struct btrfs_device *device,
- u64 chunk_tree, u64 chunk_objectid,
- u64 chunk_offset, u64 start, u64 num_bytes)
+ u64 chunk_tree, u64 chunk_offset, u64 start,
+ u64 num_bytes)
 {
int ret;
struct btrfs_path *path;
@@ -1600,7 +1600,8 @@ static int btrfs_alloc_dev_extent(struct 
btrfs_trans_handle *trans,
extent = btrfs_item_ptr(leaf, path->slots[0],
struct btrfs_dev_extent);
btrfs_set_dev_extent_chunk_tree(leaf, extent, chunk_tree);
-   btrfs_set_dev_extent_chunk_objectid(leaf, extent, chunk_objectid);
+   btrfs_set_dev_extent_chunk_objectid(leaf, extent,
+   BTRFS_FIRST_CHUNK_TREE_OBJECTID);
btrfs_set_dev_extent_chunk_offset(leaf, extent, chunk_offset);
 
btrfs_set_dev_extent_length(leaf, extent, num_bytes);
@@ -4904,7 +4905,6 @@ int btrfs_finish_chunk_alloc(struct btrfs_trans_handle 
*trans,
break;
ret = btrfs_alloc_dev_extent(trans, device,
 chunk_root->root_key.objectid,
-BTRFS_FIRST_CHUNK_TREE_OBJECTID,
 chunk_offset, dev_offset,
 stripe_size);
if (ret)
-- 
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

[PATCH 06/10] btrfs: Remove unused parameter from check_direct_IO

[Nikolay Borisov]

Introduced by 5a5f79b57069 ("Btrfs: allow unaligned DIO") and never used, 

Signed-off-by: Nikolay Borisov 
---
 fs/btrfs/inode.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 61f1ad89e97a..f1b3d6146924 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8764,7 +8764,6 @@ static void btrfs_submit_direct(struct bio *dio_bio, 
struct inode *inode,
 }
 
 static ssize_t check_direct_IO(struct btrfs_fs_info *fs_info,
-  struct kiocb *iocb,
   const struct iov_iter *iter, loff_t offset)
 {
int seg;
@@ -8811,7 +8810,7 @@ static ssize_t btrfs_direct_IO(struct kiocb *iocb, struct 
iov_iter *iter)
bool relock = false;
ssize_t ret;
 
-   if (check_direct_IO(fs_info, iocb, iter, offset))
+   if (check_direct_IO(fs_info, iter, offset))
return 0;
 
inode_dio_begin(inode);
-- 
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

[PATCH 10/10] btrfs: Remove redundant argument of __link_block_group

[Nikolay Borisov]

__link_block_group is called from only 2 places and at each call site the
space_info being passed is the same as the space info assigned to the passed
cache struct. Let's remove the redundant argument and make the function
reference the space_info from the passed block_group_cache. No functional
changes

Signed-off-by: Nikolay Borisov 
---
 fs/btrfs/extent-tree.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 1a80f6e58296..6ca75dc62a3c 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -9886,9 +9886,9 @@ int btrfs_free_block_groups(struct btrfs_fs_info *info)
return 0;
 }
 
-static void __link_block_group(struct btrfs_space_info *space_info,
-  struct btrfs_block_group_cache *cache)
+static void __link_block_group(struct btrfs_block_group_cache *cache)
 {
+   struct btrfs_space_info *space_info = cache->space_info;
int index = get_block_group_index(cache);
bool first = false;
 
@@ -10096,7 +10096,7 @@ int btrfs_read_block_groups(struct btrfs_fs_info *info)
 
cache->space_info = space_info;
 
-   __link_block_group(space_info, cache);
+   __link_block_group(cache);
 
set_avail_alloc_bits(info, cache->flags);
if (btrfs_chunk_readonly(info, cache->key.objectid)) {
@@ -10255,7 +10255,7 @@ int btrfs_make_block_group(struct btrfs_trans_handle 
*trans,
cache->bytes_super, >space_info);
update_global_block_rsv(fs_info);
 
-   __link_block_group(cache->space_info, cache);
+   __link_block_group(cache);
 
list_add_tail(>bg_list, >new_bgs);
 
-- 
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

[PATCH 09/10] btrfs: Rework error handling of add_extent_mapping in __btrfs_alloc_chunk

[Nikolay Borisov]

Currently the code executes add_extent_mapping and if it is successful it links
the new mapping, it then proceeds to unlock the extent mapping tree and check
for failure and handle them. Instead, rework the code to only perform a single
check if add_extent_mapping has failed and handle it, otherwise the code
continues in a linear fashion. No functional changes

Signed-off-by: Nikolay Borisov 
---
 fs/btrfs/volumes.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index d024f1b07282..0324c1eec19d 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4813,16 +4813,16 @@ static int __btrfs_alloc_chunk(struct 
btrfs_trans_handle *trans,
em_tree = >mapping_tree.map_tree;
write_lock(_tree->lock);
ret = add_extent_mapping(em_tree, em, 0);
-   if (!ret) {
-   list_add_tail(>list, >transaction->pending_chunks);
-   refcount_inc(>refs);
-   }
-   write_unlock(_tree->lock);
if (ret) {
+   write_unlock(_tree->lock);
free_extent_map(em);
goto error;
}
 
+   list_add_tail(>list, >transaction->pending_chunks);
+   refcount_inc(>refs);
+   write_unlock(_tree->lock);
+
ret = btrfs_make_block_group(trans, info, 0, type, start, num_bytes);
if (ret)
goto error_del_extent;
-- 
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

[PATCH 08/10] btrfs: remove unused parameter in cow_file_range

[Nikolay Borisov]

The delalloc_end parameter was only passed to extent_clear_unlock_delalloc, but
since its usage was removed form that function let's also remove it from the
caller's parameter list.

Signed-off-by: Nikolay Borisov 
---
 fs/btrfs/inode.c | 20 +---
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 98b56aca0a6f..156be9face5b 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -105,9 +105,9 @@ static int btrfs_truncate(struct inode *inode);
 static int btrfs_finish_ordered_io(struct btrfs_ordered_extent 
*ordered_extent);
 static noinline int cow_file_range(struct inode *inode,
   struct page *locked_page,
-  u64 start, u64 end, u64 delalloc_end,
-  int *page_started, unsigned long *nr_written,
-  int unlock, struct btrfs_dedupe_hash *hash);
+  u64 start, u64 end, int *page_started,
+  unsigned long *nr_written, int unlock,
+  struct btrfs_dedupe_hash *hash);
 static struct extent_map *create_io_em(struct inode *inode, u64 start, u64 len,
   u64 orig_start, u64 block_start,
   u64 block_len, u64 orig_block_len,
@@ -739,8 +739,6 @@ static noinline void submit_compressed_extents(struct inode 
*inode,
 async_extent->start,
 async_extent->start +
 async_extent->ram_size - 1,
-async_extent->start +
-async_extent->ram_size - 1,
 _started, _written, 0,
 NULL);
 
@@ -930,9 +928,9 @@ static u64 get_extent_allocation_hint(struct inode *inode, 
u64 start,
  */
 static noinline int cow_file_range(struct inode *inode,
   struct page *locked_page,
-  u64 start, u64 end, u64 delalloc_end,
-  int *page_started, unsigned long *nr_written,
-  int unlock, struct btrfs_dedupe_hash *hash)
+  u64 start, u64 end, int *page_started,
+  unsigned long *nr_written, int unlock,
+  struct btrfs_dedupe_hash *hash)
 {
struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
struct btrfs_root *root = BTRFS_I(inode)->root;
@@ -1431,7 +1429,7 @@ static noinline int run_delalloc_nocow(struct inode 
*inode,
if (cow_start != (u64)-1) {
ret = cow_file_range(inode, locked_page,
 cow_start, found_key.offset - 1,
-end, page_started, nr_written, 1,
+page_started, nr_written, 1,
 NULL);
if (ret) {
if (!nolock && nocow)
@@ -1517,7 +1515,7 @@ static noinline int run_delalloc_nocow(struct inode 
*inode,
}
 
if (cow_start != (u64)-1) {
-   ret = cow_file_range(inode, locked_page, cow_start, end, end,
+   ret = cow_file_range(inode, locked_page, cow_start, end,
 page_started, nr_written, 1, NULL);
if (ret)
goto error;
@@ -1574,7 +1572,7 @@ static int run_delalloc_range(void *private_data, struct 
page *locked_page,
ret = run_delalloc_nocow(inode, locked_page, start, end,
 page_started, 0, nr_written);
} else if (!inode_need_compress(inode, start, end)) {
-   ret = cow_file_range(inode, locked_page, start, end, end,
+   ret = cow_file_range(inode, locked_page, start, end,
  page_started, nr_written, 1, NULL);
} else {
set_bit(BTRFS_INODE_HAS_ASYNC_EXTENT,
-- 
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

[PATCH 05/10] btrfs: Remove unused arguments from btrfs_changed_cb_t

[Nikolay Borisov]

btrfs_changed_cb_t represents the signature of the callback being passed to
btrfs_compare_trees. Currently there is only one such callback, namely
changed_cb in send.c. This function doesn't really uses the first 2 parameters,
i.e. the roots. Since there are not other functions implementing the
btrfs_changed_cb_t let's remove the unused parameters from the prototype and
implementation

Signed-off-by: Nikolay Borisov 
---
 fs/btrfs/ctree.c | 15 +--
 fs/btrfs/ctree.h |  4 +---
 fs/btrfs/send.c  |  8 +++-
 3 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 6d49db7d86be..19b9c5131745 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -5496,8 +5496,7 @@ int btrfs_compare_trees(struct btrfs_root *left_root,
goto out;
} else if (left_end_reached) {
if (right_level == 0) {
-   ret = changed_cb(left_root, right_root,
-   left_path, right_path,
+   ret = changed_cb(left_path, right_path,
_key,
BTRFS_COMPARE_TREE_DELETED,
ctx);
@@ -5508,8 +5507,7 @@ int btrfs_compare_trees(struct btrfs_root *left_root,
continue;
} else if (right_end_reached) {
if (left_level == 0) {
-   ret = changed_cb(left_root, right_root,
-   left_path, right_path,
+   ret = changed_cb(left_path, right_path,
_key,
BTRFS_COMPARE_TREE_NEW,
ctx);
@@ -5523,8 +5521,7 @@ int btrfs_compare_trees(struct btrfs_root *left_root,
if (left_level == 0 && right_level == 0) {
cmp = btrfs_comp_cpu_keys(_key, _key);
if (cmp < 0) {
-   ret = changed_cb(left_root, right_root,
-   left_path, right_path,
+   ret = changed_cb(left_path, right_path,
_key,
BTRFS_COMPARE_TREE_NEW,
ctx);
@@ -5532,8 +5529,7 @@ int btrfs_compare_trees(struct btrfs_root *left_root,
goto out;
advance_left = ADVANCE;
} else if (cmp > 0) {
-   ret = changed_cb(left_root, right_root,
-   left_path, right_path,
+   ret = changed_cb(left_path, right_path,
_key,
BTRFS_COMPARE_TREE_DELETED,
ctx);
@@ -5550,8 +5546,7 @@ int btrfs_compare_trees(struct btrfs_root *left_root,
result = BTRFS_COMPARE_TREE_CHANGED;
else
result = BTRFS_COMPARE_TREE_SAME;
-   ret = changed_cb(left_root, right_root,
-left_path, right_path,
+   ret = changed_cb(left_path, right_path,
 _key, result, ctx);
if (ret < 0)
goto out;
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index ca087ad5ac48..260d8464fac4 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2811,9 +2811,7 @@ enum btrfs_compare_tree_result {
BTRFS_COMPARE_TREE_CHANGED,
BTRFS_COMPARE_TREE_SAME,
 };
-typedef int (*btrfs_changed_cb_t)(struct btrfs_root *left_root,
- struct btrfs_root *right_root,
- struct btrfs_path *left_path,
+typedef int (*btrfs_changed_cb_t)(struct btrfs_path *left_path,
  struct btrfs_path *right_path,
  struct btrfs_key *key,
  enum btrfs_compare_tree_result result,
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 85a9509a9cbd..d7a8a7aed657 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -6169,9 +6169,7 @@ static int compare_refs(struct send_ctx *sctx, struct 
btrfs_path *path,
  * Updates compare related fields in sctx and simply forwards to the actual
  * changed_xxx functions.
  */
-static int changed_cb(struct btrfs_root *left_root,
- struct btrfs_root *right_root,
-   

[PATCH 04/10] btrfs: Remove unused parameters from various functions

[Nikolay Borisov]

iterate_dir_item:found_key - introduced in 31db9f7c23fb ("Btrfs: introduce 
BTRFS_IOC_SEND for btrfs send/receive"), yet never used. 

record_ref:num - ditto

This is a first pass with the low-hanging fruit. There are still quite a few
unsued parameters in some function which have to abide by a callback interface.

Signed-off-by: Nikolay Borisov 
---
 fs/btrfs/send.c | 25 +++--
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 8f1d3d6e7087..85a9509a9cbd 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -1002,7 +1002,6 @@ typedef int (*iterate_dir_item_t)(int num, struct 
btrfs_key *di_key,
  * path must point to the dir item when called.
  */
 static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
-   struct btrfs_key *found_key,
iterate_dir_item_t iterate, void *ctx)
 {
int ret = 0;
@@ -4116,8 +4115,8 @@ static int process_recorded_refs(struct send_ctx *sctx, 
int *pending_move)
return ret;
 }
 
-static int record_ref(struct btrfs_root *root, int num, u64 dir, int index,
- struct fs_path *name, void *ctx, struct list_head *refs)
+static int record_ref(struct btrfs_root *root, u64 dir, struct fs_path *name,
+ void *ctx, struct list_head *refs)
 {
int ret = 0;
struct send_ctx *sctx = ctx;
@@ -4153,8 +4152,7 @@ static int __record_new_ref(int num, u64 dir, int index,
void *ctx)
 {
struct send_ctx *sctx = ctx;
-   return record_ref(sctx->send_root, num, dir, index, name,
- ctx, >new_refs);
+   return record_ref(sctx->send_root, dir, name, ctx, >new_refs);
 }
 
 
@@ -4163,8 +4161,8 @@ static int __record_deleted_ref(int num, u64 dir, int 
index,
void *ctx)
 {
struct send_ctx *sctx = ctx;
-   return record_ref(sctx->parent_root, num, dir, index, name,
- ctx, >deleted_refs);
+   return record_ref(sctx->parent_root, dir, name, ctx,
+ >deleted_refs);
 }
 
 static int record_new_ref(struct send_ctx *sctx)
@@ -4508,7 +4506,7 @@ static int process_new_xattr(struct send_ctx *sctx)
int ret = 0;
 
ret = iterate_dir_item(sctx->send_root, sctx->left_path,
-  sctx->cmp_key, __process_new_xattr, sctx);
+  __process_new_xattr, sctx);
 
return ret;
 }
@@ -4516,7 +4514,7 @@ static int process_new_xattr(struct send_ctx *sctx)
 static int process_deleted_xattr(struct send_ctx *sctx)
 {
return iterate_dir_item(sctx->parent_root, sctx->right_path,
-   sctx->cmp_key, __process_deleted_xattr, sctx);
+   __process_deleted_xattr, sctx);
 }
 
 struct find_xattr_ctx {
@@ -4561,7 +4559,7 @@ static int find_xattr(struct btrfs_root *root,
ctx.found_data = NULL;
ctx.found_data_len = 0;
 
-   ret = iterate_dir_item(root, path, key, __find_xattr, );
+   ret = iterate_dir_item(root, path, __find_xattr, );
if (ret < 0)
return ret;
 
@@ -4631,11 +4629,11 @@ static int process_changed_xattr(struct send_ctx *sctx)
int ret = 0;
 
ret = iterate_dir_item(sctx->send_root, sctx->left_path,
-   sctx->cmp_key, __process_changed_new_xattr, sctx);
+   __process_changed_new_xattr, sctx);
if (ret < 0)
goto out;
ret = iterate_dir_item(sctx->parent_root, sctx->right_path,
-   sctx->cmp_key, __process_changed_deleted_xattr, sctx);
+   __process_changed_deleted_xattr, sctx);
 
 out:
return ret;
@@ -4685,8 +4683,7 @@ static int process_all_new_xattrs(struct send_ctx *sctx)
goto out;
}
 
-   ret = iterate_dir_item(root, path, _key,
-  __process_new_xattr, sctx);
+   ret = iterate_dir_item(root, path, __process_new_xattr, sctx);
if (ret < 0)
goto out;
 
-- 
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

[PATCH 07/10] btrfs: Remove unused parameter from extent_clear_unlock_delalloc

[Nikolay Borisov]

This variable was added as part of ba8b04c1d4ad ("btrfs: extend
btrfs_set_extent_delalloc and its friends to support in-band dedupe and subpage 
size patchset")
however those patchsets haven't materialized yet. Let's remove the argument and 
when the time comes (e.g. whiever patchset lands first) should introduce all of 
its pre-requisites in the same series. 

Signed-off-by: Nikolay Borisov 
---

Hello David, 

I was a ambivalent whether I should send this one since it removes some
preparatory work. However, the said patchsets haven't really moved anywhere for
quite some time and will likely require substantial amount of rebasing effort, 
so let's remove *possibly* outdated leftovers. But then again, I have no 
strong opinion on this. 

 fs/btrfs/extent_io.c |  3 +--
 fs/btrfs/extent_io.h |  2 +-
 fs/btrfs/inode.c | 24 
 3 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index d17783d70228..5eab66ec74f3 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1739,8 +1739,7 @@ static int __process_pages_contig(struct address_space 
*mapping,
 }
 
 void extent_clear_unlock_delalloc(struct inode *inode, u64 start, u64 end,
-u64 delalloc_end, struct page *locked_page,
-unsigned clear_bits,
+struct page *locked_page, unsigned clear_bits,
 unsigned long page_ops)
 {
clear_extent_bit(_I(inode)->io_tree, start, end, clear_bits, 1, 0,
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index faffa28ba707..ff71256f6fae 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -492,7 +492,7 @@ int map_private_extent_buffer(const struct extent_buffer 
*eb,
 void extent_range_clear_dirty_for_io(struct inode *inode, u64 start, u64 end);
 void extent_range_redirty_for_io(struct inode *inode, u64 start, u64 end);
 void extent_clear_unlock_delalloc(struct inode *inode, u64 start, u64 end,
-u64 delalloc_end, struct page *locked_page,
+struct page *locked_page,
 unsigned bits_to_clear,
 unsigned long page_ops);
 struct bio *btrfs_bio_alloc(struct block_device *bdev, u64 first_byte);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index f1b3d6146924..98b56aca0a6f 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -583,7 +583,7 @@ static noinline void compress_file_range(struct inode 
*inode,
 * we don't need to create any more async work items.
 * Unlock and free up our temp pages.
 */
-   extent_clear_unlock_delalloc(inode, start, end, end,
+   extent_clear_unlock_delalloc(inode, start, end,
 NULL, clear_flags,
 PAGE_UNLOCK |
 PAGE_CLEAR_DIRTY |
@@ -836,8 +836,6 @@ static noinline void submit_compressed_extents(struct inode 
*inode,
extent_clear_unlock_delalloc(inode, async_extent->start,
async_extent->start +
async_extent->ram_size - 1,
-   async_extent->start +
-   async_extent->ram_size - 1,
NULL, EXTENT_LOCKED | EXTENT_DELALLOC,
PAGE_UNLOCK | PAGE_CLEAR_DIRTY |
PAGE_SET_WRITEBACK);
@@ -856,7 +854,7 @@ static noinline void submit_compressed_extents(struct inode 
*inode,
tree->ops->writepage_end_io_hook(p, start, end,
 NULL, 0);
p->mapping = NULL;
-   extent_clear_unlock_delalloc(inode, start, end, end,
+   extent_clear_unlock_delalloc(inode, start, end,
 NULL, 0,
 PAGE_END_WRITEBACK |
 PAGE_SET_ERROR);
@@ -874,8 +872,6 @@ static noinline void submit_compressed_extents(struct inode 
*inode,
extent_clear_unlock_delalloc(inode, async_extent->start,
 async_extent->start +
 async_extent->ram_size - 1,
-async_extent->start +
-async_extent->ram_size - 1,
 NULL, EXTENT_LOCKED | EXTENT_DELALLOC |
 EXTENT_DELALLOC_NEW |
 EXTENT_DEFRAG | EXTENT_DO_ACCOUNTING,
@@ -970,8 +966,7 @@ static noinline int 

[PATCH 00/10] Unused parameter cleanup

[Nikolay Borisov]

Hello, 

Here is a series that I've been sitting on for a while. It removes unused 
parameter in various functions, no functional changes. Patch 09/10 reworks
some error handling to eliminate an if branch in __btrfs_alloc_chunk, but it's
still a minor refactoring. 

Nikolay Borisov (10):
  btrfs: Remove chunk_objectid parameter of btrfs_alloc_dev_extent
  btrfs: remove superfluous chunk_tree argument from
btrfs_alloc_dev_extent
  btrfs: Remove unused variable
  btrfs: Remove unused parameters from various functions
  btrfs: Remove unused arguments from btrfs_changed_cb_t
  btrfs: Remove unused parameter from check_direct_IO
  btrfs: Remove unused parameter from extent_clear_unlock_delalloc
  btrfs: remove unused parameter in cow_file_range
  btrfs: Rework error handling of add_extent_mapping in
__btrfs_alloc_chunk
  btrfs: Remove redundant argument of __link_block_group

 fs/btrfs/ctree.c   | 15 +--
 fs/btrfs/ctree.h   |  4 +---
 fs/btrfs/extent-tree.c |  8 
 fs/btrfs/extent_io.c   |  3 +--
 fs/btrfs/extent_io.h   |  2 +-
 fs/btrfs/inode.c   | 47 ++-
 fs/btrfs/send.c| 33 ++---
 fs/btrfs/tree-log.c|  4 +---
 fs/btrfs/volumes.c | 24 +++-
 9 files changed, 56 insertions(+), 84 deletions(-)

-- 
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

Re: [PATCH 3/4] btrfs: Improve eight size determinations

[Timofey Titovets]

You use that doc [1], so it's okay, because that's style are more safe.
But i don't think that at now such cleanups are really usefull at now.
Because that's not improve anything.

Reviewed-by: Timofey Titovets 

[1] - 
https://www.kernel.org/doc/html/v4.12/process/coding-style.html#allocating-memory

2017-08-20 23:19 GMT+03:00 SF Markus Elfring :
> From: Markus Elfring 
> Date: Sun, 20 Aug 2017 21:55:56 +0200
>
> Replace the specification of data types by pointer dereferences
> as the parameter for the operator "sizeof" to make the corresponding size
> determination a bit safer according to the Linux coding style convention.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring 
> ---
>  fs/btrfs/ctree.c | 10 --
>  fs/btrfs/delayed-inode.c |  4 ++--
>  fs/btrfs/raid56.c|  2 +-
>  fs/btrfs/super.c |  2 +-
>  4 files changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index b89c101ef45e..3b49f39eaaf6 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -578,7 +578,7 @@ tree_mod_log_insert_move(struct btrfs_fs_info *fs_info,
> if (!tree_mod_need_log(fs_info, eb))
> return 0;
>
> -   tm_list = kcalloc(nr_items, sizeof(struct tree_mod_elem *), GFP_NOFS);
> +   tm_list = kcalloc(nr_items, sizeof(*tm_list), GFP_NOFS);
> if (!tm_list)
> return -ENOMEM;
>
> @@ -677,8 +677,7 @@ tree_mod_log_insert_root(struct btrfs_fs_info *fs_info,
>
> if (log_removal && btrfs_header_level(old_root) > 0) {
> nritems = btrfs_header_nritems(old_root);
> -   tm_list = kcalloc(nritems, sizeof(struct tree_mod_elem *),
> - GFP_NOFS);
> +   tm_list = kcalloc(nritems, sizeof(*tm_list), GFP_NOFS);
> if (!tm_list) {
> ret = -ENOMEM;
> goto free_tms;
> @@ -813,8 +812,7 @@ tree_mod_log_eb_copy(struct btrfs_fs_info *fs_info, 
> struct extent_buffer *dst,
> if (btrfs_header_level(dst) == 0 && btrfs_header_level(src) == 0)
> return 0;
>
> -   tm_list = kcalloc(nr_items * 2, sizeof(struct tree_mod_elem *),
> - GFP_NOFS);
> +   tm_list = kcalloc(nr_items * 2, sizeof(*tm_list), GFP_NOFS);
> if (!tm_list)
> return -ENOMEM;
>
> @@ -904,7 +902,7 @@ tree_mod_log_free_eb(struct btrfs_fs_info *fs_info, 
> struct extent_buffer *eb)
> return 0;
>
> nritems = btrfs_header_nritems(eb);
> -   tm_list = kcalloc(nritems, sizeof(struct tree_mod_elem *), GFP_NOFS);
> +   tm_list = kcalloc(nritems, sizeof(*tm_list), GFP_NOFS);
> if (!tm_list)
> return -ENOMEM;
>
> diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> index 19e4ad2f3f2e..865aaca445b9 100644
> --- a/fs/btrfs/delayed-inode.c
> +++ b/fs/btrfs/delayed-inode.c
> @@ -750,13 +750,13 @@ static int btrfs_batch_insert_items(struct btrfs_root 
> *root,
>  */
> btrfs_set_path_blocking(path);
>
> -   keys = kmalloc_array(nitems, sizeof(struct btrfs_key), GFP_NOFS);
> +   keys = kmalloc_array(nitems, sizeof(*keys), GFP_NOFS);
> if (!keys) {
> ret = -ENOMEM;
> goto out;
> }
>
> -   data_size = kmalloc_array(nitems, sizeof(u32), GFP_NOFS);
> +   data_size = kmalloc_array(nitems, sizeof(*data_size), GFP_NOFS);
> if (!data_size) {
> ret = -ENOMEM;
> goto error;
> diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
> index 208638384cd2..c85f0f534532 100644
> --- a/fs/btrfs/raid56.c
> +++ b/fs/btrfs/raid56.c
> @@ -1798,7 +1798,7 @@ static void __raid_recover_end_io(struct btrfs_raid_bio 
> *rbio)
> int err;
> int i;
>
> -   pointers = kcalloc(rbio->real_stripes, sizeof(void *), GFP_NOFS);
> +   pointers = kcalloc(rbio->real_stripes, sizeof(*pointers), GFP_NOFS);
> if (!pointers) {
> err = -ENOMEM;
> goto cleanup_io;
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 0b7a1d8cd08b..cd2ba5e3b2d0 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -1571,7 +1571,7 @@ static struct dentry *btrfs_mount(struct 
> file_system_type *fs_type, int flags,
>  * it for searching for existing supers, so this lets us do that and
>  * then open_ctree will properly initialize everything later.
>  */
> -   fs_info = kzalloc(sizeof(struct btrfs_fs_info), GFP_KERNEL);
> +   fs_info = kzalloc(sizeof(*fs_info), GFP_KERNEL);
> if (!fs_info) {
> error = -ENOMEM;
> goto error_sec_opts;
> --
> 2.14.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of 

[PATCH] Btrfs-progs: Check root before printing item

[zhangyu-fnst]

From: Zhang Yu 

[TEST/fuzz] case: 004-simple-dump-tree

Since the wrong key(DATA_RELOC_TREE CHUNK_ITEM 0) in root tree,
error calling print_chunk(), resulting in num_stripes == 0.

ERROR:
 [TEST/fuzz]   004-simple-dump-tree
ctree.h:317: btrfs_chunk_item_size: BUG_ON `num_stripes == 0`
triggered, value 1

failed (ignored, ret=134): /myproject/btrfs-progs/btrfs
inspect-internal dump-tree
/myproject/btrfs-progs/tests/fuzz-tests/images/
bko-155201-wrong-chunk-item-in-root-tree.raw.restored

test failed for case 004-simple-dump-tree
Makefile:288: recipe for target 'test-fuzz' failed
make: *** [test-fuzz] Error 1

So, before printing item, determine the root is valid or not.

Signed-off-by: Zhang Yu 
---
 print-tree.c | 57 +
 1 file changed, 57 insertions(+)

diff --git a/print-tree.c b/print-tree.c
index a0d3395..79bb9f4 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -1028,9 +1028,15 @@ void btrfs_print_leaf(struct btrfs_root *root, struct 
extent_buffer *eb)
 
switch (type) {
case BTRFS_INODE_ITEM_KEY:
+   if (root != root->fs_info->tree_root ||
+   !is_fstree(objectid))
+   break;
print_inode_item(eb, ptr);
break;
case BTRFS_INODE_REF_KEY:
+   if (root != root->fs_info->tree_root ||
+   !is_fstree(objectid))
+   break;
print_inode_ref_item(eb, item_size, ptr);
break;
case BTRFS_INODE_EXTREF_KEY:
@@ -1039,10 +1045,14 @@ void btrfs_print_leaf(struct btrfs_root *root, struct 
extent_buffer *eb)
case BTRFS_DIR_ITEM_KEY:
case BTRFS_DIR_INDEX_KEY:
case BTRFS_XATTR_ITEM_KEY:
+   if (!is_fstree(objectid))
+   break;
print_dir_item(eb, item_size, ptr);
break;
case BTRFS_DIR_LOG_INDEX_KEY:
case BTRFS_DIR_LOG_ITEM_KEY: {
+   if (root != root->fs_info->log_root_tree)
+   break;
struct btrfs_dir_log_item *dlog;
 
dlog = btrfs_item_ptr(eb, i, struct btrfs_dir_log_item);
@@ -1051,30 +1061,48 @@ void btrfs_print_leaf(struct btrfs_root *root, struct 
extent_buffer *eb)
break;
}
case BTRFS_ORPHAN_ITEM_KEY:
+   if (root != root->fs_info->tree_root)
+   break;
printf("\t\torphan item\n");
break;
case BTRFS_ROOT_ITEM_KEY:
+   if (root != root->fs_info->tree_root)
+   break;
print_root(eb, i);
break;
case BTRFS_ROOT_REF_KEY:
+   if (root != root->fs_info->tree_root)
+   break;
print_root_ref(eb, i, "ref");
break;
case BTRFS_ROOT_BACKREF_KEY:
+   if (root != root->fs_info->tree_root)
+   break;
print_root_ref(eb, i, "backref");
break;
case BTRFS_EXTENT_ITEM_KEY:
+   if (root != root->fs_info->extent_root)
+   break;
print_extent_item(eb, i, 0);
break;
case BTRFS_METADATA_ITEM_KEY:
+   if (root != root->fs_info->extent_root)
+   break;
print_extent_item(eb, i, 1);
break;
case BTRFS_TREE_BLOCK_REF_KEY:
+   if (root != root->fs_info->extent_root)
+   break;
printf("\t\ttree block backref\n");
break;
case BTRFS_SHARED_BLOCK_REF_KEY:
+   if (root != root->fs_info->extent_root)
+   break;
printf("\t\tshared block backref\n");
break;
case BTRFS_EXTENT_DATA_REF_KEY: {
+   if (root != root->fs_info->extent_root)
+   break;
struct btrfs_extent_data_ref *dref;
 
dref = btrfs_item_ptr(eb, i, struct 
btrfs_extent_data_ref);
@@ -1087,7 +1115,10 @@ void btrfs_print_leaf(struct btrfs_root *root, struct 
extent_buffer *eb)
break;
}
case 

Re: Btrfs Raid5 issue.

[Qu Wenruo]

On 2017年08月21日 12:33, Robert LeBlanc wrote:

I've been running btrfs in a raid5 for about a year now with bcache in
front of it. Yesterday, one of my drives was acting really slow, so I
was going to move it to a different port. I guess I get too
comfortable hot plugging drives in at work and didn't think twice
about what could go wrong, hey I set it up in RAID5 so it will be
fine. Well, it wasn't...


Well, Btrfs RAID5 is not that safe.
I would recommend to use RAID1 for metadata at least.
(And in your case, your metadata is damaged, so I really recommend to 
use a better profile for your metadata)




I was aware of the write hole issue, and thought it was committed to
the 4.12 branch, so I was running 4.12.5 at the time. I have two SSDs
that are in an md RAID1 that is the cache for the three backing
devices in bcache (bcache{0..2} or bcache{0,16,32} depending on the
kernel booted. I have all my critical data saved off on btrfs
snapshots on a different host, but I don't transfer my MythTV subs
that often, so I'd like to try to recover some of that if possible.

What is really interesting is that I could not boot the first time
(root on the btrfs volume), but I rebooted again and the fs was in
read-only mode, but only one of the three disks was in read-only. I
tried to reboot again and it never mounted again after that. I see
some messages in dmesg like this:

[  151.201637] BTRFS info (device bcache0): disk space caching is enabled
[  151.201640] BTRFS info (device bcache0): has skinny extents
[  151.215697] BTRFS info (device bcache0): bdev /dev/bcache16 errs:
wr 309, rd 319, flush 39, corrupt 0, gen 0
[  151.931764] BTRFS info (device bcache0): detected SSD devices,
enabling SSD mode
[  152.058915] BTRFS error (device bcache0): parent transid verify
failed on 5309837426688 wanted 1620383 found 1619473
[  152.059944] BTRFS error (device bcache0): parent transid verify
failed on 5309837426688 wanted 1620383 found 1619473


Normally transid error indicates bigger problem, and normally hard to trace.


[  152.060018] BTRFS: error (device bcache0) in
__btrfs_free_extent:6989: errno=-5 IO failure
[  152.060060] BTRFS: error (device bcache0) in
btrfs_run_delayed_refs:3009: errno=-5 IO failure
[  152.071613] BTRFS info (device bcache0): delayed_refs has NO entry
[  152.074126] BTRFS: error (device bcache0) in btrfs_replay_log:2475:
errno=-5 IO failure (Failed to recover log tree)
[  152.074244] BTRFS error (device bcache0): cleaner transaction
attach returned -30
[  152.148993] BTRFS error (device bcache0): open_ctree failed

So, I thought that the log was corrupted, I could live without the
last 30 seconds or so, I tried `btrfs rescue zero-log /dev/bcache0`
and I get a backtrace.


Yes, your idea about log is correct. It's log replay causing problem.
But the root cause seems to be corrupted extent tree, which is not easy 
to fix.



I ran `btrfs rescue chunk-recover /dev/bcache0`
and it spent hours scanning the three disks and at the end tried to
fix the logs (or tree, I can't remember exactly) and then I got
another backtrace.

Today, I compiled 4.13-rc6 to see if some of the latest fixes would
help, no dice (the dmesg above is from 4.13-rc6). I compiled the
latest master of btrfs-progs, no progress.

Things I've tried:
mount
mount -o degraded
mount -o degraded,ro
mount -o degraded (with each drive disconnected in turn to see if in
would start without one of the drives)
btrfs rescue chunk-recover
btrfs rescue super-recover (all drives report the superblocks are fine)
btrfs rescue zero-log (always has a backtrace)


I think that's some other problem causing the backtrace.
Normally extent tree corruption or transid error.


btrfs check

I know that bcache complicates things, but I'm hoping for two things.
1. Try to get what I can off the volume. 2. Provide some information
that can help make btrfs/bcache better for the future.

Here is what `btrfs rescue zero-log` outputs:

# ./btrfs rescue zero-log /dev/bcache0
Clearing log on /dev/bcache0, previous log_root 2876047507456, level 0
parent transid verify failed on 5309233872896 wanted 1620381 found 1619462
parent transid verify failed on 5309233872896 wanted 1620381 found 1619462
checksum verify failed on 5309233872896 found 6A103358 wanted 8EF38EEE
checksum verify failed on 5309233872896 found 6A103358 wanted 8EF38EEE
bytenr mismatch, want=5309233872896, have=65536
parent transid verify failed on 5309233872896 wanted 1620381 found 1619462
parent transid verify failed on 5309233872896 wanted 1620381 found 1619462
checksum verify failed on 5309233872896 found 6A103358 wanted 8EF38EEE
checksum verify failed on 5309233872896 found 6A103358 wanted 8EF38EEE
bytenr mismatch, want=5309233872896, have=65536
parent transid verify failed on 5309233872896 wanted 1620381 found 1619462
parent transid verify failed on 5309233872896 wanted 1620381 found 1619462
checksum verify failed on 5309233872896 found 6A103358 wanted 8EF38EEE
checksum verify failed on 5309233872896 

[PATCH] btrfs-progs: Doc: Fix asciidoc grammar of btrfs-rescue

[Qu Wenruo]

Code block of kernel backtrace lacks leading change line, causing the
following man page result:
--
   One can determine whether zero-log is needed according to the
   kernel backtrace:

   ? replay_one_dir_item+0xb5/0xb5 [btrfs]
   ? walk_log_tree+0x9c/0x19d [btrfs]
   ? btrfs_read_fs_root_no_radix+0x169/0x1a1 [btrfs]
   ? btrfs_recover_log_trees+0x195/0x29c [btrfs]
   ? replay_one_dir_item+0xb5/0xb5 [btrfs]
   ? btree_read_extent_buffer_pages+0x76/0xbc [btrfs]
   ? open_ctree+0xff6/0x132c [btrfs]

   + If the errors are like above, then zero-log should be used to clear
   the log and the filesystem may be mounted normally again. The keywords
--

Not only "+" is rendered as is, but also wrong indent.

Fix it by adding change line before code block.

Signed-off-by: Qu Wenruo 
---
 Documentation/btrfs-rescue.asciidoc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/btrfs-rescue.asciidoc 
b/Documentation/btrfs-rescue.asciidoc
index a9b471fe..24b619c6 100644
--- a/Documentation/btrfs-rescue.asciidoc
+++ b/Documentation/btrfs-rescue.asciidoc
@@ -57,6 +57,7 @@ or less if the commit was implied by other filesystem 
activity.
 +
 One can determine whether *zero-log* is needed according to the kernel
 backtrace:
++
 
 ? replay_one_dir_item+0xb5/0xb5 [btrfs]
 ? walk_log_tree+0x9c/0x19d [btrfs]
-- 
2.14.1

--
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