Re: Sparse Warnings about locks in extent-tree.c

[Liu Bo]

On Mon, Sep 22, 2014 at 09:57:35PM -0400, nick wrote:
> Hello Btfs Developers,
> I am new so am unsure of how to fix this but we are hitting some sparse 
> warnings about unlock/lock is having 
> a wrong count when exiting certain functions in extent-tree.c. I will paste 
> the warnings below for you guys.
> Hope this is of help.
> Cheers,
> Nick  
> fs/btrfs/extent-tree.c:6386:39: warning: context imbalance in 
> 'btrfs_lock_cluster' - wrong count at exit
> fs/btrfs/extent-tree.c:6663:44: warning: context imbalance in 
> 'find_free_extent' - unexpected unlock
> fs/btrfs/extent-tree.c:8810:9: warning: context imbalance in 
> 'btrfs_put_block_group_cache' - wrong count at exit

Hi Nick,

What program is producing these warning?  It looks like a static code analysis
tool or something.

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

[PATCH] btrfs: Make btrfs handle security mount options internally to avoid losing security label.

[Qu Wenruo]

[BUG]
Originally when mount btrfs with "-o subvol=" mount option, btrfs will
lose all security lable.
And if the btrfs fs is mounted somewhere else, due to the lost of
security lable, SELinux will refuse to mount since the same super block
is being mounted using different security lable.

[REPRODUCER]
With SELinux enabled:
 #mkfs -t btrfs /dev/sda5
 #mount -o context=system_u:object_r:nfs_t:s0 /dev/sda5 /mnt/btrfs
 #btrfs subvolume create /mnt/btrfs/subvol
 #mount -o subvol=subvol,context=system_u:object_r:nfs_t:s0 /dev/sda5
  /mnt/test

kernel message:
SELinux: mount invalid.  Same superblock, different security settings
for (dev sda5, type btrfs)

[REASON]
This happens because btrfs will call vfs_kern_mount() and then
mount_subtree() to handle subvolume name lookup.
First mount will cut off all the security lables and when it comes to
the second vfs_kern_mount(), it has no security label now.

[FIX]
This patch will makes btrfs behavior much more like nfs,
which has the type flag FS_BINARY_MOUNTDATA,
making btrfs handles the security label internally.
So security label will be set in the real mount time and won't lose
label when use with "subvol=" mount option.

Reported-by: Eryu Guan 
Signed-off-by: Qu Wenruo 
---
 fs/btrfs/ctree.h |  5 +++
 fs/btrfs/super.c | 97 +---
 2 files changed, 97 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 8e29b61..c82dd6d 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -34,6 +34,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "extent_io.h"
 #include "extent_map.h"
 #include "async-thread.h"
@@ -1723,6 +1724,9 @@ struct btrfs_fs_info {
 
/* Used to reclaim the metadata space in the background. */
struct work_struct async_reclaim_work;
+
+   /* For btrfs to record security options */
+   struct security_mnt_opts security_opts;
 };
 
 struct btrfs_subvolume_writers {
@@ -3604,6 +3608,7 @@ static inline void free_fs_info(struct btrfs_fs_info 
*fs_info)
kfree(fs_info->uuid_root);
kfree(fs_info->super_copy);
kfree(fs_info->super_for_commit);
+   security_free_mnt_opts(&fs_info->security_opts);
kfree(fs_info);
 }
 
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index c4124de..1eb7858 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1215,6 +1215,54 @@ static struct dentry *mount_subvol(const char 
*subvol_name, int flags,
return root;
 }
 
+static int parse_security_options(char *orig_opts,
+ struct security_mnt_opts *sec_opts)
+{
+   char *secdata = NULL;
+   int ret = 0;
+
+   secdata = alloc_secdata();
+   if (!secdata)
+   return -ENOMEM;
+   ret = security_sb_copy_data(orig_opts, secdata);
+   if (ret) {
+   free_secdata(secdata);
+   return ret;
+   }
+   ret = security_sb_parse_opts_str(secdata, sec_opts);
+   free_secdata(secdata);
+   return ret;
+}
+
+static int setup_security_options(struct btrfs_fs_info *fs_info,
+ struct super_block *sb,
+ struct security_mnt_opts *sec_opts)
+{
+   int ret = 0;
+
+   /*
+* Call security_sb_set_mnt_opts() to check whether new sec_opts
+* is valid.
+*/
+   ret = security_sb_set_mnt_opts(sb, sec_opts, 0, NULL);
+   if (ret)
+   return ret;
+
+   if (!fs_info->security_opts.num_mnt_opts) {
+   /* first time security setup, copy sec_opts to fs_info */
+   memcpy(&fs_info->security_opts, sec_opts, sizeof(*sec_opts));
+   } else {
+   /*
+* Since SELinux(the only one supports security_mnt_opts) does
+* NOT support changing context during remount/mount same sb,
+* This must be the same or part of the same security options,
+* just free it.
+*/
+   security_free_mnt_opts(sec_opts);
+   }
+   return ret;
+}
+
 /*
  * Find a superblock for the given device / mount point.
  *
@@ -1229,6 +1277,7 @@ static struct dentry *btrfs_mount(struct file_system_type 
*fs_type, int flags,
struct dentry *root;
struct btrfs_fs_devices *fs_devices = NULL;
struct btrfs_fs_info *fs_info = NULL;
+   struct security_mnt_opts new_sec_opts;
fmode_t mode = FMODE_READ;
char *subvol_name = NULL;
u64 subvol_objectid = 0;
@@ -1251,9 +1300,16 @@ static struct dentry *btrfs_mount(struct 
file_system_type *fs_type, int flags,
return root;
}
 
+   security_init_mnt_opts(&new_sec_opts);
+   if (data) {
+   error = parse_security_options(data, &new_sec_opts);
+   if (error)
+   return ERR_PTR(error);
+   }
+
error = btrfs_scan_one_device(device_name, mode, fs_type, &fs_devices);
if (error)

[PATCH v4 00/12] LLVMLinux: Patches to enable the kernel to be compiled with clang/LLVM

[behanw]

From: Behan Webster 

Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99
compliant equivalent. These patches allocate the appropriate amount of memory
using a char array using the SHASH_DESC_ON_STACK macro.

There are places in the kernel whose maintainers have previously taken our
patches to remove VLAIS from their crypto code. Once this patch set is accepted
into mainline, I'll go back and resubmit patches to these maintainers to use
this new macro so the same approach is used consistently in all places in the
kernel.

The LLVMLinux project aims to fully build the Linux kernel using both gcc and
clang (the C front end for the LLVM compiler infrastructure project). 


Behan Webster (6):
  crypto: LLVMLinux: Add macro to remove use of VLAIS in crypto code
  crypto: LLVMLinux: Remove VLAIS from crypto/mv_cesa.c
  crypto: LLVMLinux: Remove VLAIS from crypto/n2_core.c
  crypto: LLVMLinux: Remove VLAIS from crypto/omap_sham.c
  crypto: LLVMLinux: Remove VLAIS from crypto/.../qat_algs.c
  security, crypto: LLVMLinux: Remove VLAIS from ima_crypto.c

Jan-Simon Möller (5):
  crypto: LLVMLinux: Remove VLAIS from crypto/ccp/ccp-crypto-sha.c
  crypto, dm: LLVMLinux: Remove VLAIS usage from dm-crypt
  crypto: LLVMLinux: Remove VLAIS usage from crypto/hmac.c
  crypto: LLVMLinux: Remove VLAIS usage from libcrc32c.c
  crypto: LLVMLinux: Remove VLAIS usage from crypto/testmgr.c

Vinícius Tinti (1):
  btrfs: LLVMLinux: Remove VLAIS

 crypto/hmac.c| 25 -
 crypto/testmgr.c | 14 --
 drivers/crypto/ccp/ccp-crypto-sha.c  | 13 -
 drivers/crypto/mv_cesa.c | 41 
 drivers/crypto/n2_core.c | 11 +++-
 drivers/crypto/omap-sham.c   | 28 ---
 drivers/crypto/qat/qat_common/qat_algs.c | 31 ++---
 drivers/md/dm-crypt.c| 34 ++-
 fs/btrfs/hash.c  | 16 +--
 include/crypto/hash.h|  5 
 lib/libcrc32c.c  | 16 +--
 security/integrity/ima/ima_crypto.c  | 47 +---
 12 files changed, 122 insertions(+), 159 deletions(-)

-- 
1.9.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 05/12] crypto: LLVMLinux: Remove VLAIS from crypto/n2_core.c

[behanw]

From: Behan Webster 

Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99
compliant equivalent. This patch allocates the appropriate amount of memory
using a char array using the SHASH_DESC_ON_STACK macro.

The new code can be compiled with both gcc and clang.

Signed-off-by: Behan Webster 
Reviewed-by: Mark Charlebois 
Reviewed-by: Jan-Simon Möller 
Acked-by: Herbert Xu 
---
 drivers/crypto/n2_core.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/n2_core.c b/drivers/crypto/n2_core.c
index 7263c10..f8e3207 100644
--- a/drivers/crypto/n2_core.c
+++ b/drivers/crypto/n2_core.c
@@ -445,10 +445,7 @@ static int n2_hmac_async_setkey(struct crypto_ahash *tfm, 
const u8 *key,
struct n2_hmac_ctx *ctx = crypto_ahash_ctx(tfm);
struct crypto_shash *child_shash = ctx->child_shash;
struct crypto_ahash *fallback_tfm;
-   struct {
-   struct shash_desc shash;
-   char ctx[crypto_shash_descsize(child_shash)];
-   } desc;
+   SHASH_DESC_ON_STACK(shash, child_shash);
int err, bs, ds;
 
fallback_tfm = ctx->base.fallback_tfm;
@@ -456,15 +453,15 @@ static int n2_hmac_async_setkey(struct crypto_ahash *tfm, 
const u8 *key,
if (err)
return err;
 
-   desc.shash.tfm = child_shash;
-   desc.shash.flags = crypto_ahash_get_flags(tfm) &
+   shash->tfm = child_shash;
+   shash->flags = crypto_ahash_get_flags(tfm) &
CRYPTO_TFM_REQ_MAY_SLEEP;
 
bs = crypto_shash_blocksize(child_shash);
ds = crypto_shash_digestsize(child_shash);
BUG_ON(ds > N2_HASH_KEY_MAX);
if (keylen > bs) {
-   err = crypto_shash_digest(&desc.shash, key, keylen,
+   err = crypto_shash_digest(shash, key, keylen,
  ctx->hash_key);
if (err)
return err;
-- 
1.9.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 02/12] btrfs: LLVMLinux: Remove VLAIS

[behanw]

From: Vinícius Tinti 

Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99
compliant equivalent.  This patch instead allocates the appropriate amount of
memory using a char array using the SHASH_DESC_ON_STACK macro.

The new code can be compiled with both gcc and clang.

Signed-off-by: Vinícius Tinti 
Reviewed-by: Jan-Simon Möller 
Reviewed-by: Mark Charlebois 
Signed-off-by: Behan Webster 
Acked-by: Chris Mason 
Acked-by: Herbert Xu 
Cc: "David S. Miller" 
---
 fs/btrfs/hash.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/hash.c b/fs/btrfs/hash.c
index 85889aa..4bf4d3a 100644
--- a/fs/btrfs/hash.c
+++ b/fs/btrfs/hash.c
@@ -33,18 +33,16 @@ void btrfs_hash_exit(void)
 
 u32 btrfs_crc32c(u32 crc, const void *address, unsigned int length)
 {
-   struct {
-   struct shash_desc shash;
-   char ctx[crypto_shash_descsize(tfm)];
-   } desc;
+   SHASH_DESC_ON_STACK(shash, tfm);
+   u32 *ctx = (u32 *)shash_desc_ctx(shash);
int err;
 
-   desc.shash.tfm = tfm;
-   desc.shash.flags = 0;
-   *(u32 *)desc.ctx = crc;
+   shash->tfm = tfm;
+   shash->flags = 0;
+   *ctx = crc;
 
-   err = crypto_shash_update(&desc.shash, address, length);
+   err = crypto_shash_update(shash, address, length);
BUG_ON(err);
 
-   return *(u32 *)desc.ctx;
+   return *ctx;
 }
-- 
1.9.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 06/12] crypto: LLVMLinux: Remove VLAIS from crypto/omap_sham.c

[behanw]

From: Behan Webster 

Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99
compliant equivalent. This patch allocates the appropriate amount of memory
using a char array using the SHASH_DESC_ON_STACK macro.

The new code can be compiled with both gcc and clang.

Signed-off-by: Behan Webster 
Reviewed-by: Mark Charlebois 
Reviewed-by: Jan-Simon Möller 
Acked-by: Herbert Xu 
---
 drivers/crypto/omap-sham.c | 28 +++-
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
index 710d863..24ef489 100644
--- a/drivers/crypto/omap-sham.c
+++ b/drivers/crypto/omap-sham.c
@@ -949,17 +949,14 @@ static int omap_sham_finish_hmac(struct ahash_request 
*req)
struct omap_sham_hmac_ctx *bctx = tctx->base;
int bs = crypto_shash_blocksize(bctx->shash);
int ds = crypto_shash_digestsize(bctx->shash);
-   struct {
-   struct shash_desc shash;
-   char ctx[crypto_shash_descsize(bctx->shash)];
-   } desc;
+   SHASH_DESC_ON_STACK(shash, bctx->shash);
 
-   desc.shash.tfm = bctx->shash;
-   desc.shash.flags = 0; /* not CRYPTO_TFM_REQ_MAY_SLEEP */
+   shash->tfm = bctx->shash;
+   shash->flags = 0; /* not CRYPTO_TFM_REQ_MAY_SLEEP */
 
-   return crypto_shash_init(&desc.shash) ?:
-  crypto_shash_update(&desc.shash, bctx->opad, bs) ?:
-  crypto_shash_finup(&desc.shash, req->result, ds, req->result);
+   return crypto_shash_init(shash) ?:
+  crypto_shash_update(shash, bctx->opad, bs) ?:
+  crypto_shash_finup(shash, req->result, ds, req->result);
 }
 
 static int omap_sham_finish(struct ahash_request *req)
@@ -1118,18 +1115,15 @@ static int omap_sham_update(struct ahash_request *req)
return omap_sham_enqueue(req, OP_UPDATE);
 }
 
-static int omap_sham_shash_digest(struct crypto_shash *shash, u32 flags,
+static int omap_sham_shash_digest(struct crypto_shash *tfm, u32 flags,
  const u8 *data, unsigned int len, u8 *out)
 {
-   struct {
-   struct shash_desc shash;
-   char ctx[crypto_shash_descsize(shash)];
-   } desc;
+   SHASH_DESC_ON_STACK(shash, tfm);
 
-   desc.shash.tfm = shash;
-   desc.shash.flags = flags & CRYPTO_TFM_REQ_MAY_SLEEP;
+   shash->tfm = tfm;
+   shash->flags = flags & CRYPTO_TFM_REQ_MAY_SLEEP;
 
-   return crypto_shash_digest(&desc.shash, data, len, out);
+   return crypto_shash_digest(shash, data, len, out);
 }
 
 static int omap_sham_final_shash(struct ahash_request *req)
-- 
1.9.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 01/12] crypto: LLVMLinux: Add macro to remove use of VLAIS in crypto code

[behanw]

From: Behan Webster 

Add a macro which replaces the use of a Variable Length Array In Struct (VLAIS)
with a C99 compliant equivalent. This macro instead allocates the appropriate
amount of memory using an char array.

The new code can be compiled with both gcc and clang.

struct shash_desc contains a flexible array member member ctx declared with
CRYPTO_MINALIGN_ATTR, so sizeof(struct shash_desc) aligns the beginning
of the array declared after struct shash_desc with long long.

No trailing padding is required because it is not a struct type that can
be used in an array.

The CRYPTO_MINALIGN_ATTR is required so that desc is aligned with long long
as would be the case for a struct containing a member with
CRYPTO_MINALIGN_ATTR.

If you want to get to the ctx at the end of the shash_desc as before you can do
so using shash_desc_ctx(shash)

Signed-off-by: Behan Webster 
Reviewed-by: Mark Charlebois 
Acked-by: Herbert Xu 
Cc: Michał Mirosław 
---
 include/crypto/hash.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/include/crypto/hash.h b/include/crypto/hash.h
index a391955..74b13ec 100644
--- a/include/crypto/hash.h
+++ b/include/crypto/hash.h
@@ -58,6 +58,11 @@ struct shash_desc {
void *__ctx[] CRYPTO_MINALIGN_ATTR;
 };
 
+#define SHASH_DESC_ON_STACK(shash, ctx)  \
+   char __##shash##_desc[sizeof(struct shash_desc) + \
+   crypto_shash_descsize(ctx)] CRYPTO_MINALIGN_ATTR; \
+   struct shash_desc *shash = (struct shash_desc *)__##shash##_desc
+
 struct shash_alg {
int (*init)(struct shash_desc *desc);
int (*update)(struct shash_desc *desc, const u8 *data,
-- 
1.9.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 07/12] crypto: LLVMLinux: Remove VLAIS from crypto/.../qat_algs.c

[behanw]

From: Behan Webster 

Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99
compliant equivalent. This patch allocates the appropriate amount of memory
using a char array using the SHASH_DESC_ON_STACK macro.

The new code can be compiled with both gcc and clang.

Signed-off-by: Behan Webster 
Reviewed-by: Mark Charlebois 
Reviewed-by: Jan-Simon Möller 
Acked-by: Herbert Xu 
---
 drivers/crypto/qat/qat_common/qat_algs.c | 31 ++-
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/drivers/crypto/qat/qat_common/qat_algs.c 
b/drivers/crypto/qat/qat_common/qat_algs.c
index 59df488..9cabadd 100644
--- a/drivers/crypto/qat/qat_common/qat_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_algs.c
@@ -152,10 +152,7 @@ static int qat_alg_do_precomputes(struct 
icp_qat_hw_auth_algo_blk *hash,
  const uint8_t *auth_key,
  unsigned int auth_keylen, uint8_t *auth_state)
 {
-   struct {
-   struct shash_desc shash;
-   char ctx[crypto_shash_descsize(ctx->hash_tfm)];
-   } desc;
+   SHASH_DESC_ON_STACK(shash, ctx->hash_tfm);
struct sha1_state sha1;
struct sha256_state sha256;
struct sha512_state sha512;
@@ -167,12 +164,12 @@ static int qat_alg_do_precomputes(struct 
icp_qat_hw_auth_algo_blk *hash,
__be64 *hash512_state_out;
int i, offset;
 
-   desc.shash.tfm = ctx->hash_tfm;
-   desc.shash.flags = 0x0;
+   shash->tfm = ctx->hash_tfm;
+   shash->flags = 0x0;
 
if (auth_keylen > block_size) {
char buff[SHA512_BLOCK_SIZE];
-   int ret = crypto_shash_digest(&desc.shash, auth_key,
+   int ret = crypto_shash_digest(shash, auth_key,
  auth_keylen, buff);
if (ret)
return ret;
@@ -195,10 +192,10 @@ static int qat_alg_do_precomputes(struct 
icp_qat_hw_auth_algo_blk *hash,
*opad_ptr ^= 0x5C;
}
 
-   if (crypto_shash_init(&desc.shash))
+   if (crypto_shash_init(shash))
return -EFAULT;
 
-   if (crypto_shash_update(&desc.shash, ipad, block_size))
+   if (crypto_shash_update(shash, ipad, block_size))
return -EFAULT;
 
hash_state_out = (__be32 *)hash->sha.state1;
@@ -206,19 +203,19 @@ static int qat_alg_do_precomputes(struct 
icp_qat_hw_auth_algo_blk *hash,
 
switch (ctx->qat_hash_alg) {
case ICP_QAT_HW_AUTH_ALGO_SHA1:
-   if (crypto_shash_export(&desc.shash, &sha1))
+   if (crypto_shash_export(shash, &sha1))
return -EFAULT;
for (i = 0; i < digest_size >> 2; i++, hash_state_out++)
*hash_state_out = cpu_to_be32(*(sha1.state + i));
break;
case ICP_QAT_HW_AUTH_ALGO_SHA256:
-   if (crypto_shash_export(&desc.shash, &sha256))
+   if (crypto_shash_export(shash, &sha256))
return -EFAULT;
for (i = 0; i < digest_size >> 2; i++, hash_state_out++)
*hash_state_out = cpu_to_be32(*(sha256.state + i));
break;
case ICP_QAT_HW_AUTH_ALGO_SHA512:
-   if (crypto_shash_export(&desc.shash, &sha512))
+   if (crypto_shash_export(shash, &sha512))
return -EFAULT;
for (i = 0; i < digest_size >> 3; i++, hash512_state_out++)
*hash512_state_out = cpu_to_be64(*(sha512.state + i));
@@ -227,10 +224,10 @@ static int qat_alg_do_precomputes(struct 
icp_qat_hw_auth_algo_blk *hash,
return -EFAULT;
}
 
-   if (crypto_shash_init(&desc.shash))
+   if (crypto_shash_init(shash))
return -EFAULT;
 
-   if (crypto_shash_update(&desc.shash, opad, block_size))
+   if (crypto_shash_update(shash, opad, block_size))
return -EFAULT;
 
offset = round_up(qat_get_inter_state_size(ctx->qat_hash_alg), 8);
@@ -239,19 +236,19 @@ static int qat_alg_do_precomputes(struct 
icp_qat_hw_auth_algo_blk *hash,
 
switch (ctx->qat_hash_alg) {
case ICP_QAT_HW_AUTH_ALGO_SHA1:
-   if (crypto_shash_export(&desc.shash, &sha1))
+   if (crypto_shash_export(shash, &sha1))
return -EFAULT;
for (i = 0; i < digest_size >> 2; i++, hash_state_out++)
*hash_state_out = cpu_to_be32(*(sha1.state + i));
break;
case ICP_QAT_HW_AUTH_ALGO_SHA256:
-   if (crypto_shash_export(&desc.shash, &sha256))
+   if (crypto_shash_export(shash, &sha256))
return -EFAULT;
for (i = 0; i < digest_size >> 2; i++, hash_state_out++)
*hash_state_out = cpu_to_be32(*(sha256.state + i));
break;
case I

[PATCH v4 08/12] crypto, dm: LLVMLinux: Remove VLAIS usage from dm-crypt

[behanw]

From: Jan-Simon Möller 

Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99
compliant equivalent. This patch allocates the appropriate amount of memory
using a char array using the SHASH_DESC_ON_STACK macro.

The new code can be compiled with both gcc and clang.

Signed-off-by: Jan-Simon Möller 
Signed-off-by: Behan Webster 
Reviewed-by: Mark Charlebois 
Acked-by: Herbert Xu 
Cc: pagee...@freemail.hu
Cc: gmazyl...@gmail.com
Cc: "David S. Miller" 
---
 drivers/md/dm-crypt.c | 34 ++
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index cd15e08..fc93b93 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -526,29 +526,26 @@ static int crypt_iv_lmk_one(struct crypt_config *cc, u8 
*iv,
u8 *data)
 {
struct iv_lmk_private *lmk = &cc->iv_gen_private.lmk;
-   struct {
-   struct shash_desc desc;
-   char ctx[crypto_shash_descsize(lmk->hash_tfm)];
-   } sdesc;
+   SHASH_DESC_ON_STACK(desc, lmk->hash_tfm);
struct md5_state md5state;
__le32 buf[4];
int i, r;
 
-   sdesc.desc.tfm = lmk->hash_tfm;
-   sdesc.desc.flags = CRYPTO_TFM_REQ_MAY_SLEEP;
+   desc->tfm = lmk->hash_tfm;
+   desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
 
-   r = crypto_shash_init(&sdesc.desc);
+   r = crypto_shash_init(desc);
if (r)
return r;
 
if (lmk->seed) {
-   r = crypto_shash_update(&sdesc.desc, lmk->seed, LMK_SEED_SIZE);
+   r = crypto_shash_update(desc, lmk->seed, LMK_SEED_SIZE);
if (r)
return r;
}
 
/* Sector is always 512B, block size 16, add data of blocks 1-31 */
-   r = crypto_shash_update(&sdesc.desc, data + 16, 16 * 31);
+   r = crypto_shash_update(desc, data + 16, 16 * 31);
if (r)
return r;
 
@@ -557,12 +554,12 @@ static int crypt_iv_lmk_one(struct crypt_config *cc, u8 
*iv,
buf[1] = cpu_to_le32u64)dmreq->iv_sector >> 32) & 0x00FF) | 
0x8000);
buf[2] = cpu_to_le32(4024);
buf[3] = 0;
-   r = crypto_shash_update(&sdesc.desc, (u8 *)buf, sizeof(buf));
+   r = crypto_shash_update(desc, (u8 *)buf, sizeof(buf));
if (r)
return r;
 
/* No MD5 padding here */
-   r = crypto_shash_export(&sdesc.desc, &md5state);
+   r = crypto_shash_export(desc, &md5state);
if (r)
return r;
 
@@ -679,10 +676,7 @@ static int crypt_iv_tcw_whitening(struct crypt_config *cc,
struct iv_tcw_private *tcw = &cc->iv_gen_private.tcw;
u64 sector = cpu_to_le64((u64)dmreq->iv_sector);
u8 buf[TCW_WHITENING_SIZE];
-   struct {
-   struct shash_desc desc;
-   char ctx[crypto_shash_descsize(tcw->crc32_tfm)];
-   } sdesc;
+   SHASH_DESC_ON_STACK(desc, tcw->crc32_tfm);
int i, r;
 
/* xor whitening with sector number */
@@ -691,16 +685,16 @@ static int crypt_iv_tcw_whitening(struct crypt_config *cc,
crypto_xor(&buf[8], (u8 *)§or, 8);
 
/* calculate crc32 for every 32bit part and xor it */
-   sdesc.desc.tfm = tcw->crc32_tfm;
-   sdesc.desc.flags = CRYPTO_TFM_REQ_MAY_SLEEP;
+   desc->tfm = tcw->crc32_tfm;
+   desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
for (i = 0; i < 4; i++) {
-   r = crypto_shash_init(&sdesc.desc);
+   r = crypto_shash_init(desc);
if (r)
goto out;
-   r = crypto_shash_update(&sdesc.desc, &buf[i * 4], 4);
+   r = crypto_shash_update(desc, &buf[i * 4], 4);
if (r)
goto out;
-   r = crypto_shash_final(&sdesc.desc, &buf[i * 4]);
+   r = crypto_shash_final(desc, &buf[i * 4]);
if (r)
goto out;
}
-- 
1.9.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 03/12] crypto: LLVMLinux: Remove VLAIS from crypto/ccp/ccp-crypto-sha.c

[behanw]

From: Jan-Simon Möller 

Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99
compliant equivalent. This patch allocates the appropriate amount of memory
using a char array using the SHASH_DESC_ON_STACK macro.

The new code can be compiled with both gcc and clang.

Signed-off-by: Jan-Simon Möller 
Signed-off-by: Behan Webster 
Reviewed-by: Mark Charlebois 
Acked-by: Herbert Xu 
---
 drivers/crypto/ccp/ccp-crypto-sha.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/ccp/ccp-crypto-sha.c 
b/drivers/crypto/ccp/ccp-crypto-sha.c
index 873f234..9653157 100644
--- a/drivers/crypto/ccp/ccp-crypto-sha.c
+++ b/drivers/crypto/ccp/ccp-crypto-sha.c
@@ -198,10 +198,9 @@ static int ccp_sha_setkey(struct crypto_ahash *tfm, const 
u8 *key,
 {
struct ccp_ctx *ctx = crypto_tfm_ctx(crypto_ahash_tfm(tfm));
struct crypto_shash *shash = ctx->u.sha.hmac_tfm;
-   struct {
-   struct shash_desc sdesc;
-   char ctx[crypto_shash_descsize(shash)];
-   } desc;
+
+   SHASH_DESC_ON_STACK(sdesc, shash);
+
unsigned int block_size = crypto_shash_blocksize(shash);
unsigned int digest_size = crypto_shash_digestsize(shash);
int i, ret;
@@ -216,11 +215,11 @@ static int ccp_sha_setkey(struct crypto_ahash *tfm, const 
u8 *key,
 
if (key_len > block_size) {
/* Must hash the input key */
-   desc.sdesc.tfm = shash;
-   desc.sdesc.flags = crypto_ahash_get_flags(tfm) &
+   sdesc->tfm = shash;
+   sdesc->flags = crypto_ahash_get_flags(tfm) &
CRYPTO_TFM_REQ_MAY_SLEEP;
 
-   ret = crypto_shash_digest(&desc.sdesc, key, key_len,
+   ret = crypto_shash_digest(sdesc, key, key_len,
  ctx->u.sha.key);
if (ret) {
crypto_ahash_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
-- 
1.9.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 09/12] crypto: LLVMLinux: Remove VLAIS usage from crypto/hmac.c

[behanw]

From: Jan-Simon Möller 

Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99
compliant equivalent. This patch allocates the appropriate amount of memory
using a char array using the SHASH_DESC_ON_STACK macro.

The new code can be compiled with both gcc and clang.

Signed-off-by: Jan-Simon Möller 
Signed-off-by: Behan Webster 
Reviewed-by: Mark Charlebois 
Acked-by: Herbert Xu 
Cc: pagee...@freemail.hu
---
 crypto/hmac.c | 25 +++--
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/crypto/hmac.c b/crypto/hmac.c
index 8d9544c..e392219 100644
--- a/crypto/hmac.c
+++ b/crypto/hmac.c
@@ -52,20 +52,17 @@ static int hmac_setkey(struct crypto_shash *parent,
struct hmac_ctx *ctx = align_ptr(opad + ss,
 crypto_tfm_ctx_alignment());
struct crypto_shash *hash = ctx->hash;
-   struct {
-   struct shash_desc shash;
-   char ctx[crypto_shash_descsize(hash)];
-   } desc;
+   SHASH_DESC_ON_STACK(shash, hash);
unsigned int i;
 
-   desc.shash.tfm = hash;
-   desc.shash.flags = crypto_shash_get_flags(parent) &
-   CRYPTO_TFM_REQ_MAY_SLEEP;
+   shash->tfm = hash;
+   shash->flags = crypto_shash_get_flags(parent)
+   & CRYPTO_TFM_REQ_MAY_SLEEP;
 
if (keylen > bs) {
int err;
 
-   err = crypto_shash_digest(&desc.shash, inkey, keylen, ipad);
+   err = crypto_shash_digest(shash, inkey, keylen, ipad);
if (err)
return err;
 
@@ -81,12 +78,12 @@ static int hmac_setkey(struct crypto_shash *parent,
opad[i] ^= 0x5c;
}
 
-   return crypto_shash_init(&desc.shash) ?:
-  crypto_shash_update(&desc.shash, ipad, bs) ?:
-  crypto_shash_export(&desc.shash, ipad) ?:
-  crypto_shash_init(&desc.shash) ?:
-  crypto_shash_update(&desc.shash, opad, bs) ?:
-  crypto_shash_export(&desc.shash, opad);
+   return crypto_shash_init(shash) ?:
+  crypto_shash_update(shash, ipad, bs) ?:
+  crypto_shash_export(shash, ipad) ?:
+  crypto_shash_init(shash) ?:
+  crypto_shash_update(shash, opad, bs) ?:
+  crypto_shash_export(shash, opad);
 }
 
 static int hmac_export(struct shash_desc *pdesc, void *out)
-- 
1.9.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 04/12] crypto: LLVMLinux: Remove VLAIS from crypto/mv_cesa.c

[behanw]

From: Behan Webster 

Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99
compliant equivalent. This patch allocates the appropriate amount of memory
using a char array using the SHASH_DESC_ON_STACK macro.

The new code can be compiled with both gcc and clang.

Signed-off-by: Behan Webster 
Reviewed-by: Mark Charlebois 
Reviewed-by: Jan-Simon Möller 
Acked-by: Herbert Xu 
---
 drivers/crypto/mv_cesa.c | 41 ++---
 1 file changed, 18 insertions(+), 23 deletions(-)

diff --git a/drivers/crypto/mv_cesa.c b/drivers/crypto/mv_cesa.c
index 29d0ee5..032c72c 100644
--- a/drivers/crypto/mv_cesa.c
+++ b/drivers/crypto/mv_cesa.c
@@ -402,26 +402,23 @@ static int mv_hash_final_fallback(struct ahash_request 
*req)
 {
const struct mv_tfm_hash_ctx *tfm_ctx = crypto_tfm_ctx(req->base.tfm);
struct mv_req_hash_ctx *req_ctx = ahash_request_ctx(req);
-   struct {
-   struct shash_desc shash;
-   char ctx[crypto_shash_descsize(tfm_ctx->fallback)];
-   } desc;
+   SHASH_DESC_ON_STACK(shash, tfm_ctx->fallback);
int rc;
 
-   desc.shash.tfm = tfm_ctx->fallback;
-   desc.shash.flags = CRYPTO_TFM_REQ_MAY_SLEEP;
+   shash->tfm = tfm_ctx->fallback;
+   shash->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
if (unlikely(req_ctx->first_hash)) {
-   crypto_shash_init(&desc.shash);
-   crypto_shash_update(&desc.shash, req_ctx->buffer,
+   crypto_shash_init(shash);
+   crypto_shash_update(shash, req_ctx->buffer,
req_ctx->extra_bytes);
} else {
/* only SHA1 for now
 */
-   rc = mv_hash_import_sha1_ctx(req_ctx, &desc.shash);
+   rc = mv_hash_import_sha1_ctx(req_ctx, shash);
if (rc)
goto out;
}
-   rc = crypto_shash_final(&desc.shash, req->result);
+   rc = crypto_shash_final(shash, req->result);
 out:
return rc;
 }
@@ -794,23 +791,21 @@ static int mv_hash_setkey(struct crypto_ahash *tfm, const 
u8 * key,
ss = crypto_shash_statesize(ctx->base_hash);
 
{
-   struct {
-   struct shash_desc shash;
-   char ctx[crypto_shash_descsize(ctx->base_hash)];
-   } desc;
+   SHASH_DESC_ON_STACK(shash, ctx->base_hash);
+
unsigned int i;
char ipad[ss];
char opad[ss];
 
-   desc.shash.tfm = ctx->base_hash;
-   desc.shash.flags = crypto_shash_get_flags(ctx->base_hash) &
+   shash->tfm = ctx->base_hash;
+   shash->flags = crypto_shash_get_flags(ctx->base_hash) &
CRYPTO_TFM_REQ_MAY_SLEEP;
 
if (keylen > bs) {
int err;
 
err =
-   crypto_shash_digest(&desc.shash, key, keylen, ipad);
+   crypto_shash_digest(shash, key, keylen, ipad);
if (err)
return err;
 
@@ -826,12 +821,12 @@ static int mv_hash_setkey(struct crypto_ahash *tfm, const 
u8 * key,
opad[i] ^= 0x5c;
}
 
-   rc = crypto_shash_init(&desc.shash) ? :
-   crypto_shash_update(&desc.shash, ipad, bs) ? :
-   crypto_shash_export(&desc.shash, ipad) ? :
-   crypto_shash_init(&desc.shash) ? :
-   crypto_shash_update(&desc.shash, opad, bs) ? :
-   crypto_shash_export(&desc.shash, opad);
+   rc = crypto_shash_init(shash) ? :
+   crypto_shash_update(shash, ipad, bs) ? :
+   crypto_shash_export(shash, ipad) ? :
+   crypto_shash_init(shash) ? :
+   crypto_shash_update(shash, opad, bs) ? :
+   crypto_shash_export(shash, opad);
 
if (rc == 0)
mv_hash_init_ivs(ctx, ipad, opad);
-- 
1.9.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 11/12] security, crypto: LLVMLinux: Remove VLAIS from ima_crypto.c

[behanw]

From: Behan Webster 

Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99
compliant equivalent. This patch allocates the appropriate amount of memory
using a char array using the SHASH_DESC_ON_STACK macro.

The new code can be compiled with both gcc and clang.

Signed-off-by: Behan Webster 
Reviewed-by: Mark Charlebois 
Reviewed-by: Jan-Simon Möller 
Acked-by: Herbert Xu 
Cc: t...@linutronix.de
---
 security/integrity/ima/ima_crypto.c | 47 +++--
 1 file changed, 19 insertions(+), 28 deletions(-)

diff --git a/security/integrity/ima/ima_crypto.c 
b/security/integrity/ima/ima_crypto.c
index 0bd7328..e35f5d9 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -380,17 +380,14 @@ static int ima_calc_file_hash_tfm(struct file *file,
loff_t i_size, offset = 0;
char *rbuf;
int rc, read = 0;
-   struct {
-   struct shash_desc shash;
-   char ctx[crypto_shash_descsize(tfm)];
-   } desc;
+   SHASH_DESC_ON_STACK(shash, tfm);
 
-   desc.shash.tfm = tfm;
-   desc.shash.flags = 0;
+   shash->tfm = tfm;
+   shash->flags = 0;
 
hash->length = crypto_shash_digestsize(tfm);
 
-   rc = crypto_shash_init(&desc.shash);
+   rc = crypto_shash_init(shash);
if (rc != 0)
return rc;
 
@@ -420,7 +417,7 @@ static int ima_calc_file_hash_tfm(struct file *file,
break;
offset += rbuf_len;
 
-   rc = crypto_shash_update(&desc.shash, rbuf, rbuf_len);
+   rc = crypto_shash_update(shash, rbuf, rbuf_len);
if (rc)
break;
}
@@ -429,7 +426,7 @@ static int ima_calc_file_hash_tfm(struct file *file,
kfree(rbuf);
 out:
if (!rc)
-   rc = crypto_shash_final(&desc.shash, hash->digest);
+   rc = crypto_shash_final(shash, hash->digest);
return rc;
 }
 
@@ -487,18 +484,15 @@ static int ima_calc_field_array_hash_tfm(struct 
ima_field_data *field_data,
 struct ima_digest_data *hash,
 struct crypto_shash *tfm)
 {
-   struct {
-   struct shash_desc shash;
-   char ctx[crypto_shash_descsize(tfm)];
-   } desc;
+   SHASH_DESC_ON_STACK(shash, tfm);
int rc, i;
 
-   desc.shash.tfm = tfm;
-   desc.shash.flags = 0;
+   shash->tfm = tfm;
+   shash->flags = 0;
 
hash->length = crypto_shash_digestsize(tfm);
 
-   rc = crypto_shash_init(&desc.shash);
+   rc = crypto_shash_init(shash);
if (rc != 0)
return rc;
 
@@ -508,7 +502,7 @@ static int ima_calc_field_array_hash_tfm(struct 
ima_field_data *field_data,
u32 datalen = field_data[i].len;
 
if (strcmp(td->name, IMA_TEMPLATE_IMA_NAME) != 0) {
-   rc = crypto_shash_update(&desc.shash,
+   rc = crypto_shash_update(shash,
(const u8 *) &field_data[i].len,
sizeof(field_data[i].len));
if (rc)
@@ -518,13 +512,13 @@ static int ima_calc_field_array_hash_tfm(struct 
ima_field_data *field_data,
data_to_hash = buffer;
datalen = IMA_EVENT_NAME_LEN_MAX + 1;
}
-   rc = crypto_shash_update(&desc.shash, data_to_hash, datalen);
+   rc = crypto_shash_update(shash, data_to_hash, datalen);
if (rc)
break;
}
 
if (!rc)
-   rc = crypto_shash_final(&desc.shash, hash->digest);
+   rc = crypto_shash_final(shash, hash->digest);
 
return rc;
 }
@@ -565,15 +559,12 @@ static int __init ima_calc_boot_aggregate_tfm(char 
*digest,
 {
u8 pcr_i[TPM_DIGEST_SIZE];
int rc, i;
-   struct {
-   struct shash_desc shash;
-   char ctx[crypto_shash_descsize(tfm)];
-   } desc;
+   SHASH_DESC_ON_STACK(shash, tfm);
 
-   desc.shash.tfm = tfm;
-   desc.shash.flags = 0;
+   shash->tfm = tfm;
+   shash->flags = 0;
 
-   rc = crypto_shash_init(&desc.shash);
+   rc = crypto_shash_init(shash);
if (rc != 0)
return rc;
 
@@ -581,10 +572,10 @@ static int __init ima_calc_boot_aggregate_tfm(char 
*digest,
for (i = TPM_PCR0; i < TPM_PCR8; i++) {
ima_pcrread(i, pcr_i);
/* now accumulate with current aggregate */
-   rc = crypto_shash_update(&desc.shash, pcr_i, TPM_DIGEST_SIZE);
+   rc = crypto_shash_update(shash, pcr_i, TPM_DIGEST_SIZE);
}
if (!rc)
-   crypto_shash_final(&desc.shash, digest);
+   crypto_shash_final(shash, digest);
return rc;
 }
 
-- 
1.9.1

--
To unsubscribe

[PATCH v4 10/12] crypto: LLVMLinux: Remove VLAIS usage from libcrc32c.c

[behanw]

From: Jan-Simon Möller 

Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99
compliant equivalent. This patch allocates the appropriate amount of memory
using a char array using the SHASH_DESC_ON_STACK macro.

The new code can be compiled with both gcc and clang.

Signed-off-by: Jan-Simon Möller 
Signed-off-by: Behan Webster 
Reviewed-by: Mark Charlebois 
Acked-by: Herbert Xu 
Cc: pagee...@freemail.hu
Cc: "David S. Miller" 
---
 lib/libcrc32c.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/lib/libcrc32c.c b/lib/libcrc32c.c
index b3131f5..6a08ce7 100644
--- a/lib/libcrc32c.c
+++ b/lib/libcrc32c.c
@@ -41,20 +41,18 @@ static struct crypto_shash *tfm;
 
 u32 crc32c(u32 crc, const void *address, unsigned int length)
 {
-   struct {
-   struct shash_desc shash;
-   char ctx[crypto_shash_descsize(tfm)];
-   } desc;
+   SHASH_DESC_ON_STACK(shash, tfm);
+   u32 *ctx = (u32 *)shash_desc_ctx(shash);
int err;
 
-   desc.shash.tfm = tfm;
-   desc.shash.flags = 0;
-   *(u32 *)desc.ctx = crc;
+   shash->tfm = tfm;
+   shash->flags = 0;
+   *ctx = crc;
 
-   err = crypto_shash_update(&desc.shash, address, length);
+   err = crypto_shash_update(shash, address, length);
BUG_ON(err);
 
-   return *(u32 *)desc.ctx;
+   return *ctx;
 }
 
 EXPORT_SYMBOL(crc32c);
-- 
1.9.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 12/12] crypto: LLVMLinux: Remove VLAIS usage from crypto/testmgr.c

[behanw]

From: Jan-Simon Möller 

Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99
compliant equivalent. This patch allocates the appropriate amount of memory
using a char array using the SHASH_DESC_ON_STACK macro.

The new code can be compiled with both gcc and clang.

Signed-off-by: Jan-Simon Möller 
Signed-off-by: Behan Webster 
Reviewed-by: Mark Charlebois 
Acked-by: Herbert Xu 
Cc: pagee...@freemail.hu
---
 crypto/testmgr.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index ac2b631..b959c0c 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -1714,16 +1714,14 @@ static int alg_test_crc32c(const struct alg_test_desc 
*desc,
}
 
do {
-   struct {
-   struct shash_desc shash;
-   char ctx[crypto_shash_descsize(tfm)];
-   } sdesc;
+   SHASH_DESC_ON_STACK(shash, tfm);
+   u32 *ctx = (u32 *)shash_desc_ctx(shash);
 
-   sdesc.shash.tfm = tfm;
-   sdesc.shash.flags = 0;
+   shash->tfm = tfm;
+   shash->flags = 0;
 
-   *(u32 *)sdesc.ctx = le32_to_cpu(420553207);
-   err = crypto_shash_final(&sdesc.shash, (u8 *)&val);
+   *ctx = le32_to_cpu(420553207);
+   err = crypto_shash_final(shash, (u8 *)&val);
if (err) {
printk(KERN_ERR "alg: crc32c: Operation failed for "
   "%s: %d\n", driver, err);
-- 
1.9.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] btrfs-progs: fix page align issue for lzo compress in restore

[Gui Hecheng]

When runing restore under lzo compression, "bad compress length"
problems are encountered.
It is because there is a page align problem with the @decompress_lzo,
as follows:
|--| ||-| |--|...|--|
  page ^page   page
   |
  3 bytes left

When lzo compress pages im RAM, lzo will ensure that
the 4 bytes len will be in one page as a whole.
There is a situation that 3 (or less) bytes are left
at the end of a page, and then the 4 bytes len is
stored at the start of the next page.
But the @decompress_lzo doesn't goto the start of
the next page and continue to read the next 4 bytes
which is across two pages, so a random value is fetched
as a "bad compress length".

So we check page alignment every time before we are going to
fetch the next @len and after the former piece of data is decompressed.
If the current page that we reach has less than 4 bytes left,
then we should fetch the next @len at the start of next page.

Signed-off-by: Gui Hecheng 
Reviewed-by: Marc Dietrich 
---
changelog
v1->v2: adopt alignment check method suggested by Marc
v2->v3: make code more readable
v3->v4: keep type safety
---
 cmds-restore.c | 29 +++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/cmds-restore.c b/cmds-restore.c
index 38a131e..fa5d5d1 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -56,7 +56,10 @@ static int get_xattrs = 0;
 static int dry_run = 0;
 
 #define LZO_LEN 4
-#define PAGE_CACHE_SIZE 4096
+#define PAGE_CACHE_SIZE 4096UL
+#define PAGE_CACHE_MASK (~(PAGE_CACHE_SIZE - 1))
+#define PAGE_CACHE_ALIGN(addr) (((addr) + PAGE_CACHE_SIZE - 1) \
+   & PAGE_CACHE_MASK)
 #define lzo1x_worst_compress(x) ((x) + ((x) / 16) + 64 + 3)
 
 static int decompress_zlib(char *inbuf, char *outbuf, u64 compress_len,
@@ -93,6 +96,28 @@ static inline size_t read_compress_length(unsigned char *buf)
return le32_to_cpu(dlen);
 }
 
+static void align_if_need(size_t *tot_in, size_t *in_len)
+{
+   size_t tot_in_aligned;
+   size_t bytes_left;
+
+   tot_in_aligned = PAGE_CACHE_ALIGN(*tot_in);
+   bytes_left = tot_in_aligned - *tot_in;
+
+   if (bytes_left >= LZO_LEN)
+   return;
+
+   /*
+* The LZO_LEN bytes is guaranteed to be
+* in one page as a whole, so if a page
+* has fewer than LZO_LEN bytes left,
+* the LZO_LEN bytes should be fetched
+* at the start of the next page
+*/
+   *in_len += bytes_left;
+   *tot_in = tot_in_aligned;
+}
+
 static int decompress_lzo(unsigned char *inbuf, char *outbuf, u64 compress_len,
  u64 *decompress_len)
 {
@@ -135,8 +160,8 @@ static int decompress_lzo(unsigned char *inbuf, char 
*outbuf, u64 compress_len,
}
out_len += new_len;
outbuf += new_len;
+   align_if_need(&tot_in, &in_len);
inbuf += in_len;
-   tot_in += in_len;
}
 
*decompress_len = out_len;
-- 
1.8.1.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

Sparse Warnings about locks in extent-tree.c

[nick]

Hello Btfs Developers,
I am new so am unsure of how to fix this but we are hitting some sparse 
warnings about unlock/lock is having 
a wrong count when exiting certain functions in extent-tree.c. I will paste the 
warnings below for you guys.
Hope this is of help.
Cheers,
Nick  
fs/btrfs/extent-tree.c:6386:39: warning: context imbalance in 
'btrfs_lock_cluster' - wrong count at exit
fs/btrfs/extent-tree.c:6663:44: warning: context imbalance in 
'find_free_extent' - unexpected unlock
fs/btrfs/extent-tree.c:8810:9: warning: context imbalance in 
'btrfs_put_block_group_cache' - wrong count at exit
--
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-progs: fix page align issue for lzo compress in restore

[Gui Hecheng]

On Mon, 2014-09-22 at 15:41 +0200, David Sterba wrote:
> On Mon, Sep 22, 2014 at 04:58:26PM +0800, Gui Hecheng wrote:
> > So we check page alignment every time before we are going to
> > fetch the next @len and after the former piece of data is decompressed.
> > If the current page that we reach has less than 4 bytes left,
> > then we should fetch the next @len at the start of next page.
> 
> Thanks for the fix.
> 
> > --- a/cmds-restore.c
> > +++ b/cmds-restore.c
> > @@ -57,6 +57,9 @@ static int dry_run = 0;
> >  
> >  #define LZO_LEN 4
> >  #define PAGE_CACHE_SIZE 4096
> > +#define PAGE_CACHE_MASK (~(PAGE_CACHE_SIZE - 1))
> > +#define PAGE_CACHE_ALIGN(addr) (((addr) + PAGE_CACHE_SIZE - 1) \
> > +   & PAGE_CACHE_MASK)
> 
> This is not type-safe, the PAGE_CACHE_SIZE should be unsigned long.
> 
> >  #define lzo1x_worst_compress(x) ((x) + ((x) / 16) + 64 + 3)
> >  
> >  static int decompress_zlib(char *inbuf, char *outbuf, u64 compress_len,
> > @@ -93,6 +96,28 @@ static inline size_t read_compress_length(unsigned char 
> > *buf)
> > return le32_to_cpu(dlen);
> >  }
> >  
> > +static void align_if_need(size_t *tot_in, size_t *in_len)
> > +{
> > +   int tot_in_aligned;
> > +   int bytes_left;
> > +
> > +   tot_in_aligned = PAGE_CACHE_ALIGN(*tot_in);
> 
> size_t -> int, plus other tricks that happen inside the macro
> 
> > +   bytes_left = tot_in_aligned - *tot_in;
> 
> int = int - size_t
> 
> > +
> > +   if (bytes_left >= LZO_LEN)
> > +   return;
> > +
> > +   /*
> > +* The LZO_LEN bytes is guaranteed to be
> > +* in one page as a whole, so if a page
> > +* has fewer than LZO_LEN bytes left,
> > +* the LZO_LEN bytes should be fetched
> > +* at the start of the next page
> > +*/
> 
> Nitpick, the comment can use the whole width of the line
> 
>   /*
>* The LZO_LEN bytes is guaranteed to be in one page as a whole,
>* so if a page has fewer than LZO_LEN bytes left, the LZO_LEN
>* bytes should be fetched at the start of the next page
>*/
> 
> > +   *in_len += bytes_left;
> > +   *tot_in = tot_in_aligned;
> > +}

Thanks David, I will pay more attention to the type-safe issue and
resend.

-Gui

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

[no subject]

[Erdal KÜÇÜK]

Your mailbox is almost full.
20GB23GB

Your mailbox needed to update now (Click 
Update) Update it now,and 
Increase your Mail Quota.
IT Services Help Desk
--
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: general thoughts and questions + general and RAID5/6 stability?

[Stefan G. Weichinger]

Am 20.09.2014 um 11:32 schrieb Duncan:

> What I do as part of my regular backup regime, is every few kernel cycles 
> I wipe the (first level) backup and do a fresh mkfs.btrfs, activating new 
> optional features as I believe appropriate.  Then I boot to the new 
> backup and run a bit to test it, then wipe the normal working copy and do 
> a fresh mkfs.btrfs on it, again with the new optional features enabled 
> that I want.

Is re-creating btrfs-filesystems *recommended* in any way?

Does that actually make a difference in the fs-structure?

So far I assumed it was enough to keep the kernel up2date, use current
(stable) btrfs-progs and run some scrub every week or so (not to mention
backups .. if it ain't backed up, it was/isn't important).

Stefan




--
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: super-recover: fix double free fs_devices memory

[Eric Sandeen]

On 9/18/14 4:01 AM, Wang Shilong wrote:
> super-recover collects btrfs devices infomation using existed
> functions scan_one_devices().
> 
> Problem is fs_devices is freed twice in close_ctree() and
> free_recover_superblock() for super correction path.
> 
> Fix this problem by checking whether fs_devices memory
> have been freed before we free it.
> 
> Cc: Eric Sandeen 
> Cc: Chris Murphy 
> Signed-off-by: Wang Shilong 

That does seem to fix the testcase.  Thanks!

Acked-by: Eric Sandeen 

> ---
>  super-recover.c | 13 +++--
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/super-recover.c b/super-recover.c
> index 767de4b..419b86a 100644
> --- a/super-recover.c
> +++ b/super-recover.c
> @@ -69,21 +69,11 @@ void init_recover_superblock(struct 
> btrfs_recover_superblock *recover)
>  static
>  void free_recover_superblock(struct btrfs_recover_superblock *recover)
>  {
> - struct btrfs_device *device;
>   struct super_block_record *record;
>  
>   if (!recover->fs_devices)
>   return;
>  
> - while (!list_empty(&recover->fs_devices->devices)) {
> - device = list_entry(recover->fs_devices->devices.next,
> - struct btrfs_device, dev_list);
> - list_del_init(&device->dev_list);
> - free(device->name);
> - free(device);
> - }
> - free(recover->fs_devices);
> -
>   while (!list_empty(&recover->good_supers)) {
>   record = list_entry(recover->good_supers.next,
>   struct super_block_record, list);
> @@ -341,6 +331,9 @@ int btrfs_recover_superblocks(const char *dname,
>  no_recover:
>   recover_err_str(ret);
>   free_recover_superblock(&recover);
> + /* check if we have freed fs_deivces in close_ctree() */
> + if (!root)
> + btrfs_close_devices(recover.fs_devices);
>   return ret;
>  }
>  
> 

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

Corrupted metadata. 'btrfs check' fails repair due to an assertion failure: '!(root->ref_cows && trans->transid != root->last_trans)'

[Jaap Pieroen]

Also filed on bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=85011

My log files are spammed with these messages until the drive is full:
Sep 22 21:27:38 nasbak kernel: [ 1911.486180] BTRFS critical (device sdg): 
corrupt leaf, bad key order: block=13415158087680,root=1, slot=7
Sep 22 21:27:38 nasbak kernel: [ 1911.487179] BTRFS critical (device sdg): 
corrupt leaf, bad key order: block=13415158087680,root=1, slot=7
Sep 22 21:27:38 nasbak kernel: [ 1911.488334] BTRFS critical (device sdg): 
corrupt leaf, bad key order: block=13415158087680,root=1, slot=7
Sep 22 21:27:38 nasbak kernel: [ 1911.489497] BTRFS critical (device sdg): 
corrupt leaf, bad key order: block=13415158087680,root=1, slot=7
Sep 22 21:27:38 nasbak kernel: [ 1911.490512] BTRFS critical (device sdg): 
corrupt leaf, bad key order: block=13415158087680,root=1, slot=7
Sep 22 21:27:38 nasbak kernel: [ 1911.491551] BTRFS critical (device sdg): 
corrupt leaf, bad key order: block=13415158087680,root=1, slot=7
Sep 22 21:27:38 nasbak kernel: [ 1911.492557] BTRFS critical (device sdg): 
corrupt leaf, bad key order: block=13415158087680,root=1, slot=7

Memory cards have been tested for 72hours with no problems to be found. 
Visiting the IRC channel learned me that I have corrupted meta data, which 
should be fixed by 'btrfs check'. Unfortunately 'btrfs check' stops due to an 
assertion failure:

# btrfs check --repair /dev/sdh
enabling repair mode
Checking filesystem on /dev/sdh
UUID: 20ccaf09-54ea-486e-9495-9dc91b933e9c
checking extents
bad key ordering 7 8
btrfs: ctree.c:267: __btrfs_cow_block: Assertion '!(root->ref_cows && 
trans->transid != root->last_trans)' failed.

I'm also unable to provide an image due to another assertion failure:

# btrfs-image -c9 -t4 /dev/sdh btrfs-image
btrfs-image: disk-io.c:155: readahead_tree_block: Assertion `!(ret)' failed.

I'm using the latest tools and latest stable kernel:

# uname -a
Linux computername 3.16.2-031602-generic #201409052035 SMP Sat Sep 6 00:36:44 
UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
# btrfs --version
Btrfs v3.16

Other relevant information:

# cat /etc/fstab
UUID=f6162bfd-f793-4541-828c-be31550e0271 /   ext4
discard,noatime,errors=remount-ro 0   1
UUID=20ccaf09-54ea-486e-9495-9dc91b933e9c /home   btrfs   
defaults,subvol=home   0   0
# sudo mount /home/
# dmesg
[  414.659393] BTRFS info (device sdh): disk space caching is enabled
[  700.423512] init: smbd main process (1006) killed by TERM signal
[  738.454018] BTRFS info (device sdh): disk space caching is enabled
[  739.318381] BTRFS: bdev /dev/sde errs: wr 0, rd 0, flush 0, corrupt 6, gen 0
[  739.318389] BTRFS: bdev /dev/sdd errs: wr 0, rd 0, flush 0, corrupt 6, gen 0
[  739.318393] BTRFS: bdev /dev/sdb errs: wr 0, rd 0, flush 0, corrupt 16, gen 0
[  739.318397] BTRFS: bdev /dev/sda errs: wr 0, rd 0, flush 0, corrupt 24, gen 0
# btrfs fi show
Label: 'btrfs_storage_2'  uuid: 20ccaf09-54ea-486e-9495-9dc91b933e9c
Total devices 6 FS bytes used 5.08TiB
devid1 size 2.73TiB used 2.73TiB path /dev/sdb
devid2 size 2.73TiB used 2.73TiB path /dev/sda
devid3 size 1.82TiB used 1.82TiB path /dev/sdd
devid4 size 1.82TiB used 1.82TiB path /dev/sde
devid6 size 2.73TiB used 2.73TiB path /dev/sdg
devid7 size 2.73TiB used 2.73TiB path /dev/sdh
# btrfs fi df /home
Data, RAID10: total=7.27TiB, used=5.07TiB
System, RAID10: total=64.00MiB, used=1.14MiB
Metadata, RAID10: total=8.12GiB, used=6.90GiB
unknown, single: total=512.00MiB, used=0.00

Attached you'll find the output of the command: '# btrfs-debug-tree -b 
13415158087680 /dev/sdh'

I'm now locked into a situation where my filesystem is buggy(locking up), slow 
and logfiles are continuously filled. There is no way out because btrfs doesn't 
allow me to delete the files in the affected block or fix the metadata 
corruption. I'd very much appreciate a patch for btrfs check, or perhaps a 
workaround which allows me to get rid of the block. I'm very willing to help 
debug the issue further.

Thanks for your time.--
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: device delete progress

[Duncan]

Duncan posted on Mon, 22 Sep 2014 16:29:26 + as excerpted:

> David Sterba posted on Mon, 22 Sep 2014 11:49:55 +0200 as excerpted:
> 
>> The upcomming 3.17 release should contain all the 'new df' patches that
>> were unfortunatelly delayed. There are some minor issues to be fixed
>> but I hope it'll be ready close to the kernel 3.17 release.
> 
> Will that fix my "unknown" chunk profiles too?

Never mind.  I see that in the 3.16.1 announcement, which I imagine I'll 
get next update.

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman

--
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: device delete progress

[Duncan]

David Sterba posted on Mon, 22 Sep 2014 11:49:55 +0200 as excerpted:

> The upcomming 3.17 release should contain all the 'new df' patches that
> were unfortunatelly delayed. There are some minor issues to be fixed but
> I hope it'll be ready close to the kernel 3.17 release.

Will that fix my "unknown" chunk profiles too?

I expected either userspace 3.16 or kernel 3.17 to fix that, but I'm 
running both (well, kernel 3.17 git, since the release hasn't happened 
yet) and still seeing the unknown chunk profiles.

Either that, or if userspace 3.16 and kernel 3.17 should have fixed it, 
I'm triggering a bug somewhere.

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman

--
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: be aware of btree inode write errors to avoid fs corruption

[Filipe Manana]

While we have a transaction ongoing, the VM might decide at any time
to call btree_inode->i_mapping->a_ops->writepages(), which will start
writeback of dirty pages belonging to btree nodes/leafs. This call
might return an error or the writeback might finish with an error
before we attempt to commit the running transaction. If this happens,
we might have no way of knowing that such error happened when we are
committing the transaction - because the pages might no longer be
marked dirty nor tagged for writeback (if a subsequent modification
to the extent buffer didn't happen before the transaction commit) which
makes filemap_fdata[write|wait]_range unable to find such pages (even
if they're marked with SetPageError).
So if this happens we must abort the transaction, otherwise we commit
a super block with btree roots that point to btree nodes/leafs whose
content on disk is invalid - either garbage or the content of some
node/leaf from a past generation that got cowed or deleted and is no
longer valid (for this later case we end up getting error messages like
"parent transid verify failed on 10826481664 wanted 25748 found 29562"
when reading btree nodes/leafs from disk).

Note that setting and checking AS_EIO/AS_ENOSPC in the btree inode's
i_mapping would not be enough because we need to distinguish between
log tree extents (not fatal) vs non-log tree extents (fatal) and
because the next call to filemap_fdatawait_range() will catch and clear
such errors in the mapping - and that call might be from a log sync and
not from a transaction commit, which means we would not know about the
error at transaction commit time. Also, checking for the eb flag
EXTENT_BUFFER_IOERR at transaction commit time isn't done and would
not be completely reliable, as the eb might be removed from memory and
read back when trying to get it, which clears that flag right before
reading the eb's pages from disk, making us not know about the previous
write error.

Using the BTRFS_INODE_BTREE_IO_ERR and BTRFS_INODE_BTREE_LOG_IO_ERR
inode flags also makes us achieve the goal of AS_EIO/AS_ENOSPC when
writepages() returns success, started writeback for all dirty pages
and before filemap_fdatawait_range() is called, the writeback for
all dirty pages had already finished with errors - because we were
not using AS_EIO/AS_ENOSPC, filemap_fdatawait_range() would return
success, as it could not know that writeback errors happened (the
pages were no longer tagged for writeback).

Signed-off-by: Filipe Manana 
---
 fs/btrfs/btrfs_inode.h |  2 ++
 fs/btrfs/extent_io.c   | 69 +++---
 fs/btrfs/transaction.c | 20 ---
 fs/btrfs/transaction.h |  3 +--
 fs/btrfs/tree-log.c| 13 ++
 5 files changed, 93 insertions(+), 14 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 3511031..dbe37dc 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -44,6 +44,8 @@
 #define BTRFS_INODE_IN_DELALLOC_LIST   9
 #define BTRFS_INODE_READDIO_NEED_LOCK  10
 #define BTRFS_INODE_HAS_PROPS  11
+#define BTRFS_INODE_BTREE_IO_ERR   12
+#define BTRFS_INODE_BTREE_LOG_IO_ERR   13
 
 /* in memory btrfs inode */
 struct btrfs_inode {
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 91f866c..33b113b 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -20,6 +20,7 @@
 #include "locking.h"
 #include "rcu-string.h"
 #include "backref.h"
+#include "transaction.h"
 
 static struct kmem_cache *extent_state_cache;
 static struct kmem_cache *extent_buffer_cache;
@@ -3606,6 +3607,68 @@ static void end_extent_buffer_writeback(struct 
extent_buffer *eb)
wake_up_bit(&eb->bflags, EXTENT_BUFFER_WRITEBACK);
 }
 
+static void set_btree_ioerr(struct page *page, int err)
+{
+   struct extent_buffer *eb = (struct extent_buffer *)page->private;
+   const u64 start = eb->start;
+   const u64 end = eb->start + eb->len - 1;
+   struct btrfs_fs_info *fs_info = eb->fs_info;
+   int ret;
+
+   set_bit(EXTENT_BUFFER_IOERR, &eb->bflags);
+   SetPageError(page);
+
+   /*
+* If writeback for a btree extent that doesn't belong to a log tree
+* failed, set the bit BTRFS_INODE_BTREE_IO_ERR in the inode btree.
+* We do this because while the transaction is running and before it's
+* committing (when we call filemap_fdata[write|wait]_range against
+* the btree inode), we might have
+* btree_inode->i_mapping->a_ops->writepages() called by the VM - if it
+* returns an error or an error happens during writeback, when we're
+* committing the transaction we wouldn't know about it, since the pages
+* can be no longer dirty nor marked anymore for writeback (if a
+* subsequent modification to the extent buffer didn't happen before the
+* transaction commit), which makes filemap_fdata[write|wait]_range not
+* able t

[PATCH 1/2] Btrfs: add missing end_page_writeback on submit_extent_page failure

[Filipe Manana]

If submit_extent_page() fails in write_one_eb(), we end up with the current
page not marked dirty anymore, unlocked and marked for writeback. But we never
end up calling end_page_writeback() against the page, which will make calls to
filemap_fdatawait_range (e.g. at transaction commit time) hang forever waiting
for the writeback bit to be cleared from the page.

Signed-off-by: Filipe Manana 
---
 fs/btrfs/extent_io.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 3af4966..91f866c 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3668,6 +3668,7 @@ static noinline_for_stack int write_one_eb(struct 
extent_buffer *eb,
if (ret) {
set_bit(EXTENT_BUFFER_IOERR, &eb->bflags);
SetPageError(p);
+   end_page_writeback(p);
if (atomic_sub_and_test(num_pages - i, &eb->io_pages))
end_extent_buffer_writeback(eb);
ret = -EIO;
-- 
1.9.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: Btrfs-progs release 3.16.1

[Chris Mason]

On 09/22/2014 11:24 AM, David Sterba wrote:
> Hi,
> 
> this is is a minor release collecting a few bugfixes and enhancements.
> User visible changes:
> 
> * the 'uknown' blockgroup in 'fi df' is properly named 'GlobalReserve'
> * -R option for listing received UUID
> 
> Other updates:
> 
> * library version defines
> * static build is fixed
> * build without documentation is possible
> 
> The kernel.org btrfs-progs git repository is at
> 
> https://git.kernel.org/cgit/linux/kernel/git/kdave/btrfs-progs.git
> 
> and eventually synced to Chris' git tree.
> 

Thanks Dave!  I've sync'd the v3.16.1 git commits over to my tree.

-chris

--
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-progs release 3.16.1

[David Sterba]

Hi,

this is is a minor release collecting a few bugfixes and enhancements.
User visible changes:

* the 'uknown' blockgroup in 'fi df' is properly named 'GlobalReserve'
* -R option for listing received UUID

Other updates:

* library version defines
* static build is fixed
* build without documentation is possible

The kernel.org btrfs-progs git repository is at

https://git.kernel.org/cgit/linux/kernel/git/kdave/btrfs-progs.git/

and eventually synced to Chris' git tree.

The release tarballs are at

https://www.kernel.org/pub/linux/kernel/people/kdave/btrfs-progs/

The tarballs and release commits are signed by my GPG key, fingerprint
 F2B4 1200 C54E FB30 380C  1756 C565 D5F9 D76D 583B

I'll send a separate mail about the expected patchflow and future
releases. I'm not expecting to do 3.16.2 unless something serious pops
up.

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: fs corruption report

[Zooko Wilcox-OHearn]

I guess I'll wait until there's a git repo (maybe one of these
branches https://github.com/kdave/btrfs-progs/branches/active ?) with
the fix integrated and then I'll test "btrfs restore" using that.
Thanks!

Regards,

Zooko
--
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-progs: fix page align issue for lzo compress in restore

[David Sterba]

On Mon, Sep 22, 2014 at 04:58:26PM +0800, Gui Hecheng wrote:
> So we check page alignment every time before we are going to
> fetch the next @len and after the former piece of data is decompressed.
> If the current page that we reach has less than 4 bytes left,
> then we should fetch the next @len at the start of next page.

Thanks for the fix.

> --- a/cmds-restore.c
> +++ b/cmds-restore.c
> @@ -57,6 +57,9 @@ static int dry_run = 0;
>  
>  #define LZO_LEN 4
>  #define PAGE_CACHE_SIZE 4096
> +#define PAGE_CACHE_MASK (~(PAGE_CACHE_SIZE - 1))
> +#define PAGE_CACHE_ALIGN(addr) (((addr) + PAGE_CACHE_SIZE - 1)   \
> + & PAGE_CACHE_MASK)

This is not type-safe, the PAGE_CACHE_SIZE should be unsigned long.

>  #define lzo1x_worst_compress(x) ((x) + ((x) / 16) + 64 + 3)
>  
>  static int decompress_zlib(char *inbuf, char *outbuf, u64 compress_len,
> @@ -93,6 +96,28 @@ static inline size_t read_compress_length(unsigned char 
> *buf)
>   return le32_to_cpu(dlen);
>  }
>  
> +static void align_if_need(size_t *tot_in, size_t *in_len)
> +{
> + int tot_in_aligned;
> + int bytes_left;
> +
> + tot_in_aligned = PAGE_CACHE_ALIGN(*tot_in);

size_t -> int, plus other tricks that happen inside the macro

> + bytes_left = tot_in_aligned - *tot_in;

int = int - size_t

> +
> + if (bytes_left >= LZO_LEN)
> + return;
> +
> + /*
> +  * The LZO_LEN bytes is guaranteed to be
> +  * in one page as a whole, so if a page
> +  * has fewer than LZO_LEN bytes left,
> +  * the LZO_LEN bytes should be fetched
> +  * at the start of the next page
> +  */

Nitpick, the comment can use the whole width of the line

/*
 * The LZO_LEN bytes is guaranteed to be in one page as a whole,
 * so if a page has fewer than LZO_LEN bytes left, the LZO_LEN
 * bytes should be fetched at the start of the next page
 */

> + *in_len += bytes_left;
> + *tot_in = tot_in_aligned;
> +}
--
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: Performance Issues

[David Sterba]

On Mon, Sep 22, 2014 at 12:37:59PM +, Holger Hoffstätte wrote:
> Thanks Dave - that confirms everything I (unscientifically ;) observed so
> far, since I also tried to use "find" to warm up (in the hope it would
> cache the relevant metadata blocks), but running with strace showed that
> it does - of course! - not call stat on each inode, and just quickly reads
> the directory entry list (via getdents()).
> 
> This meant that even after a full "find" a subsequent "du" would still be
> slow(er). Both the cold "find" and a cold "du" also *sound* noticeably
> different, in terms of disk head scratching; find is significantly less
> seeky.

The default find still calls some variant of stat quite a lot because it
needs to decide whether to recurse to the directories, but when compared
to 'du -s' it's still not called for all files.

> Interesting that you also mention the readahead. I've run the "du" warmup
> under Brendan Gregg's iosnoop and it shows that most stat()-heavy I/O is
> done in 16k blocks, while ext4 only seems to use 4k.

Most probably, 16k is the size of the metadata blocks, mkfs option
--nodesize or default since progs v3.12.
--
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] Default to acting like fsck.

[David Sterba]

On Mon, Sep 22, 2014 at 09:58:34AM +0100, Dimitri John Ledkov wrote:
> On 21 September 2014 13:59, Tobias Geerinckx-Rice
>  wrote:
> > On 21 September 2014 03:01, Dimitri John Ledkov  wrote:
> >>
> >> Inspect arguments, if we are not called as btrfs, then assume we are
> >> called to act like fsck.
> > [...]
> >> -   if (!strcmp(bname, "btrfsck")) {
> >> +   if (strcmp(bname, "btrfs") != 0) {
> >
> > That's assuming a lot.
> >
> > Silently (!) breaking people's btrfs-3.15_patched-DontRandomlyPanicV2
> > is a recipe for needless hair-pulling. Is there a reason for not using
> > something less like strstr(bname, "fsck") that I am missing?
> >
> 
> Quite. This is verbatim patch as I have currently applied in Debian
> packaging, and it was a fast fix to prevent breakage we had at one
> point.
> 
> Indeed using "strstr(bname, "fsck")" would be better and sufficient to
> resolve the problem we encountered (specifically fsck.btrfs -> btrfs
> not acting like btrfs). Also using strstr, would fix btrfsck.my-build
> to act like fsck tool.

The intention was to provide backward compatibility shortcut for
'btrfsck' -> 'btrfs check' and nothing else. The referenced bug is again
for 0.19 but there's an upstream-shipped stub fsck.btrfs (since 3.14)
that should avoid any packaging tricks.
--
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] Fixing unaligned memory accesses.

[David Sterba]

On Sun, Sep 21, 2014 at 02:01:21AM +0100, Dimitri John Ledkov wrote:
> From: Shawn Landen 
> 
> Bug-Debian: http://bugs.debian.org/656955

The bug seems old (2012) and agains 0.19. We've fixed a few unaligned
access bugs in the meantime. Can you please retest with 3.16?

> --- a/ctree.h
> +++ b/ctree.h
> @@ -19,6 +19,8 @@
>  #ifndef __BTRFS__
>  #define __BTRFS__
>  
> +#include 
> +
>  #if BTRFS_FLAT_INCLUDES
>  #include "list.h"
>  #include "kerncompat.h"
> @@ -1191,13 +1193,17 @@ struct btrfs_root {
>  static inline u##bits btrfs_##name(const struct extent_buffer *eb)   \
>  {\
>   const struct btrfs_header *h = (struct btrfs_header *)eb->data; \
> - return le##bits##_to_cpu(h->member);\
> + uint##bits##_t t;   \
> + memcpy(&t, &h->member, sizeof(h->member));  \
> + return le##bits##_to_cpu(t);\

The change to memcpy is safe, the compiler is smart enough to not emit
any memcpy call for x86_64 and there's no change to the leXX_to_cpu
macros.

However, I'd like to check first if this is really necessary due to the
old version in the bugreport. I'd prefer using the u8/.../u64 types
instead of the stdint.h ones, for sake of consistency with the rest of
the codebase.

> --- a/volumes.c
> +++ b/volumes.c
> @@ -472,10 +472,11 @@ static int find_next_chunk(struct btrfs_root *root, u64 
> objectid, u64 *offset)
>   if (found_key.objectid != objectid)
>   *offset = 0;
>   else {
> + u64 t;
>   chunk = btrfs_item_ptr(path->nodes[0], path->slots[0],
>  struct btrfs_chunk);
> - *offset = found_key.offset +
> - btrfs_chunk_length(path->nodes[0], chunk);
> + t = found_key.offset + 
> btrfs_chunk_length(path->nodes[0], chunk);
> + memcpy(offset, &t, sizeof(found_key.offset));

That's not enough, there are more direct assignments to *offset in that
function. The preferred way is to add 'put_unaligned' helper into
kerncompat.h and use it.
--
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: Performance Issues

[Holger Hoffstätte]

On Mon, 22 Sep 2014 13:59:03 +0200, David Sterba wrote:

> On Fri, Sep 19, 2014 at 01:34:38PM +, Holger Hoffstätte wrote:
>> 
>> I'd also love a technical explanation why this happens and how it could
>> be fixed. Maybe it's just a consequence of how the metadata tree(s)
>> are laid out on disk.
> 
> The stat() call is the most severe slowdown factor in combination with
> fragmentation and random order of the inodes that are being stat()ed.
> 
> A 'ls -f' (that does not stat) goes only through the DIR_INDEX_KEY items
> in the b-tree that are usually packed together and reading is sequential
> and fast.
> 
> A 'ls' that calls stat() in turn will have to seek for the INODE_ITEM
> item that can be far from all the currently read metadata blocks.

Thanks Dave - that confirms everything I (unscientifically ;) observed so
far, since I also tried to use "find" to warm up (in the hope it would
cache the relevant metadata blocks), but running with strace showed that
it does - of course! - not call stat on each inode, and just quickly reads
the directory entry list (via getdents()).

This meant that even after a full "find" a subsequent "du" would still be
slow(er). Both the cold "find" and a cold "du" also *sound* noticeably
different, in terms of disk head scratching; find is significantly less
seeky.

Interesting that you also mention the readahead. I've run the "du" warmup
under Brendan Gregg's iosnoop and it shows that most stat()-heavy I/O is
done in 16k blocks, while ext4 only seems to use 4k.

Time to look at the trees in detail.. :)

-h

--
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] Fixes FTBFS with --no-add-needed.

[David Sterba]

On Sun, Sep 21, 2014 at 02:01:20AM +0100, Dimitri John Ledkov wrote:
> --- a/Makefile
> +++ b/Makefile
> @@ -26,7 +26,7 @@ TESTS = fsck-tests.sh convert-tests.sh
>  INSTALL = install
>  prefix ?= /usr/local
>  bindir = $(prefix)/bin
> -lib_LIBS = -luuid -lblkid -lm -lz -llzo2 -L.
> +lib_LIBS = -luuid -lblkid -lm -lz -llzo2 -lcom_err -L.

This will add the com_err library to all binaries, while it's actually
used only by the btrfs-convert utility and this dependency has been
properly added in ef85e7e285daf (released in 3.12).

One can add a per-target libraries like

btrfs_convert_libs = -lext2fs -lcom_err

Also please be more descriptive what the actual error is in the
changelog itself so I don't have to look it up in the referenced bug.
--
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: Unable to mount multiple subvolumes of a single disk

[Stefan G. Weichinger]

Am 17.09.2014 um 15:21 schrieb Chris Mason:

> No problem, the original patch looked right to me too.  We're getting
> closer to rc6, I think at this point I'll revert the original patch
> until the next merge window.  Then we can step back and nail down
> exactly what is going on.

running git-sources 3.17-rc6 now and didn't need to apply the patch.
So you fixed it already?

"btrfs fi show" also displays devices correctly again.

Thanks, Stefan

--
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: export __btrfs_set_prop

[David Sterba]

On Fri, Sep 19, 2014 at 05:52:17PM +0900, Satoru Takeuchi wrote:
> @@ -99,7 +99,7 @@ find_prop_handler(const char *name,
>   return NULL;
>  }
>  
> -static int __btrfs_set_prop(struct btrfs_trans_handle *trans,
> +int __btrfs_set_prop(struct btrfs_trans_handle *trans,

It's common for static helpers to use the __ prefix, but please drop it
for an exported function(s).
--
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: Performance Issues

[David Sterba]

On Fri, Sep 19, 2014 at 01:34:38PM +, Holger Hoffstätte wrote:
> On Fri, 19 Sep 2014 13:18:34 +0100, Rob Spanton wrote:
> 
> > I have a particularly uncomplicated setup (a desktop PC with a hard
> > disk) and I'm seeing particularly slow performance from btrfs.  A `git
> > status` in the linux source tree takes about 46 seconds after dropping
> > caches, whereas on other machines using ext4 this takes about 13s.  My
> 
> This is - unfortunately - a particular btrfs oddity/characteristic/flaw,
> whatever you want to call it. git relies a lot on fast stat() calls,
> and those seem to be particularly slow with btrfs esp. on rotational
> media. I have the same problem with rsync on a freshly mounted volume;
> it gets fast (quite so!) after the first run.
> 
> The simplest thing to fix this is a "du -s >/dev/null" to pre-cache all
> file inodes.
> 
> I'd also love a technical explanation why this happens and how it could
> be fixed. Maybe it's just a consequence of how the metadata tree(s)
> are laid out on disk.

The stat() call is the most severe slowdown factor in combination with
fragmentation and random order of the inodes that are being stat()ed.

A 'ls -f' (that does not stat) goes only through the DIR_INDEX_KEY items
in the b-tree that are usually packed together and reading is sequential
and fast.

A 'ls' that calls stat() in turn will have to seek for the INODE_ITEM
item that can be far from all the currently read metadata blocks.

What could help here is a directory readahead, pre-read the inode items
during the readdir() call. I tried this with current readahead (some
timeago) code but this didn't lead to improvement. The reason is that
the readahead has low priority and is not able to cache the metadata
blocks in time.

So the fix or rather workaround is to implement high priority readahead
and trigger it for all DIR_INDEX keys that get returned by readdir().
This will hurt the 'ls -f' usecase.

In your example, 'du -s' acts as the readahead step.
--
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: XFS Tests for Btrfs

[nick]

On 14-09-22 04:17 AM, Qu Wenruo wrote:
> 
>  Original Message 
> Subject: Re: XFS Tests for Btrfs
> From: nick 
> To: Qu Wenruo 
> Date: 2014年09月19日 18:49
>>
>> On 14-09-18 10:46 PM, Qu Wenruo wrote:
>>>  Original Message 
>>> Subject: Re: XFS Tests for Btrfs
>>> From: nick 
>>> To: Qu Wenruo 
>>> Date: 2014年09月19日 10:40
 [snip]
>> Sorry Qu,ur local _scratch_mkfs routine ...
>> Error: unable to open /dev/sdc: Device or resource busy
>> check: failed to mkfs $SCRATCH_DEV using specified options
>> Passed all 0 tests
>> Is happening to me now. How do I fix this ?
>> Nick
>>
> Full local.conf please.
>
> Also lsblk output is needed.
>
> Thanks,
> Qu
 Thank for the Help Qu,
 Here is the output of lsblk:
 NAME   MAJ:MIN RM   SIZE RO TYPE MOUNTPOINT
 sda  8:00 111.8G  0 disk
 └─sda1   8:10 111.8G  0 part /
 sdb  8:16   0 465.8G  0 disk
 ├─sdb1   8:17   0   7.6G  0 part [SWAP]
 └─sdb2   8:18   0 458.1G  0 part /home
 sdc  8:32   0 931.5G  0 disk
 └─sdc1   8:33   0 931.5G  0 part
 sr0 11:01  1024M  0 rom
 # Ideally define at least these 4 to match your environment
 # The first 2 are required.
 # See README for other variables which can be set.
 #
 # Note: SCRATCH_DEV >will< get overwritten!

 export TEST_DEV=/dev/sdX1
 export TEST_DIR=/mnt/test
 export SCRATCH_DEV=/dev/sdX2
 export SCRATCH_MNT=/mnt/scratch.
 I am new to this so sorry for the troubles.
 Nick
>>> Part your /dev/sdc into the following layout:
>>> sdc
>>>  sdc540G<- Use as TEST_DEV
>>>  sdc615G
>>>  sdc715G
>>>  sdc815G
>>>  sdc915G
>>>
>>> then
>>> # mkfs.btrfs -f /dev/sdc5
>>> # mkfs.btrfs -f /dev/sdc[6-9]
>>> # mkdir -p /mnt/test
>>> # mkdir -p /mnt/scratch
>>>
>>>
>>> then write your local.conf like the following:
>>> export FSTYP=btrfs
>>> export TEST_DEV=/dev/sdc5
>>> export TEST_MNT=/mnt/test
>>> export SCRATCH_DEV_POOL="/dev/sdc6 /dev/sdc7 /dev/sdc8 /dev/sdc9"
>>> export SCRATCH_MNT=/mnt/scratch
>>>
>>> Then, you should be able to run ./check -g auto.
>>>
>>> Thanks,
>>> Qu
>> I got three failing tests now, would you like me to post the logs.
>> Nick
> There are some tests that are already known to fail but not bugs or already 
> fixed.
> 
> As far as I can tell, if using 3.17-rc1,
> btrfs/010, btrfs/047, btrfs/054 are always failure under all mount options.
> btrfs/010 is somewhat outdated, since kernel now disables defrage with 
> subvolume, so it will never pass.
> btrfs/047 and btrfs/054, as you can see in the comments of these tests, it's 
> already fixed.
> Maybe next release or later RCs.
> 
> Generic/015 and generic/027 will sometims fail using 
> nodatasum/nodatacow/compress=lzo mount options
> Also generic/275 will sometimes fail with compress=lzo, none of them is bug.
> Some btrfs features or designs make them not pass.
> 
> Geneirc/018 will always fails with nodatacow mount option, but the patch is 
> already sent.
> 
> So if you encounters other bugs, I'll be happen to see the failed test number 
> and test log.
> 
> BTW, as I mentioned, different mount options may cause different test results,
> so it's high recommended to run xfstest against all supported mount options.
> 
> Thanks,
> Qu
> 
> 
Qu,
I will run the tests later and see if any of them are failing beside the ones 
you mentioned.
Nick
--
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: device delete progress

[David Sterba]

On Sun, Sep 21, 2014 at 08:34:33AM +, Duncan wrote:
> Russell Coker posted on Sun, 21 Sep 2014 12:09:11 +1000 as excerpted:
> 
> > We need to have a way to determine the progress of a device delete
> > operation.
> > Also for a balance of a RAID-1 that has more than 2 devices it would be
> > good to know how much space is used on each device.
> > 
> > Could btrfs fi df be extended to show information separately for each
> > device?
> 
> btrfs fi show should give you at least some minimal per-device stats, 
> today.  Enough to at least have some idea of the progress of a balance 
> when adding/removing devices.

Up to now this is the only way to see the progress.

> Longer term, there has been discussion of extending/changing the fi df 
> format and making it far more verbose, including the information found in 
> btrfs fi show as well, and making everything potentially per-device.  I 
> hadn't paid a whole lot of attention to the details, however.
> 
> Alternatively, leave df more or less as it is (perhaps extending it a bit 
> but attempting not to kill existing scripts using it) and put the detail 
> in a new btrfs filesystem usage.  This sounds rather more reasonable to 
> me.
> 
> Either way the idea is to give people a single command that combines the 
> current output of fi show and fi df, ideally displaying per-device and 
> filesystem totals both, in enough verbosity to avoid the unintuitive and 
> arcane btrfs specific knowledge required today to interpret it.
> 
> I had originally presumed that such a change would happen before the 
> experimental labels came off, but it didn't.  I don't know the timetable 
> for it now, or even if it's still planned, as IIRC the discussion died 
> away back in the btrfs-progs 3.12 era and I expected to see it in 3.14 
> and it wasn't there, so I don't know...

The upcomming 3.17 release should contain all the 'new df' patches that
were unfortunatelly delayed. There are some minor issues to be fixed but
I hope it'll be ready close to the kernel 3.17 release.
--
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-progs: fix page align issue for lzo compress in restore

[Gui Hecheng]

When runing restore under lzo compression, "bad compress length"
problems are encountered.
It is because there is a page align problem with the @decompress_lzo,
as follows:
|--| ||-| |--|...|--|
  page ^page   page
   |
  3 bytes left

When lzo compress pages im RAM, lzo will ensure that
the 4 bytes len will be in one page as a whole.
There is a situation that 3 (or less) bytes are left
at the end of a page, and then the 4 bytes len is
stored at the start of the next page.
But the @decompress_lzo doesn't goto the start of
the next page and continue to read the next 4 bytes
which is across two pages, so a random value is fetched
as a "bad compress length".

So we check page alignment every time before we are going to
fetch the next @len and after the former piece of data is decompressed.
If the current page that we reach has less than 4 bytes left,
then we should fetch the next @len at the start of next page.

Signed-off-by: Gui Hecheng 
Reviewed-by: Marc Dietrich 
---
changelog
v1->v2: adopt alignment check method suggested by Marc
v2->v3: make code more readable
---
 cmds-restore.c | 27 ++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/cmds-restore.c b/cmds-restore.c
index 38a131e..5094b05 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -57,6 +57,9 @@ static int dry_run = 0;
 
 #define LZO_LEN 4
 #define PAGE_CACHE_SIZE 4096
+#define PAGE_CACHE_MASK (~(PAGE_CACHE_SIZE - 1))
+#define PAGE_CACHE_ALIGN(addr) (((addr) + PAGE_CACHE_SIZE - 1) \
+   & PAGE_CACHE_MASK)
 #define lzo1x_worst_compress(x) ((x) + ((x) / 16) + 64 + 3)
 
 static int decompress_zlib(char *inbuf, char *outbuf, u64 compress_len,
@@ -93,6 +96,28 @@ static inline size_t read_compress_length(unsigned char *buf)
return le32_to_cpu(dlen);
 }
 
+static void align_if_need(size_t *tot_in, size_t *in_len)
+{
+   int tot_in_aligned;
+   int bytes_left;
+
+   tot_in_aligned = PAGE_CACHE_ALIGN(*tot_in);
+   bytes_left = tot_in_aligned - *tot_in;
+
+   if (bytes_left >= LZO_LEN)
+   return;
+
+   /*
+* The LZO_LEN bytes is guaranteed to be
+* in one page as a whole, so if a page
+* has fewer than LZO_LEN bytes left,
+* the LZO_LEN bytes should be fetched
+* at the start of the next page
+*/
+   *in_len += bytes_left;
+   *tot_in = tot_in_aligned;
+}
+
 static int decompress_lzo(unsigned char *inbuf, char *outbuf, u64 compress_len,
  u64 *decompress_len)
 {
@@ -135,8 +160,8 @@ static int decompress_lzo(unsigned char *inbuf, char 
*outbuf, u64 compress_len,
}
out_len += new_len;
outbuf += new_len;
+   align_if_need(&tot_in, &in_len);
inbuf += in_len;
-   tot_in += in_len;
}
 
*decompress_len = out_len;
-- 
1.8.1.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 4/4] Default to acting like fsck.

[Dimitri John Ledkov]

On 21 September 2014 13:59, Tobias Geerinckx-Rice
 wrote:
> On 21 September 2014 03:01, Dimitri John Ledkov  wrote:
>>
>> Inspect arguments, if we are not called as btrfs, then assume we are
>> called to act like fsck.
> [...]
>> -   if (!strcmp(bname, "btrfsck")) {
>> +   if (strcmp(bname, "btrfs") != 0) {
>
> That's assuming a lot.
>
> Silently (!) breaking people's btrfs-3.15_patched-DontRandomlyPanicV2
> is a recipe for needless hair-pulling. Is there a reason for not using
> something less like strstr(bname, "fsck") that I am missing?
>

Quite. This is verbatim patch as I have currently applied in Debian
packaging, and it was a fast fix to prevent breakage we had at one
point.

Indeed using "strstr(bname, "fsck")" would be better and sufficient to
resolve the problem we encountered (specifically fsck.btrfs -> btrfs
not acting like btrfs). Also using strstr, would fix btrfsck.my-build
to act like fsck tool.

I'll update this one patch.

-- 
Regards,

Dimitri.
--
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: fs corruption report

[Gui Hecheng]

On Mon, 2014-09-22 at 10:49 +0200, Marc Dietrich wrote:
> Am Montag, 22. September 2014, 16:33:56 schrieb Gui Hecheng:
> > On Mon, 2014-09-22 at 10:19 +0200, Marc Dietrich wrote:
> > > Hi,
> > > 
> > > Am Freitag, 19. September 2014, 09:30:30 schrieb Gui Hecheng:
> > > > On Thu, 2014-09-18 at 12:47 +, Zooko Wilcox-OHearn wrote:
> > > > > Thank you! I will try to restore using this patch.
> > > > > 
> > > > > What branch of what btrfs tools git repo should I apply the patch to?
> > > > > 
> > > > > Regards,
> > > > > 
> > > > > Zooko
> > > > 
> > > > At least I think the following repo/v3.17.x branch has all
> > > > restore-related patches included.
> > > > 
> > > > git://github.com/kdave/btrfs-progs.git
> > > > 
> > > > This is a mirror of the latest devel repo that I am on.
> > > 
> > > restore of my corrupted partition finished. No "bad compress" nor valgrind
> > > errors anymore. I'll close the bug.
> > > 
> > > Many thanks!
> > > 
> > > Marc
> > 
> > Hi Marc,
> > I'm very glad to hear the result.
> > I've sent a v2 patch which adopts your idea. I also add your
> > sign-off-by, please help me to check whether it matches your thoughts,
> > or more could be done to improve.
> 
> well, I didn't wrote any code. So it's enough to just add my reviewed-by.
> 
> Thanks!
> 
> Marc

OK, I will add a reviewed-by instead.

--
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: fs corruption report

[Marc Dietrich]

Am Montag, 22. September 2014, 16:33:56 schrieb Gui Hecheng:
> On Mon, 2014-09-22 at 10:19 +0200, Marc Dietrich wrote:
> > Hi,
> > 
> > Am Freitag, 19. September 2014, 09:30:30 schrieb Gui Hecheng:
> > > On Thu, 2014-09-18 at 12:47 +, Zooko Wilcox-OHearn wrote:
> > > > Thank you! I will try to restore using this patch.
> > > > 
> > > > What branch of what btrfs tools git repo should I apply the patch to?
> > > > 
> > > > Regards,
> > > > 
> > > > Zooko
> > > 
> > > At least I think the following repo/v3.17.x branch has all
> > > restore-related patches included.
> > > 
> > > git://github.com/kdave/btrfs-progs.git
> > > 
> > > This is a mirror of the latest devel repo that I am on.
> > 
> > restore of my corrupted partition finished. No "bad compress" nor valgrind
> > errors anymore. I'll close the bug.
> > 
> > Many thanks!
> > 
> > Marc
> 
> Hi Marc,
> I'm very glad to hear the result.
> I've sent a v2 patch which adopts your idea. I also add your
> sign-off-by, please help me to check whether it matches your thoughts,
> or more could be done to improve.

well, I didn't wrote any code. So it's enough to just add my reviewed-by.

Thanks!

Marc


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

Re: [PATCH v2] btrfs-progs: fix page align issue for lzo compress in restore

[Gui Hecheng]

On Mon, 2014-09-22 at 10:44 +0200, Marc Dietrich wrote:
> Am Montag, 22. September 2014, 16:29:28 schrieb Gui Hecheng:
> > When runing restore under lzo compression, "bad compress length"
> > problems are encountered.
> > It is because there is a page align problem with the @decompress_lzo,
> > as follows:
> > |--| ||-| |--|...|--|
> >   page ^page   page
> >|
> >   3 bytes left
> > 
> > When lzo compress pages im RAM, lzo will ensure that
> > the 4 bytes len will be in one page as a whole.
> > There is a situation that 3 (or less) bytes are left
> > at the end of a page, and then the 4 bytes len is
> > stored at the start of the next page.
> > But the @decompress_lzo doesn't goto the start of
> > the next page and continue to read the next 4 bytes
> > which is across two pages, so a random value is fetched
> > as a "bad compress length".
> > 
> > So we check page alignment every time before we are going to
> > fetch the next @len and after the former piece of data is decompressed.
> > If the current page that we reach has less than 4 bytes left,
> > then we should fetch the next @len at the start of next page.
> > 
> > Signed-off-by: Marc Dietrich 
> > Signed-off-by: Gui Hecheng 
> > ---
> > changelog
> > v1->v2: adopt alignment check method suggested by Marc
> > ---
> >  cmds-restore.c | 27 ++-
> >  1 file changed, 26 insertions(+), 1 deletion(-)
> > 
> > diff --git a/cmds-restore.c b/cmds-restore.c
> > index 38a131e..974f45d 100644
> > --- a/cmds-restore.c
> > +++ b/cmds-restore.c
> > @@ -57,6 +57,9 @@ static int dry_run = 0;
> >  
> >  #define LZO_LEN 4
> >  #define PAGE_CACHE_SIZE 4096
> > +#define PAGE_CACHE_MASK (~(PAGE_CACHE_SIZE - 1))
> > +#define PAGE_CACHE_ALIGN(addr) (((addr) + PAGE_CACHE_SIZE - 1) \
> > +   & PAGE_CACHE_MASK)
> >  #define lzo1x_worst_compress(x) ((x) + ((x) / 16) + 64 + 3)
> >  
> >  static int decompress_zlib(char *inbuf, char *outbuf, u64 compress_len,
> > @@ -93,6 +96,28 @@ static inline size_t read_compress_length(unsigned char 
> > *buf)
> > return le32_to_cpu(dlen);
> >  }
> >  
> > +static void align_if_need(size_t *tot_in, size_t *in_len)
> > +{
> > +   int tot_in_aligned;
> > +   int bytes_left;
> > +
> > +   tot_in_aligned = PAGE_CACHE_ALIGN(*tot_in);
> > +   bytes_left = tot_in_aligned - *tot_in;
> > +
> > +   if (bytes_left >= LZO_LEN)
> > +   return;
> > +
> > +   /*
> > +* The LZO_LEN bytes is guaranteed to be
> > +* in one page as a whole, so if a page
> > +* has fewer than LZO_LEN bytes left,
> > +* the LZO_LEN bytes should be fetched
> > +* at the start of the next page
> > +*/
> > +   *in_len += tot_in_aligned - *tot_in;
> 
> in_len += bytes_left; // makes it more readable

Oh, yes, that's my carelessness, Thanks!

> > +   *tot_in = tot_in_aligned;
> > +}
> > +
> >  static int decompress_lzo(unsigned char *inbuf, char *outbuf, u64 
> > compress_len,
> >   u64 *decompress_len)
> >  {
> > @@ -135,8 +160,8 @@ static int decompress_lzo(unsigned char *inbuf, char 
> > *outbuf, u64 compress_len,
> > }
> > out_len += new_len;
> > outbuf += new_len;
> > +   align_if_need(&tot_in, &in_len);
> > inbuf += in_len;
> > -   tot_in += in_len;
> > }
> >  
> > *decompress_len = out_len;
> 
> otherwise, looks good to me.
> 
> Thanks!
> 
> Marc


--
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-progs: fix page align issue for lzo compress in restore

[Marc Dietrich]

Am Montag, 22. September 2014, 16:29:28 schrieb Gui Hecheng:
> When runing restore under lzo compression, "bad compress length"
> problems are encountered.
> It is because there is a page align problem with the @decompress_lzo,
> as follows:
>   |--| ||-| |--|...|--|
> page ^page   page
>  |
> 3 bytes left
> 
>   When lzo compress pages im RAM, lzo will ensure that
>   the 4 bytes len will be in one page as a whole.
>   There is a situation that 3 (or less) bytes are left
>   at the end of a page, and then the 4 bytes len is
>   stored at the start of the next page.
>   But the @decompress_lzo doesn't goto the start of
>   the next page and continue to read the next 4 bytes
>   which is across two pages, so a random value is fetched
>   as a "bad compress length".
> 
> So we check page alignment every time before we are going to
> fetch the next @len and after the former piece of data is decompressed.
> If the current page that we reach has less than 4 bytes left,
> then we should fetch the next @len at the start of next page.
> 
> Signed-off-by: Marc Dietrich 
> Signed-off-by: Gui Hecheng 
> ---
> changelog
>   v1->v2: adopt alignment check method suggested by Marc
> ---
>  cmds-restore.c | 27 ++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/cmds-restore.c b/cmds-restore.c
> index 38a131e..974f45d 100644
> --- a/cmds-restore.c
> +++ b/cmds-restore.c
> @@ -57,6 +57,9 @@ static int dry_run = 0;
>  
>  #define LZO_LEN 4
>  #define PAGE_CACHE_SIZE 4096
> +#define PAGE_CACHE_MASK (~(PAGE_CACHE_SIZE - 1))
> +#define PAGE_CACHE_ALIGN(addr) (((addr) + PAGE_CACHE_SIZE - 1)   \
> + & PAGE_CACHE_MASK)
>  #define lzo1x_worst_compress(x) ((x) + ((x) / 16) + 64 + 3)
>  
>  static int decompress_zlib(char *inbuf, char *outbuf, u64 compress_len,
> @@ -93,6 +96,28 @@ static inline size_t read_compress_length(unsigned char 
> *buf)
>   return le32_to_cpu(dlen);
>  }
>  
> +static void align_if_need(size_t *tot_in, size_t *in_len)
> +{
> + int tot_in_aligned;
> + int bytes_left;
> +
> + tot_in_aligned = PAGE_CACHE_ALIGN(*tot_in);
> + bytes_left = tot_in_aligned - *tot_in;
> +
> + if (bytes_left >= LZO_LEN)
> + return;
> +
> + /*
> +  * The LZO_LEN bytes is guaranteed to be
> +  * in one page as a whole, so if a page
> +  * has fewer than LZO_LEN bytes left,
> +  * the LZO_LEN bytes should be fetched
> +  * at the start of the next page
> +  */
> + *in_len += tot_in_aligned - *tot_in;

in_len += bytes_left; // makes it more readable

> + *tot_in = tot_in_aligned;
> +}
> +
>  static int decompress_lzo(unsigned char *inbuf, char *outbuf, u64 
> compress_len,
> u64 *decompress_len)
>  {
> @@ -135,8 +160,8 @@ static int decompress_lzo(unsigned char *inbuf, char 
> *outbuf, u64 compress_len,
>   }
>   out_len += new_len;
>   outbuf += new_len;
> + align_if_need(&tot_in, &in_len);
>   inbuf += in_len;
> - tot_in += in_len;
>   }
>  
>   *decompress_len = out_len;

otherwise, looks good to me.

Thanks!

Marc


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

Re: fs corruption report

[Gui Hecheng]

On Mon, 2014-09-22 at 10:19 +0200, Marc Dietrich wrote:
> Hi,
> 
> Am Freitag, 19. September 2014, 09:30:30 schrieb Gui Hecheng:
> > On Thu, 2014-09-18 at 12:47 +, Zooko Wilcox-OHearn wrote:
> > > Thank you! I will try to restore using this patch.
> > > 
> > > What branch of what btrfs tools git repo should I apply the patch to?
> > > 
> > > Regards,
> > > 
> > > Zooko
> > 
> > At least I think the following repo/v3.17.x branch has all
> > restore-related patches included.
> > 
> > git://github.com/kdave/btrfs-progs.git
> > 
> > This is a mirror of the latest devel repo that I am on.
> 
> restore of my corrupted partition finished. No "bad compress" nor valgrind
> errors anymore. I'll close the bug.
> 
> Many thanks!
> 
> Marc

Hi Marc,
I'm very glad to hear the result.
I've sent a v2 patch which adopts your idea. I also add your
sign-off-by, please help me to check whether it matches your thoughts,
or more could be done to improve.

Thanks,
-Gui

--
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-progs: fix page align issue for lzo compress in restore

[Gui Hecheng]

When runing restore under lzo compression, "bad compress length"
problems are encountered.
It is because there is a page align problem with the @decompress_lzo,
as follows:
|--| ||-| |--|...|--|
  page ^page   page
   |
  3 bytes left

When lzo compress pages im RAM, lzo will ensure that
the 4 bytes len will be in one page as a whole.
There is a situation that 3 (or less) bytes are left
at the end of a page, and then the 4 bytes len is
stored at the start of the next page.
But the @decompress_lzo doesn't goto the start of
the next page and continue to read the next 4 bytes
which is across two pages, so a random value is fetched
as a "bad compress length".

So we check page alignment every time before we are going to
fetch the next @len and after the former piece of data is decompressed.
If the current page that we reach has less than 4 bytes left,
then we should fetch the next @len at the start of next page.

Signed-off-by: Marc Dietrich 
Signed-off-by: Gui Hecheng 
---
changelog
v1->v2: adopt alignment check method suggested by Marc
---
 cmds-restore.c | 27 ++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/cmds-restore.c b/cmds-restore.c
index 38a131e..974f45d 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -57,6 +57,9 @@ static int dry_run = 0;
 
 #define LZO_LEN 4
 #define PAGE_CACHE_SIZE 4096
+#define PAGE_CACHE_MASK (~(PAGE_CACHE_SIZE - 1))
+#define PAGE_CACHE_ALIGN(addr) (((addr) + PAGE_CACHE_SIZE - 1) \
+   & PAGE_CACHE_MASK)
 #define lzo1x_worst_compress(x) ((x) + ((x) / 16) + 64 + 3)
 
 static int decompress_zlib(char *inbuf, char *outbuf, u64 compress_len,
@@ -93,6 +96,28 @@ static inline size_t read_compress_length(unsigned char *buf)
return le32_to_cpu(dlen);
 }
 
+static void align_if_need(size_t *tot_in, size_t *in_len)
+{
+   int tot_in_aligned;
+   int bytes_left;
+
+   tot_in_aligned = PAGE_CACHE_ALIGN(*tot_in);
+   bytes_left = tot_in_aligned - *tot_in;
+
+   if (bytes_left >= LZO_LEN)
+   return;
+
+   /*
+* The LZO_LEN bytes is guaranteed to be
+* in one page as a whole, so if a page
+* has fewer than LZO_LEN bytes left,
+* the LZO_LEN bytes should be fetched
+* at the start of the next page
+*/
+   *in_len += tot_in_aligned - *tot_in;
+   *tot_in = tot_in_aligned;
+}
+
 static int decompress_lzo(unsigned char *inbuf, char *outbuf, u64 compress_len,
  u64 *decompress_len)
 {
@@ -135,8 +160,8 @@ static int decompress_lzo(unsigned char *inbuf, char 
*outbuf, u64 compress_len,
}
out_len += new_len;
outbuf += new_len;
+   align_if_need(&tot_in, &in_len);
inbuf += in_len;
-   tot_in += in_len;
}
 
*decompress_len = out_len;
-- 
1.8.1.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: fs corruption report

[Marc Dietrich]

Hi,

Am Freitag, 19. September 2014, 09:30:30 schrieb Gui Hecheng:
> On Thu, 2014-09-18 at 12:47 +, Zooko Wilcox-OHearn wrote:
> > Thank you! I will try to restore using this patch.
> > 
> > What branch of what btrfs tools git repo should I apply the patch to?
> > 
> > Regards,
> > 
> > Zooko
> 
> At least I think the following repo/v3.17.x branch has all
> restore-related patches included.
> 
> git://github.com/kdave/btrfs-progs.git
> 
> This is a mirror of the latest devel repo that I am on.

restore of my corrupted partition finished. No "bad compress" nor valgrind
errors anymore. I'll close the bug.

Many thanks!

Marc


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