Page MenuHomeFreeBSD

nfsclient: Make per-entry directory positions usable
Needs ReviewPublic

Authored by hayzam_gmail.com on Sat, Oct 10, 10:43 PM.
Tags
None
Referenced Files
F175848772: D60603.id189506.diff
Tue, Oct 13, 9:02 PM
F175834500: D60603.diff
Tue, Oct 13, 6:14 PM
F175813763: D60603.id189472.diff
Tue, Oct 13, 2:24 PM
F175757062: D60603.id189506.diff
Tue, Oct 13, 2:29 AM
F175746493: D60603.id189472.diff
Tue, Oct 13, 12:07 AM
F175742211: D60603.id189472.diff
Mon, Oct 12, 11:15 PM
Unknown Object (File)
Mon, Oct 12, 8:13 PM
Unknown Object (File)
Mon, Oct 12, 8:03 PM
Subscribers

Details

Reviewers
markj
rmacklem
Summary

The NFS client leaves d_off zero. Populate it with next-entry offsets
into the client's synthesized directory stream when filling the cache
block, allowing reads to resume at individual entry boundaries.

Validate that records cover the buffer without crossing DIRBLKSIZ
boundaries. A saved position can point inside a record when the block
is rebuilt from a directory that changed on the server; move back to
the start of the record which contains it so that only whole records
are returned. Re-anchoring avoids returning fragments but does not
preserve the identity of a saved position across changes to the
synthesized layout: entries can be repeated or skipped. If the record
containing the position does not fit in the caller's buffer, fail the
read instead of returning a partial record, and leave the file position
unchanged so that a larger buffer can retry it.

Since the client now guarantees whole records, nfs_readdir() no longer
needs to round the requested size to a DIRBLKSIZ boundary.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 78029
Build 74912: arc lint + arc unit

Event Timeline

Clarify the commit message

Make the description consumer-agnostic. The Linuxulator change to use
these offsets for partial getdents returns will come in a different patch.

Rick is much more qualified to review this than me. I'm happy to go through the implementation if the idea seems sound.

This gets ugly when another client modifies the directory
by adding or deleting an entry.
Once that happens, all these "logical offsets" become bogus.

It also gets ugly when one of the directory blocks falls out
of the buffer cache. I theory, once read in from the server,
the buffer cache block should look identical to what was there
before (assuming the directory hasn't changed), but I'm not
sure that's guaranteed and could be different for non-FreeBSD
servers.
--> I consider it just plain lucky that no one complains about

readdir issues against non-FreeBSD servers.

There is something called a "directory offset cookie", which is
supposed to change whenever the "real offset cookies" change.
(This worked long ago, when Sun and most others used UFS,
where the offsets didn't change when directory entries were
removed. The "real" definition for "directory offset cookie" was
it changed when files were added to a directory. But this is no
longer the case and the "directory offset cookie" is essentially
useless.)

The problem with doing anything like this is trying to determine
if it works ok for all servers and not just FreeBSD ones.
(Bottom line, it's nearly impossible to test against these non-FreeBSD
servers, although if it behaves well for FreeBSD and Linux servers, that
might be ok?)

Thanks, Rick. I've updated the diff: it walks record boundaries and only returns complete records, and a saved position that lands inside a rebuilt record gets moved back to the start of that record. That stops fragments when a position lands inside a record, but it doesn't make the logical offsets stable when the layout changes.

I tested it on a FreeBSD box (UFS) and a Linux box running knfsd on ext4, over v3, v3 with READDIRPLUS, and v4 (v4.2 on Linux), with a second client deleting, adding, replacing and churning entries while a reader resumed from saved positions.

For unchanged directories, continuation matched by name and inode on both servers after forced cache eviction and reload. The cookie map stayed intact (hash-verified), and nfsstat confirmed fresh READDIR/READDIRPLUS RPCs; no re-anchoring occurred.

There were no malformed records in the revised tests on either server; the old revision returned them in about 9,300 of the 14,500 resumptions. Continuation is still best effort: after deletes or replaces the reader can skip surviving entries, sometimes even when the saved offset is still a record boundary. In the FreeBSD delete case, 220 of 222 checks missed surviving entries, 76 of those while still aligned; add-only changes preserved continuation in the tested cases. When the first record doesn't fit, EINVAL leaves the file position unchanged, so a larger-buffer retry works.

So this addresses record fragments, not stale-position tracking. Is that an acceptable tradeoff within the existing logical-offset scheme, or do you think per-entry continuation needs a different design? Happy to send the reproducer and logs.