Brandon Williams <bmw...@google.com> writes:
>> In my mind, the value of having a constant check_attr is primarily
>> that it gives us a stable pointer to serve as a hashmap key,
>> i.e. the identifier for each call site, in a later iteration.
>
> We didn't really discuss this notion of having the pointer be a key into
> a hashmap, what sort of information are you envisioning being stored in
> this sort of hashmap?
The "entries relevant to this attr_check() call, that is specific to
the <check_attr instance, the thread> tuple" (aka "what used to be
called the global attr_stack") we discussed would be the primary
example. A thread is likely be looping in a caller that has many
paths inside a directory, calling a function that has a call to
attr_check() for each path. Having something that can use to
identify the check_attr instance in a stable way, even when the
inner function is called and returns many times, would allow us to
populate the "attr stack" just once for the thread when it enters a
directory for the first time (remember, another thread may be
executing the same codepath, checking for paths in a different
directory) and keep using it. There may be other mechanisms you can
come up with, so I wouldn't say it is the only valid way, but it is
a way. That is why I said:
>> But all of the above comes from my intuition, and I'll very much
>> welcome to be proven wrong with an alternative design, or better
>> yet, a working code based on an alternative design ;-).
near the end of my message.
> One issue I can see with this is that the
> functions which have a static attr_check struct would still not be thread
> safe if the initialization of the structure isn't surrounded by a mutex
> itself. ie
Yes, that goes without saying. That is why I suggested Stefan to do
not this:
> static struct attr_check *check;
>
> if (!check)
> init(check);
>
> would need to be:
>
> lock()
> if (!check)
> init(check);
> unlock();
but this:
static struct attr_check *check;
init(&check);
and hide the lock/unlock gymnastics inside the API. I thought that
already was in what you inherited from him and started your work
on top of?
