|
This bug prompted several, fairly large changes to the of OneFS's
readdirplus() within Samba.
One fundamental problem is that we kept our cache cursor pointed at the
next entry to be returned from onefs_readdir(), while the resume cookie
needed to refill the cache such that our cursor would be on this entry,
was located in the previous cache entry. This meant that to correctly handle
seekdir() cases which could be found within the existing cache, and cases
where a cache reload was needed, required that the cache always hold
at least two entries: the entry we wished to return, and the previous entry
which held the resume cookie. Since the readdirplus() syscall gives us no
guarantee that it will always return these two direntries, there was a
fundamental problem with this design.
To fix this problem, I have rearchitected the onefs_readdir() path to keep
its pointer on the entry which contains the resume_cookie, not the entry
which will be returned next. Essentially, I changed onefs_readdir() from a
"return an entry then increment the cursor" model to "increment the cursor
then return an entry". By doing this, we only require that a single entry
be within the cache: the entry containing the resume cookie.
Second, there have been numerous off-by-one bugs in my implementation of
onefs_seekdir() which did a mapping between the 64-bit resume cookie
returned by readdirplus() and its own monotonically increasing "location"
offset. Furthermore, this design caused a somewhat frequent waste of
cycles, as in some cases we'd need to re-enumerate the entire directory to
recover the current "location" from an old resume cookie. As this code was
somewhat difficult to understand, prone to bugs, and innefficient in some
cases I decided it was better to wholesale replace it now, rather than later.
It is possible to algorithmically map the 64-bit resume cookies from
readdirplus() into 32-bit offset values which SMB requires. The onefs.so
module now calls into a system library to do this conversion. This greatly
simplifies both the seekdir() and telldir() paths and is more efficient.
|