On 3/4/19 4:02 PM, Frank Filz wrote:
On 3/4/19 1:52 PM, katcherw@gmail.com wrote:
Daniel,
I don't think this assumption is always correct. In mdcache_refresh_attrs:
/* We will want all the requested attributes in the entry */ entry->attrs.request_mask = attrs.request_mask;
In my situation, this is turning off the ATTR_ACL bit in the entry's request mask.
Okay, I see. Something like this:
I've been looking over the code. That simple fix might be enough, but I'm not sure.
I think our attribute tracking has gotten overly complex and some things are disconnected.
One thought I have is for the time stamp of an attribute to actually be supplied by the FSAL, at least conceptually (i.e. we could easily set it in mdcache_refresh_attrs just after calling FSAL getattrs). But make the time stamps be part of struct attrlist.
Then each attribute (or group of attributes) has a bit in valid_mask, and a time stamp. When the cached attributes are updated, the bit in the valid mask, the attribute(s), and the time stamp are all copied together.
I wonder if we really need separate trust flags in mde_flags?
And we should make it such that request_mask is ONLY used to indicate to getattrs which attributes are of interest (and of course mdcache may pass a different request_mask to the underlying FSAL than it was passed).
In the current code, I see that fs_locations_time isn't ever checked, and there is no separate sec_label_time...
Frank
I'm not adverse to changing how we handle attribute validity. However, I think a timestamp-per-attribute would more than double the size of the attrlist, and so I think something else is necessary. It would also lead to a *ton* of timestamp comparisons for any getattr operation, which would add to latency. Daniel