diff mbox

[7/7] Btrfs: warn if total_bytes_pinned is non-zero on unmount

Message ID 0a015ca3c5d799b501b9c23475ed9e96d16de671.1496792333.git.osandov@fb.com (mailing list archive)
State New, archived
Headers show

Commit Message

Omar Sandoval June 6, 2017, 11:45 p.m. UTC
From: Omar Sandoval <osandov@fb.com>

Catch any future/remaining leaks or underflows of total_bytes_pinned.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 fs/btrfs/extent-tree.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Liu Bo June 7, 2017, 8:22 p.m. UTC | #1
On Tue, Jun 06, 2017 at 04:45:32PM -0700, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> 
> Catch any future/remaining leaks or underflows of total_bytes_pinned.
>

This might be a little bit late, what about checking it after
btrfs_finish_extetn_commit()?

-liubo

> Signed-off-by: Omar Sandoval <osandov@fb.com>
> ---
>  fs/btrfs/extent-tree.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 75ad24f8d253..5fb2fb27eda6 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -9860,6 +9860,7 @@ int btrfs_free_block_groups(struct btrfs_fs_info *info)
>  			    space_info->bytes_reserved > 0 ||
>  			    space_info->bytes_may_use > 0))
>  			dump_space_info(info, space_info, 0, 0);
> +		WARN_ON(percpu_counter_sum(&space_info->total_bytes_pinned) != 0);
>  		list_del(&space_info->list);
>  		for (i = 0; i < BTRFS_NR_RAID_TYPES; i++) {
>  			struct kobject *kobj;
> -- 
> 2.13.0
> 
--
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
Omar Sandoval June 9, 2017, 11:45 p.m. UTC | #2
On Wed, Jun 07, 2017 at 01:22:04PM -0700, Liu Bo wrote:
> On Tue, Jun 06, 2017 at 04:45:32PM -0700, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> > 
> > Catch any future/remaining leaks or underflows of total_bytes_pinned.
> >
> 
> This might be a little bit late, what about checking it after
> btrfs_finish_extetn_commit()?
> 
> -liubo

Only reason I didn't put it there was because then I'd have to spend
time to convince myself that it couldn't be modified concurrently :)
I'll try it and make sure. Thanks for the review!
--
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
Jeff Mahoney June 13, 2017, 6:35 p.m. UTC | #3
On 6/6/17 7:45 PM, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> 
> Catch any future/remaining leaks or underflows of total_bytes_pinned.
> 
> Signed-off-by: Omar Sandoval <osandov@fb.com>
> ---
>  fs/btrfs/extent-tree.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 75ad24f8d253..5fb2fb27eda6 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -9860,6 +9860,7 @@ int btrfs_free_block_groups(struct btrfs_fs_info *info)
>  			    space_info->bytes_reserved > 0 ||
>  			    space_info->bytes_may_use > 0))
>  			dump_space_info(info, space_info, 0, 0);
> +		WARN_ON(percpu_counter_sum(&space_info->total_bytes_pinned) != 0);
>  		list_del(&space_info->list);
>  		for (i = 0; i < BTRFS_NR_RAID_TYPES; i++) {
>  			struct kobject *kobj;
> 

Can we group this in with the other WARN_ON and add printing
total_bytes_pinned to dump_space_info?  Understanding the magnitude and
whether we're underflowed or haven't released enough is helpful.  While
testing your patchset, I did this and it found a few bugs in cleanup
after error.  I'll post those patches shortly.

-Jeff
David Sterba June 21, 2017, 5:40 p.m. UTC | #4
On Tue, Jun 06, 2017 at 04:45:32PM -0700, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> 
> Catch any future/remaining leaks or underflows of total_bytes_pinned.
> 
> Signed-off-by: Omar Sandoval <osandov@fb.com>

This patch received some objections. As it's a debugging aid, I'd rather
merge a patch that other agree is useful for that purpose.
--
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 mbox

Patch

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 75ad24f8d253..5fb2fb27eda6 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -9860,6 +9860,7 @@  int btrfs_free_block_groups(struct btrfs_fs_info *info)
 			    space_info->bytes_reserved > 0 ||
 			    space_info->bytes_may_use > 0))
 			dump_space_info(info, space_info, 0, 0);
+		WARN_ON(percpu_counter_sum(&space_info->total_bytes_pinned) != 0);
 		list_del(&space_info->list);
 		for (i = 0; i < BTRFS_NR_RAID_TYPES; i++) {
 			struct kobject *kobj;