Re: [PATCH v2 25/27] attr: store attribute stack in attr_check structure

Brandon Williams [off-list ref] writes:

quoted

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:

quoted

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?