| Message ID | 1498488155-9941-2-git-send-email-nborisov@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 26.06.2017 17:42, Nikolay Borisov wrote: > With this patch applied pahole stats look like: > > /* size: 840, cachelines: 14, members: 40 */ > /* sum members: 833, holes: 1, sum holes: 7 */ > /* bit holes: 1, sum bit holes: 28 bits */ > /* last cacheline: 8 bytes */ > > No functional changes. > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/ctree.h | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h > index cdd3775e930b..bdd06bbeb9aa 100644 > --- a/fs/btrfs/ctree.h > +++ b/fs/btrfs/ctree.h > @@ -586,6 +586,11 @@ struct btrfs_block_group_cache { > unsigned int iref:1; > unsigned int has_caching_ctl:1; > unsigned int removed:1; > + /* > + * Does the block group need to be added to the free space tree? > + * Protected by free_space_lock. > + */ > + unsigned int needs_free_space:1; Upon closer inspection of memory-barriers.txt I'm not confident in this change. This puts fields protected by different locks in the same bitfield which can lead to corrupted values. > > int disk_cache_state; > > @@ -608,6 +613,8 @@ struct btrfs_block_group_cache { > /* usage count */ > atomic_t count; > > + atomic_t trimming This one will likely eliminated 1 hole in the struct so I might end up sending v2 of this patch. > + > /* List of struct btrfs_free_clusters for this block group. > * Today it will only have one thing on it, but that may change > */ > @@ -619,8 +626,6 @@ struct btrfs_block_group_cache { > /* For read-only block groups */ > struct list_head ro_list; > > - atomic_t trimming; > - > /* For dirty block groups */ > struct list_head dirty_list; > struct list_head io_list; > @@ -651,11 +656,6 @@ struct btrfs_block_group_cache { > /* Lock for free space tree operations. */ > struct mutex free_space_lock; > > - /* > - * Does the block group need to be added to the free space tree? > - * Protected by free_space_lock. > - */ > - int needs_free_space; > > /* Record locked full stripes for RAID5/6 block group */ > struct btrfs_full_stripe_locks_tree full_stripe_locks_root; > -- 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
On Mon, Jun 26, 2017 at 06:30:34PM +0300, Nikolay Borisov wrote: > > > On 26.06.2017 17:42, Nikolay Borisov wrote: > > With this patch applied pahole stats look like: > > > > /* size: 840, cachelines: 14, members: 40 */ > > /* sum members: 833, holes: 1, sum holes: 7 */ > > /* bit holes: 1, sum bit holes: 28 bits */ > > /* last cacheline: 8 bytes */ > > > > No functional changes. > > > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> > > --- > > fs/btrfs/ctree.h | 14 +++++++------- > > 1 file changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h > > index cdd3775e930b..bdd06bbeb9aa 100644 > > --- a/fs/btrfs/ctree.h > > +++ b/fs/btrfs/ctree.h > > @@ -586,6 +586,11 @@ struct btrfs_block_group_cache { > > unsigned int iref:1; > > unsigned int has_caching_ctl:1; > > unsigned int removed:1; > > + /* > > + * Does the block group need to be added to the free space tree? > > + * Protected by free_space_lock. > > + */ > > + unsigned int needs_free_space:1; > Upon closer inspection of memory-barriers.txt I'm not confident in this > change. This puts fields protected by different locks in the same > bitfield which can lead to corrupted values. Alternatively, you can switch the flags to unsigned long and access them using set_bit/test_bit, they're atomic regarding updates. -- 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
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index cdd3775e930b..bdd06bbeb9aa 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -586,6 +586,11 @@ struct btrfs_block_group_cache { unsigned int iref:1; unsigned int has_caching_ctl:1; unsigned int removed:1; + /* + * Does the block group need to be added to the free space tree? + * Protected by free_space_lock. + */ + unsigned int needs_free_space:1; int disk_cache_state; @@ -608,6 +613,8 @@ struct btrfs_block_group_cache { /* usage count */ atomic_t count; + atomic_t trimming; + /* List of struct btrfs_free_clusters for this block group. * Today it will only have one thing on it, but that may change */ @@ -619,8 +626,6 @@ struct btrfs_block_group_cache { /* For read-only block groups */ struct list_head ro_list; - atomic_t trimming; - /* For dirty block groups */ struct list_head dirty_list; struct list_head io_list; @@ -651,11 +656,6 @@ struct btrfs_block_group_cache { /* Lock for free space tree operations. */ struct mutex free_space_lock; - /* - * Does the block group need to be added to the free space tree? - * Protected by free_space_lock. - */ - int needs_free_space; /* Record locked full stripes for RAID5/6 block group */ struct btrfs_full_stripe_locks_tree full_stripe_locks_root;
With this patch applied pahole stats look like: /* size: 840, cachelines: 14, members: 40 */ /* sum members: 833, holes: 1, sum holes: 7 */ /* bit holes: 1, sum bit holes: 28 bits */ /* last cacheline: 8 bytes */ No functional changes. Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- fs/btrfs/ctree.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)