lkml.org 
[lkml]   [2022]   [Dec]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2 2/2] trace: allocate space from temparary trace sequence buffer
From

On 12/15/2022 9:23 PM, Steven Rostedt wrote:
> On Thu, 15 Dec 2022 12:33:27 +0800
> Linyu Yuan <quic_linyyuan@quicinc.com> wrote:
>
>> --- a/include/linux/trace_seq.h
>> +++ b/include/linux/trace_seq.h
>> @@ -95,6 +95,7 @@ extern void trace_seq_bitmask(struct trace_seq *s, const unsigned long *maskp,
>> extern int trace_seq_hex_dump(struct trace_seq *s, const char *prefix_str,
>> int prefix_type, int rowsize, int groupsize,
>> const void *buf, size_t len, bool ascii);
>> +void *trace_seq_alloc_buffer(struct trace_seq *s, int len);
> So, I really don't like the name with "alloc" in it. That makes the
> assumption that it also needs to be freed. Which it does not, and why it
> confused me last night.
>
> A better name would be trace_seq_acquire(s, len);
>
> And it should return a char *, as it it process stings and not arbitrary
> binary data.
thanks, will update next version.
>
>> +/**
>> + * trace_seq_alloc_buffer - allocate seq buffer with size len
>> + * @s: trace sequence descriptor
>> + * @len: size of buffer to be allocated
>> + *
>> + * allocate space with size of @len from seq buffer for output usage,
>> + * On success, it returns start address of the allocated buffer,
>> + * user can fill data start from the address, user should make sure the
>> + * data length not exceed the @len, if it exceed, behavior is undefined.
>> + *
>> + * Returns NULL if no buffer can be allocated, it also means system will
>> + * crash, it is user responsiblity to make sure total buffer used will
>> + * not exceed PAGE_SIZE.
>> + *
>> + * it allow multiple usage in one trace output function call.
>> + */
>> +void *trace_seq_alloc_buffer(struct trace_seq *s, int len)
> char *trace_seq_acquire(struct trace_seq *s, int len)
>
>> +{
>> + char *buf = trace_seq_buffer_ptr(s);
>> +
> You need to check the length first before committing:
>
> if (seq_buf_buffer_left(&s->seq) < len)
> return NULL;
will change next version.
>
>> + seq_buf_commit(&s->seq, len);
> Because the above is a bug if len is too big.
>
>> +
>> + if (unlikely(seq_buf_has_overflowed(&s->seq))) {
> And then we don't need this either.
>
> I must apologize for the response last night. It was past my normal bed
> time, and I must really avoid reviewing patches when I should be going to
> bed ;-)

sorry to bring your attention near your bed time.

I accept late response next time 😁.

thanks again for all your comment today, it is very good.

>
> -- Steve
>
>
>> + s->full = 1;
>> + return NULL;
>> + }
>> +
>> + return (void *)buf;
>> +}
>> +EXPORT_SYMBOL(trace_seq_alloc_buffer);

\
 
 \ /
  Last update: 2022-12-16 03:05    [W:0.077 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site