-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 5/23/14, 3:04 PM, Jeff Mahoney wrote:
> We are currently allocating space_info objects in an array when we
> allocate space_info. When a user does something like:
>
> # btrfs balance start -mconvert=raid1 -dconvert=raid1 /mnt # btrfs
> balance start -mconvert=single -dconvert=single /mnt -f # btrfs
> balance start -mconvert=raid1 -dconvert=raid1 /
>
> We can end up with memory corruption since the kobject hasn't been
> reinitialized properly and the name pointer was left set.
>
> The rationale behind allocating them statically was to avoid
> creating a separate kobject container that just contained the raid
> type. It used the index in the array to determine the index.
>
> Ultimately, though, this wastes more memory than it saves in all
> but the most complex scenarios and introduces kobject lifetime
> questions.
>
> This patch allocates the kobjects dynamically instead. Note that we
> also remove the kobject_get/put of the parent kobject since
> kobject_add and kobject_del do that internally.
Nack.
Works with switching raid groups but crashes on umount.
- -Jeff
> Signed-off-by: Jeff Mahoney <je...@suse.com> --- fs/btrfs/ctree.h
> | 8 +++++++- fs/btrfs/extent-tree.c | 33
> +++++++++++++++++++++------------ fs/btrfs/sysfs.c | 5
> +++-- 3 files changed, 31 insertions(+), 15 deletions(-)
>
> --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -1113,6 +1113,12
> @@ struct btrfs_qgroup_limit_item { __le64 rsv_excl; }
> __attribute__ ((__packed__));
>
> +/* For raid type sysfs entries */ +struct raid_kobject { + int
> raid_type; + struct kobject kobj; +}; + struct btrfs_space_info {
> spinlock_t lock;
>
> @@ -1163,7 +1169,7 @@ struct btrfs_space_info { wait_queue_head_t
> wait;
>
> struct kobject kobj; - struct kobject
> block_group_kobjs[BTRFS_NR_RAID_TYPES]; + struct kobject
> *block_group_kobjs[BTRFS_NR_RAID_TYPES]; };
>
> #define BTRFS_BLOCK_RSV_GLOBAL 1 --- a/fs/btrfs/extent-tree.c
> +++
> b/fs/btrfs/extent-tree.c @@ -3401,10 +3401,8 @@ static int
> update_space_info(struct btrf return ret; }
>
> - for (i = 0; i < BTRFS_NR_RAID_TYPES; i++) { + for (i = 0; i <
> BTRFS_NR_RAID_TYPES; i++) INIT_LIST_HEAD(&found->block_groups[i]);
> - kobject_init(&found->block_group_kobjs[i], &btrfs_raid_ktype); -
> } init_rwsem(&found->groups_sem); spin_lock_init(&found->lock);
> found->flags = flags & BTRFS_BLOCK_GROUP_TYPE_MASK; @@ -8327,7
> +8325,8 @@ int btrfs_free_block_groups(struct btrfs
> list_del(&space_info->list); for (i = 0; i < BTRFS_NR_RAID_TYPES;
> i++) { struct kobject *kobj; - kobj =
> &space_info->block_group_kobjs[i]; + kobj =
> space_info->block_group_kobjs[i]; +
> space_info->block_group_kobjs[i] = NULL; if (kobj->parent) {
> kobject_del(kobj); kobject_put(kobj); @@ -8352,17 +8351,26 @@
> static void __link_block_group(struct bt
> up_write(&space_info->groups_sem);
>
> if (first) { - struct kobject *kobj =
> &space_info->block_group_kobjs[index]; + struct raid_kobject
> *rkobj; int ret;
>
> - kobject_get(&space_info->kobj); /* put in release */ -
> ret =
> kobject_add(kobj, &space_info->kobj, "%s", -
> get_raid_name(index)); + rkobj = kzalloc(sizeof(*rkobj),
> GFP_KERNEL); + if (!rkobj) + goto out_err; +
> rkobj->raid_type =
> index; + kobject_init(&rkobj->kobj, &btrfs_raid_ktype); +
> ret =
> kobject_add(&rkobj->kobj, &space_info->kobj, +
> "%s",
> get_raid_name(index)); if (ret) { - pr_warn("BTRFS: failed
> to add
> kobject for block cache. ignoring.\n"); -
> kobject_put(&space_info->kobj); +
> kobject_put(&rkobj->kobj); +
> goto out_err; } + space_info->block_group_kobjs[index] =
> &rkobj->kobj; } + + return; +out_err: + pr_warn("BTRFS: failed to
> add kobject for block cache. ignoring.\n"); }
>
> static struct btrfs_block_group_cache * @@ -8796,8 +8804,9 @@ int
> btrfs_remove_block_group(struct btrf */
> list_del_init(&block_group->list); if
> (list_empty(&block_group->space_info->block_groups[index])) { -
> kobject_del(&block_group->space_info->block_group_kobjs[index]); -
> kobject_put(&block_group->space_info->block_group_kobjs[index]); +
> kobject_del(block_group->space_info->block_group_kobjs[index]); +
> kobject_put(block_group->space_info->block_group_kobjs[index]); +
> block_group->space_info->block_group_kobjs[index] = NULL;
> clear_avail_alloc_bits(root->fs_info, block_group->flags); }
> up_write(&block_group->space_info->groups_sem); ---
> a/fs/btrfs/sysfs.c +++ b/fs/btrfs/sysfs.c @@ -254,6 +254,7 @@
> static ssize_t global_rsv_reserved_show(
> BTRFS_ATTR(global_rsv_reserved, 0444, global_rsv_reserved_show);
>
> #define to_space_info(_kobj) container_of(_kobj, struct
> btrfs_space_info, kobj) +#define to_raid_kobj(_kobj)
> container_of(_kobj, struct raid_kobject, kobj)
>
> static ssize_t raid_bytes_show(struct kobject *kobj, struct
> kobj_attribute *attr, char *buf); @@ -266,7 +267,7 @@ static
> ssize_t raid_bytes_show(struct ko { struct btrfs_space_info *sinfo
> = to_space_info(kobj->parent); struct btrfs_block_group_cache
> *block_group; - int index = kobj - sinfo->block_group_kobjs; + int
> index = to_raid_kobj(kobj)->raid_type; u64 val = 0;
>
> down_read(&sinfo->groups_sem); @@ -288,7 +289,7 @@ static struct
> attribute *raid_attributes
>
> static void release_raid_kobj(struct kobject *kobj) { -
> kobject_put(kobj->parent); + kfree(to_raid_kobj(kobj)); }
>
> struct kobj_type btrfs_raid_ktype = {
>
>
- --
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.22 (Darwin)
iQIcBAEBAgAGBQJTf6RhAAoJEB57S2MheeWyMMsP/0qXabjwUETeCxa/VIKbAMP8
C/xzcOPrVIsxK/3EjD865o8dEoUI56FT0pWgxYxysafwdG8VVTCTJi1GXb8gl+yo
JQZOS/JzLSPM+KElzBDZ+2KkDxz3wtx/XVrWQruOCQ+ycx1KVSS5TQ5vNFPMucik
/MBYSx3rDmD9Np5iyv2McbZLqZDlqe57dsuCjZd0p9cBR1DguhZKuhgl1Bf4j7RR
+O7Gip15SAuwYbU+Qntwe6e4i7OAuVIqeYDXP/EJtN/PWzoopwnSatYIHUFhjAXj
Y+GIzxNAZP9rpCjc0akZM1zxgHi9Xrowg/2bWWGfu0GU+ChL02+CdQFt8ZUylsQQ
AndOwTW8QU+PMV9qOzkDj38yrSTWTWeh9SAE85JSzaP4bRaFKqNudnwGrlYUwRqe
TFbuR4JMubGCM4+vSzVgPznY7tiapNtRuoE7Ij5TYQW6arB4aXSUuk351qFGozI+
7N1G1S7KOSFvXGCPgIiz7QqREq9uahb9nEtEBOdkuSZWxMJBmaxfBI5o6n4E0MQA
UEt2IDsZV6p6FbEZGG49eXBVV6JYD0UGrW4I0HAgYFbIRx/0E+E+VuxMsTqjpGl1
NprRDOO6UdzbGM+hCYIc++wXJgq0PYJYBbOM7PKLaCLdTyRlgeZWmmUoIcXkm1oh
tODf44NEMvOVF/sKIMZz
=3iv4
-----END PGP SIGNATURE-----
--
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
