Hi, On Mon, May 30, 2022 at 10:55 AM Alexander Aring <aahri...@redhat.com> wrote: > > This patch adds the resource name to dlm tracepoints. In case of > dlm_lock() we might end in a short time situation (if flags is not > convert) that a lkb is not associated with a resource at the time > when the tracepoint is called. Therefore we have a a prioritzation of > getting the resource name. If the resource in a lkb isn't set we use the > name and namelen passed as parameter as fallback. > > The dlm resource name can be decoded as a string representated ascii > codec. DLM itself stores the name with as a null terminated array but > the user does not required to pass a null terminated resource name. To > have somehow a similar behaviour and the user knows that the resource > name of the dlm user is ascii codec we store the resource name array in > a null terminated array but pass the array length for the trace user > without the null terminated byte. The advantage here is that the user > can run directly a printf() on the resource name array with a optional > check on isascii(). > > The TP_printk() call however use always a hexadecimal string > representation for the resource name array. > > Signed-off-by: Alexander Aring <aahri...@redhat.com> > --- > fs/dlm/lock.c | 4 +- > include/trace/events/dlm.h | 134 ++++++++++++++++++++++++++++++++----- > 2 files changed, 118 insertions(+), 20 deletions(-) > > diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c > index 226822f49d30..e80d42ba64ae 100644 > --- a/fs/dlm/lock.c > +++ b/fs/dlm/lock.c > @@ -3472,7 +3472,7 @@ int dlm_lock(dlm_lockspace_t *lockspace, > if (error) > goto out; > > - trace_dlm_lock_start(ls, lkb, mode, flags); > + trace_dlm_lock_start(ls, lkb, name, namelen, mode, flags); > > error = set_lock_args(mode, lksb, flags, namelen, 0, ast, > astarg, bast, &args); > @@ -3487,7 +3487,7 @@ int dlm_lock(dlm_lockspace_t *lockspace, > if (error == -EINPROGRESS) > error = 0; > out_put: > - trace_dlm_lock_end(ls, lkb, mode, flags, error); > + trace_dlm_lock_end(ls, lkb, name, namelen, mode, flags, error); > > if (convert || error) > __put_lkb(ls, lkb); > diff --git a/include/trace/events/dlm.h b/include/trace/events/dlm.h > index e333176ecfaf..180a24481929 100644 > --- a/include/trace/events/dlm.h > +++ b/include/trace/events/dlm.h > @@ -49,38 +49,56 @@ > /* note: we begin tracing dlm_lock_start() only if ls and lkb are found */ > TRACE_EVENT(dlm_lock_start, > > - TP_PROTO(struct dlm_ls *ls, struct dlm_lkb *lkb, int mode, > - __u32 flags), > + TP_PROTO(struct dlm_ls *ls, struct dlm_lkb *lkb, void *name, > + unsigned int namelen, int mode, __u32 flags), > > - TP_ARGS(ls, lkb, mode, flags), > + TP_ARGS(ls, lkb, name, namelen, mode, flags), > > TP_STRUCT__entry( > __field(__u32, ls_id) > __field(__u32, lkb_id) > __field(int, mode) > __field(__u32, flags) > + __dynamic_array(unsigned char, res_name, > + lkb->lkb_resource ? > lkb->lkb_resource->res_length + 1 : namelen + 1)
I will send a v2 series for this. I think we should remove the additional byte here and let the user handle it to parse it in some way and add a null termination on its own (if necessary). The array length in trace api is encoded in their trace UAPI/data structure, we can't make it ending '\0' and make the array length without the null termination. Or we add another field for the length and have two lengths there, whereas one is -1 of the other... sounds not right. However..., I will change that again. The user should not assume to have a '\0' terminated array here, _whereas_ in dlm_rsb kernel the user can assume it... but can't assume it's ascii code. > ), > > TP_fast_assign( > + struct dlm_rsb *r; > + char *c; > + > __entry->ls_id = ls->ls_global_id; > __entry->lkb_id = lkb->lkb_id; > __entry->mode = mode; > __entry->flags = flags; > + > + r = lkb->lkb_resource; > + if (r) > + memcpy(__get_dynamic_array(res_name), r->res_name, > + __get_dynamic_array_len(res_name)); > + else if (name) > + memcpy(__get_dynamic_array(res_name), name, > + __get_dynamic_array_len(res_name)); > + > + c = __get_dynamic_array(res_name); > + c[__get_dynamic_array_len(res_name) - 1] = '\0'; Hopefully we can remove this then... > ), > > - TP_printk("ls_id=%u lkb_id=%x mode=%s flags=%s", > + TP_printk("ls_id=%u lkb_id=%x mode=%s flags=%s res_name=%s", > __entry->ls_id, __entry->lkb_id, > show_lock_mode(__entry->mode), > - show_lock_flags(__entry->flags)) > + show_lock_flags(__entry->flags), > + __print_hex_str(__get_dynamic_array(res_name), > + __get_dynamic_array_len(res_name))) also wrong that it will always print the 00 byte at the end. :-/ - Alex