this is a rough check I used for testing. Basically we want the d_off value to be the lseek value *after* reading a given entry. To check for lseek we must only iterate over getdirentries one entry at a time, otherwise the lseek offset will be too far off.

Maybe a sweep with opengrok is a good idea to see where we should be using this.

Can you please check lib/libc/sys/getdirentries.c line 60:

dst->d_off = 0;	/* nothing uses it yet, so safe for now */

The main feasibility test is in-kernel nfs server.

In D17917#382778, @pfg wrote:

Maybe a sweep with opengrok is a good idea to see where we should be using this.

I don't know about opengrok, but this is a new feature only affecting a small part of the ABI so it seems pretty well contained.

Can you please check lib/libc/sys/getdirentries.c line 60:

dst->d_off = 0;	/* nothing uses it yet, so safe for now */

This is compatibility code called in __cvt_dirents_from11, it only called for kernels previous to the 64bit inode switch (see below). In this case there's no way userspace can expect anything meaningful in d_off.

lib/libc/sys/getdirentries.c line 87:

if (__getosreldate() >= INO64_FIRST)
    return (__sys_getdirentries( ... ))

.
.
.

__cvt_dirents_from11( ... )

In D17917#382796, @jack_gandi.net wrote:

In D17917#382778, @pfg wrote:

Maybe a sweep with opengrok is a good idea to see where we should be using this.

I don't know about opengrok, but this is a new feature only affecting a small part of the ABI so it seems pretty well contained.

opengrok is a web based grep, see src.illumos.org, just for curiosity ;).

Can you please check lib/libc/sys/getdirentries.c line 60:

dst->d_off = 0;	/* nothing uses it yet, so safe for now */

This is compatibility code called in __cvt_dirents_from11, it only called for kernels previous to the 64bit inode switch (see below). In this case there's no way userspace can expect anything meaningful in d_off.

lib/libc/sys/getdirentries.c line 87:

if (__getosreldate() >= INO64_FIRST)
    return (__sys_getdirentries( ... ))

.
.
.

__cvt_dirents_from11( ... )

Ahh, OK. I only read the comment in a quick sweep., thanks for checking.

While you're at it, would you mind documenting d_off in getdirentries.2? The field is included in the structure definition, but its semantics are not defined there.

I can't seem to remember how to do inline comments (if I ever knew), so I'll stick it here...

I doubt it will matter, but for udf, the last argument to udf_uiodir() is the "offset cookie".
The code sets it to 1 and 2 for "." and "..", since it notes that the real offset is zero and NFS doesn't like that.
Then, for subsequent entries it sets it to "ds->this_off". If you look in udf_getfid(),
it sets "ds->this_off" to "ds->offset + ds->off", so I think your "else" case is correct,
but might be more consistent if you used "ds->this_off".
(SInce I don't even know what UDF is, I suspect no one exports it via NFS and it doesn't matter anyhow?)

The other changes look pretty much the same as what I did a couple of years ago for a patch that never
made it into head (I never tested the UDF case, so I can't say I got it right using "1", "2" and "ds->this_off"?).

Once this goes in, I can look at using d_off in the kernel nfsd. (I have old patches for that stuff too.)

Nice change. Can you bump the .Dd of the man page as this a content change. Thanks!

Added an inline suggestion related to the man page update.

Also please add an implementation note to mention readdir plus NFS RPC response, as suggested by Rick.

My intent when adding the d_off field was that it be used for this purpose, so I am glad to see someone actually making it happen.

Some filesystems use a B-tree or other non-linear structure for their directories, so my thought was that d_off be treated as a cookie that would have to be passed to seekdir (rather than lseek) to get back to the next entry. So I am not entirely sure that describing it as an lseek offset is the correct semantics. Passing it as a cookie to seekdir rather than as an offset to lseek would allow us to support a wider range of filesystems. Of course, at the moment seekdir is just using it as an offset to lseek, so if that is what Linux does, then we should probably not diverge from that.

In D17917#383763, @mckusick wrote:

Some filesystems use a B-tree or other non-linear structure for their directories, so my thought was that d_off be treated as a cookie that would have to be passed to seekdir (rather than lseek) to get back to the next entry.

I agree that d_off should be an arbitrary cookie, but don't understand the suggestion for seekdir vs lseek.

So I am not entirely sure that describing it as an lseek offset is the correct semantics. Passing it as a cookie to seekdir rather than as an offset to lseek would allow us to support a wider range of filesystems.

I believe for OneFS we would be fine interpreting defining d_off as a valid lseek offset.

Of course, at the moment seekdir is just using it as an offset to lseek, so if that is what Linux does, then we should probably not diverge from that.

Right — seekdir is just some additional complexity and seatbelt around lseek() to validate that inputs match outputs from telldir(3).

In D17917#383719, @kib wrote:

Also please add an implementation note to mention readdir plus NFS RPC response, as suggested by Rick.

I'm not sure what you mean by this, should I add a new section to the manpage?

In D17917#383881, @jack_gandi.net wrote:

In D17917#383719, @kib wrote:

Also please add an implementation note to mention readdir plus NFS RPC response, as suggested by Rick.

I'm not sure what you mean by this, should I add a new section to the manpage?

Yes, this is most suitable for 'IMPLEMENTATION NOTES section.

"Cookie" or "handle" are definitely good words here. The part "to position the directory descriptor to the start of the entry next to the current" could be simplified a bit to "to position the directory descriptor to the next entry" since the position cannot be in the middle of an entry.

The man page share/man/man5/dir.5 incorrectly states that d_off is /* directory offset of entry */; this must be "of next entry".

In D17917#383965, @kib wrote:

In D17917#383881, @jack_gandi.net wrote:

In D17917#383719, @kib wrote:

Also please add an implementation note to mention readdir plus NFS RPC response, as suggested by Rick.

I'm not sure what you mean by this, should I add a new section to the manpage?

Yes, this is most suitable for 'IMPLEMENTATION NOTES section.

I've added an example of the NFS usage, although after looking at other man (see pipe(2)) it looks like IMPLEMENTATION NOTES is used for details about the implementation rather than how the feature is used.

In D17917#384160, @jilles wrote:

"Cookie" or "handle" are definitely good words here. The part "to position the directory descriptor to the start of the entry next to the current" could be simplified a bit to "to position the directory descriptor to the next entry" since the position cannot be in the middle of an entry.

The man page share/man/man5/dir.5 incorrectly states that d_off is /* directory offset of entry */; this must be "of next entry".

Finally settled for cookie. sorry for the double upload. I felt like handle sounded like a ressource/allocated which it is not.

Note the request for attributes/metadata for the change. I will commit after you provide the info.

In D17917#384287, @kib wrote:

Note the request for attributes/metadata for the change. I will commit after you provide the info.

Uploaded in the latest diff in the commit message, if you can't see it here it is also:

Submitted by:  Jack Halford <jack@gandi.net>
Sponsored by:  Gandi.net

I was trying to svn commit on my own as described on the wiki, no wonder it didn't work.