lookup: do not return vp_crossmp as dvp for mount over the reg file
Needs ReviewPublic
Actions

Authored by kib on Sun, Jul 12, 12:04 PM.

Details

Reviewers

olce
markj
jah

Summary

Return the covered vnode instead.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

kib created this revision.Sun, Jul 12, 12:04 PM

Herald added subscribers: olce, imp. · View Herald TranscriptSun, Jul 12, 12:04 PM

kib requested review of this revision.Sun, Jul 12, 12:04 PM

kib mentioned this in D58178: vfs: handle file mounts in vn_fullpath_hardlink().Sun, Jul 12, 12:06 PM

nick_spun.io added a subscriber: nick_spun.io.Sun, Jul 12, 6:30 PM

I think this is a good structural fix - running it locally with no issues so far

kib added reviewers: olce, markj, jah.Mon, Jul 13, 6:17 AM

kib removed a subscriber: olce.

Can we add a regression test case for this? I will write one if someone will sketch out what needs to happen.

In D58191#1335065, @markj wrote:

Can we add a regression test case for this? I will write one if someone will sketch out what needs to happen.

I missed D58178.

In D58191#1335084, @markj wrote:

In D58191#1335065, @markj wrote:

Can we add a regression test case for this? I will write one if someone will sketch out what needs to happen.

I missed D58178.

I think it makes more sense to add this to stress2. Also it should get the rename() test. I will talk with Peter about it.

kib added a subscriber: pho.Mon, Jul 13, 1:57 PM

markj added inline comments.Mon, Jul 13, 6:12 PM

sys/kern/vfs_lookup.c
1492
1509	I think you have the lock flags and lock state parameters flipped around here.
1517	We did not acquire a usecount ref on ndp->ni_dvp, but in the error path we call vrele(ndp->ni_dvp).

kib marked 3 inline comments as done.Mon, Jul 13, 6:21 PM

kib added inline comments.

sys/kern/vfs_lookup.c
1509	Yes, Peter also reported the issue, I fixed it on the branch.

Сorrect arguments and simplify the vn_lock_pair() call. No need for the locked variable.
vref(mvp) unconditionally to pacify the error path.

I ran all of the stress2 tests without seeing any issues.

In D58191#1337590, @pho wrote:

I ran all of the stress2 tests without seeing any issues.

Also the specific tests for the situation in PR282596 were run. It seems that the change is enough.

Revision Contents
Changeset List

Path

Size

sys/

kern/

vfs_lookup.c

39 lines

vfs_syscalls.c

9 lines

Diff 181829

View Options

sys/kern/vfs_lookup.c