On Sat, Jan 30, 2016 at 12:51:46PM -0800, Santosh Madiraju wrote:
> From: madiraju <madiraju.santosh@xxxxxxxxx>
>
> While shrinking the dentry list in shrink_dentry_list function,
> if DCACHE_MAY_FREE flag is set, it frees the dentry, and it again
> tries to do it in dentry_kill.
[snip]
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 92d5140..7aa2252 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -986,8 +986,8 @@ static void shrink_dentry_list(struct list_head *list)
> spin_unlock(&parent->d_lock);
> continue;
> }
> -
> - __dentry_kill(dentry);
> + if (dentry)
> + __dentry_kill(dentry);
Considering the fact that this call of __dentry_kill() is immediately preceded
by
inode = dentry->d_inode;
if (inode && unlikely(!spin_trylock(&inode->i_lock))) {
d_shrink_add(dentry, list);
spin_unlock(&dentry->d_lock);
if (parent)
spin_unlock(&parent->d_lock);
continue;
}
it is flat-out impossible to get to that call with dentry equal to NULL -
we would've oopsed on attempt to fetch dentry->d_inode, not to mention that
the value in dentry ultimately comes from
dentry = list_entry(list->prev, struct dentry, d_lru);
which _never_ yields NULL. And that access of dentry->d_inode is not the
only place in between where we would've oopsed with dentry being NULL,
while we are at it.
And description also makes no sense whatsoever - if we run into DCACHE_MAY_FREE
(which can't be set without DCACHE_DENTRY_KILLED having been set first), we
won't even reach that __dentry_kill() - we'll run into
if (unlikely(dentry->d_flags & DCACHE_DENTRY_KILLED)) {
bool can_free = dentry->d_flags & DCACHE_MAY_FREE;
spin_unlock(&dentry->d_lock);
if (parent)
spin_unlock(&parent->d_lock);
if (can_free)
dentry_free(dentry);
continue;
}
and that's it as far as this dentry is concerned. We'd done
d_shrink_del(dentry) - that's the very first thing we do and we do it
unconditionally.
What's more, making no sense is about the only thing description and patch
have in common. NAK.
--
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
