[Linux-cachefs] PROBLEM: ASSERT(object->dentry) fails in cachefiles_delete_object()

[David Howells]

Manuel Schölling <manuel.schoelling at gmx.de> wrote:

>  #5 [ffff880575a03d80] cachefiles_delete_object at ffffffffa0433d4d
> [cachefiles]
>  #6 [ffff880575a03da0] cachefiles_drop_object at ffffffffa043280f
> [cachefiles]
>  #7 [ffff880575a03dc0] fscache_drop_object at ffffffffa02ac511 [fscache]
>  #8 [ffff880575a03df0] fscache_object_work_func at ffffffffa02ac697
> [fscache]

Okay, we're in the DROP_OBJECT state.  This cleans the object memory record up
before we free it and saves any data and metadata to the backing device or
deletes the object if retired.

>   dentry = 0x0, 
>   backer = 0x0, 

To get here with object->dentry set to NULL, I think we must've got to the
DROP_OBJECT state from one of the following states:

	STATE		TRANSITION PATH
	=============	===========================
	WAIT_FOR_INIT	oob via ABORT_INIT
	INIT_OBJECT	directly
	WAIT_FOR_PARENT	oob via ABORT_INIT
	PARENT_READY	oob via ABORT_INIT
	LOOKUP_OBJECT	directly via LOOKUP_FAILURE
	CREATE_OBJECT	directly via LOOKUP_FAILURE
	
The object->dentry pointer is:

 (1) set in cachefiles_walk_to_object(), which is called from
     cachefiles_lookup_object();

 (2) cleared in cachefiles_walk_to_object() upon failure; and

 (3) cleared in cachefiles_drop_object() after the call to
     cachefiles_delete_object() - in which the crash happened.

object->dentry is NULL, which suggests that we either haven't started the
on-disk object lookup/creation process yet or we did start it and we failed at
it.  Either way, we go via DROP_OBJECT to clean up with a NULL dentry pointer.

>     flags = 64, 

FSCACHE_OBJECT_RETIRED is set in cachefiles->fscache.flags which suggest that
the file may have been obsoleted before we finished setting it up - maybe we
were still waiting for the parent object to be created on disk.

So, what I think is probably happening is that the object is being obsoleted
by the netfs before we manage to finish setting it up.

Does the attached patch help?

David
---
commit eac30680e912d223841dfc220824b53446bdf235
Author: David Howells <dhowells at redhat.com>
Date:   Wed Sep 17 22:24:18 2014 +0100

    CacheFiles: Handle object being killed before being set up

diff --git a/fs/cachefiles/interface.c b/fs/cachefiles/interface.c
index 584743d456c3..94fe77b5dd99 100644
--- a/fs/cachefiles/interface.c
+++ b/fs/cachefiles/interface.c
@@ -268,20 +268,26 @@ static void cachefiles_drop_object(struct fscache_object *_object)
 	ASSERT((atomic_read(&object->usage) & 0xffff0000) != 0x6b6b0000);
 #endif
 
-	/* delete retired objects */
-	if (test_bit(FSCACHE_OBJECT_RETIRED, &object->fscache.flags) &&
-	    _object != cache->cache.fsdef
-	    ) {
-		_debug("- retire object OBJ%x", object->fscache.debug_id);
-		cachefiles_begin_secure(cache, &saved_cred);
-		cachefiles_delete_object(cache, object);
-		cachefiles_end_secure(cache, saved_cred);
-	}
+	/* We need to tidy the object up if we did in fact manage to open it.
+	 * It's possible for us to get here before the object is fully
+	 * initialised if the parent goes away.
+	 */
+	if (object->dentry) {
+		/* delete retired objects */
+		if (test_bit(FSCACHE_OBJECT_RETIRED, &object->fscache.flags) &&
+		    _object != cache->cache.fsdef
+		    ) {
+			_debug("- retire object OBJ%x", object->fscache.debug_id);
+			cachefiles_begin_secure(cache, &saved_cred);
+			cachefiles_delete_object(cache, object);
+			cachefiles_end_secure(cache, saved_cred);
+		}
 
-	/* close the filesystem stuff attached to the object */
-	if (object->backer != object->dentry)
-		dput(object->backer);
-	object->backer = NULL;
+		/* close the filesystem stuff attached to the object */
+		if (object->backer != object->dentry)
+			dput(object->backer);
+		object->backer = NULL;
+	}
 
 	/* note that the object is now inactive */
 	if (test_bit(CACHEFILES_OBJECT_ACTIVE, &object->flags)) {