date:20180626

[PATCH v4 3/5] btrfs: remove unnecessary -ERANGE check in btrfs_get_acl()

2018-06-26 Thread Chengguang Xu

There is no chance to get into -ERANGE error condition because
we first call btrfs_getxattr to get the length of the attribute,
then we do a subsequent call with the size from the first call.
Between the 2 calls the size shouldn't change. So remove the
unnecessary -ERANGE error check.

Signed-off-by: Chengguang Xu 
Reviewed-by: Nikolay Borisov 
---
v4:
- Split patch to series.

v3:
- Fix some toher bad practices.
- Add more information to commit log.

v2:
- Avoid errno overriding instead of print error message in error case.
- Chagne commit log for better understanding.

 fs/btrfs/acl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
index 83fdd80c51c6..a1d7211c8884 100644
--- a/fs/btrfs/acl.c
+++ b/fs/btrfs/acl.c
@@ -42,7 +42,7 @@ struct posix_acl *btrfs_get_acl(struct inode *inode, int type)
}
if (size > 0) {
acl = posix_acl_from_xattr(_user_ns, value, size);
-   } else if (size == -ERANGE || size == -ENODATA || size == 0) {
+   } else if (size == -ENODATA || size == 0) {
acl = NULL;
} else {
acl = ERR_PTR(-EIO);
-- 
2.17.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

[PATCH v4 5/5] btrfs: remove unnecessary bracket in btrfs_get_acl()

2018-06-26 Thread Chengguang Xu

It's only coding style fix not functinal change.
When if/else has only one statement then the bracket
is not needed.

Signed-off-by: Chengguang Xu 
Reviewed-by: Nikolay Borisov 
---
v4:
- Split patch to series.

v3:
- Fix some toher bad practices.
- Add more information to commit log.

v2:
- Avoid errno overriding instead of print error message in error case.
- Chagne commit log for better understanding.

 fs/btrfs/acl.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
index 7d673ec9e54a..3b66c957ea6f 100644
--- a/fs/btrfs/acl.c
+++ b/fs/btrfs/acl.c
@@ -40,13 +40,12 @@ struct posix_acl *btrfs_get_acl(struct inode *inode, int 
type)
return ERR_PTR(-ENOMEM);
size = btrfs_getxattr(inode, name, value, size);
}
-   if (size > 0) {
+   if (size > 0)
acl = posix_acl_from_xattr(_user_ns, value, size);
-   } else if (size == -ENODATA || size == 0) {
+   else if (size == -ENODATA || size == 0)
acl = NULL;
-   } else {
+   else
acl = ERR_PTR(size);
-   }
kfree(value);
 
return acl;
-- 
2.17.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

[PATCH v4 4/5] btrfs: avoid error code overriding in btrfs_get_acl()

2018-06-26 Thread Chengguang Xu

It's no good to override error code when failing from
btrfs_getxattr() in btrfs_get_acl() because it will hide
real root cause.

Signed-off-by: Chengguang Xu 
Reviewed-by: Nikolay Borisov 
---
v4:
- Split patch to series.

v3:
- Fix some toher bad practices.
- Add more information to commit log.

v2:
- Avoid errno overriding instead of print error message in error case.
- Chagne commit log for better understanding.

 fs/btrfs/acl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
index a1d7211c8884..7d673ec9e54a 100644
--- a/fs/btrfs/acl.c
+++ b/fs/btrfs/acl.c
@@ -45,7 +45,7 @@ struct posix_acl *btrfs_get_acl(struct inode *inode, int type)
} else if (size == -ENODATA || size == 0) {
acl = NULL;
} else {
-   acl = ERR_PTR(-EIO);
+   acl = ERR_PTR(size);
}
kfree(value);
 
-- 
2.17.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

[PATCH v4 1/5] btrfs: return error instead of crash when detecting unexpected type in btrfs_get_acl()

2018-06-26 Thread Chengguang Xu

The caller of btrfs_get_acl() checks error condition so there is no
impact from this change. In practice there is no chance to get into
default case of switch statement because VFS has already checked
the type.

Signed-off-by: Chengguang Xu 
Reviewed-by: Nikolay Borisov 
---
v4:
- Split patch to series.

v3:
- Fix some toher bad practices.
- Add more information to commit log.

v2:
- Avoid errno overriding instead of print error message in error case.
- Chagne commit log for better understanding.

 fs/btrfs/acl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
index 15e1dfef56a5..60f83a3bd77c 100644
--- a/fs/btrfs/acl.c
+++ b/fs/btrfs/acl.c
@@ -30,7 +30,7 @@ struct posix_acl *btrfs_get_acl(struct inode *inode, int type)
name = XATTR_NAME_POSIX_ACL_DEFAULT;
break;
default:
-   BUG();
+   return ERR_PTR(-EINVAL);
}
 
size = btrfs_getxattr(inode, name, "", 0);
-- 
2.17.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

[PATCH v4 2/5] btrfs: replace empty string with NULL when getting attribute length in btrfs_get_acl()

2018-06-26 Thread Chengguang Xu

In btrfs_get_acl() the first call of btr_getxattr() is for getting
the length of attribute, the value buffer is never used in this
case. So it's better to replace empty string with NULL.

Signed-off-by: Chengguang Xu 
Reviewed-by: Nikolay Borisov 
---
v4:
- Split patch to series.

v3:
- Fix some toher bad practices.
- Add more information to commit log.

v2:
- Avoid errno overriding instead of print error message in error case.
- Chagne commit log for better understanding.

 fs/btrfs/acl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
index 60f83a3bd77c..83fdd80c51c6 100644
--- a/fs/btrfs/acl.c
+++ b/fs/btrfs/acl.c
@@ -33,7 +33,7 @@ struct posix_acl *btrfs_get_acl(struct inode *inode, int type)
return ERR_PTR(-EINVAL);
}
 
-   size = btrfs_getxattr(inode, name, "", 0);
+   size = btrfs_getxattr(inode, name, NULL, 0);
if (size > 0) {
value = kzalloc(size, GFP_KERNEL);
if (!value)
-- 
2.17.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

[PATCH v4 0/5] code cleanups for btrfs_get_acl()

2018-06-26 Thread Chengguang Xu

This patch set does code cleanups for btrfs_get_acl().

Chengguang Xu (5):
  btrfs: return error instead of crash when detecting unexpected type in
btrfs_get_acl()
  btrfs: replace empty string with NULL when getting attribute length in
btrfs_get_acl()
  btrfs: remove unnecessary -ERANGE check in btrfs_get_acl()
  btrfs: avoid error code overriding in btrfs_get_acl()
  btrfs: remove unnecessary bracket in btrfs_get_acl()

 fs/btrfs/acl.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

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

Re: Mailing list archives links update

2018-06-26 Thread Lu Fengqi

On Tue, Jun 26, 2018 at 09:25:22PM +0300, Eugene Bright wrote:
>Greetings!
>
>I've found one more mailing list archive.
>I do not have wiki access, so could someone else add one more link?

I have already added this link for you.

-- 
Thanks,
Lu

>
>Wiki page:
>https://btrfs.wiki.kernel.org/index.php/Btrfs_mailing_list
>
>Additional archive link:
>https://www.spinics.net/lists/linux-btrfs/
>
>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

Mailing list archives links update

2018-06-26 Thread Eugene Bright

Greetings!

I've found one more mailing list archive.
I do not have wiki access, so could someone else add one more link?

Wiki page:
https://btrfs.wiki.kernel.org/index.php/Btrfs_mailing_list

Additional archive link:
https://www.spinics.net/lists/linux-btrfs/

Thanks!

signature.asc
Description: This is a digitally signed message part.

Re: [PATCH v2] btrfs: Add graceful handling of V0 extents

2018-06-26 Thread kbuild test robot

Hi Nikolay,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20180626]
[cannot apply to btrfs/next v4.18-rc2 v4.18-rc1 v4.17 v4.18-rc2]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Nikolay-Borisov/btrfs-Add-graceful-handling-of-V0-extents/20180626-231445
config: i386-randconfig-s1-201825 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   fs/btrfs/extent-tree.c: In function 'btrfs_lookup_extent_info':
>> fs/btrfs/extent-tree.c:871:4: error: implicit declaration of function 
>> 'btrfs_print_v0_err' [-Werror=implicit-function-declaration]
   btrfs_print_v0_err(fs_info);
   ^~
   fs/btrfs/extent-tree.c: In function 'remove_extent_data_ref':
>> fs/btrfs/extent-tree.c:1308:22: error: 'fs_info' undeclared (first use in 
>> this function)
  btrfs_print_v0_err(fs_info);
 ^~~
   fs/btrfs/extent-tree.c:1308:22: note: each undeclared identifier is reported 
only once for each function it appears in
   cc1: some warnings being treated as errors
--
   fs/btrfs/print-tree.c: In function 'print_extent_item':
>> fs/btrfs/print-tree.c:56:3: error: implicit declaration of function 
>> 'btrfs_print_v0_err' [-Werror=implicit-function-declaration]
  btrfs_print_v0_err(eb->fs_info);
  ^~
   cc1: some warnings being treated as errors
--
   fs/btrfs/relocation.c: In function 'find_inline_backref':
>> fs/btrfs/relocation.c:602:3: error: implicit declaration of function 
>> 'btrfs_print_v0_err' [-Werror=implicit-function-declaration]
  btrfs_print_v0_err(leaf->fs_info);
  ^~
   cc1: some warnings being treated as errors

vim +/btrfs_print_v0_err +871 fs/btrfs/extent-tree.c

   794  
   795  /*
   796   * helper function to lookup reference count and flags of a tree block.
   797   *
   798   * the head node for delayed ref is used to store the sum of all the
   799   * reference count modifications queued up in the rbtree. the head
   800   * node may also store the extent flags to set. This way you can check
   801   * to see what the reference count and extent flags would be if all of
   802   * the delayed refs are not processed.
   803   */
   804  int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
   805   struct btrfs_fs_info *fs_info, u64 bytenr,
   806   u64 offset, int metadata, u64 *refs, u64 
*flags)
   807  {
   808  struct btrfs_delayed_ref_head *head;
   809  struct btrfs_delayed_ref_root *delayed_refs;
   810  struct btrfs_path *path;
   811  struct btrfs_extent_item *ei;
   812  struct extent_buffer *leaf;
   813  struct btrfs_key key;
   814  u32 item_size;
   815  u64 num_refs;
   816  u64 extent_flags;
   817  int ret;
   818  
   819  /*
   820   * If we don't have skinny metadata, don't bother doing anything
   821   * different
   822   */
   823  if (metadata && !btrfs_fs_incompat(fs_info, SKINNY_METADATA)) {
   824  offset = fs_info->nodesize;
   825  metadata = 0;
   826  }
   827  
   828  path = btrfs_alloc_path();
   829  if (!path)
   830  return -ENOMEM;
   831  
   832  if (!trans) {
   833  path->skip_locking = 1;
   834  path->search_commit_root = 1;
   835  }
   836  
   837  search_again:
   838  key.objectid = bytenr;
   839  key.offset = offset;
   840  if (metadata)
   841  key.type = BTRFS_METADATA_ITEM_KEY;
   842  else
   843  key.type = BTRFS_EXTENT_ITEM_KEY;
   844  
   845  ret = btrfs_search_slot(trans, fs_info->extent_root, , 
path, 0, 0);
   846  if (ret < 0)
   847  goto out_free;
   848  
   849  if (ret > 0 && metadata && key.type == BTRFS_METADATA_ITEM_KEY) 
{
   850  if (path->slots[0]) {
   851  path->slots[0]--;
   852  btrfs_item_key_to_cpu(path->nodes[0], ,
   853path->slots[0]);
   854  if (key.objectid == bytenr &&
   855  key.type == BTRFS_EXTENT_ITEM_KEY &&
   856  key.offset == fs_info->nodesize)
   857  ret = 0;
   858  }
   859  }
   860  
   861  if (ret == 0) {
   862  leaf

Re: [PATCH v2] Btrfs: fix regression in btrfs_page_mkwrite() from vm_fault_t conversion

2018-06-26 Thread David Sterba

On Mon, Jun 25, 2018 at 10:03:41AM -0700, Chris Mason wrote:
> The vm_fault_t conversion commit introduced a ret2 variable for tracking
> the integer return values from internal btrfs functions.  It was
> sometimes returning VM_FAULT_LOCKED for pages that were actually invalid
> and had been removed from the radix.  Something like this:
> 
> ret2 = btrfs_delalloc_reserve_space() // returns zero on success
> 
> lock_page(page)
> if (page->mapping != inode->i_mapping)
>   goto out_unlock;
> 
> ...
> 
> out_unlock:
> if (!ret2) {
>   ...
>   return VM_FAULT_LOCKED;
> }
> 
> This ends up triggering this WARNING in btrfs_destroy_inode()
> WARN_ON(BTRFS_I(inode)->block_rsv.size);
> 
> xfstests generic/095 was able to reliably reproduce the errors.
> 
> Since out_unlock: is only used for errors, this fix moves it below the
> if (!ret2) check we use to return VM_FAULT_LOCKED for success.
> 
> Fixes: a528a2415087 (btrfs: change return type of btrfs_page_mkwrite to 
> vm_fault_t)
> Signed-off-by: Chris Mason 

Reviewed-by: David Sterba 

also passed the tests here standalone and with the fixup worker fixes.
--
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: Add graceful handling of V0 extents

2018-06-26 Thread David Sterba

On Wed, Jun 27, 2018 at 12:05:39AM +0800, kbuild test robot wrote:
> Hi Nikolay,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on next-20180626]
> [cannot apply to btrfs/next v4.18-rc2 v4.18-rc1 v4.17 v4.18-rc2]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Nikolay-Borisov/btrfs-Add-graceful-handling-of-V0-extents/20180626-231445
> config: x86_64-randconfig-x015-201825 (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64 
> 
> All errors (new ones prefixed by >>):
> 
>fs//btrfs/extent-tree.c: In function 'btrfs_lookup_extent_info':
> >> fs//btrfs/extent-tree.c:871:4: error: implicit declaration of function 
> >> 'btrfs_print_v0_err'; did you mean 'btrfs_print_tree'? 
> >> [-Werror=implicit-function-declaration]
>btrfs_print_v0_err(fs_info);
>^~

Fixed by using trans->fs_info in the original patch so the followup that
removes 'fs_info' does not break compilation. Updated for-next branch
pushed.
--
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] btrfs: code cleanups for btrfs_get_acl()

2018-06-26 Thread David Sterba

On Tue, Jun 26, 2018 at 02:08:27PM +0800, Chengguang Xu wrote:
> There is no chance to get into -ERANGE error condition because
> we first call btrfs_getxattr to get the length of the attribute,
> then we do a subsequent call with the size from the first call.
> Between the 2 calls the size shouldn't change. So remove the
> unnecessary -ERANGE error check and meanwhile fix some other
> bad practices as well.
> 
> Detail fixes in this patch.
> - return ERR_PTR(-EINVAL) instead of crashing kernel in default
> case of switch.
> - return original error code when failing from btrfs_getxattr().
> - remove unnecessary bracket.
> - remove unnecessary -ERANGE check.

Hm, that should be really 3 patches, but Nikolay is to blame as he
should have mentioned that when asking for the other things to fix.

> Signed-off-by: Chengguang Xu 
> ---
> v3:
> - Fix some other bad practices.
> - Add more information to commit log.
> 
> v2:
> - Avoid errno overriding instead of print error message in error case.
> - Change commit log for better understanding.
> 
>  fs/btrfs/acl.c | 13 ++---
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
> index 15e1dfef56a5..3b66c957ea6f 100644
> --- a/fs/btrfs/acl.c
> +++ b/fs/btrfs/acl.c
> @@ -30,23 +30,22 @@ struct posix_acl *btrfs_get_acl(struct inode *inode, int 
> type)
>   name = XATTR_NAME_POSIX_ACL_DEFAULT;
>   break;
>   default:
> - BUG();
> + return ERR_PTR(-EINVAL);

First patch

>   }
>  
> - size = btrfs_getxattr(inode, name, "", 0);
> + size = btrfs_getxattr(inode, name, NULL, 0);

Second patch

>   if (size > 0) {
>   value = kzalloc(size, GFP_KERNEL);
>   if (!value)
>   return ERR_PTR(-ENOMEM);
>   size = btrfs_getxattr(inode, name, value, size);
>   }
> - if (size > 0) {
> + if (size > 0)
>   acl = posix_acl_from_xattr(_user_ns, value, size);
> - } else if (size == -ERANGE || size == -ENODATA || size == 0) {
> + else if (size == -ENODATA || size == 0)
>   acl = NULL;
> - } else {
> - acl = ERR_PTR(-EIO);
> - }
> + else
> + acl = ERR_PTR(size);

And third patch.

>From current changelog only the 3rd is covered, otherwise 1st and 2nd
are under the "and other bad things", but this deserves a more specific
explanation.

So please split and resend. 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 v2] btrfs: Add graceful handling of V0 extents

2018-06-26 Thread kbuild test robot

Hi Nikolay,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20180626]
[cannot apply to btrfs/next v4.18-rc2 v4.18-rc1 v4.17 v4.18-rc2]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Nikolay-Borisov/btrfs-Add-graceful-handling-of-V0-extents/20180626-231445
config: x86_64-randconfig-x015-201825 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   fs//btrfs/extent-tree.c: In function 'btrfs_lookup_extent_info':
>> fs//btrfs/extent-tree.c:871:4: error: implicit declaration of function 
>> 'btrfs_print_v0_err'; did you mean 'btrfs_print_tree'? 
>> [-Werror=implicit-function-declaration]
   btrfs_print_v0_err(fs_info);
   ^~
   btrfs_print_tree
   fs//btrfs/extent-tree.c: In function 'remove_extent_data_ref':
>> fs//btrfs/extent-tree.c:1308:22: error: 'fs_info' undeclared (first use in 
>> this function); did you mean 'qc_info'?
  btrfs_print_v0_err(fs_info);
 ^~~
 qc_info
   fs//btrfs/extent-tree.c:1308:22: note: each undeclared identifier is 
reported only once for each function it appears in
   cc1: some warnings being treated as errors
--
   fs//btrfs/print-tree.c: In function 'print_extent_item':
>> fs//btrfs/print-tree.c:56:3: error: implicit declaration of function 
>> 'btrfs_print_v0_err'; did you mean 'btrfs_print_tree'? 
>> [-Werror=implicit-function-declaration]
  btrfs_print_v0_err(eb->fs_info);
  ^~
  btrfs_print_tree
   cc1: some warnings being treated as errors
--
   fs//btrfs/relocation.c: In function 'find_inline_backref':
>> fs//btrfs/relocation.c:602:3: error: implicit declaration of function 
>> 'btrfs_print_v0_err'; did you mean 'btrfs_print_tree'? 
>> [-Werror=implicit-function-declaration]
  btrfs_print_v0_err(leaf->fs_info);
  ^~
  btrfs_print_tree
   cc1: some warnings being treated as errors

vim +871 fs//btrfs/extent-tree.c

   794  
   795  /*
   796   * helper function to lookup reference count and flags of a tree block.
   797   *
   798   * the head node for delayed ref is used to store the sum of all the
   799   * reference count modifications queued up in the rbtree. the head
   800   * node may also store the extent flags to set. This way you can check
   801   * to see what the reference count and extent flags would be if all of
   802   * the delayed refs are not processed.
   803   */
   804  int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
   805   struct btrfs_fs_info *fs_info, u64 bytenr,
   806   u64 offset, int metadata, u64 *refs, u64 
*flags)
   807  {
   808  struct btrfs_delayed_ref_head *head;
   809  struct btrfs_delayed_ref_root *delayed_refs;
   810  struct btrfs_path *path;
   811  struct btrfs_extent_item *ei;
   812  struct extent_buffer *leaf;
   813  struct btrfs_key key;
   814  u32 item_size;
   815  u64 num_refs;
   816  u64 extent_flags;
   817  int ret;
   818  
   819  /*
   820   * If we don't have skinny metadata, don't bother doing anything
   821   * different
   822   */
   823  if (metadata && !btrfs_fs_incompat(fs_info, SKINNY_METADATA)) {
   824  offset = fs_info->nodesize;
   825  metadata = 0;
   826  }
   827  
   828  path = btrfs_alloc_path();
   829  if (!path)
   830  return -ENOMEM;
   831  
   832  if (!trans) {
   833  path->skip_locking = 1;
   834  path->search_commit_root = 1;
   835  }
   836  
   837  search_again:
   838  key.objectid = bytenr;
   839  key.offset = offset;
   840  if (metadata)
   841  key.type = BTRFS_METADATA_ITEM_KEY;
   842  else
   843  key.type = BTRFS_EXTENT_ITEM_KEY;
   844  
   845  ret = btrfs_search_slot(trans, fs_info->extent_root, , 
path, 0, 0);
   846  if (ret < 0)
   847  goto out_free;
   848  
   849  if (ret > 0 && metadata && key.type == BTRFS_METADATA_ITEM_KEY) 
{
   850  if (path->slots[0]) {
   851  path->slots[0]--;
   852  btrfs_item_key_to_cpu(path->nodes[0], ,
   853path->slots[0]);
   854  if (key.objectid == bytenr &&
   855  key.type == BTRFS_EXT

Re: [PATCH v3] btrfs: code cleanups for btrfs_get_acl()

2018-06-26 Thread Nikolay Borisov




On 26.06.2018 09:08, Chengguang Xu wrote:
> There is no chance to get into -ERANGE error condition because
> we first call btrfs_getxattr to get the length of the attribute,
> then we do a subsequent call with the size from the first call.
> Between the 2 calls the size shouldn't change. So remove the
> unnecessary -ERANGE error check and meanwhile fix some other
> bad practices as well.
> 
> Detail fixes in this patch.
> - return ERR_PTR(-EINVAL) instead of crashing kernel in default
> case of switch.
> - return original error code when failing from btrfs_getxattr().
> - remove unnecessary bracket.
> - remove unnecessary -ERANGE check.
> 
> Signed-off-by: Chengguang Xu 

Reviewed-by: Nikolay Borisov 

> ---
> v3:
> - Fix some other bad practices.
> - Add more information to commit log.
> 
> v2:
> - Avoid errno overriding instead of print error message in error case.
> - Change commit log for better understanding.
> 
>  fs/btrfs/acl.c | 13 ++---
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
> index 15e1dfef56a5..3b66c957ea6f 100644
> --- a/fs/btrfs/acl.c
> +++ b/fs/btrfs/acl.c
> @@ -30,23 +30,22 @@ struct posix_acl *btrfs_get_acl(struct inode *inode, int 
> type)
>   name = XATTR_NAME_POSIX_ACL_DEFAULT;
>   break;
>   default:
> - BUG();
> + return ERR_PTR(-EINVAL);
>   }
>  
> - size = btrfs_getxattr(inode, name, "", 0);
> + size = btrfs_getxattr(inode, name, NULL, 0);
>   if (size > 0) {
>   value = kzalloc(size, GFP_KERNEL);
>   if (!value)
>   return ERR_PTR(-ENOMEM);
>   size = btrfs_getxattr(inode, name, value, size);
>   }
> - if (size > 0) {
> + if (size > 0)
>   acl = posix_acl_from_xattr(_user_ns, value, size);
> - } else if (size == -ERANGE || size == -ENODATA || size == 0) {
> + else if (size == -ENODATA || size == 0)
>   acl = NULL;
> - } else {
> - acl = ERR_PTR(-EIO);
> - }
> + else
> + acl = ERR_PTR(size);
>   kfree(value);
>  
>   return acl;
> 
--
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: annotate unlikely branches after V0 extent type removal

2018-06-26 Thread David Sterba

On Tue, Jun 26, 2018 at 05:31:40PM +0300, Nikolay Borisov wrote:
> 
> 
> On 26.06.2018 17:24, David Sterba wrote:
> > The v0 extent type checks are the right case for the unlikely
> > annotations as we don't expect to ever see them, so let's give the
> > compiler some hint.
> > 
> > Signed-off-by: David Sterba 
> 
> The question is how helpful those unlikelies are in the generated code.

They typically lead to inverse condition (so the jump is not taken) and
the body of the loop is moved farther away.

> Also according to the comment about __cold annotation this already gives
> the compiler a hint that any code leading to such a cold function is
> already unlikely.

Yes, that's another hint. The condition annotation affects the whole
statement block regardless. I also think that the unlikely in this case
serves as a code documentation (as long as this is consistent with other
uses of likely/unlikely).
--
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/3] btrfs: fix race between mkfs and mount

2018-06-26 Thread Anand Jain





On 06/26/2018 08:19 PM, David Sterba wrote:

On Tue, Jun 26, 2018 at 02:25:11PM +0800, Anand Jain wrote:



(sorry for the delay in reply due to my vacation and, other
   urgent things took my priority too).

   Please see inline below.

On 06/19/2018 09:53 PM, David Sterba wrote:

On Thu, Jun 07, 2018 at 06:39:32PM +0200, David Sterba wrote:

On Mon, Jun 04, 2018 at 11:00:30PM +0800, Anand Jain wrote:

In an instrumented testing it is possible that the mount and
a newer mkfs.btrfs thread on the same device can race and if the new
mkfs.btrfs wins it will free the older fs_devices, then the mount thread
will lead to oops.

Thread1 Thread2
--- ---
mkfs.btrfs -fq /dev/sdb
mount /dev/sdb /btrfs
|_btrfs_mount_root()
|_btrfs_scan_one_device(... _devices)

mkfs.btrfs -fq /dev/sdb
|_btrfs_contol_ioctl()
  |_btrfs_scan_one_device(... 
_devices)
|_::
  
|_btrfs_free_stale_devices()

|_btrfs_open_devices(fs_devices ..) <-- stale fs_devices.

Fix this with a mutually exclusive flag BTRFS_VOL_FLAG_EXCL_OPS.


I'm not sure this is the right way to fix it, adding another bit to the
uuid_mutex and device_list_mutex combo.


   Hmm I wonder why?


To fix the race between mount and mkfs we can add a bit of logic to the
device scanning so that mount calling scan will track the purpose and
mkfs scan will not be allowed to free that device.


   Right. To implement such a logic requisites are..
#1 The lock must be FSID local so that concurrent mount and or scan
   of two independent FSID+DEV is possible.
#2 It should not return EBUSY when either of scan or mount is in
   progress but smart enough to complete the (re)scan and or mount
   in parallel

   #1 is must, but #2 is good to have and if EBUSY is returned its not
   wrong as well.



Last version of the proposed fix is to extend the uuid_mutex over the
whole mount callback and use it around calls to btrfs_scan_one_device.
That way we'll be sure the mount will always get to the device it
scanned and that will not be freed by the parallel scan.


   That will break the requisite #1 as above.


I don't see how it breaks 'mount and or scan of two independent fsid+dev
is possible'. It is possible, but the lock does not need to be
filesystem local.

Concurrent mount or scan will block on the uuid_mutex,


 As uuid_mutex is global, if fsid1 is being mounted, the fsid2 mount
 will wait upto certain extent with this code. My efforts here was to
 use uuid_mutex only to protect the fs_uuid update part, in this way
 there is concurrency in the mount process of fsid1 and fsid2 and
 provides shorter bootup time when the user uses the mount at boot
 option.

Thanks, Anand


so all callers
will proceed once the lock is released and there's no unexpected
behaviour.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] btrfs: annotate unlikely branches after V0 extent type removal

2018-06-26 Thread Nikolay Borisov




On 26.06.2018 17:24, David Sterba wrote:
> The v0 extent type checks are the right case for the unlikely
> annotations as we don't expect to ever see them, so let's give the
> compiler some hint.
> 
> Signed-off-by: David Sterba 

The question is how helpful those unlikelies are in the generated code.
Also according to the comment about __cold annotation this already gives
the compiler a hint that any code leading to such a cold function is
already unlikely.

Anyways:

Reviewed-by: Nikolay Borisov 

> ---
>  fs/btrfs/extent-tree.c | 8 
>  fs/btrfs/print-tree.c  | 2 +-
>  fs/btrfs/relocation.c  | 8 
>  3 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 8713a1693dfc..d3f7afe3a548 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -1310,7 +1310,7 @@ static noinline int remove_extent_data_ref(struct 
> btrfs_trans_handle *trans,
>   ref2 = btrfs_item_ptr(leaf, path->slots[0],
> struct btrfs_shared_data_ref);
>   num_refs = btrfs_shared_data_ref_count(leaf, ref2);
> - } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
> + } else if (unlikely(key.type == BTRFS_EXTENT_REF_V0_KEY)) {
>   btrfs_print_v0_err(fs_info);
>   btrfs_abort_transaction(trans, -EINVAL);
>   return -EINVAL;
> @@ -1563,7 +1563,7 @@ int lookup_inline_extent_backref(struct 
> btrfs_trans_handle *trans,
>  
>   leaf = path->nodes[0];
>   item_size = btrfs_item_size_nr(leaf, path->slots[0]);
> - if (item_size < sizeof(*ei)) {
> + if (unlikely(item_size < sizeof(*ei))) {
>   err = -EINVAL;
>   btrfs_print_v0_err(fs_info);
>   btrfs_abort_transaction(trans, err);
> @@ -2269,7 +2269,7 @@ static int run_delayed_extent_op(struct 
> btrfs_trans_handle *trans,
>   leaf = path->nodes[0];
>   item_size = btrfs_item_size_nr(leaf, path->slots[0]);
>  
> - if (item_size < sizeof(*ei)) {
> + if (unlikely(item_size < sizeof(*ei))) {
>   err = -EINVAL;
>   btrfs_print_v0_err(fs_info);
>   btrfs_abort_transaction(trans, err);
> @@ -6811,7 +6811,7 @@ static int __btrfs_free_extent(struct 
> btrfs_trans_handle *trans,
>  
>   leaf = path->nodes[0];
>   item_size = btrfs_item_size_nr(leaf, extent_slot);
> - if (item_size < sizeof(*ei)) {
> + if (unlikely(item_size < sizeof(*ei))) {
>   ret = -EINVAL;
>   btrfs_print_v0_err(info);
>   btrfs_abort_transaction(trans, ret);
> diff --git a/fs/btrfs/print-tree.c b/fs/btrfs/print-tree.c
> index 772560eecfa6..0dd44d161935 100644
> --- a/fs/btrfs/print-tree.c
> +++ b/fs/btrfs/print-tree.c
> @@ -52,7 +52,7 @@ static void print_extent_item(struct extent_buffer *eb, int 
> slot, int type)
>   u64 offset;
>   int ref_index = 0;
>  
> - if (item_size < sizeof(*ei)) {
> + if (unlikely(item_size < sizeof(*ei))) {
>   btrfs_print_v0_err(eb->fs_info);
>   btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
>   }
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index a8dddefaae99..381bd3c24249 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -832,7 +832,7 @@ struct backref_node *build_backref_tree(struct 
> reloc_control *rc,
>   goto next;
>   } else if (key.type != BTRFS_TREE_BLOCK_REF_KEY) {
>   goto next;
> - } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
> + } else if (unlikely(key.type == BTRFS_EXTENT_REF_V0_KEY)) {
>   err = -EINVAL;
>   btrfs_print_v0_err(rc->extent_root->fs_info);
>   btrfs_handle_fs_error(rc->extent_root->fs_info, err,
> @@ -3325,7 +3325,7 @@ static int add_tree_block(struct reloc_control *rc,
>   level = (int)extent_key->offset;
>   }
>   generation = btrfs_extent_generation(eb, ei);
> - } else if (item_size == sizeof(struct btrfs_extent_item_v0)) {
> + } else if (unlikely(item_size == sizeof(struct btrfs_extent_item_v0))) {
>   btrfs_print_v0_err(eb->fs_info);
>   btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
>   return -EINVAL;
> @@ -3742,7 +3742,7 @@ int add_data_references(struct reloc_control *rc,
> struct btrfs_extent_data_ref);
>   ret = find_data_references(rc, extent_key,
>  eb, dref, blocks);
> - } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
> + } else if (unlikely(key.type == BTRFS_EXTENT_REF_V0_KEY)) {
>   btrfs_print_v0_err(eb->fs_info);
>   btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
>   ret = -EINVAL;
> @@ -3984,7

[PATCH] btrfs: annotate unlikely branches after V0 extent type removal

2018-06-26 Thread David Sterba

The v0 extent type checks are the right case for the unlikely
annotations as we don't expect to ever see them, so let's give the
compiler some hint.

Signed-off-by: David Sterba 
---
 fs/btrfs/extent-tree.c | 8 
 fs/btrfs/print-tree.c  | 2 +-
 fs/btrfs/relocation.c  | 8 
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 8713a1693dfc..d3f7afe3a548 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -1310,7 +1310,7 @@ static noinline int remove_extent_data_ref(struct 
btrfs_trans_handle *trans,
ref2 = btrfs_item_ptr(leaf, path->slots[0],
  struct btrfs_shared_data_ref);
num_refs = btrfs_shared_data_ref_count(leaf, ref2);
-   } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
+   } else if (unlikely(key.type == BTRFS_EXTENT_REF_V0_KEY)) {
btrfs_print_v0_err(fs_info);
btrfs_abort_transaction(trans, -EINVAL);
return -EINVAL;
@@ -1563,7 +1563,7 @@ int lookup_inline_extent_backref(struct 
btrfs_trans_handle *trans,
 
leaf = path->nodes[0];
item_size = btrfs_item_size_nr(leaf, path->slots[0]);
-   if (item_size < sizeof(*ei)) {
+   if (unlikely(item_size < sizeof(*ei))) {
err = -EINVAL;
btrfs_print_v0_err(fs_info);
btrfs_abort_transaction(trans, err);
@@ -2269,7 +2269,7 @@ static int run_delayed_extent_op(struct 
btrfs_trans_handle *trans,
leaf = path->nodes[0];
item_size = btrfs_item_size_nr(leaf, path->slots[0]);
 
-   if (item_size < sizeof(*ei)) {
+   if (unlikely(item_size < sizeof(*ei))) {
err = -EINVAL;
btrfs_print_v0_err(fs_info);
btrfs_abort_transaction(trans, err);
@@ -6811,7 +6811,7 @@ static int __btrfs_free_extent(struct btrfs_trans_handle 
*trans,
 
leaf = path->nodes[0];
item_size = btrfs_item_size_nr(leaf, extent_slot);
-   if (item_size < sizeof(*ei)) {
+   if (unlikely(item_size < sizeof(*ei))) {
ret = -EINVAL;
btrfs_print_v0_err(info);
btrfs_abort_transaction(trans, ret);
diff --git a/fs/btrfs/print-tree.c b/fs/btrfs/print-tree.c
index 772560eecfa6..0dd44d161935 100644
--- a/fs/btrfs/print-tree.c
+++ b/fs/btrfs/print-tree.c
@@ -52,7 +52,7 @@ static void print_extent_item(struct extent_buffer *eb, int 
slot, int type)
u64 offset;
int ref_index = 0;
 
-   if (item_size < sizeof(*ei)) {
+   if (unlikely(item_size < sizeof(*ei))) {
btrfs_print_v0_err(eb->fs_info);
btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
}
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index a8dddefaae99..381bd3c24249 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -832,7 +832,7 @@ struct backref_node *build_backref_tree(struct 
reloc_control *rc,
goto next;
} else if (key.type != BTRFS_TREE_BLOCK_REF_KEY) {
goto next;
-   } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
+   } else if (unlikely(key.type == BTRFS_EXTENT_REF_V0_KEY)) {
err = -EINVAL;
btrfs_print_v0_err(rc->extent_root->fs_info);
btrfs_handle_fs_error(rc->extent_root->fs_info, err,
@@ -3325,7 +3325,7 @@ static int add_tree_block(struct reloc_control *rc,
level = (int)extent_key->offset;
}
generation = btrfs_extent_generation(eb, ei);
-   } else if (item_size == sizeof(struct btrfs_extent_item_v0)) {
+   } else if (unlikely(item_size == sizeof(struct btrfs_extent_item_v0))) {
btrfs_print_v0_err(eb->fs_info);
btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
return -EINVAL;
@@ -3742,7 +3742,7 @@ int add_data_references(struct reloc_control *rc,
  struct btrfs_extent_data_ref);
ret = find_data_references(rc, extent_key,
   eb, dref, blocks);
-   } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
+   } else if (unlikely(key.type == BTRFS_EXTENT_REF_V0_KEY)) {
btrfs_print_v0_err(eb->fs_info);
btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
ret = -EINVAL;
@@ -3984,7 +3984,7 @@ static noinline_for_stack int relocate_block_group(struct 
reloc_control *rc)
flags = btrfs_extent_flags(path->nodes[0], ei);
ret = check_extent_flags(flags);
BUG_ON(ret);
-   } else if (item_size == sizeof(struct btrfs_extent_item_v0)) {
+   } else if (unlikely(item_size == sizeof(struct

Re: [PATCH v2] btrfs: Add graceful handling of V0 extents

2018-06-26 Thread David Sterba

On Tue, Jun 26, 2018 at 04:57:36PM +0300, Nikolay Borisov wrote:
> Following the removal of the v0 handling code let's be courteous and
> print an error message when such extents are handled. In the cases
> where we have a transaction just abort it, otherwise just call
> btrfs_handle_fs_error. Both cases result in the FS being re-mounted RO.
> 
> Signed-off-by: Nikolay Borisov 

> - ASSERT(key.type != BTRFS_EXTENT_REF_V0_KEY);
> - if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
> + if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
> + err = -EINVAL;
> + btrfs_print_v0_err(rc->extent_root->fs_info);
> + btrfs_handle_fs_error(rc->extent_root->fs_info, err,
> +   NULL);
> + goto out;
> + } else if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {

The v0 check should be made last as it's not expected to happen. I'm
commiting with this diff

--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -787,13 +787,7 @@ struct backref_node *build_backref_tree(struct 
reloc_control *rc,
goto next;
}
 
-   if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
-   err = -EINVAL;
-   btrfs_print_v0_err(rc->extent_root->fs_info);
-   btrfs_handle_fs_error(rc->extent_root->fs_info, err,
- NULL);
-   goto out;
-   } else if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
+   if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
if (key.objectid == key.offset) {
/*
 * only root blocks of reloc trees use
@@ -838,6 +832,12 @@ struct backref_node *build_backref_tree(struct 
reloc_control *rc,
goto next;
} else if (key.type != BTRFS_TREE_BLOCK_REF_KEY) {
goto next;
+   } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
+   err = -EINVAL;
+   btrfs_print_v0_err(rc->extent_root->fs_info);
+   btrfs_handle_fs_error(rc->extent_root->fs_info, err,
+ NULL);
+   goto out;
}
 
/* key.type == BTRFS_TREE_BLOCK_REF_KEY */
@@ -3734,11 +3734,7 @@ int add_data_references(struct reloc_control *rc,
if (key.objectid != extent_key->objectid)
break;
 
-   if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
-   btrfs_print_v0_err(eb->fs_info);
-   btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
-   ret = -EINVAL;
-   } else if (key.type == BTRFS_SHARED_DATA_REF_KEY) {
+   if (key.type == BTRFS_SHARED_DATA_REF_KEY) {
ret = __add_tree_block(rc, key.offset, blocksize,
   blocks);
} else if (key.type == BTRFS_EXTENT_DATA_REF_KEY) {
@@ -3746,6 +3742,10 @@ int add_data_references(struct reloc_control *rc,
  struct btrfs_extent_data_ref);
ret = find_data_references(rc, extent_key,
   eb, dref, blocks);
+   } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
+   btrfs_print_v0_err(eb->fs_info);
+   btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
+   ret = -EINVAL;
} else {
ret = 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] btrfs: remove warnings superseded by refcount_t usage

2018-06-26 Thread Nikolay Borisov




On 25.06.2018 19:38, David Sterba wrote:
> There are several WARN_ON calls that catch incrorrect reference counter
> updates, but this is what the refcount_t does already:
> 
> * refcount_inc from 0 will warn
> * refcount_dec_and_test from 0 will warn
> 

But these warnings are only going to be triggered in the case of
CONFIG_REFCOUNT_FULL otherwise refcount falls back to plain atomic
operations.

> Signed-off-by: David Sterba 
> ---
>  fs/btrfs/delayed-ref.h | 1 -
>  fs/btrfs/extent_map.c  | 2 +-
>  fs/btrfs/transaction.c | 1 -
>  fs/btrfs/volumes.c | 1 -
>  4 files changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h
> index ea1aecb6a50d..16ee92c541bf 100644
> --- a/fs/btrfs/delayed-ref.h
> +++ b/fs/btrfs/delayed-ref.h
> @@ -210,7 +210,6 @@ btrfs_free_delayed_extent_op(struct 
> btrfs_delayed_extent_op *op)
>  
>  static inline void btrfs_put_delayed_ref(struct btrfs_delayed_ref_node *ref)
>  {
> - WARN_ON(refcount_read(>refs) == 0);
>   if (refcount_dec_and_test(>refs)) {
>   WARN_ON(ref->in_tree);
>   switch (ref->type) {
> diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
> index 6648d55e5339..6808ad4ed3c9 100644
> --- a/fs/btrfs/extent_map.c
> +++ b/fs/btrfs/extent_map.c
> @@ -72,7 +72,7 @@ void free_extent_map(struct extent_map *em)
>  {
>   if (!em)
>   return;
> - WARN_ON(refcount_read(>refs) == 0);
> +
>   if (refcount_dec_and_test(>refs)) {
>   WARN_ON(extent_map_in_tree(em));
>   WARN_ON(!list_empty(>list));
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index 4485eae41e88..f6b68099b767 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -41,7 +41,6 @@ static const unsigned int 
> btrfs_blocked_trans_types[TRANS_STATE_MAX] = {
>  
>  void btrfs_put_transaction(struct btrfs_transaction *transaction)
>  {
> - WARN_ON(refcount_read(>use_count) == 0);
>   if (refcount_dec_and_test(>use_count)) {
>   BUG_ON(!list_empty(>list));
>   WARN_ON(!RB_EMPTY_ROOT(>delayed_refs.href_root));
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index e034ad9e23b4..f64d26b4f278 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -5330,7 +5330,6 @@ static struct btrfs_bio *alloc_btrfs_bio(int 
> total_stripes, int real_stripes)
>  
>  void btrfs_get_bbio(struct btrfs_bio *bbio)
>  {
> - WARN_ON(!refcount_read(>refs));
>   refcount_inc(>refs);
>  }
>  
> 
--
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: Add graceful handling of V0 extents

2018-06-26 Thread Nikolay Borisov

Following the removal of the v0 handling code let's be courteous and
print an error message when such extents are handled. In the cases
where we have a transaction just abort it, otherwise just call
btrfs_handle_fs_error. Both cases result in the FS being re-mounted RO.

Signed-off-by: Nikolay Borisov 
---

V2:
 * Replaced open-coded error printing with a helper function for consistency. 

 fs/btrfs/ctree.h   |  7 +++
 fs/btrfs/extent-tree.c | 39 +++
 fs/btrfs/print-tree.c  |  9 ++---
 fs/btrfs/relocation.c  | 31 ++-
 4 files changed, 74 insertions(+), 12 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index bc52bf7ac572..629ae1977d2c 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3457,6 +3457,13 @@ static inline void assfail(char *expr, char *file, int 
line)
BUG();
 }
 
+__cold
+static inline void btrfs_print_v0_err(struct btrfs_fs_info *fs_info)
+{
+   btrfs_err(fs_info,
+   "Unsupported V0 extent filesystem detected. Aborting... Please 
re-create your filesystem with a newer kernel");
+}
+
 #define ASSERT(expr)   \
(likely(expr) ? (void)0 : assfail(#expr, __FILE__, __LINE__))
 #else
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 4129831523a2..dd3ef8699b67 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -870,8 +870,16 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle 
*trans,
num_refs = btrfs_extent_refs(leaf, ei);
extent_flags = btrfs_extent_flags(leaf, ei);
} else {
-   BUG();
+   ret = -EINVAL;
+   btrfs_print_v0_err(fs_info);
+   if (trans)
+   btrfs_abort_transaction(trans, ret);
+   else
+   btrfs_handle_fs_error(fs_info, ret, NULL);
+
+   goto out_free;
}
+
BUG_ON(num_refs == 0);
} else {
num_refs = 0;
@@ -1302,6 +1310,10 @@ static noinline int remove_extent_data_ref(struct 
btrfs_trans_handle *trans,
ref2 = btrfs_item_ptr(leaf, path->slots[0],
  struct btrfs_shared_data_ref);
num_refs = btrfs_shared_data_ref_count(leaf, ref2);
+   } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
+   btrfs_print_v0_err(fs_info);
+   btrfs_abort_transaction(trans, -EINVAL);
+   return -EINVAL;
} else {
BUG();
}
@@ -1334,6 +1346,8 @@ static noinline u32 extent_data_ref_count(struct 
btrfs_path *path,
 
leaf = path->nodes[0];
btrfs_item_key_to_cpu(leaf, , path->slots[0]);
+
+   BUG_ON(key.type == BTRFS_EXTENT_REF_V0_KEY);
if (iref) {
/*
 * If type is invalid, we should have bailed out earlier than
@@ -1549,7 +1563,12 @@ int lookup_inline_extent_backref(struct 
btrfs_trans_handle *trans,
 
leaf = path->nodes[0];
item_size = btrfs_item_size_nr(leaf, path->slots[0]);
-   BUG_ON(item_size < sizeof(*ei));
+   if (item_size < sizeof(*ei)) {
+   err = -EINVAL;
+   btrfs_print_v0_err(fs_info);
+   btrfs_abort_transaction(trans, err);
+   goto out;
+   }
 
ei = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_extent_item);
flags = btrfs_extent_flags(leaf, ei);
@@ -2282,7 +2301,14 @@ static int run_delayed_extent_op(struct 
btrfs_trans_handle *trans,
 
leaf = path->nodes[0];
item_size = btrfs_item_size_nr(leaf, path->slots[0]);
-   BUG_ON(item_size < sizeof(*ei));
+
+   if (item_size < sizeof(*ei)) {
+   err = -EINVAL;
+   btrfs_print_v0_err(fs_info);
+   btrfs_abort_transaction(trans, err);
+   goto out;
+   }
+
ei = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_extent_item);
__run_delayed_extent_op(extent_op, leaf, ei);
 
@@ -6815,7 +6841,12 @@ static int __btrfs_free_extent(struct btrfs_trans_handle 
*trans,
 
leaf = path->nodes[0];
item_size = btrfs_item_size_nr(leaf, extent_slot);
-   BUG_ON(item_size < sizeof(*ei));
+   if (item_size < sizeof(*ei)) {
+   ret = -EINVAL;
+   btrfs_print_v0_err(info);
+   btrfs_abort_transaction(trans, ret);
+   goto out;
+   }
ei = btrfs_item_ptr(leaf, extent_slot,
struct btrfs_extent_item);
if (owner_objectid < BTRFS_FIRST_FREE_OBJECTID &&
diff --git a/fs/btrfs/print-tree.c b/fs/btrfs/print-tree.c
index b03040b84fe9..772560eecfa6 100644
--- a/fs/btrfs/print-tree.c
+++ b/fs/btrfs/print-tree.c
@@ -52,8 +52,10 @@ static void print_extent_item(struct extent_buffer *eb, int 
slot, int type)
u64 offset;
int ref_index

Re: [PATCH 3/3] btrfs: fix race between mkfs and mount

2018-06-26 Thread David Sterba

On Tue, Jun 26, 2018 at 02:25:11PM +0800, Anand Jain wrote:
> 
> 
> (sorry for the delay in reply due to my vacation and, other
>   urgent things took my priority too).
> 
>   Please see inline below.
> 
> On 06/19/2018 09:53 PM, David Sterba wrote:
> > On Thu, Jun 07, 2018 at 06:39:32PM +0200, David Sterba wrote:
> >> On Mon, Jun 04, 2018 at 11:00:30PM +0800, Anand Jain wrote:
> >>> In an instrumented testing it is possible that the mount and
> >>> a newer mkfs.btrfs thread on the same device can race and if the new
> >>> mkfs.btrfs wins it will free the older fs_devices, then the mount thread
> >>> will lead to oops.
> >>>
> >>> Thread1   Thread2
> >>> ---   ---
> >>> mkfs.btrfs -fq /dev/sdb
> >>> mount /dev/sdb /btrfs
> >>> |_btrfs_mount_root()
> >>>|_btrfs_scan_one_device(... _devices)
> >>>
> >>>   mkfs.btrfs -fq /dev/sdb
> >>>   |_btrfs_contol_ioctl()
> >>> |_btrfs_scan_one_device(... 
> >>> _devices)
> >>>   |_::
> >>> 
> >>> |_btrfs_free_stale_devices()
> >>>
> >>>|_btrfs_open_devices(fs_devices ..) <-- stale fs_devices.
> >>>
> >>> Fix this with a mutually exclusive flag BTRFS_VOL_FLAG_EXCL_OPS.
> >>
> >> I'm not sure this is the right way to fix it, adding another bit to the
> >> uuid_mutex and device_list_mutex combo.
> 
>   Hmm I wonder why?
> 
> >> To fix the race between mount and mkfs we can add a bit of logic to the
> >> device scanning so that mount calling scan will track the purpose and
> >> mkfs scan will not be allowed to free that device.
> 
>   Right. To implement such a logic requisites are..
>#1 The lock must be FSID local so that concurrent mount and or scan
>   of two independent FSID+DEV is possible.
>#2 It should not return EBUSY when either of scan or mount is in
>   progress but smart enough to complete the (re)scan and or mount
>   in parallel
> 
>   #1 is must, but #2 is good to have and if EBUSY is returned its not
>   wrong as well.
> 
> 
> > Last version of the proposed fix is to extend the uuid_mutex over the
> > whole mount callback and use it around calls to btrfs_scan_one_device.
> > That way we'll be sure the mount will always get to the device it
> > scanned and that will not be freed by the parallel scan.
> 
>   That will break the requisite #1 as above.

I don't see how it breaks 'mount and or scan of two independent fsid+dev
is possible'. It is possible, but the lock does not need to be
filesystem local.

Concurrent mount or scan will block on the uuid_mutex, so all callers
will proceed once the lock is released and there's no unexpected
behaviour.
--
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 raid10 performance

2018-06-26 Thread Austin S. Hemmelgarn


On 2018-06-25 21:05, Sterling Windmill wrote:

I am running a single btrfs RAID10 volume of eight LUKS devices, each
using a 2TB SATA hard drive as a backing store. The SATA drives are a
mixture of Seagate and Western Digital drives, some with RPMs ranging
from 5400 to 7200. Each seems to individually performance test where I
would expect for drives of this caliber. They are all attached to an
LSI PCIe SAS controller and configured in JBOD.

I have a relatively beefy quad core Xeon CPU that supports AES-NI and
don't think LUKS is my bottleneck.

Here's some info from the resulting filesystem:

   btrfs fi df /storage
   Data, RAID10: total=6.30TiB, used=6.29TiB
   System, RAID10: total=8.00MiB, used=560.00KiB
   Metadata, RAID10: total=9.00GiB, used=7.64GiB
   GlobalReserve, single: total=512.00MiB, used=0.00B

In general I see good performance, especially read performance which
is enough to regularly saturate my gigabit network when copying files
from this host via samba. Reads are definitely taking advantage of the
multiple copies of data available and spreading the load among all
drives.

Writes aren't quite as rosy, however.

When writing files using dd like in this example:

   dd if=/dev/zero of=tempfile bs=1M count=10240 conv=fdatasync,notrun
c status=progress

And running a command like:

   iostat -m 1

to monitor disk I/O, writes seem to only focus on one of the eight
disks at a time, moving from one drive to the next. This results in a
sustained 55-90 MB/sec throughput depending on which disk is being
written to (remember, some have faster spindle speed than others).

Am I wrong to expect btrfs' RAID10 mode to write to multiple disks
simultaneously and to break larger writes into smaller stripes across
my four pairs of disks?

I had trouble identifying whether btrfs RAID10 is writing (64K?)
stripes or (1GB?) blocks to disk in this mode. The latter might make
more sense based upon what I'm seeing?

Anything else I should be trying to narrow down the bottleneck?
First, you're probably incorrect that the disk access is being 
parallelized.  Given that BTRFS still doesn't parallelize writes in 
raid1 mode, I very much doubt it does so in raid10 mode.  Parallelizing 
writes is a performance optimization that still hasn't really been 
tackled by anyone.  Realistically, BTRFS writes to exactly one disk at a 
time.  So, in a four disk raid10 array, it first writes to disk 1, waits 
for that to finish, then writes to disk 2, waits for that to finish, 
then 3, waits, and then four.  Overall, this makes writes rather slow.


As far as striping across multiple disks, yes, that does happen.  The 
specifics of this are a bit complicated though, and require explaining a 
bit about how BTRFS works in general.


BTRFS uses a two-stage allocator, first allocating 'large' regions of 
disk space to be used for a specific type of data called chunks, and 
then allocating blocks out of those regions to actually store the data. 
There are three chunk types, data (used for storing actual file 
contents), metadata (used for storing things like filenames, access 
times, directory structure, etc), and system (used to store the 
allocation information for all the other chunks in the filesystem). 
Data chunks are typically 1 GB in size, metadata are typically 256 MB in 
size, and system chunks are highly variable but don't really matter for 
this explanation.  The chunk level is where the actual replication and 
striping happen, and the chunk size represents what is exposed to the 
block allocator (so every 1 GB data chunk exposes 1 GB of space to the 
block allocator).


Now, replicated (raid1 or dup profiles) chunks work just like you would 
expect, each of the two allocations for the chunk is 1 GB, and each byte 
is stored as-is in both.  Striped (raid0 or raid10 profiles) are 
somewhat more complicated, and I actually don't know exactly how they 
end up allocated at the lower level.  However, I do know how the 
striping works.  In short, you can treat each striped set (either a full 
raid0 chunk, or half a raid10 chunk) as being functionally identical in 
operation to a conventional RAID0 array, striping occurs at a small 
block granularity (I think it's equal to the block size, which would be 
4k in most cases), which unfortunately compounds the performance issues 
caused by BTRFS only writing to one disk at a time.


As far as improving the performance, I've got two suggestions for 
alternative storage arrangements:


* If you want to just stick with only BTRFS for storage, try just using 
raid1 mode.  It will give you the same theoretical total capacity as 
raid10 does and will slow down reads somewhat, but should speed up 
writes significantly (because you're only writing to two devices, not 
striping across two sets of four).


* If you're willing to try something a bit different, convert your 
storage array to two LVM or MD RAID0 volumes composed of four devices 
each, and then run BTRFS in raid1 mode on top of

Re: [PATCH 7/7] btrfs: fix mount and ioctl device scan ioctl race

2018-06-26 Thread Anand Jain





On 06/21/2018 01:51 AM, David Sterba wrote:

Technically this extends the critical section covered by uuid_mutex to:

- parse early mount options -- here we can call device scan on paths
   that can be passed as 'device=/dev/...'

- scan the device passed to mount

- open the devices related to the fs_devices -- this increases
   fs_devices::opened

The race can happen when mount calls one of the scans and there's
another one called eg. by mkfs or 'btrfs dev scan':

Mount  Scan
-  
scan_one_device (dev1, fsid1)
scan_one_device (dev2, fsid2)

   
   dev1
typo?



   add the device
   free stale devices
   fsid1 fs_devices::opened == 0
   find fsid1:dev1
   free fsid1:dev1
   if it's the last one,
free fs_devices of fsid1
too

open_devices (dev1, fsid1)
dev1 not found

When fixed, the uuid mutex will make sure that mount will increase
fs_devices::opened and this will not be touched by the racing scan
ioctl.


 Using uuid_mutex will unnecessarily serialize mount across different
 fsids.

 Unfortunately we don't have a test case to measure concurrency across
 btrfs fsids. When we have that, this shall fail.

 Expecting different fsids to be able to mount concurrently is a fair
 expectation. And is certainly important for large servers running
 btrfs on few luns which shall start to mount at bootup.

 These changes is kind of going in an opposite direction as I
 originally planned to improve concurrency (across fsids) by reducing
 the unnecessary uuid_mutex footprints.

 And fix the other necessaries using the fsid local atomic volume
 exclusive operations flag. Which in the long term can replace
 fs_info::BTRFS_FS_EXCL_OP as well.

 As both of these approaches fix the issue, its a trade off between the
 concerns of atomic volume exclusive operations flag (except for the
 -EBUSY part [1]) VS serialize mount across different fsids, and IMO,
 its better to make sure different fsids are concurrent in their
 scan-mount operations as it is critical to the boot-up time.

 [1]
 Though returning -EBUSY (for one of the racing mount, scan and or ready
 threads) is theoretically correct but its blunt, and it may wrongly
 categorize as regression, let me try to fix that part and ask for
 comments.

Thanks, Anand



Reported-and-tested-by: syzbot+909a5177749d7990f...@syzkaller.appspotmail.com
Reported-and-tested-by: syzbot+ceb2606025ec1cc34...@syzkaller.appspotmail.com

>

Signed-off-by: David Sterba 
---
  fs/btrfs/super.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 1780eb41f203..b13b871bc584 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1557,19 +1557,19 @@ static struct dentry *btrfs_mount_root(struct 
file_system_type *fs_type,
  
  	mutex_lock(_mutex);

error = btrfs_parse_early_options(data, mode, fs_type, _devices);
-   mutex_unlock(_mutex);
-   if (error)
+   if (error) {
+   mutex_unlock(_mutex);
goto error_fs_info;
+   }
  
-	mutex_lock(_mutex);

error = btrfs_scan_one_device(device_name, mode, fs_type, _devices);
-   mutex_unlock(_mutex);
-   if (error)
+   if (error) {
+   mutex_unlock(_mutex);
goto error_fs_info;
+   }
  
  	fs_info->fs_devices = fs_devices;
  
-	mutex_lock(_mutex);

error = btrfs_open_devices(fs_devices, mode, fs_type);
mutex_unlock(_mutex);
if (error)


--
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: qgroups: Move transaction managed inside btrfs_quota_enable

2018-06-26 Thread Nikolay Borisov




On 26.06.2018 11:46, Misono Tomohiro wrote:
> On 2018/06/26 16:09, Nikolay Borisov wrote:
>> Commit 5d23515be669 ("btrfs: Move qgroup rescan on quota enable to
>> btrfs_quota_enable") not only resulted in an easier to follow code but
>> it also introduced a subtle bug. It changed the timing when the initial
>> transaction rescan was happening - before the commit it would happen
>> after transaction commit had occured but after the commit it might happen
>> before the transaction was committed. This results in failure to
>> correctly rescan the quota since there could be data which is still not
>> committed on disk.
>>
>> This patch aims to fix this by movign the transaction creation/commit
>> inside btrfs_quota_enable, which allows to schedule the quota commit
>> after the transaction has been committed.
>>
>> Fixes: 5d23515be669 ("btrfs: Move qgroup rescan on quota enable to 
>> btrfs_quota_enable")
>> Reported-by: Misono Tomohiro 
>> Signed-off-by: Nikolay Borisov 
>> ---
>> Hi Misono, 
>>
>> Care to test the following patch ? If you say it's ok I will do a similar 
>> one 
>> for the btrfs_quota_disable function. This will also allow me to get rid of 
>> the extra err variable in btrfs_ioctl_quota_ctl. Additionally I think the 
>> number of blocks (2) passed to the transaction for reservation might be 
>> wrong. 
> 
> Hi,
> 
> The patch does not removes start_transaction() from btrfs_ioctl_quota_ctl(),
> so this does not work but I understand your approach (continue to  below).

Ah you are right, the reason I didn't do it is because in a 2nd patch I
wanted to also remove the trans argument to btrfs_quota_disable. So
yeah, I will have to do that in one go.
> 
>>
>>  fs/btrfs/ioctl.c  |  2 +-
>>  fs/btrfs/qgroup.c | 17 ++---
>>  fs/btrfs/qgroup.h |  3 +--
>>  3 files changed, 16 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index a399750b9e41..bf99d7aae3ae 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -5161,7 +5161,7 @@ static long btrfs_ioctl_quota_ctl(struct file *file, 
>> void __user *arg)
>>  
>>  switch (sa->cmd) {
>>  case BTRFS_QUOTA_CTL_ENABLE:
>> -ret = btrfs_quota_enable(trans, fs_info);
>> +ret = btrfs_quota_enable(fs_info);
>>  break;
>>  case BTRFS_QUOTA_CTL_DISABLE:
>>  ret = btrfs_quota_disable(trans, fs_info);
>> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
>> index 1874a6d2e6f5..91bb7e97c0d0 100644
>> --- a/fs/btrfs/qgroup.c
>> +++ b/fs/btrfs/qgroup.c
>> @@ -875,8 +875,7 @@ static int btrfs_clean_quota_tree(struct 
>> btrfs_trans_handle *trans,
>>  return ret;
>>  }
>>  
>> -int btrfs_quota_enable(struct btrfs_trans_handle *trans,
>> -   struct btrfs_fs_info *fs_info)
>> +int btrfs_quota_enable(struct btrfs_fs_info *fs_info)
>>  {
>>  struct btrfs_root *quota_root;
>>  struct btrfs_root *tree_root = fs_info->tree_root;
>> @@ -886,6 +885,7 @@ int btrfs_quota_enable(struct btrfs_trans_handle *trans,
>>  struct btrfs_key key;
>>  struct btrfs_key found_key;
>>  struct btrfs_qgroup *qgroup = NULL;
>> +struct btrfs_trans_handle *trans = NULL;
>>  int ret = 0;
>>  int slot;
>>  
>> @@ -893,6 +893,12 @@ int btrfs_quota_enable(struct btrfs_trans_handle *trans,
>>  if (fs_info->quota_root)
>>  goto out;
>>  
>> +trans = btrfs_start_transaction(tree_root, 2);
> 
> (Should we use fs_root for quota?)

Good question, I just copied the code. The thing is however, when
enabling the quota we might also have to insert the quota tree root item
in the tree_root, I guess that's why it was originally set to tree_root.
> 
>> +if (IS_ERR(trans)) {
>> +ret = PTR_ERR(trans);
>> +goto out;
>> +}
>> +
>>  fs_info->qgroup_ulist = ulist_alloc(GFP_KERNEL);
>>  if (!fs_info->qgroup_ulist) {
>>  ret = -ENOMEM;
>> @@ -987,6 +993,11 @@ int btrfs_quota_enable(struct btrfs_trans_handle *trans,
>>  fs_info->quota_root = quota_root;
>>  set_bit(BTRFS_FS_QUOTA_ENABLED, _info->flags);
>>  spin_unlock(_info->qgroup_lock);
>> +
>> +ret = btrfs_commit_transaction(trans);
>> +if (ret)
>> +goto out_free_path;
>> +
>>  ret = qgroup_rescan_init(fs_info, 0, 1);
> 
> However, I'm not sure if this approach completely works well when some files 
> are
> concurrently written while quota is being enabled.
> Since before the commit 5d23515be669, quota_rescan_init() is called during 
> transaction
> commit, but now quota_rescan_init() is called outside of transacation.
> So, is there still a slight possibility that the same problem occurs here?

Good point, I will have to defer to Qu since he seems to be a quota
expert. My thinking would be that quota will be consistent w.r.t the
last committed transaction. Commit 5d23515be669 just moved the initial
quota rescan and not the running of dirty qgroups. We still have the
btrfs_run_qgroups call in

Re: [PATCH] btrfs: qgroups: Move transaction managed inside btrfs_quota_enable

2018-06-26 Thread Misono Tomohiro

On 2018/06/26 16:09, Nikolay Borisov wrote:
> Commit 5d23515be669 ("btrfs: Move qgroup rescan on quota enable to
> btrfs_quota_enable") not only resulted in an easier to follow code but
> it also introduced a subtle bug. It changed the timing when the initial
> transaction rescan was happening - before the commit it would happen
> after transaction commit had occured but after the commit it might happen
> before the transaction was committed. This results in failure to
> correctly rescan the quota since there could be data which is still not
> committed on disk.
> 
> This patch aims to fix this by movign the transaction creation/commit
> inside btrfs_quota_enable, which allows to schedule the quota commit
> after the transaction has been committed.
> 
> Fixes: 5d23515be669 ("btrfs: Move qgroup rescan on quota enable to 
> btrfs_quota_enable")
> Reported-by: Misono Tomohiro 
> Signed-off-by: Nikolay Borisov 
> ---
> Hi Misono, 
> 
> Care to test the following patch ? If you say it's ok I will do a similar one 
> for the btrfs_quota_disable function. This will also allow me to get rid of 
> the extra err variable in btrfs_ioctl_quota_ctl. Additionally I think the 
> number of blocks (2) passed to the transaction for reservation might be 
> wrong. 

Hi,

The patch does not removes start_transaction() from btrfs_ioctl_quota_ctl(),
so this does not work but I understand your approach (continue to  below).

> 
>  fs/btrfs/ioctl.c  |  2 +-
>  fs/btrfs/qgroup.c | 17 ++---
>  fs/btrfs/qgroup.h |  3 +--
>  3 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index a399750b9e41..bf99d7aae3ae 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -5161,7 +5161,7 @@ static long btrfs_ioctl_quota_ctl(struct file *file, 
> void __user *arg)
>  
>   switch (sa->cmd) {
>   case BTRFS_QUOTA_CTL_ENABLE:
> - ret = btrfs_quota_enable(trans, fs_info);
> + ret = btrfs_quota_enable(fs_info);
>   break;
>   case BTRFS_QUOTA_CTL_DISABLE:
>   ret = btrfs_quota_disable(trans, fs_info);
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 1874a6d2e6f5..91bb7e97c0d0 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -875,8 +875,7 @@ static int btrfs_clean_quota_tree(struct 
> btrfs_trans_handle *trans,
>   return ret;
>  }
>  
> -int btrfs_quota_enable(struct btrfs_trans_handle *trans,
> -struct btrfs_fs_info *fs_info)
> +int btrfs_quota_enable(struct btrfs_fs_info *fs_info)
>  {
>   struct btrfs_root *quota_root;
>   struct btrfs_root *tree_root = fs_info->tree_root;
> @@ -886,6 +885,7 @@ int btrfs_quota_enable(struct btrfs_trans_handle *trans,
>   struct btrfs_key key;
>   struct btrfs_key found_key;
>   struct btrfs_qgroup *qgroup = NULL;
> + struct btrfs_trans_handle *trans = NULL;
>   int ret = 0;
>   int slot;
>  
> @@ -893,6 +893,12 @@ int btrfs_quota_enable(struct btrfs_trans_handle *trans,
>   if (fs_info->quota_root)
>   goto out;
>  
> + trans = btrfs_start_transaction(tree_root, 2);

(Should we use fs_root for quota?)

> + if (IS_ERR(trans)) {
> + ret = PTR_ERR(trans);
> + goto out;
> + }
> +
>   fs_info->qgroup_ulist = ulist_alloc(GFP_KERNEL);
>   if (!fs_info->qgroup_ulist) {
>   ret = -ENOMEM;
> @@ -987,6 +993,11 @@ int btrfs_quota_enable(struct btrfs_trans_handle *trans,
>   fs_info->quota_root = quota_root;
>   set_bit(BTRFS_FS_QUOTA_ENABLED, _info->flags);
>   spin_unlock(_info->qgroup_lock);
> +
> + ret = btrfs_commit_transaction(trans);
> + if (ret)
> + goto out_free_path;
> +
>   ret = qgroup_rescan_init(fs_info, 0, 1);

However, I'm not sure if this approach completely works well when some files are
concurrently written while quota is being enabled.
Since before the commit 5d23515be669, quota_rescan_init() is called during 
transaction
commit, but now quota_rescan_init() is called outside of transacation.
So, is there still a slight possibility that the same problem occurs here?

(I don't completely understands how quota works yet , so correct me if I'm 
wrong.)

>   if (!ret) {
>   qgroup_rescan_zero_tracking(fs_info);
> @@ -3061,7 +3072,7 @@ static int __btrfs_qgroup_release_data(struct inode 
> *inode,
>   if (free && reserved)
>   return qgroup_free_reserved_data(inode, reserved, start, len);
>   extent_changeset_init();
> - ret = clear_record_extent_bits(_I(inode)->io_tree, start, 
> + ret = clear_record_extent_bits(_I(inode)->io_tree, start,
>   start + len -1, EXTENT_QGROUP_RESERVED, );
>   if (ret < 0)
>   goto out;
> diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h
> index d60dd06445ce..3900efab0e70 100644
> --- a/fs/btrfs/qgroup.h
> +++ b/fs/btrfs/qgroup.h
> @@ -141,8 +141,7 @@ struct btrfs_qgroup {
>  #define

Re: [PATCH 2/7] btrfs: extend critical section when scanning a new device

2018-06-26 Thread Anand Jain





On 06/21/2018 01:51 AM, David Sterba wrote:

The stale device list removal needs to be protected by device_list_mutex
too as this could delete from the list and could race with another list
modification and cause crash.


 Its is very less likely or almost practically impossible (unless
 instrumented) to have same device with same fsid but with different
 devid, which then it would delete the device which is under the same
 fs_devices which device_list_add just operated. Otherwise in most of
 the common cases it shall delete the device which does not belong to
 the fs_devices which the device_list_add() just added a device. So
 fs_devices::device_list_mutex lock won't help.

 I have few patches to fix this. I shall send it. But then it was all
 based on the atomic volume exclusive flag which in stages I had
 plans to replace fs_info::BTRFS_FS_EXCL_OP with volume exclusive flag
 as well.

 Also we need locks to manage with in btrfs_free_stale_devices() so
 that we can reuse this code to support user land forget device feature,
 (38cf665d338fca33af4b16f9ec7cad6637fc0fec btrfs: make
 btrfs_free_stale_device() to iterate all stales).

Thanks, Anand



The device needs to be fully initialized before it's added to the list
so the fs_devices also need to be set under the mutex.

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

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 1da162928d1a..02246f9af0a3 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -791,12 +791,11 @@ static noinline struct btrfs_device 
*device_list_add(const char *path,
rcu_assign_pointer(device->name, name);
  
  		mutex_lock(_devices->device_list_mutex);

+   device->fs_devices = fs_devices;
list_add_rcu(>dev_list, _devices->devices);
fs_devices->num_devices++;
-   mutex_unlock(_devices->device_list_mutex);
-
-   device->fs_devices = fs_devices;
btrfs_free_stale_devices(path, device);
+   mutex_unlock(_devices->device_list_mutex);
  
  		if (disk_super->label[0])

pr_info("BTRFS: device label %s devid %llu transid %llu 
%s\n",


--
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: qgroups: Move transaction managed inside btrfs_quota_enable

2018-06-26 Thread Nikolay Borisov

Commit 5d23515be669 ("btrfs: Move qgroup rescan on quota enable to
btrfs_quota_enable") not only resulted in an easier to follow code but
it also introduced a subtle bug. It changed the timing when the initial
transaction rescan was happening - before the commit it would happen
after transaction commit had occured but after the commit it might happen
before the transaction was committed. This results in failure to
correctly rescan the quota since there could be data which is still not
committed on disk.

This patch aims to fix this by movign the transaction creation/commit
inside btrfs_quota_enable, which allows to schedule the quota commit
after the transaction has been committed.

Fixes: 5d23515be669 ("btrfs: Move qgroup rescan on quota enable to 
btrfs_quota_enable")
Reported-by: Misono Tomohiro 
Signed-off-by: Nikolay Borisov 
---
Hi Misono, 

Care to test the following patch ? If you say it's ok I will do a similar one 
for the btrfs_quota_disable function. This will also allow me to get rid of 
the extra err variable in btrfs_ioctl_quota_ctl. Additionally I think the 
number of blocks (2) passed to the transaction for reservation might be 
wrong. 

 fs/btrfs/ioctl.c  |  2 +-
 fs/btrfs/qgroup.c | 17 ++---
 fs/btrfs/qgroup.h |  3 +--
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index a399750b9e41..bf99d7aae3ae 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -5161,7 +5161,7 @@ static long btrfs_ioctl_quota_ctl(struct file *file, void 
__user *arg)
 
switch (sa->cmd) {
case BTRFS_QUOTA_CTL_ENABLE:
-   ret = btrfs_quota_enable(trans, fs_info);
+   ret = btrfs_quota_enable(fs_info);
break;
case BTRFS_QUOTA_CTL_DISABLE:
ret = btrfs_quota_disable(trans, fs_info);
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 1874a6d2e6f5..91bb7e97c0d0 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -875,8 +875,7 @@ static int btrfs_clean_quota_tree(struct btrfs_trans_handle 
*trans,
return ret;
 }
 
-int btrfs_quota_enable(struct btrfs_trans_handle *trans,
-  struct btrfs_fs_info *fs_info)
+int btrfs_quota_enable(struct btrfs_fs_info *fs_info)
 {
struct btrfs_root *quota_root;
struct btrfs_root *tree_root = fs_info->tree_root;
@@ -886,6 +885,7 @@ int btrfs_quota_enable(struct btrfs_trans_handle *trans,
struct btrfs_key key;
struct btrfs_key found_key;
struct btrfs_qgroup *qgroup = NULL;
+   struct btrfs_trans_handle *trans = NULL;
int ret = 0;
int slot;
 
@@ -893,6 +893,12 @@ int btrfs_quota_enable(struct btrfs_trans_handle *trans,
if (fs_info->quota_root)
goto out;
 
+   trans = btrfs_start_transaction(tree_root, 2);
+   if (IS_ERR(trans)) {
+   ret = PTR_ERR(trans);
+   goto out;
+   }
+
fs_info->qgroup_ulist = ulist_alloc(GFP_KERNEL);
if (!fs_info->qgroup_ulist) {
ret = -ENOMEM;
@@ -987,6 +993,11 @@ int btrfs_quota_enable(struct btrfs_trans_handle *trans,
fs_info->quota_root = quota_root;
set_bit(BTRFS_FS_QUOTA_ENABLED, _info->flags);
spin_unlock(_info->qgroup_lock);
+
+   ret = btrfs_commit_transaction(trans);
+   if (ret)
+   goto out_free_path;
+
ret = qgroup_rescan_init(fs_info, 0, 1);
if (!ret) {
qgroup_rescan_zero_tracking(fs_info);
@@ -3061,7 +3072,7 @@ static int __btrfs_qgroup_release_data(struct inode 
*inode,
if (free && reserved)
return qgroup_free_reserved_data(inode, reserved, start, len);
extent_changeset_init();
-   ret = clear_record_extent_bits(_I(inode)->io_tree, start, 
+   ret = clear_record_extent_bits(_I(inode)->io_tree, start,
start + len -1, EXTENT_QGROUP_RESERVED, );
if (ret < 0)
goto out;
diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h
index d60dd06445ce..3900efab0e70 100644
--- a/fs/btrfs/qgroup.h
+++ b/fs/btrfs/qgroup.h
@@ -141,8 +141,7 @@ struct btrfs_qgroup {
 #define QGROUP_RELEASE (1<<1)
 #define QGROUP_FREE(1<<2)
 
-int btrfs_quota_enable(struct btrfs_trans_handle *trans,
-  struct btrfs_fs_info *fs_info);
+int btrfs_quota_enable(struct btrfs_fs_info *fs_info);
 int btrfs_quota_disable(struct btrfs_trans_handle *trans,
struct btrfs_fs_info *fs_info);
 int btrfs_qgroup_rescan(struct btrfs_fs_info *fs_info);
-- 
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: Enabling quota may not correctly rescan on 4.17

2018-06-26 Thread Nikolay Borisov




On 26.06.2018 09:00, Misono Tomohiro wrote:
> Hello Nikolay,
> 
> I noticed that commit 5d23515be669 ("btrfs: Move qgroup rescan
> on quota enable to btrfs_quota_enable") in 4.17 sometimes causes
> to fail correctly rescanning quota when quota is enabled.

Everytime I hear anything quota-related and I cringe ...

> 
> Simple reproducer:
> 
> $ mkfs.btrfs -f $DEV
> $ mount $DEV /mnt
> $ dd if=/dev/urandom of=/mnt/file bs=1000 count=1000
> $ btrfs quota enbale /mnt
> $ umount /mnt
> $ btrfs check $DEV
> ...
> checking quota groups
> Counts for qgroup id: 0/5 are different
> our:referenced 1019904 referenced compressed 1019904
> disk:   referenced 16384 referenced compressed 16384
> diff:   referenced 1003520 referenced compressed 1003520
> our:exclusive 1019904 exclusive compressed 1019904
> disk:   exclusive 16384 exclusive compressed 16384
> diff:   exclusive 1003520 exclusive compressed 1003520
> found 1413120 bytes used, error(s) found
> ...
> 
> This can be also observed in btrfs/114. (Note that progs < 4.17
> returns error code 0 even if quota is not consistency and therefore
> test will incorrectly pass.)
> 
> My observation is that this commit changed to call initial quota rescan
> when quota is enabeld instead of first comit transaction after enabling
> quota, and therefore if there is something not commited at that time,
> their usage will not be accounted.
> 
> Actually this can be simply fixed by calling "btrfs rescan" again or
> calling "btrfs fi sync" before "btrfs quota enable".
> 
> I think the commit itself makes the code much easier to read, so it may
> be better to fix the problem in progs (i.e. calling sync before quota enable).
> 
> Do you have any thoughts?

So fixing it in progs will indeed work, however any 3rd party code which
relies on the scan/enable ioctl will be "broken" so that's not good.
Ideally the correct thing should be that quota scanning can also take
into account the in-memory state i.e any delayed refs but I think this
will be way too much work.

Alternatively, what about moving the transaction start from
btrfs_ioctl_quota_ctl to btrfs_quota_enable/btrfs_quota_disable
functions. That way what we can do is perform the transaction commit in
btrfs_quota_enable before queuing the rescan?

> 
> Thanks,
> Tomohiro Misono
> 
> 
> 
--
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/3] btrfs: fix race between mkfs and mount

2018-06-26 Thread Anand Jain





On 06/20/2018 10:06 PM, David Sterba wrote:

On Mon, Jun 04, 2018 at 11:00:30PM +0800, Anand Jain wrote:

In an instrumented testing it is possible that the mount and
a newer mkfs.btrfs thread on the same device can race and if the new
mkfs.btrfs wins it will free the older fs_devices, then the mount thread
will lead to oops.

Thread1 Thread2
--- ---
mkfs.btrfs -fq /dev/sdb
mount /dev/sdb /btrfs
|_btrfs_mount_root()
   |_btrfs_scan_one_device(... _devices)

mkfs.btrfs -fq /dev/sdb
|_btrfs_contol_ioctl()
  |_btrfs_scan_one_device(... 
_devices)
|_::
  
|_btrfs_free_stale_devices()

   |_btrfs_open_devices(fs_devices ..) <-- stale fs_devices.

Fix this with a mutually exclusive flag BTRFS_VOL_FLAG_EXCL_OPS.

Signed-off-by: Anand Jain 
---
  fs/btrfs/super.c   |  6 ++
  fs/btrfs/volumes.c | 10 +-
  fs/btrfs/volumes.h |  1 +
  3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index f0c13defc9eb..b60e7cbe39f5 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1565,7 +1565,13 @@ static struct dentry *btrfs_mount_root(struct 
file_system_type *fs_type,
goto error_fs_info;
}
  
+	if (test_and_set_bit(BTRFS_VOLUME_STATE_EXCL_OPS, _devices->volume_state)) {

+   error = -EBUSY;


We'd need to wait until the bit is not set instead of BUSY, as the
parallel scan is not really a reason to fail the whole mount.



I'll post the patch series to address this problem today, it utilizes
the uuid_mutex in a similar way you try to do with the new bit, but it
will not lead to EBUSY.


 Ok. Shall review.

Thanks, Anand


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] btrfs: fix race between mkfs and mount

2018-06-26 Thread Anand Jain





(sorry for the delay in reply due to my vacation and, other
 urgent things took my priority too).

 Please see inline below.

On 06/19/2018 09:53 PM, David Sterba wrote:

On Thu, Jun 07, 2018 at 06:39:32PM +0200, David Sterba wrote:

On Mon, Jun 04, 2018 at 11:00:30PM +0800, Anand Jain wrote:

In an instrumented testing it is possible that the mount and
a newer mkfs.btrfs thread on the same device can race and if the new
mkfs.btrfs wins it will free the older fs_devices, then the mount thread
will lead to oops.

Thread1 Thread2
--- ---
mkfs.btrfs -fq /dev/sdb
mount /dev/sdb /btrfs
|_btrfs_mount_root()
   |_btrfs_scan_one_device(... _devices)

mkfs.btrfs -fq /dev/sdb
|_btrfs_contol_ioctl()
  |_btrfs_scan_one_device(... 
_devices)
|_::
  
|_btrfs_free_stale_devices()

   |_btrfs_open_devices(fs_devices ..) <-- stale fs_devices.

Fix this with a mutually exclusive flag BTRFS_VOL_FLAG_EXCL_OPS.


I'm not sure this is the right way to fix it, adding another bit to the
uuid_mutex and device_list_mutex combo.


 Hmm I wonder why?


To fix the race between mount and mkfs we can add a bit of logic to the
device scanning so that mount calling scan will track the purpose and
mkfs scan will not be allowed to free that device.


 Right. To implement such a logic requisites are..
  #1 The lock must be FSID local so that concurrent mount and or scan
 of two independent FSID+DEV is possible.
  #2 It should not return EBUSY when either of scan or mount is in
 progress but smart enough to complete the (re)scan and or mount
 in parallel

 #1 is must, but #2 is good to have and if EBUSY is returned its not
 wrong as well.



Last version of the proposed fix is to extend the uuid_mutex over the
whole mount callback and use it around calls to btrfs_scan_one_device.
That way we'll be sure the mount will always get to the device it
scanned and that will not be freed by the parallel scan.


 That will break the requisite #1 as above.

Thanks, Anand

--
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 v3] btrfs: code cleanups for btrfs_get_acl()

2018-06-26 Thread Chengguang Xu

There is no chance to get into -ERANGE error condition because
we first call btrfs_getxattr to get the length of the attribute,
then we do a subsequent call with the size from the first call.
Between the 2 calls the size shouldn't change. So remove the
unnecessary -ERANGE error check and meanwhile fix some other
bad practices as well.

Detail fixes in this patch.
- return ERR_PTR(-EINVAL) instead of crashing kernel in default
case of switch.
- return original error code when failing from btrfs_getxattr().
- remove unnecessary bracket.
- remove unnecessary -ERANGE check.

Signed-off-by: Chengguang Xu 
---
v3:
- Fix some other bad practices.
- Add more information to commit log.

v2:
- Avoid errno overriding instead of print error message in error case.
- Change commit log for better understanding.

 fs/btrfs/acl.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
index 15e1dfef56a5..3b66c957ea6f 100644
--- a/fs/btrfs/acl.c
+++ b/fs/btrfs/acl.c
@@ -30,23 +30,22 @@ struct posix_acl *btrfs_get_acl(struct inode *inode, int 
type)
name = XATTR_NAME_POSIX_ACL_DEFAULT;
break;
default:
-   BUG();
+   return ERR_PTR(-EINVAL);
}
 
-   size = btrfs_getxattr(inode, name, "", 0);
+   size = btrfs_getxattr(inode, name, NULL, 0);
if (size > 0) {
value = kzalloc(size, GFP_KERNEL);
if (!value)
return ERR_PTR(-ENOMEM);
size = btrfs_getxattr(inode, name, value, size);
}
-   if (size > 0) {
+   if (size > 0)
acl = posix_acl_from_xattr(_user_ns, value, size);
-   } else if (size == -ERANGE || size == -ENODATA || size == 0) {
+   else if (size == -ENODATA || size == 0)
acl = NULL;
-   } else {
-   acl = ERR_PTR(-EIO);
-   }
+   else
+   acl = ERR_PTR(size);
kfree(value);
 
return acl;
-- 
2.17.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

Enabling quota may not correctly rescan on 4.17

2018-06-26 Thread Misono Tomohiro

Hello Nikolay,

I noticed that commit 5d23515be669 ("btrfs: Move qgroup rescan
on quota enable to btrfs_quota_enable") in 4.17 sometimes causes
to fail correctly rescanning quota when quota is enabled.

Simple reproducer:

$ mkfs.btrfs -f $DEV
$ mount $DEV /mnt
$ dd if=/dev/urandom of=/mnt/file bs=1000 count=1000
$ btrfs quota enbale /mnt
$ umount /mnt
$ btrfs check $DEV
...
checking quota groups
Counts for qgroup id: 0/5 are different
our:referenced 1019904 referenced compressed 1019904
disk:   referenced 16384 referenced compressed 16384
diff:   referenced 1003520 referenced compressed 1003520
our:exclusive 1019904 exclusive compressed 1019904
disk:   exclusive 16384 exclusive compressed 16384
diff:   exclusive 1003520 exclusive compressed 1003520
found 1413120 bytes used, error(s) found
...

This can be also observed in btrfs/114. (Note that progs < 4.17
returns error code 0 even if quota is not consistency and therefore
test will incorrectly pass.)

My observation is that this commit changed to call initial quota rescan
when quota is enabeld instead of first comit transaction after enabling
quota, and therefore if there is something not commited at that time,
their usage will not be accounted.

Actually this can be simply fixed by calling "btrfs rescan" again or
calling "btrfs fi sync" before "btrfs quota enable".

I think the commit itself makes the code much easier to read, so it may
be better to fix the problem in progs (i.e. calling sync before quota enable).

Do you have any thoughts?

Thanks,
Tomohiro Misono


--
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 v4 3/5] btrfs: remove unnecessary -ERANGE check in btrfs_get_acl()

[PATCH v4 5/5] btrfs: remove unnecessary bracket in btrfs_get_acl()

[PATCH v4 4/5] btrfs: avoid error code overriding in btrfs_get_acl()

[PATCH v4 1/5] btrfs: return error instead of crash when detecting unexpected type in btrfs_get_acl()

[PATCH v4 2/5] btrfs: replace empty string with NULL when getting attribute length in btrfs_get_acl()

[PATCH v4 0/5] code cleanups for btrfs_get_acl()

Re: Mailing list archives links update

Mailing list archives links update

Re: [PATCH v2] btrfs: Add graceful handling of V0 extents

Re: [PATCH v2] Btrfs: fix regression in btrfs_page_mkwrite() from vm_fault_t conversion

Re: [PATCH v2] btrfs: Add graceful handling of V0 extents

Re: [PATCH v3] btrfs: code cleanups for btrfs_get_acl()

Re: [PATCH v2] btrfs: Add graceful handling of V0 extents

Re: [PATCH v3] btrfs: code cleanups for btrfs_get_acl()

Re: [PATCH] btrfs: annotate unlikely branches after V0 extent type removal

Re: [PATCH 3/3] btrfs: fix race between mkfs and mount

Re: [PATCH] btrfs: annotate unlikely branches after V0 extent type removal

[PATCH] btrfs: annotate unlikely branches after V0 extent type removal

Re: [PATCH v2] btrfs: Add graceful handling of V0 extents

Re: [PATCH] btrfs: remove warnings superseded by refcount_t usage

[PATCH v2] btrfs: Add graceful handling of V0 extents

Re: [PATCH 3/3] btrfs: fix race between mkfs and mount

Re: btrfs raid10 performance

Re: [PATCH 7/7] btrfs: fix mount and ioctl device scan ioctl race

Re: [PATCH] btrfs: qgroups: Move transaction managed inside btrfs_quota_enable

Re: [PATCH] btrfs: qgroups: Move transaction managed inside btrfs_quota_enable

Re: [PATCH 2/7] btrfs: extend critical section when scanning a new device

[PATCH] btrfs: qgroups: Move transaction managed inside btrfs_quota_enable

Re: Enabling quota may not correctly rescan on 4.17

Re: [PATCH 3/3] btrfs: fix race between mkfs and mount

Re: [PATCH 3/3] btrfs: fix race between mkfs and mount

[PATCH v3] btrfs: code cleanups for btrfs_get_acl()

Enabling quota may not correctly rescan on 4.17

33 matches

Site Navigation

Mail list logo

Footer information