Re: [PATCH 2/2] btrfs: prefix fsid to all trace events

[Jeff Mahoney]

On 6/13/16 11:48 AM, David Sterba wrote:
> On Thu, Jun 09, 2016 at 07:48:01PM -0400, je...@suse.com wrote:
>> +#define TP_printk_btrfs(fmt, args...) \
>> +TP_printk("%pU: " fmt, __entry->fsid, args)
> 
> So it's identified by the UUID. As there's no previous format of the
> filesystem identifier (unlike the syslog messages), I'm ok with using
> the UUID.

Buried in the patch is actually the single use of a previous format for
trace events that I've converted to use the macros.  It's the
btrfs_space_reservation that Josef added in 2012[1].

I wouldn't have minded a shorter identifier but given how prevalent its
usage is internal to the FS, its exported interfaces, and tools
surrounding it, it seems like it's the only real choice.

-Jeff

[1] 8c2a3ca20f6 (Btrfs: space leak tracepoints)

-- 
Jeff Mahoney
SUSE Labs



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 2/2] btrfs: prefix fsid to all trace events

[David Sterba]

On Thu, Jun 09, 2016 at 07:48:01PM -0400, je...@suse.com wrote:
> +#define TP_printk_btrfs(fmt, args...) \
> + TP_printk("%pU: " fmt, __entry->fsid, args)

So it's identified by the UUID. As there's no previous format of the
filesystem identifier (unlike the syslog messages), I'm ok with using
the UUID.
--
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/2] btrfs: prefix fsid to all trace events

[Chris Mason]

On Thu, Jun 09, 2016 at 07:48:01PM -0400, je...@suse.com wrote:
> From: Jeff Mahoney 
> 
> When using trace events to debug a problem, it's impossible to determine
> which file system generated a particular event.  This patch adds a
> macro to prefix standard information to the head of a trace event.
> 
> The extent_state alloc/free events are all that's left without an
> fs_info available.

No complaints here.  Do we know of anyone that depends on the current
trace output?

-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

Re: [PATCH 2/2] btrfs: prefix fsid to all trace events

[Liu Bo]

On Thu, Jun 09, 2016 at 07:48:01PM -0400, je...@suse.com wrote:
> From: Jeff Mahoney 
> 
> When using trace events to debug a problem, it's impossible to determine
> which file system generated a particular event.  This patch adds a
> macro to prefix standard information to the head of a trace event.
> 
> The extent_state alloc/free events are all that's left without an
> fs_info available.

Looks good to me.

Reviewed-by: Liu Bo 

Thanks,

-liubo

> 
> Signed-off-by: Jeff Mahoney 
> ---
>  fs/btrfs/delayed-ref.c   |   9 +-
>  fs/btrfs/extent-tree.c   |  10 +-
>  fs/btrfs/qgroup.c|  19 +--
>  fs/btrfs/qgroup.h|   9 +-
>  fs/btrfs/super.c |   2 +-
>  include/trace/events/btrfs.h | 282 
> ---
>  6 files changed, 182 insertions(+), 149 deletions(-)
> 
> diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
> index 430b368..e7b1ec0 100644
> --- a/fs/btrfs/delayed-ref.c
> +++ b/fs/btrfs/delayed-ref.c
> @@ -606,7 +606,8 @@ add_delayed_ref_head(struct btrfs_fs_info *fs_info,
>   qrecord->num_bytes = num_bytes;
>   qrecord->old_roots = NULL;
>  
> - qexisting = btrfs_qgroup_insert_dirty_extent(delayed_refs,
> + qexisting = btrfs_qgroup_insert_dirty_extent(fs_info,
> +  delayed_refs,
>qrecord);
>   if (qexisting)
>   kfree(qrecord);
> @@ -615,7 +616,7 @@ add_delayed_ref_head(struct btrfs_fs_info *fs_info,
>   spin_lock_init(_ref->lock);
>   mutex_init(_ref->mutex);
>  
> - trace_add_delayed_ref_head(ref, head_ref, action);
> + trace_add_delayed_ref_head(fs_info, ref, head_ref, action);
>  
>   existing = htree_insert(_refs->href_root,
>   _ref->href_node);
> @@ -682,7 +683,7 @@ add_delayed_tree_ref(struct btrfs_fs_info *fs_info,
>   ref->type = BTRFS_TREE_BLOCK_REF_KEY;
>   full_ref->level = level;
>  
> - trace_add_delayed_tree_ref(ref, full_ref, action);
> + trace_add_delayed_tree_ref(fs_info, ref, full_ref, action);
>  
>   ret = add_delayed_ref_tail_merge(trans, delayed_refs, head_ref, ref);
>  
> @@ -739,7 +740,7 @@ add_delayed_data_ref(struct btrfs_fs_info *fs_info,
>   full_ref->objectid = owner;
>   full_ref->offset = offset;
>  
> - trace_add_delayed_data_ref(ref, full_ref, action);
> + trace_add_delayed_data_ref(fs_info, ref, full_ref, action);
>  
>   ret = add_delayed_ref_tail_merge(trans, delayed_refs, head_ref, ref);
>  
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 689d25a..ecb68bb 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -2194,7 +2194,7 @@ static int run_delayed_data_ref(struct 
> btrfs_trans_handle *trans,
>   ins.type = BTRFS_EXTENT_ITEM_KEY;
>  
>   ref = btrfs_delayed_node_to_data_ref(node);
> - trace_run_delayed_data_ref(node, ref, node->action);
> + trace_run_delayed_data_ref(root->fs_info, node, ref, node->action);
>  
>   if (node->type == BTRFS_SHARED_DATA_REF_KEY)
>   parent = ref->parent;
> @@ -2349,7 +2349,7 @@ static int run_delayed_tree_ref(struct 
> btrfs_trans_handle *trans,
>SKINNY_METADATA);
>  
>   ref = btrfs_delayed_node_to_tree_ref(node);
> - trace_run_delayed_tree_ref(node, ref, node->action);
> + trace_run_delayed_tree_ref(root->fs_info, node, ref, node->action);
>  
>   if (node->type == BTRFS_SHARED_BLOCK_REF_KEY)
>   parent = ref->parent;
> @@ -2413,7 +2413,8 @@ static int run_one_delayed_ref(struct 
> btrfs_trans_handle *trans,
>*/
>   BUG_ON(extent_op);
>   head = btrfs_delayed_node_to_head(node);
> - trace_run_delayed_ref_head(node, head, node->action);
> + trace_run_delayed_ref_head(root->fs_info, node, head,
> +node->action);
>  
>   if (insert_reserved) {
>   btrfs_pin_extent(root, node->bytenr,
> @@ -8316,7 +8317,8 @@ static int record_one_subtree_extent(struct 
> btrfs_trans_handle *trans,
>  
>   delayed_refs = >transaction->delayed_refs;
>   spin_lock(_refs->lock);
> - if (btrfs_qgroup_insert_dirty_extent(delayed_refs, qrecord))
> + if (btrfs_qgroup_insert_dirty_extent(trans->root->fs_info,
> +  delayed_refs, qrecord))
>   kfree(qrecord);
>   spin_unlock(_refs->lock);
>  
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 9d4c05b..13e28d8 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -1453,9 +1453,10 @@ int btrfs_qgroup_prepare_account_extents(struct 
> btrfs_trans_handle *trans,
>   return ret;
>  }
>  
> -struct 

[PATCH 2/2] btrfs: prefix fsid to all trace events

[jeffm]

From: Jeff Mahoney 

When using trace events to debug a problem, it's impossible to determine
which file system generated a particular event.  This patch adds a
macro to prefix standard information to the head of a trace event.

The extent_state alloc/free events are all that's left without an
fs_info available.

Signed-off-by: Jeff Mahoney 
---
 fs/btrfs/delayed-ref.c   |   9 +-
 fs/btrfs/extent-tree.c   |  10 +-
 fs/btrfs/qgroup.c|  19 +--
 fs/btrfs/qgroup.h|   9 +-
 fs/btrfs/super.c |   2 +-
 include/trace/events/btrfs.h | 282 ---
 6 files changed, 182 insertions(+), 149 deletions(-)

diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
index 430b368..e7b1ec0 100644
--- a/fs/btrfs/delayed-ref.c
+++ b/fs/btrfs/delayed-ref.c
@@ -606,7 +606,8 @@ add_delayed_ref_head(struct btrfs_fs_info *fs_info,
qrecord->num_bytes = num_bytes;
qrecord->old_roots = NULL;
 
-   qexisting = btrfs_qgroup_insert_dirty_extent(delayed_refs,
+   qexisting = btrfs_qgroup_insert_dirty_extent(fs_info,
+delayed_refs,
 qrecord);
if (qexisting)
kfree(qrecord);
@@ -615,7 +616,7 @@ add_delayed_ref_head(struct btrfs_fs_info *fs_info,
spin_lock_init(_ref->lock);
mutex_init(_ref->mutex);
 
-   trace_add_delayed_ref_head(ref, head_ref, action);
+   trace_add_delayed_ref_head(fs_info, ref, head_ref, action);
 
existing = htree_insert(_refs->href_root,
_ref->href_node);
@@ -682,7 +683,7 @@ add_delayed_tree_ref(struct btrfs_fs_info *fs_info,
ref->type = BTRFS_TREE_BLOCK_REF_KEY;
full_ref->level = level;
 
-   trace_add_delayed_tree_ref(ref, full_ref, action);
+   trace_add_delayed_tree_ref(fs_info, ref, full_ref, action);
 
ret = add_delayed_ref_tail_merge(trans, delayed_refs, head_ref, ref);
 
@@ -739,7 +740,7 @@ add_delayed_data_ref(struct btrfs_fs_info *fs_info,
full_ref->objectid = owner;
full_ref->offset = offset;
 
-   trace_add_delayed_data_ref(ref, full_ref, action);
+   trace_add_delayed_data_ref(fs_info, ref, full_ref, action);
 
ret = add_delayed_ref_tail_merge(trans, delayed_refs, head_ref, ref);
 
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 689d25a..ecb68bb 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -2194,7 +2194,7 @@ static int run_delayed_data_ref(struct btrfs_trans_handle 
*trans,
ins.type = BTRFS_EXTENT_ITEM_KEY;
 
ref = btrfs_delayed_node_to_data_ref(node);
-   trace_run_delayed_data_ref(node, ref, node->action);
+   trace_run_delayed_data_ref(root->fs_info, node, ref, node->action);
 
if (node->type == BTRFS_SHARED_DATA_REF_KEY)
parent = ref->parent;
@@ -2349,7 +2349,7 @@ static int run_delayed_tree_ref(struct btrfs_trans_handle 
*trans,
 SKINNY_METADATA);
 
ref = btrfs_delayed_node_to_tree_ref(node);
-   trace_run_delayed_tree_ref(node, ref, node->action);
+   trace_run_delayed_tree_ref(root->fs_info, node, ref, node->action);
 
if (node->type == BTRFS_SHARED_BLOCK_REF_KEY)
parent = ref->parent;
@@ -2413,7 +2413,8 @@ static int run_one_delayed_ref(struct btrfs_trans_handle 
*trans,
 */
BUG_ON(extent_op);
head = btrfs_delayed_node_to_head(node);
-   trace_run_delayed_ref_head(node, head, node->action);
+   trace_run_delayed_ref_head(root->fs_info, node, head,
+  node->action);
 
if (insert_reserved) {
btrfs_pin_extent(root, node->bytenr,
@@ -8316,7 +8317,8 @@ static int record_one_subtree_extent(struct 
btrfs_trans_handle *trans,
 
delayed_refs = >transaction->delayed_refs;
spin_lock(_refs->lock);
-   if (btrfs_qgroup_insert_dirty_extent(delayed_refs, qrecord))
+   if (btrfs_qgroup_insert_dirty_extent(trans->root->fs_info,
+delayed_refs, qrecord))
kfree(qrecord);
spin_unlock(_refs->lock);
 
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 9d4c05b..13e28d8 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1453,9 +1453,10 @@ int btrfs_qgroup_prepare_account_extents(struct 
btrfs_trans_handle *trans,
return ret;
 }
 
-struct btrfs_qgroup_extent_record
-*btrfs_qgroup_insert_dirty_extent(struct btrfs_delayed_ref_root *delayed_refs,
- struct btrfs_qgroup_extent_record *record)
+struct btrfs_qgroup_extent_record *
+btrfs_qgroup_insert_dirty_extent(struct btrfs_fs_info *fs_info,
+