On Thu, Dec 1, 2016 at 6:18 AM, Alexey Lyashkov
<alexey.lyashkov@xxxxxxxxx> wrote:
> rehash process protected with d_seq and d_lock locks, but VFS have access to the
> d_hashed field without any locks sometimes. It produce errors with get cwd
> operations or fsnotify may report an unlink event sometimes. d_seq lock isn’t
> used to protect due possibility to sleep with holding locks, and d_lock is too
> hot to check a simple state, lets return a DCACHE_DENTRY_UNHASHED flag to
> ability to check a unhashed state without locks held.
>
>
If I am counting correctly, you modified all call sites of __d_drop() except
for those in d_move() and d_delete(). Right?
I suggest that you keep all those call sites in tact and only modify the
2 call sites with changed behavior.
Also, best give the new helper a name that is a bit more meaningful to
understand the changed behavior (maybe __d_unhash(), do_d_drop()).
Then you may add a comment to explain why you need to use the
modified behavior when you use it. Why do you need it in d_delete()?
Incidentally, I just wrote a helper I needed to find out if a dentry I got from
exportfs_decode_fh() is "lookable" as opposed to a "connected zombie".
So your patch is certainly usable for my use case.
Can anyone say if my helper is correct and whether is usable for anyone
else?
/* Check if p1 is connected with a chain of hashed dentries to p2 */
static bool ovl_is_lookable(struct dentry *p1, struct dentry *p2)
{
struct dentry *p;
for (p = p2; !IS_ROOT(p); p = p->d_parent) {
/*
* FIXME: there is a small window inside d_move()
* where moved entry is unhashed and rehashed and
* this can fail.
*/
if (d_unhashed(p))
return false;
if (p->d_parent == p1)
return true;
}
return false;
}
[...]
> @@ -326,11 +327,12 @@ extern struct dentry *dget_parent(struct dentry *dentry);
> * @dentry: entry to check
> *
> * Returns true if the dentry passed is not currently hashed.
> + * hlist_bl_unhashed can't be used as race with d_move().
> */
>
remove this newline while at it.
> static inline int d_unhashed(const struct dentry *dentry)
> {
> - return hlist_bl_unhashed(&dentry->d_hash);
> + return dentry->d_flags & DCACHE_DENTRY_UNHASHED;
> }
>
Thanks,
Amir.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
