Re: [PATCH] cachefiles: Fix excess dput() after end_removing()

[David Howells <dhowells@redhat.com>]

Marc Dionne <marc.c.dionne@gmail.com> wrote:

> I think it is the correct Fixes tag, but I'm not sure that this is
> actually the right fix.  7bb1eb45e43c switched other callers of
> cachefiles_bury_object to use start_removing_dentry, which gets an
> additional ref, and removed the extra dget from
> cachefiles_bury_object.  In the cachefiles_cull case however, the
> dentry is from start_removing and has a single ref on entry to
> cachefiles_bury_object, which is an issue as "rep" may be used there
> after end_removing may have put the last ref.  So the correct is
> probably for cachefiles_cull to add a dget() before the call to
> cachefiles_bury_object.

Ugh.  You're right.

The problem is that we're calling start_removing() without knowing whether we
can just unlink the object.  I wonder if I need to do the lookup in
cachefiles_lookup_for_cull() and only then call start_removing_dentry() if
it's not a directory (directories get moved to the graveyard for cachefilesd
to tear down).

I think the right solution is actually to move start_removing_dentry() down
into cachefiles_bury_object() and make it contingent on the dentry being a
non-dir.

David