On Mon, 5 Apr 2021 18:01:41 +0300
Yordan Karadzhov <y.karadz@xxxxxxxxx> wrote:
> > This makes the API not thread safe. There should be no problem with
> > supported options, as they cannot be changed at runtime. I think it is
> > OK to limit the APIs , as I see no use case to call them from multiple
> > threads on the same instance, but this should be described in the
> > documentation.
>
> Discussing is this API is thread safe or not is not really relevant.
> Even if you make tracefs_options_get_enabled() thread safe, this does
> not change anything since the options can be already changed by the time
> when you examine the bits of the mask.
You are correct, but you also do not want the update of the bitmask to be
corrupted. Thus I think a pthread_mutex() should be added to the updating
of the value, such that we don't have a read/modify/write corruption. Sure
the reader of the mask may get some stale data if its being updated. But it
shouldn't get corrupted data.
void tracefs_option_set(struct tracefs_options_mask *options, enum tracefs_option_id id)
{
if (options && id > TRACEFS_OPTION_INVALID)
options->mask |= (1ULL << (id - 1));
}
If two threads are doing this to the same mask, you can have an option set
that gets cleared by another writer, and even though the option is set
after the call, it will be clear in the mask. That is, it's not stale data,
it's incorrect data. That is, the readers after that corruption will get an
option not set, even though it is.
-- Steve
|