All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL][PATCH 0/4] Sysfs fixes to incompat bits
@ 2016-01-21 18:31 David Sterba
  2016-01-21 18:32 ` [PATCH 1/4] btrfs: sysfs: fix typo in compat_ro attribute definition David Sterba
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: David Sterba @ 2016-01-21 18:31 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba, clm

Hi,

I've noticed that the free space tree compat bit is missing from the sysfs
files, this patch series fixes that. Incidentally I found out that any incompat
bits set during filesystem lifetime are not tracked there either, so that's
also fixed.

This is not critical for using the free space tree feature itself, I'm
targeting 4.5-rc2. Thanks.

----------------------------------------------------------------
The following changes since commit 988f1f576d4f7531cb2175ee1b7cb7afd6d95d22:

  Merge branch 'for-chris-4.5' of git://git.kernel.org/pub/scm/linux/kernel/git/fdmanana/linux into for-linus-4.5 (2016-01-11 08:39:28 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git fix/fst-sysfs

for you to fetch changes up to 14e46e04958df740c6c6a94849f176159a333f13:

  btrfs: synchronize incompat feature bits with sysfs files (2016-01-21 18:54:41 +0100)

----------------------------------------------------------------
David Sterba (4):
  btrfs: sysfs: fix typo in compat_ro attribute definition
  btrfs: sysfs: add free-space-tree bit attribute
  btrfs: sysfs: introduce helper for syncing bits with sysfs files
  btrfs: synchronize incompat feature bits with sysfs files

 fs/btrfs/free-space-tree.c |  7 +++++++
 fs/btrfs/ioctl.c           |  4 ++++
 fs/btrfs/super.c           |  4 ++++
 fs/btrfs/sysfs.c           | 32 ++++++++++++++++++++++++++++++++
 fs/btrfs/sysfs.h           |  5 ++++-
 fs/btrfs/volumes.c         |  2 ++
 6 files changed, 53 insertions(+), 1 deletion(-)

-- 
2.6.3


^ permalink raw reply	[flat|nested] 11+ messages in thread
* [PATCH 1/4] btrfs: sysfs: fix typo in compat_ro attribute definition
  2016-01-21 18:31 [PULL][PATCH 0/4] Sysfs fixes to incompat bits David Sterba
@ 2016-01-21 18:32 ` David Sterba
  2016-01-21 18:32 ` [PATCH 2/4] btrfs: sysfs: add free-space-tree bit attribute David Sterba
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: David Sterba @ 2016-01-21 18:32 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/sysfs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
index 9c09522125a6..72408e2c4ea8 100644
--- a/fs/btrfs/sysfs.h
+++ b/fs/btrfs/sysfs.h
@@ -56,7 +56,7 @@ static struct btrfs_feature_attr btrfs_attr_##_name = {			     \
 #define BTRFS_FEAT_ATTR_COMPAT(name, feature) \
 	BTRFS_FEAT_ATTR(name, FEAT_COMPAT, BTRFS_FEATURE_COMPAT, feature)
 #define BTRFS_FEAT_ATTR_COMPAT_RO(name, feature) \
-	BTRFS_FEAT_ATTR(name, FEAT_COMPAT_RO, BTRFS_FEATURE_COMPAT, feature)
+	BTRFS_FEAT_ATTR(name, FEAT_COMPAT_RO, BTRFS_FEATURE_COMPAT_RO, feature)
 #define BTRFS_FEAT_ATTR_INCOMPAT(name, feature) \
 	BTRFS_FEAT_ATTR(name, FEAT_INCOMPAT, BTRFS_FEATURE_INCOMPAT, feature)
 
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [PATCH 2/4] btrfs: sysfs: add free-space-tree bit attribute
  2016-01-21 18:31 [PULL][PATCH 0/4] Sysfs fixes to incompat bits David Sterba
  2016-01-21 18:32 ` [PATCH 1/4] btrfs: sysfs: fix typo in compat_ro attribute definition David Sterba
@ 2016-01-21 18:32 ` David Sterba
  2016-01-21 18:32 ` [PATCH 3/4] btrfs: sysfs: introduce helper for syncing bits with sysfs files David Sterba
  2016-01-21 18:32 ` [PATCH 4/4] btrfs: synchronize incompat feature " David Sterba
  3 siblings, 0 replies; 11+ messages in thread
From: David Sterba @ 2016-01-21 18:32 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

The incompat bit representing the newly added free space tree feature is
missing. Right now it will be listed only among features supported by
the module, not per-fs.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/sysfs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index e0ac85949067..906f7ed6fc80 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -202,6 +202,7 @@ BTRFS_FEAT_ATTR_INCOMPAT(extended_iref, EXTENDED_IREF);
 BTRFS_FEAT_ATTR_INCOMPAT(raid56, RAID56);
 BTRFS_FEAT_ATTR_INCOMPAT(skinny_metadata, SKINNY_METADATA);
 BTRFS_FEAT_ATTR_INCOMPAT(no_holes, NO_HOLES);
+BTRFS_FEAT_ATTR_COMPAT_RO(free_space_tree, FREE_SPACE_TREE);
 
 static struct attribute *btrfs_supported_feature_attrs[] = {
 	BTRFS_FEAT_ATTR_PTR(mixed_backref),
@@ -213,6 +214,7 @@ static struct attribute *btrfs_supported_feature_attrs[] = {
 	BTRFS_FEAT_ATTR_PTR(raid56),
 	BTRFS_FEAT_ATTR_PTR(skinny_metadata),
 	BTRFS_FEAT_ATTR_PTR(no_holes),
+	BTRFS_FEAT_ATTR_PTR(free_space_tree),
 	NULL
 };
 
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [PATCH 3/4] btrfs: sysfs: introduce helper for syncing bits with sysfs files
  2016-01-21 18:31 [PULL][PATCH 0/4] Sysfs fixes to incompat bits David Sterba
  2016-01-21 18:32 ` [PATCH 1/4] btrfs: sysfs: fix typo in compat_ro attribute definition David Sterba
  2016-01-21 18:32 ` [PATCH 2/4] btrfs: sysfs: add free-space-tree bit attribute David Sterba
@ 2016-01-21 18:32 ` David Sterba
  2016-01-29 13:55   ` Filipe Manana
  2016-01-21 18:32 ` [PATCH 4/4] btrfs: synchronize incompat feature " David Sterba
  3 siblings, 1 reply; 11+ messages in thread
From: David Sterba @ 2016-01-21 18:32 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

The files under /sys/fs/UUID/features get out of sync with the actual
incompat bits set for the filesystem if they change after mount. We're
going to sync them and need a helper to do that.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/sysfs.c | 30 ++++++++++++++++++++++++++++++
 fs/btrfs/sysfs.h |  3 +++
 2 files changed, 33 insertions(+)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 906f7ed6fc80..6986886243bf 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -782,6 +782,36 @@ int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info)
 	return error;
 }
 
+
+/*
+ * Change per-fs features in /sys/fs/btrfs/UUID/features to match current
+ * values in superblock. Call after any changes to incompat/compat_ro flags
+ */
+void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
+		u64 bit, enum btrfs_feature_set set)
+{
+	struct btrfs_fs_devices *fs_devs;
+	struct kobject *fsid_kobj;
+	u64 features;
+	int ret;
+
+	if (!fs_info)
+		return;
+
+	features = get_features(fs_info, set);
+	ASSERT(bit & supported_feature_masks[set]);
+
+	fs_devs = fs_info->fs_devices;
+	fsid_kobj = &fs_devs->fsid_kobj;
+
+	/*
+	 * FIXME: this is too heavy to update just one value, ideally we'd like
+	 * to use sysfs_update_group but some refactoring is needed first.
+	 */
+	sysfs_remove_group(fsid_kobj, &btrfs_feature_attr_group);
+	ret = sysfs_create_group(fsid_kobj, &btrfs_feature_attr_group);
+}
+
 static int btrfs_init_debugfs(void)
 {
 #ifdef CONFIG_DEBUG_FS
diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
index 72408e2c4ea8..d7da1a4c2f6c 100644
--- a/fs/btrfs/sysfs.h
+++ b/fs/btrfs/sysfs.h
@@ -90,4 +90,7 @@ int btrfs_sysfs_add_fsid(struct btrfs_fs_devices *fs_devs,
 				struct kobject *parent);
 int btrfs_sysfs_add_device(struct btrfs_fs_devices *fs_devs);
 void btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs);
+void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
+		u64 bit, enum btrfs_feature_set set);
+
 #endif /* _BTRFS_SYSFS_H_ */
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [PATCH 4/4] btrfs: synchronize incompat feature bits with sysfs files
  2016-01-21 18:31 [PULL][PATCH 0/4] Sysfs fixes to incompat bits David Sterba
                   ` (2 preceding siblings ...)
  2016-01-21 18:32 ` [PATCH 3/4] btrfs: sysfs: introduce helper for syncing bits with sysfs files David Sterba
@ 2016-01-21 18:32 ` David Sterba
  2016-01-26 20:23   ` Chris Mason
  3 siblings, 1 reply; 11+ messages in thread
From: David Sterba @ 2016-01-21 18:32 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

The files under /sys/fs/UUID/features get out of sync with the actual
incompat bits set for the filesystem if they change after mount (eg. the
LZO compression).

Synchronize the feature bits with the sysfs files representing them
right after we set/clear them.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/free-space-tree.c | 7 +++++++
 fs/btrfs/ioctl.c           | 4 ++++
 fs/btrfs/super.c           | 4 ++++
 fs/btrfs/volumes.c         | 2 ++
 4 files changed, 17 insertions(+)

diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index 393e36bd5845..94e887f5ec4e 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -23,6 +23,7 @@
 #include "locking.h"
 #include "free-space-tree.h"
 #include "transaction.h"
+#include "sysfs.h"
 
 static int __add_block_group_free_space(struct btrfs_trans_handle *trans,
 					struct btrfs_fs_info *fs_info,
@@ -1169,6 +1170,9 @@ int btrfs_create_free_space_tree(struct btrfs_fs_info *fs_info)
 	}
 
 	btrfs_set_fs_compat_ro(fs_info, FREE_SPACE_TREE);
+	btrfs_sysfs_feature_update(fs_info,
+		BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE, FEAT_COMPAT_RO);
+
 	fs_info->creating_free_space_tree = 0;
 
 	ret = btrfs_commit_transaction(trans, tree_root);
@@ -1237,6 +1241,9 @@ int btrfs_clear_free_space_tree(struct btrfs_fs_info *fs_info)
 		return PTR_ERR(trans);
 
 	btrfs_clear_fs_compat_ro(fs_info, FREE_SPACE_TREE);
+	btrfs_sysfs_feature_update(fs_info,
+		BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE, FEAT_COMPAT_RO);
+
 	fs_info->free_space_root = NULL;
 
 	ret = clear_free_space_tree(trans, free_space_root);
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index e392dd67f0ba..209dcfa9ab33 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1455,6 +1455,8 @@ int btrfs_defrag_file(struct inode *inode, struct file *file,
 
 	if (range->compress_type == BTRFS_COMPRESS_LZO) {
 		btrfs_set_fs_incompat(root->fs_info, COMPRESS_LZO);
+		btrfs_sysfs_feature_update(root->fs_info,
+			BTRFS_FEATURE_INCOMPAT_COMPRESS_LZO, FEAT_INCOMPAT);
 	}
 
 	ret = defrag_count;
@@ -4063,6 +4065,8 @@ static long btrfs_ioctl_default_subvol(struct file *file, void __user *argp)
 	btrfs_free_path(path);
 
 	btrfs_set_fs_incompat(root->fs_info, DEFAULT_SUBVOL);
+	btrfs_sysfs_feature_update(root->fs_info,
+		BTRFS_FEATURE_INCOMPAT_DEFAULT_SUBVOL, FEAT_INCOMPAT);
 	btrfs_end_transaction(trans, root);
 out:
 	mnt_drop_write_file(file);
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 86f7fdc05633..5a1bab11984d 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -58,6 +58,7 @@
 #include "dev-replace.h"
 #include "free-space-cache.h"
 #include "backref.h"
+#include "sysfs.h"
 #include "tests/btrfs-tests.h"
 
 #include "qgroup.h"
@@ -477,6 +478,9 @@ int btrfs_parse_options(struct btrfs_root *root, char *options)
 				btrfs_clear_opt(info->mount_opt, NODATACOW);
 				btrfs_clear_opt(info->mount_opt, NODATASUM);
 				btrfs_set_fs_incompat(info, COMPRESS_LZO);
+				btrfs_sysfs_feature_update(root->fs_info,
+					BTRFS_FEATURE_INCOMPAT_COMPRESS_LZO,
+					FEAT_INCOMPAT);
 			} else if (strncmp(args[0].from, "no", 2) == 0) {
 				compress_type = "no";
 				btrfs_clear_opt(info->mount_opt, COMPRESS);
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index c32abbca9d77..73bcd1322c1d 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4468,6 +4468,8 @@ static void check_raid56_incompat_flag(struct btrfs_fs_info *info, u64 type)
 		return;
 
 	btrfs_set_fs_incompat(info, RAID56);
+	btrfs_sysfs_feature_update(info, BTRFS_FEATURE_INCOMPAT_RAID56,
+		FEAT_INCOMPAT);
 }
 
 #define BTRFS_MAX_DEVS(r) ((BTRFS_LEAF_DATA_SIZE(r)		\
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* Re: [PATCH 4/4] btrfs: synchronize incompat feature bits with sysfs files
  2016-01-21 18:32 ` [PATCH 4/4] btrfs: synchronize incompat feature " David Sterba
@ 2016-01-26 20:23   ` Chris Mason
  0 siblings, 0 replies; 11+ messages in thread
From: Chris Mason @ 2016-01-26 20:23 UTC (permalink / raw)
  To: David Sterba; +Cc: linux-btrfs

On Thu, Jan 21, 2016 at 07:32:11PM +0100, David Sterba wrote:
> The files under /sys/fs/UUID/features get out of sync with the actual
> incompat bits set for the filesystem if they change after mount (eg. the
> LZO compression).
> 
> Synchronize the feature bits with the sysfs files representing them
> right after we set/clear them.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/free-space-tree.c | 7 +++++++
>  fs/btrfs/ioctl.c           | 4 ++++
>  fs/btrfs/super.c           | 4 ++++
>  fs/btrfs/volumes.c         | 2 ++
>  4 files changed, 17 insertions(+)
> 
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 86f7fdc05633..5a1bab11984d 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -58,6 +58,7 @@
>  #include "dev-replace.h"
>  #include "free-space-cache.h"
>  #include "backref.h"
> +#include "sysfs.h"
>  #include "tests/btrfs-tests.h"
>  
>  #include "qgroup.h"
> @@ -477,6 +478,9 @@ int btrfs_parse_options(struct btrfs_root *root, char *options)
>  				btrfs_clear_opt(info->mount_opt, NODATACOW);
>  				btrfs_clear_opt(info->mount_opt, NODATASUM);
>  				btrfs_set_fs_incompat(info, COMPRESS_LZO);
> +				btrfs_sysfs_feature_update(root->fs_info,
> +					BTRFS_FEATURE_INCOMPAT_COMPRESS_LZO,
> +					FEAT_INCOMPAT);
>  			} else if (strncmp(args[0].from, "no", 2) == 0) {
>  				compress_type = "no";
>  				btrfs_clear_opt(info->mount_opt, COMPRESS);

Thanks for tackling this Dave.  I'm having trouble with this one change,
crashing in btrfs/004.  The problem is that our sysfs files aren't fully
setup for the mount until later in open_ctree()

I think btrfs_sysfs_add_fsid() needs to be called before we can call
btrfs_sysfs_feature_update().

-chris


^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] btrfs: sysfs: introduce helper for syncing bits with sysfs files
  2016-01-21 18:32 ` [PATCH 3/4] btrfs: sysfs: introduce helper for syncing bits with sysfs files David Sterba
@ 2016-01-29 13:55   ` Filipe Manana
  2016-01-29 14:32     ` David Sterba
                       ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Filipe Manana @ 2016-01-29 13:55 UTC (permalink / raw)
  To: David Sterba; +Cc: linux-btrfs

On Thu, Jan 21, 2016 at 6:32 PM, David Sterba <dsterba@suse.com> wrote:
> The files under /sys/fs/UUID/features get out of sync with the actual
> incompat bits set for the filesystem if they change after mount. We're
> going to sync them and need a helper to do that.
>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/sysfs.c | 30 ++++++++++++++++++++++++++++++
>  fs/btrfs/sysfs.h |  3 +++
>  2 files changed, 33 insertions(+)
>
> diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
> index 906f7ed6fc80..6986886243bf 100644
> --- a/fs/btrfs/sysfs.c
> +++ b/fs/btrfs/sysfs.c
> @@ -782,6 +782,36 @@ int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info)
>         return error;
>  }
>
> +
> +/*
> + * Change per-fs features in /sys/fs/btrfs/UUID/features to match current
> + * values in superblock. Call after any changes to incompat/compat_ro flags
> + */
> +void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
> +               u64 bit, enum btrfs_feature_set set)
> +{
> +       struct btrfs_fs_devices *fs_devs;
> +       struct kobject *fsid_kobj;
> +       u64 features;
> +       int ret;
> +
> +       if (!fs_info)
> +               return;
> +
> +       features = get_features(fs_info, set);
> +       ASSERT(bit & supported_feature_masks[set]);
> +
> +       fs_devs = fs_info->fs_devices;
> +       fsid_kobj = &fs_devs->fsid_kobj;
> +
> +       /*
> +        * FIXME: this is too heavy to update just one value, ideally we'd like
> +        * to use sysfs_update_group but some refactoring is needed first.
> +        */
> +       sysfs_remove_group(fsid_kobj, &btrfs_feature_attr_group);
> +       ret = sysfs_create_group(fsid_kobj, &btrfs_feature_attr_group);


Did you try to run all xfstests with this?

I'm getting very often lots of warnings in btrfs tests 060 to 074:

[16283.551143] WARNING: CPU: 2 PID: 734 at fs/sysfs/group.c:237
sysfs_remove_group+0x53/0x86()
[16283.554774] sysfs group ffffffffa03e20e0 not found for kobject
'f849fa25-50d1-491b-a7eb-aea06667b62b'
[16283.556232] Modules linked in: btrfs dm_flakey dm_mod ppdev
sha256_generic hmac xor drbg ansi_cprng aesni_intel raid6_pq
aes_x86_64 ablk_helper cryptd lrw gf128mul acpi_cpufreq glue_helper
parport_pc sg evdev tpm_tis parport i2c_piix4 tpm psmouse i2c_core
pcspkr processor serio_raw button loop autofs4 ext4 crc16 mbcache jbd2
sd_mod sr_mod cdrom ata_generic virtio_scsi ata_piix virtio_pci libata
virtio_ring virtio e1000 crc32c_intel scsi_mod floppy [last unloaded:
btrfs]
[16283.563564] CPU: 2 PID: 734 Comm: btrfs Tainted: G        W
4.4.0-rc6-btrfs-next-18+ #1
[16283.564994] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS by qemu-project.org 04/01/2014
[16283.566609]  0000000000000000 ffff8803178df9d0 ffffffff8125d4fd
ffff8803178dfa18
[16283.567832]  ffff8803178dfa08 ffffffff8105055e ffffffff811da7ee
0000000000000000
[16283.569315]  ffffffffa03e20e0 ffff8803dcd4af28 ffff88018c6bc220
ffff8803178dfa70
[16283.571227] Call Trace:
[16283.571663]  [<ffffffff8125d4fd>] dump_stack+0x4e/0x79
[16283.572549]  [<ffffffff8105055e>] warn_slowpath_common+0x9f/0xb8
[16283.573501]  [<ffffffff811da7ee>] ? sysfs_remove_group+0x53/0x86
[16283.574420]  [<ffffffff810505bf>] warn_slowpath_fmt+0x48/0x50
[16283.575272]  [<ffffffff81485330>] ? mutex_unlock+0xe/0x10
[16283.576122]  [<ffffffff811da7ee>] sysfs_remove_group+0x53/0x86
[16283.577124]  [<ffffffffa037ff55>]
btrfs_sysfs_feature_update+0x62/0x74 [btrfs]
[16283.578292]  [<ffffffffa038d4d3>] __btrfs_alloc_chunk+0x6ed/0x7be [btrfs]
[16283.579458]  [<ffffffffa038fef6>] btrfs_alloc_chunk+0x50/0x56 [btrfs]
[16283.580502]  [<ffffffffa0358f86>] do_chunk_alloc+0x1e9/0x279 [btrfs]
[16283.581584]  [<ffffffffa035bb81>] btrfs_force_chunk_alloc+0x30/0x35 [btrfs]
[16283.582589]  [<ffffffffa038f885>] btrfs_balance+0xda1/0xe4a [btrfs]
[16283.583502]  [<ffffffff81276712>] ? debug_smp_processor_id+0x17/0x19
[16283.584499]  [<ffffffffa03992b4>] btrfs_ioctl_balance+0x255/0x2d3 [btrfs]
[16283.585624]  [<ffffffffa039c349>] btrfs_ioctl+0x136d/0x27a9 [btrfs]
[16283.586536]  [<ffffffff8108a8b0>] ? arch_local_irq_save+0x9/0xc
[16283.587398]  [<ffffffff81276727>] ? __this_cpu_preempt_check+0x13/0x15
[16283.588423]  [<ffffffff81143aea>] ? handle_mm_fault+0x44f/0xd40
[16283.589587]  [<ffffffff81486ab7>] ? _raw_spin_unlock+0x31/0x44
[16283.590428]  [<ffffffff8108a8b0>] ? arch_local_irq_save+0x9/0xc
[16283.591282]  [<ffffffff811822f8>] do_vfs_ioctl+0x42b/0x4ea
[16283.592122]  [<ffffffff810fc6b0>] ? time_hardirqs_on+0x15/0x28
[16283.593247]  [<ffffffff81001017>] ? trace_hardirqs_on_thunk+0x17/0x19
[16283.594177]  [<ffffffff8118b4de>] ? __fget_light+0x4d/0x71
[16283.603702]  [<ffffffff8118240e>] SyS_ioctl+0x57/0x79
[16283.604649]  [<ffffffff814872d7>] entry_SYSCALL_64_fastpath+0x12/0x6f
[16283.605670] ---[ end trace 60f318fdba512bac ]---
[16283.606384] ------------[ cut here ]------------
[16283.607082] WARNING: CPU: 2 PID: 734 at fs/sysfs/dir.c:31
sysfs_warn_dup+0x64/0x73()
[16283.608305] sysfs: cannot create duplicate filename
'/fs/btrfs/f849fa25-50d1-491b-a7eb-aea06667b62b/features'
[16283.609835] Modules linked in: btrfs dm_flakey dm_mod ppdev
sha256_generic hmac xor drbg ansi_cprng aesni_intel raid6_pq
aes_x86_64 ablk_helper cryptd lrw gf128mul acpi_cpufreq glue_helper
parport_pc sg evdev tpm_tis parport i2c_piix4 tpm psmouse i2c_core
pcspkr processor serio_raw button loop autofs4 ext4 crc16 mbcache jbd2
sd_mod sr_mod cdrom ata_generic virtio_scsi ata_piix virtio_pci libata
virtio_ring virtio e1000 crc32c_intel scsi_mod floppy [last unloaded:
btrfs]
[16283.616694] CPU: 2 PID: 734 Comm: btrfs Tainted: G        W
4.4.0-rc6-btrfs-next-18+ #1
[16283.617974] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS by qemu-project.org 04/01/2014
[16283.619343]  0000000000000000 ffff8803178df978 ffffffff8125d4fd
ffff8803178df9c0
[16283.620627]  ffff8803178df9b0 ffffffff8105055e ffffffff811d9e55
ffff8802f6c4f000
[16283.621838]  ffffffffa03e7fd5 ffff88040246df58 ffff88018c6bc220
ffff8803178dfa18
[16283.623040] Call Trace:
[16283.623459]  [<ffffffff8125d4fd>] dump_stack+0x4e/0x79
[16283.624257]  [<ffffffff8105055e>] warn_slowpath_common+0x9f/0xb8
[16283.625366]  [<ffffffff811d9e55>] ? sysfs_warn_dup+0x64/0x73
[16283.626202]  [<ffffffff810505bf>] warn_slowpath_fmt+0x48/0x50
[16283.627045]  [<ffffffff811d6ab9>] ? kernfs_path+0x4d/0x58
[16283.627844]  [<ffffffff811d9e55>] sysfs_warn_dup+0x64/0x73
[16283.628701]  [<ffffffff811da4b0>] internal_create_group+0xc7/0x26f
[16283.629924]  [<ffffffff811da66b>] sysfs_create_group+0x13/0x15
[16283.631466]  [<ffffffffa037ff64>]
btrfs_sysfs_feature_update+0x71/0x74 [btrfs]
[16283.632659]  [<ffffffffa038d4d3>] __btrfs_alloc_chunk+0x6ed/0x7be [btrfs]
[16283.633763]  [<ffffffffa038fef6>] btrfs_alloc_chunk+0x50/0x56 [btrfs]
[16283.634710]  [<ffffffffa0358f86>] do_chunk_alloc+0x1e9/0x279 [btrfs]
[16283.635644]  [<ffffffffa035bb81>] btrfs_force_chunk_alloc+0x30/0x35 [btrfs]
[16283.636704]  [<ffffffffa038f885>] btrfs_balance+0xda1/0xe4a [btrfs]
[16283.637766]  [<ffffffff81276712>] ? debug_smp_processor_id+0x17/0x19
[16283.638697]  [<ffffffffa03992b4>] btrfs_ioctl_balance+0x255/0x2d3 [btrfs]
[16283.639675]  [<ffffffffa039c349>] btrfs_ioctl+0x136d/0x27a9 [btrfs]
[16283.640801]  [<ffffffff8108a8b0>] ? arch_local_irq_save+0x9/0xc
[16283.642074]  [<ffffffff81276727>] ? __this_cpu_preempt_check+0x13/0x15
[16283.643015]  [<ffffffff81143aea>] ? handle_mm_fault+0x44f/0xd40
[16283.643879]  [<ffffffff81486ab7>] ? _raw_spin_unlock+0x31/0x44
[16283.644819]  [<ffffffff8108a8b0>] ? arch_local_irq_save+0x9/0xc
[16283.645981]  [<ffffffff811822f8>] do_vfs_ioctl+0x42b/0x4ea
[16283.646799]  [<ffffffff810fc6b0>] ? time_hardirqs_on+0x15/0x28
[16283.647660]  [<ffffffff81001017>] ? trace_hardirqs_on_thunk+0x17/0x19
[16283.648643]  [<ffffffff8118b4de>] ? __fget_light+0x4d/0x71
[16283.649601]  [<ffffffff8118240e>] SyS_ioctl+0x57/0x79
(...)

I'm getting really many of these, about 10+ per test when it happens
(which makes the tests fail).

> +}
> +
>  static int btrfs_init_debugfs(void)
>  {
>  #ifdef CONFIG_DEBUG_FS
> diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
> index 72408e2c4ea8..d7da1a4c2f6c 100644
> --- a/fs/btrfs/sysfs.h
> +++ b/fs/btrfs/sysfs.h
> @@ -90,4 +90,7 @@ int btrfs_sysfs_add_fsid(struct btrfs_fs_devices *fs_devs,
>                                 struct kobject *parent);
>  int btrfs_sysfs_add_device(struct btrfs_fs_devices *fs_devs);
>  void btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs);
> +void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
> +               u64 bit, enum btrfs_feature_set set);
> +
>  #endif /* _BTRFS_SYSFS_H_ */
> --
> 2.6.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] btrfs: sysfs: introduce helper for syncing bits with sysfs files
  2016-01-29 13:55   ` Filipe Manana
@ 2016-01-29 14:32     ` David Sterba
  2016-01-29 14:33     ` Chris Mason
  2016-01-29 18:34     ` David Sterba
  2 siblings, 0 replies; 11+ messages in thread
From: David Sterba @ 2016-01-29 14:32 UTC (permalink / raw)
  To: Filipe Manana; +Cc: David Sterba, linux-btrfs

On Fri, Jan 29, 2016 at 01:55:31PM +0000, Filipe Manana wrote:
> On Thu, Jan 21, 2016 at 6:32 PM, David Sterba <dsterba@suse.com> wrote:
> > The files under /sys/fs/UUID/features get out of sync with the actual
> > incompat bits set for the filesystem if they change after mount. We're
> > going to sync them and need a helper to do that.
> >
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> >  fs/btrfs/sysfs.c | 30 ++++++++++++++++++++++++++++++
> >  fs/btrfs/sysfs.h |  3 +++
> >  2 files changed, 33 insertions(+)
> >
> > diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
> > index 906f7ed6fc80..6986886243bf 100644
> > --- a/fs/btrfs/sysfs.c
> > +++ b/fs/btrfs/sysfs.c
> > @@ -782,6 +782,36 @@ int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info)
> >         return error;
> >  }
> >
> > +
> > +/*
> > + * Change per-fs features in /sys/fs/btrfs/UUID/features to match current
> > + * values in superblock. Call after any changes to incompat/compat_ro flags
> > + */
> > +void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
> > +               u64 bit, enum btrfs_feature_set set)
> > +{
> > +       struct btrfs_fs_devices *fs_devs;
> > +       struct kobject *fsid_kobj;
> > +       u64 features;
> > +       int ret;
> > +
> > +       if (!fs_info)
> > +               return;
> > +
> > +       features = get_features(fs_info, set);
> > +       ASSERT(bit & supported_feature_masks[set]);
> > +
> > +       fs_devs = fs_info->fs_devices;
> > +       fsid_kobj = &fs_devs->fsid_kobj;
> > +
> > +       /*
> > +        * FIXME: this is too heavy to update just one value, ideally we'd like
> > +        * to use sysfs_update_group but some refactoring is needed first.
> > +        */
> > +       sysfs_remove_group(fsid_kobj, &btrfs_feature_attr_group);
> > +       ret = sysfs_create_group(fsid_kobj, &btrfs_feature_attr_group);
> 
> 
> Did you try to run all xfstests with this?
> 
> I'm getting very often lots of warnings in btrfs tests 060 to 074:

I did a few rounds. After an overnight test I saw the warning as well,
during test btrfs/063. Running it standalone did not reproduce it
immediatelly, but it triggered while the sysfs files were accessed.

> [16283.551143] WARNING: CPU: 2 PID: 734 at fs/sysfs/group.c:237
> sysfs_remove_group+0x53/0x86()
> [16283.554774] sysfs group ffffffffa03e20e0 not found for kobject
> 'f849fa25-50d1-491b-a7eb-aea06667b62b'
> [16283.556232] Modules linked in: btrfs dm_flakey dm_mod ppdev
> sha256_generic hmac xor drbg ansi_cprng aesni_intel raid6_pq
> aes_x86_64 ablk_helper cryptd lrw gf128mul acpi_cpufreq glue_helper
> parport_pc sg evdev tpm_tis parport i2c_piix4 tpm psmouse i2c_core
> pcspkr processor serio_raw button loop autofs4 ext4 crc16 mbcache jbd2
> sd_mod sr_mod cdrom ata_generic virtio_scsi ata_piix virtio_pci libata
> virtio_ring virtio e1000 crc32c_intel scsi_mod floppy [last unloaded:
> btrfs]
> [16283.563564] CPU: 2 PID: 734 Comm: btrfs Tainted: G        W
> 4.4.0-rc6-btrfs-next-18+ #1
> [16283.564994] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS by qemu-project.org 04/01/2014
> [16283.566609]  0000000000000000 ffff8803178df9d0 ffffffff8125d4fd
> ffff8803178dfa18
> [16283.567832]  ffff8803178dfa08 ffffffff8105055e ffffffff811da7ee
> 0000000000000000
> [16283.569315]  ffffffffa03e20e0 ffff8803dcd4af28 ffff88018c6bc220
> ffff8803178dfa70
> [16283.571227] Call Trace:
> [16283.571663]  [<ffffffff8125d4fd>] dump_stack+0x4e/0x79
> [16283.572549]  [<ffffffff8105055e>] warn_slowpath_common+0x9f/0xb8
> [16283.573501]  [<ffffffff811da7ee>] ? sysfs_remove_group+0x53/0x86
> [16283.574420]  [<ffffffff810505bf>] warn_slowpath_fmt+0x48/0x50
> [16283.575272]  [<ffffffff81485330>] ? mutex_unlock+0xe/0x10
> [16283.576122]  [<ffffffff811da7ee>] sysfs_remove_group+0x53/0x86
> [16283.577124]  [<ffffffffa037ff55>]
> btrfs_sysfs_feature_update+0x62/0x74 [btrfs]
> [16283.578292]  [<ffffffffa038d4d3>] __btrfs_alloc_chunk+0x6ed/0x7be [btrfs]
> [16283.579458]  [<ffffffffa038fef6>] btrfs_alloc_chunk+0x50/0x56 [btrfs]
> [16283.580502]  [<ffffffffa0358f86>] do_chunk_alloc+0x1e9/0x279 [btrfs]
> [16283.581584]  [<ffffffffa035bb81>] btrfs_force_chunk_alloc+0x30/0x35 [btrfs]
> [16283.582589]  [<ffffffffa038f885>] btrfs_balance+0xda1/0xe4a [btrfs]
> [16283.583502]  [<ffffffff81276712>] ? debug_smp_processor_id+0x17/0x19
> [16283.584499]  [<ffffffffa03992b4>] btrfs_ioctl_balance+0x255/0x2d3 [btrfs]
> [16283.585624]  [<ffffffffa039c349>] btrfs_ioctl+0x136d/0x27a9 [btrfs]
> [16283.586536]  [<ffffffff8108a8b0>] ? arch_local_irq_save+0x9/0xc
> [16283.587398]  [<ffffffff81276727>] ? __this_cpu_preempt_check+0x13/0x15
> [16283.588423]  [<ffffffff81143aea>] ? handle_mm_fault+0x44f/0xd40
> [16283.589587]  [<ffffffff81486ab7>] ? _raw_spin_unlock+0x31/0x44
> [16283.590428]  [<ffffffff8108a8b0>] ? arch_local_irq_save+0x9/0xc
> [16283.591282]  [<ffffffff811822f8>] do_vfs_ioctl+0x42b/0x4ea
> [16283.592122]  [<ffffffff810fc6b0>] ? time_hardirqs_on+0x15/0x28
> [16283.593247]  [<ffffffff81001017>] ? trace_hardirqs_on_thunk+0x17/0x19
> [16283.594177]  [<ffffffff8118b4de>] ? __fget_light+0x4d/0x71
> [16283.603702]  [<ffffffff8118240e>] SyS_ioctl+0x57/0x79
> [16283.604649]  [<ffffffff814872d7>] entry_SYSCALL_64_fastpath+0x12/0x6f
> [16283.605670] ---[ end trace 60f318fdba512bac ]---
> [16283.606384] ------------[ cut here ]------------
> [16283.607082] WARNING: CPU: 2 PID: 734 at fs/sysfs/dir.c:31
> sysfs_warn_dup+0x64/0x73()
> [16283.608305] sysfs: cannot create duplicate filename
> '/fs/btrfs/f849fa25-50d1-491b-a7eb-aea06667b62b/features'
> [16283.609835] Modules linked in: btrfs dm_flakey dm_mod ppdev
> sha256_generic hmac xor drbg ansi_cprng aesni_intel raid6_pq
> aes_x86_64 ablk_helper cryptd lrw gf128mul acpi_cpufreq glue_helper
> parport_pc sg evdev tpm_tis parport i2c_piix4 tpm psmouse i2c_core
> pcspkr processor serio_raw button loop autofs4 ext4 crc16 mbcache jbd2
> sd_mod sr_mod cdrom ata_generic virtio_scsi ata_piix virtio_pci libata
> virtio_ring virtio e1000 crc32c_intel scsi_mod floppy [last unloaded:
> btrfs]
> [16283.616694] CPU: 2 PID: 734 Comm: btrfs Tainted: G        W
> 4.4.0-rc6-btrfs-next-18+ #1
> [16283.617974] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS by qemu-project.org 04/01/2014
> [16283.619343]  0000000000000000 ffff8803178df978 ffffffff8125d4fd
> ffff8803178df9c0
> [16283.620627]  ffff8803178df9b0 ffffffff8105055e ffffffff811d9e55
> ffff8802f6c4f000
> [16283.621838]  ffffffffa03e7fd5 ffff88040246df58 ffff88018c6bc220
> ffff8803178dfa18
> [16283.623040] Call Trace:
> [16283.623459]  [<ffffffff8125d4fd>] dump_stack+0x4e/0x79
> [16283.624257]  [<ffffffff8105055e>] warn_slowpath_common+0x9f/0xb8
> [16283.625366]  [<ffffffff811d9e55>] ? sysfs_warn_dup+0x64/0x73
> [16283.626202]  [<ffffffff810505bf>] warn_slowpath_fmt+0x48/0x50
> [16283.627045]  [<ffffffff811d6ab9>] ? kernfs_path+0x4d/0x58
> [16283.627844]  [<ffffffff811d9e55>] sysfs_warn_dup+0x64/0x73
> [16283.628701]  [<ffffffff811da4b0>] internal_create_group+0xc7/0x26f
> [16283.629924]  [<ffffffff811da66b>] sysfs_create_group+0x13/0x15
> [16283.631466]  [<ffffffffa037ff64>]
> btrfs_sysfs_feature_update+0x71/0x74 [btrfs]
> [16283.632659]  [<ffffffffa038d4d3>] __btrfs_alloc_chunk+0x6ed/0x7be [btrfs]
> [16283.633763]  [<ffffffffa038fef6>] btrfs_alloc_chunk+0x50/0x56 [btrfs]
> [16283.634710]  [<ffffffffa0358f86>] do_chunk_alloc+0x1e9/0x279 [btrfs]
> [16283.635644]  [<ffffffffa035bb81>] btrfs_force_chunk_alloc+0x30/0x35 [btrfs]
> [16283.636704]  [<ffffffffa038f885>] btrfs_balance+0xda1/0xe4a [btrfs]
> [16283.637766]  [<ffffffff81276712>] ? debug_smp_processor_id+0x17/0x19
> [16283.638697]  [<ffffffffa03992b4>] btrfs_ioctl_balance+0x255/0x2d3 [btrfs]
> [16283.639675]  [<ffffffffa039c349>] btrfs_ioctl+0x136d/0x27a9 [btrfs]
> [16283.640801]  [<ffffffff8108a8b0>] ? arch_local_irq_save+0x9/0xc
> [16283.642074]  [<ffffffff81276727>] ? __this_cpu_preempt_check+0x13/0x15
> [16283.643015]  [<ffffffff81143aea>] ? handle_mm_fault+0x44f/0xd40
> [16283.643879]  [<ffffffff81486ab7>] ? _raw_spin_unlock+0x31/0x44
> [16283.644819]  [<ffffffff8108a8b0>] ? arch_local_irq_save+0x9/0xc
> [16283.645981]  [<ffffffff811822f8>] do_vfs_ioctl+0x42b/0x4ea
> [16283.646799]  [<ffffffff810fc6b0>] ? time_hardirqs_on+0x15/0x28
> [16283.647660]  [<ffffffff81001017>] ? trace_hardirqs_on_thunk+0x17/0x19
> [16283.648643]  [<ffffffff8118b4de>] ? __fget_light+0x4d/0x71
> [16283.649601]  [<ffffffff8118240e>] SyS_ioctl+0x57/0x79
> (...)
> 
> I'm getting really many of these, about 10+ per test when it happens
> (which makes the tests fail).

Sorry about that, I hoped I could get away with minimum of new sysfs
code to cover the missing feature bits. I'll get to fix it.

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] btrfs: sysfs: introduce helper for syncing bits with sysfs files
  2016-01-29 13:55   ` Filipe Manana
  2016-01-29 14:32     ` David Sterba
@ 2016-01-29 14:33     ` Chris Mason
  2016-01-29 18:34     ` David Sterba
  2 siblings, 0 replies; 11+ messages in thread
From: Chris Mason @ 2016-01-29 14:33 UTC (permalink / raw)
  To: Filipe Manana; +Cc: David Sterba, linux-btrfs

On Fri, Jan 29, 2016 at 01:55:31PM +0000, Filipe Manana wrote:
> On Thu, Jan 21, 2016 at 6:32 PM, David Sterba <dsterba@suse.com> wrote:
> > The files under /sys/fs/UUID/features get out of sync with the actual
> > incompat bits set for the filesystem if they change after mount. We're
> > going to sync them and need a helper to do that.
> >
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> >  fs/btrfs/sysfs.c | 30 ++++++++++++++++++++++++++++++
> >  fs/btrfs/sysfs.h |  3 +++
> >  2 files changed, 33 insertions(+)
> >
> > diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
> > index 906f7ed6fc80..6986886243bf 100644
> > --- a/fs/btrfs/sysfs.c
> > +++ b/fs/btrfs/sysfs.c
> > @@ -782,6 +782,36 @@ int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info)
> >         return error;
> >  }
> >
> > +
> > +/*
> > + * Change per-fs features in /sys/fs/btrfs/UUID/features to match current
> > + * values in superblock. Call after any changes to incompat/compat_ro flags
> > + */
> > +void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
> > +               u64 bit, enum btrfs_feature_set set)
> > +{
> > +       struct btrfs_fs_devices *fs_devs;
> > +       struct kobject *fsid_kobj;
> > +       u64 features;
> > +       int ret;
> > +
> > +       if (!fs_info)
> > +               return;
> > +
> > +       features = get_features(fs_info, set);
> > +       ASSERT(bit & supported_feature_masks[set]);
> > +
> > +       fs_devs = fs_info->fs_devices;
> > +       fsid_kobj = &fs_devs->fsid_kobj;
> > +
> > +       /*
> > +        * FIXME: this is too heavy to update just one value, ideally we'd like
> > +        * to use sysfs_update_group but some refactoring is needed first.
> > +        */
> > +       sysfs_remove_group(fsid_kobj, &btrfs_feature_attr_group);
> > +       ret = sysfs_create_group(fsid_kobj, &btrfs_feature_attr_group);
> 
> 
> Did you try to run all xfstests with this?
> 
> I'm getting very often lots of warnings in btrfs tests 060 to 074:

I did, but I have the compression remount tests ratelimited a bit
because of bugs in mount on my test box.

I might wait on the whole pull until before rc3, so we can sort this out
properly.

-chris

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] btrfs: sysfs: introduce helper for syncing bits with sysfs files
  2016-01-29 13:55   ` Filipe Manana
  2016-01-29 14:32     ` David Sterba
  2016-01-29 14:33     ` Chris Mason
@ 2016-01-29 18:34     ` David Sterba
  2016-01-29 20:28       ` Filipe Manana
  2 siblings, 1 reply; 11+ messages in thread
From: David Sterba @ 2016-01-29 18:34 UTC (permalink / raw)
  To: Filipe Manana; +Cc: David Sterba, linux-btrfs

On Fri, Jan 29, 2016 at 01:55:31PM +0000, Filipe Manana wrote:
> Did you try to run all xfstests with this?
> 
> I'm getting very often lots of warnings in btrfs tests 060 to 074:

Could you please test the attached patch in your testing setup? I was
not able to trigger the crashes with btrfs/060 062 063, running hundreds
of times with concurrent "find /sys/fs/btrfs -exec cat '{}' \;" in a
tight loop.

Out of all approaches how update the sysfs files, the one based on
workques looks best to me. The allocation context is safe wrt internal
sysfs allocations with GFP_KERNEL.

----------------------------8<-------------------------------
From: David Sterba <dsterba@suse.com>
Subject: [PATCH] btrfs: sysfs: update features from a workqueue

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/sysfs.c | 37 +++++++++++++++++++++++++++++++------
 fs/btrfs/sysfs.h |  7 +++++++
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 539e7b5e3f86..ced2570fdf6a 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -782,12 +782,7 @@ failure:
 	return error;
 }
 
-
-/*
- * Change per-fs features in /sys/fs/btrfs/UUID/features to match current
- * values in superblock. Call after any changes to incompat/compat_ro flags
- */
-void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
+static void btrfs_sysfs_do_feature_update(struct btrfs_fs_info *fs_info,
 		u64 bit, enum btrfs_feature_set set)
 {
 	struct btrfs_fs_devices *fs_devs;
@@ -815,6 +810,36 @@ void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
 	ret = sysfs_create_group(fsid_kobj, &btrfs_feature_attr_group);
 }
 
+static void btrfs_sysfs_feature_update_callback(struct work_struct *work)
+{
+	struct btrfs_feature_update *bfu = container_of(work,
+			struct btrfs_feature_update, work);
+
+	btrfs_sysfs_do_feature_update(bfu->fs_info, bfu->bit, bfu->set);
+	kfree(bfu);
+}
+
+/*
+ * Change per-fs features in /sys/fs/btrfs/UUID/features to match current
+ * values in superblock. Call after any changes to incompat/compat_ro flags
+ */
+void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
+		u64 bit, enum btrfs_feature_set set)
+{
+	struct btrfs_feature_update *bfu;
+
+	bfu = kmalloc(sizeof(*bfu), GFP_NOFS);
+	if (!bfu)
+		return;
+
+	INIT_WORK(&bfu->work, btrfs_sysfs_feature_update_callback);
+	bfu->fs_info = fs_info;
+	bfu->bit = bit;
+	bfu->set = set;
+
+	schedule_work(&bfu->work);
+}
+
 static int btrfs_init_debugfs(void)
 {
 #ifdef CONFIG_DEBUG_FS
diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
index d7da1a4c2f6c..d9772f2e5c9a 100644
--- a/fs/btrfs/sysfs.h
+++ b/fs/btrfs/sysfs.h
@@ -93,4 +93,11 @@ void btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs);
 void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
 		u64 bit, enum btrfs_feature_set set);
 
+struct btrfs_feature_update {
+	struct work_struct work;
+	struct btrfs_fs_info *fs_info;
+	u64 bit;
+	enum btrfs_feature_set set;
+};
+
 #endif /* _BTRFS_SYSFS_H_ */
-- 
1.8.4.5


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] btrfs: sysfs: introduce helper for syncing bits with sysfs files
  2016-01-29 18:34     ` David Sterba
@ 2016-01-29 20:28       ` Filipe Manana
  0 siblings, 0 replies; 11+ messages in thread
From: Filipe Manana @ 2016-01-29 20:28 UTC (permalink / raw)
  To: dsterba, Filipe Manana, David Sterba, linux-btrfs

On Fri, Jan 29, 2016 at 6:34 PM, David Sterba <dsterba@suse.cz> wrote:
> On Fri, Jan 29, 2016 at 01:55:31PM +0000, Filipe Manana wrote:
>> Did you try to run all xfstests with this?
>>
>> I'm getting very often lots of warnings in btrfs tests 060 to 074:
>
> Could you please test the attached patch in your testing setup? I was
> not able to trigger the crashes with btrfs/060 062 063, running hundreds
> of times with concurrent "find /sys/fs/btrfs -exec cat '{}' \;" in a
> tight loop.

Just bump the number of fsstress operation for 063 for example, makes
the issue easier to hit.

>
> Out of all approaches how update the sysfs files, the one based on
> workques looks best to me. The allocation context is safe wrt internal
> sysfs allocations with GFP_KERNEL.

I don't think the problem has anything to do with memory
allocations... Just concurrent calls to that new function or
concurrent remounts will trigger the warning in the new sysfs
functions - we're updating the sysfs group non-atomically - a remove +
add. So when a task attempts to remove it, some other has removed it
already, triggering one of the warnings. Same about the add operation.

So yes, the new patch triggers exactly the same warnings:

[41676.531816] BTRFS info (device sdg): disk space caching is enabled
[41676.535301] ------------[ cut here ]------------
[41676.544911] WARNING: CPU: 5 PID: 4499 at fs/sysfs/dir.c:31
sysfs_warn_dup+0x64/0x73()
[41676.546552] sysfs: cannot create duplicate filename
'/fs/btrfs/8024a413-cc1a-4e9b-a09a-255bd60a23da/features'
[41676.550614] Modules linked in: btrfs dm_flakey dm_mod ppdev
sha256_generic hmac xor drbg ansi_cprng aesni_intel raid6_pq
aes_x86_64 ablk_helper cryptd lrw gf128mul
 acpi_cpufreq glue_helper parport_pc sg evdev tpm_tis parport
i2c_piix4 tpm psmouse i2c_core pcspkr processor serio_raw button loop
autofs4 ext4 crc16 mbcache jbd2 sd
_mod sr_mod cdrom ata_generic virtio_scsi ata_piix virtio_pci libata
virtio_ring virtio e1000 crc32c_intel scsi_mod floppy [last unloaded:
btrfs]
[41676.570670] CPU: 5 PID: 4499 Comm: kworker/5:2 Tainted: G        W
     4.4.0-rc6-btrfs-next-18+ #1
[41676.573490] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS by qemu-project.org 04/01/2014
[41676.574947] Workqueue: events btrfs_sysfs_feature_update_callback [btrfs]
[41676.575944]  0000000000000000 ffff8803c5073c60 ffffffff8125d4fd
ffff8803c5073ca8
[41676.578680]  ffff8803c5073c98 ffffffff8105055e ffffffff811d9e55
ffff88037d669000
[41676.580851]  ffffffffa04d504c ffff8803e3e259d8 ffff8800b89841c8
ffff8803c5073d00
[41676.582641] Call Trace:
[41676.583059]  [<ffffffff8125d4fd>] dump_stack+0x4e/0x79
[41676.583814]  [<ffffffff8105055e>] warn_slowpath_common+0x9f/0xb8
[41676.585975]  [<ffffffff811d9e55>] ? sysfs_warn_dup+0x64/0x73
[41676.588266]  [<ffffffff810505bf>] warn_slowpath_fmt+0x48/0x50
[41676.589556]  [<ffffffff811d6ab9>] ? kernfs_path+0x4d/0x58
[41676.590323]  [<ffffffff811d9e55>] sysfs_warn_dup+0x64/0x73
[41676.591103]  [<ffffffff811da4b0>] internal_create_group+0xc7/0x26f
[41676.592134]  [<ffffffff811da66b>] sysfs_create_group+0x13/0x15
[41676.594360]  [<ffffffffa046c79f>]
btrfs_sysfs_feature_update_callback+0x7f/0x8c [btrfs]
[41676.594420] BTRFS info (device sdg): use no compression
[41676.594477] BTRFS info (device sdg): disk space caching is enabled
[41676.599794]  [<ffffffff81066d52>] process_one_work+0x24a/0x4ac
[41676.601319]  [<ffffffff810674b4>] worker_thread+0x206/0x2c2
[41676.602441]  [<ffffffff810672ae>] ? rescuer_thread+0x2cb/0x2cb
[41676.603364]  [<ffffffff8106c24d>] kthread+0xef/0xf7
[41676.605520]  [<ffffffff8106c15e>] ? kthread_parkme+0x24/0x24
[41676.606352]  [<ffffffff8148763f>] ret_from_fork+0x3f/0x70
[41676.607128]  [<ffffffff8106c15e>] ? kthread_parkme+0x24/0x24
[41676.607926] ---[ end trace 60f318fdba512bc7 ]---


>
> ----------------------------8<-------------------------------
> From: David Sterba <dsterba@suse.com>
> Subject: [PATCH] btrfs: sysfs: update features from a workqueue
>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/sysfs.c | 37 +++++++++++++++++++++++++++++++------
>  fs/btrfs/sysfs.h |  7 +++++++
>  2 files changed, 38 insertions(+), 6 deletions(-)
>
> diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
> index 539e7b5e3f86..ced2570fdf6a 100644
> --- a/fs/btrfs/sysfs.c
> +++ b/fs/btrfs/sysfs.c
> @@ -782,12 +782,7 @@ failure:
>         return error;
>  }
>
> -
> -/*
> - * Change per-fs features in /sys/fs/btrfs/UUID/features to match current
> - * values in superblock. Call after any changes to incompat/compat_ro flags
> - */
> -void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
> +static void btrfs_sysfs_do_feature_update(struct btrfs_fs_info *fs_info,
>                 u64 bit, enum btrfs_feature_set set)
>  {
>         struct btrfs_fs_devices *fs_devs;
> @@ -815,6 +810,36 @@ void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
>         ret = sysfs_create_group(fsid_kobj, &btrfs_feature_attr_group);
>  }
>
> +static void btrfs_sysfs_feature_update_callback(struct work_struct *work)
> +{
> +       struct btrfs_feature_update *bfu = container_of(work,
> +                       struct btrfs_feature_update, work);
> +
> +       btrfs_sysfs_do_feature_update(bfu->fs_info, bfu->bit, bfu->set);
> +       kfree(bfu);
> +}
> +
> +/*
> + * Change per-fs features in /sys/fs/btrfs/UUID/features to match current
> + * values in superblock. Call after any changes to incompat/compat_ro flags
> + */
> +void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
> +               u64 bit, enum btrfs_feature_set set)
> +{
> +       struct btrfs_feature_update *bfu;
> +
> +       bfu = kmalloc(sizeof(*bfu), GFP_NOFS);
> +       if (!bfu)
> +               return;
> +
> +       INIT_WORK(&bfu->work, btrfs_sysfs_feature_update_callback);
> +       bfu->fs_info = fs_info;
> +       bfu->bit = bit;
> +       bfu->set = set;
> +
> +       schedule_work(&bfu->work);
> +}
> +
>  static int btrfs_init_debugfs(void)
>  {
>  #ifdef CONFIG_DEBUG_FS
> diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
> index d7da1a4c2f6c..d9772f2e5c9a 100644
> --- a/fs/btrfs/sysfs.h
> +++ b/fs/btrfs/sysfs.h
> @@ -93,4 +93,11 @@ void btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs);
>  void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
>                 u64 bit, enum btrfs_feature_set set);
>
> +struct btrfs_feature_update {
> +       struct work_struct work;
> +       struct btrfs_fs_info *fs_info;
> +       u64 bit;
> +       enum btrfs_feature_set set;
> +};
> +
>  #endif /* _BTRFS_SYSFS_H_ */
> --
> 1.8.4.5
>



-- 
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."

^ permalink raw reply	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-01-29 20:28 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-21 18:31 [PULL][PATCH 0/4] Sysfs fixes to incompat bits David Sterba
2016-01-21 18:32 ` [PATCH 1/4] btrfs: sysfs: fix typo in compat_ro attribute definition David Sterba
2016-01-21 18:32 ` [PATCH 2/4] btrfs: sysfs: add free-space-tree bit attribute David Sterba
2016-01-21 18:32 ` [PATCH 3/4] btrfs: sysfs: introduce helper for syncing bits with sysfs files David Sterba
2016-01-29 13:55   ` Filipe Manana
2016-01-29 14:32     ` David Sterba
2016-01-29 14:33     ` Chris Mason
2016-01-29 18:34     ` David Sterba
2016-01-29 20:28       ` Filipe Manana
2016-01-21 18:32 ` [PATCH 4/4] btrfs: synchronize incompat feature " David Sterba
2016-01-26 20:23   ` Chris Mason

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.