N when mounted linrdlnk
Needs ReviewPublic
Actions

Authored by nick_spun.io on Fri, Sep 4, 4:28 PM.

Details

Reviewers

kib
adrian

Summary

Linux resolves a path below /proc/self/fd/N in the directory the
descriptor names, and linprocfs makes /proc/<pid>/fd a symlink to
/dev/fd. Under linrdlnk the fdescfs node carries only VV_READLINK,
which namei will not walk through, so such a path fails with ENOTDIR.

Return the underlying vnode from fdesc_lookup for a non-final component,
or a trailing slash, reusing the machinery the nodup option already
uses. The last component is untouched, so open("/dev/fd/N") keeps its
dup(2) semantic; a descriptor with no vnode behind it, such as a pipe,
yields ENOTDIR.

Add ATF coverage for traversal, descriptor reuse, and preservation of
last-component and mount-option semantics.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 76602
Build 73485: arc lint + arc unit

Event Timeline

nick_spun.io created this revision.Fri, Sep 4, 4:28 PM

Herald added a subscriber: imp. · View Herald TranscriptFri, Sep 4, 4:28 PM

nick_spun.io requested review of this revision.Fri, Sep 4, 4:28 PM

Harbormaster completed remote builds in B76500: Diff 185827.Fri, Sep 4, 4:28 PM

Linked from D59271

nick_spun.io edited the summary of this revision. (Show Details)Fri, Sep 4, 4:38 PM

Yes I think this is quite close to what I though would work.
But don't you need to reclaim the vnodes aggressively to not have the non-last component lookup to not affect the last component lookup?
This might be handled by providing fdescfs_inactive() VOP that does the reclaim.

In D59393#1363727, @kib wrote:

Yes I think this is quite close to what I though would work.
But don't you need to reclaim the vnodes aggressively to not have the non-last component lookup to not affect the last component lookup?
This might be handled by providing fdescfs_inactive() VOP that does the reclaim.

If I'm looking at this correctly I don't think we need to change reclaim behavior - the non-last just returns the underlying vnode (same branch nodup uses) so we're not creating an fdescfs node, and we're avoiding fdhashtbl entirely with the lookup. Does that seem right, or am I off base here?

In D59393#1363743, @nick_spun.io wrote:

In D59393#1363727, @kib wrote:

Yes I think this is quite close to what I though would work.
But don't you need to reclaim the vnodes aggressively to not have the non-last component lookup to not affect the last component lookup?
This might be handled by providing fdescfs_inactive() VOP that does the reclaim.

If I'm looking at this correctly I don't think we need to change reclaim behavior - the non-last just returns the underlying vnode (same branch nodup uses) so we're not creating an fdescfs node, and we're avoiding fdhashtbl entirely with the lookup. Does that seem right, or am I off base here?

Yes, you are right.

I suggest to ask Peter Holm (pho@) to run the stress2 parts relevant for fdescfs. You may also explain what additional tests are useful to add for this case.

kib accepted this revision.Sat, Sep 5, 10:13 PM

kib added a subscriber: pho.

This revision is now accepted and ready to land.Sat, Sep 5, 10:13 PM

In D59393#1363763, @kib wrote:

In D59393#1363743, @nick_spun.io wrote:

In D59393#1363727, @kib wrote:

Yes I think this is quite close to what I though would work.
But don't you need to reclaim the vnodes aggressively to not have the non-last component lookup to not affect the last component lookup?
This might be handled by providing fdescfs_inactive() VOP that does the reclaim.

If I'm looking at this correctly I don't think we need to change reclaim behavior - the non-last just returns the underlying vnode (same branch nodup uses) so we're not creating an fdescfs node, and we're avoiding fdhashtbl entirely with the lookup. Does that seem right, or am I off base here?

Yes, you are right.

I suggest to ask Peter Holm (pho@) to run the stress2 parts relevant for fdescfs. You may also explain what additional tests are useful to add for this case.