Details

Reviewers

trasz
markj
mckusick
chs

Commits

rGf19063ab029b: vfs_hash_rehash(): require the vnode to be exclusively locked
rG7c1e4aab7934: vfs_hash_insert: ensure that predicate is true
rG85fb840ebf3c: msdosfs: drop now unused DE_RENAME
rG6ae13c0feb96: msdosfs: add doscheckpath lock
rG95d42526e92c: msdosfs: fix rename
rGae7e8a02e6e9: msdosfs deget(): add locking flags argument
rG92d4e088274c: msdosfs: unstaticise msdosfs_lookup_

Summary

msdosfs_rename: Use the same locking algorithm for msdosfs_rename() as used by ufs_rename(). Convert doscheckpath() to non-sleeping version.

PR: 257522

msdosfs_rename: add a lock to ensure stability of the path check (ufs_checkpath and doscheckpath).

vfs_hash_insert: ensure that predicate is true

After vnode lock, recheck v_hash. When vfs_hash_insert() is used with a predicate, recheck it after the selected vnode is locked. Since vfs_hash_lock is dropped, vnode could be rehashed during the sleep for the vnode lock, which could go unnoticed there.

[The addition of the excl lock assert into vfs_hash_rehash()] is not going to be committed right now]

Tested by: pho

Per-commit view is available at https://kib.kiev.ua/git/gitweb.cgi?p=deviant3.git;a=shortlog;h=refs/heads/msdosfs_rename

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

kib requested review of this revision.Aug 8 2021, 7:45 PM

kib created this revision.

ufs fix was committed
Upload rebased patch, with the accumulated bug fixes.

Upload patch which passed pho run.

This looks like a correct adaptation of the UFS code to the MSDOS filesystem implementation. Given that I have been struggling with getting UFS rename correct since the mid-1980s I am sure there are still bugs lurking in this implementation, but at least they are closer to being the same bugs in both filesystem implementations.

This revision is now accepted and ready to land.Aug 20 2021, 1:00 AM

Fix problems with corrupted lookup cached data in directory vnodes. Basically, add non-sleeping calls to msdosfs_lookup_ino() into several places in msdosfs_rename() to refresh de offsets.

This revision now requires review to proceed.Aug 25 2021, 9:35 PM

This revision was not accepted when it landed; it landed in state Needs Review.Aug 27 2021, 3:40 PM

Closed by commit rG92d4e088274c: msdosfs: unstaticise msdosfs_lookup_ (authored by kib). · Explain Why

This revision was automatically updated to reflect the committed changes.

kib added a commit: rG92d4e088274c: msdosfs: unstaticise msdosfs_lookup_.

kib added a commit: rGae7e8a02e6e9: msdosfs deget(): add locking flags argument.

kib added a commit: rG95d42526e92c: msdosfs: fix rename.

kib added a commit: rG6ae13c0feb96: msdosfs: add doscheckpath lock.

kib added a commit: rG85fb840ebf3c: msdosfs: drop now unused DE_RENAME.

kib added a commit: rG7c1e4aab7934: vfs_hash_insert: ensure that predicate is true.

kib added a commit: rGf19063ab029b: vfs_hash_rehash(): require the vnode to be exclusively locked.

trasz mentioned this in D31366: msdosfs: drop DE_RENAME.Sep 1 2021, 9:38 AM