9:10 a.m.
On 3/4/19 4:02 PM, Frank Filz wrote:
>> On 3/4/19 1:52 PM, katcherw(a)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:
>>
>>
>> https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/446980
>
> 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.
That's why I said "or group of attributes". In fact, other than adding a
time stamp for security label, I'm not proposing adding any more time stamps (though I
don't know if there might be a case for dividing the POSIX attributes out that are
updated by things like layout commit, the question there would be do clients ever request
a subset of attributes that are easily maintained by Ganesha without having to go to the
file system. Outside of that, all the traditional POSIX attributes need only be covered by
a single time stamp. Hmm, another place that might call for more granularity would be
really using STATX to request subsets of the POSIX attributes. These ideas would need to
be explored in more detail.
Frank