All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Yordan Karadzhov <y.karadz@gmail.com>
Cc: linux-trace-devel@vger.kernel.org, linux-kernel@vger.kernel.org,
	Tom Zanussi <zanussi@kernel.org>,
	Daniel Bristot de Oliveira <bristot@redhat.com>,
	Masami Hiramatsu <mhiramat@kernel.org>
Subject: Re: [PATCH 1/2] libtracefs: Add random number to keep synthetic variables unique
Date: Thu, 12 Aug 2021 10:00:10 -0400	[thread overview]
Message-ID: <20210812100010.0f3922d1@oasis.local.home> (raw)
In-Reply-To: <13bed3a6-7e39-eec2-2421-a80081302a81@gmail.com>

On Thu, 12 Aug 2021 11:34:57 +0300
Yordan Karadzhov <y.karadz@gmail.com> wrote:

> > @@ -957,7 +960,15 @@ static char *new_arg(struct tracefs_synth *synth)
> >   	char *arg;
> >   	int ret;
> >   
> > -	ret = asprintf(&arg, "__arg__%d", cnt);
> > +	/* Create a unique argument name */
> > +	if (!synth->arg_name[0]) {
> > +		srand(time(NULL));  
> 
> Nit: Have in mind that time(NULL) has 1 second resolution. Fast consecutive calls (within a second) of this function can 
> generate identical random numbers.
> This can be mitigated if we do something like this:
> 
> 		struct timeval now;
> 
> 		gettimeofday(&now, NULL);
> 		srand(now.tv_usec);

So you are saying that if one thread created two synthetic events
within a second, then this could give the same value. Yeah, I can see
that could happen. I was hoping to avoid the declaring the "now" and
calling gettimeofday().

Also, looking more into this, I see that rand() is not safe in thread
context (it may not be a problem, but there's no guarantee), and
perhaps we should just open code it, to be on the safe side.

Thanks for the review.

-- Steve

  reply	other threads:[~2021-08-12 14:00 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-12  0:55 [PATCH 0/2] libtracefs: Make hist variable names unique Steven Rostedt
2021-08-12  0:55 ` [PATCH 1/2] libtracefs: Add random number to keep synthetic variables unique Steven Rostedt
2021-08-12  8:34   ` Yordan Karadzhov
2021-08-12 14:00     ` Steven Rostedt [this message]
2021-08-12  0:55 ` [PATCH 2/2] libtracefs: Have end event variables not be the end event field name Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210812100010.0f3922d1@oasis.local.home \
    --to=rostedt@goodmis.org \
    --cc=bristot@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=y.karadz@gmail.com \
    --cc=zanussi@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.