Re: [PATCH v4 07/14] VFS: introduce start_removing_dentry()

On Thu, 30 Oct 2025, Al Viro wrote:
> On Thu, Oct 30, 2025 at 10:31:07AM +1100, NeilBrown wrote:
> 
> > @@ -428,11 +429,14 @@ static bool cachefiles_invalidate_cookie(struct fscache_cookie *cookie)
> >  		if (!old_tmpfile) {
> >  			struct cachefiles_volume *volume = object->volume;
> >  			struct dentry *fan = volume->fanout[(u8)cookie->key_hash];
> > -
> > -			inode_lock_nested(d_inode(fan), I_MUTEX_PARENT);
> > -			cachefiles_bury_object(volume->cache, object, fan,
> > -					       old_file->f_path.dentry,
> > -					       FSCACHE_OBJECT_INVALIDATED);
> > +			struct dentry *obj;
> > +
> > +			obj = start_removing_dentry(fan, old_file->f_path.dentry);
> > +			if (!IS_ERR(obj))
> > +				cachefiles_bury_object(volume->cache, object,
> > +						       fan, obj,
> > +						       FSCACHE_OBJECT_INVALIDATED);
> > +			end_removing(obj);
> 
> Huh?  Where did you change cachefiles_bury_object to *not* unlock the parent?
> Not in this commit, AFAICS, and that means at least a bisection hazard around
> here...
> 
> Confused...
> 

Thanks for the review and for catching that error.
This incremental patch should fix it.

Thanks,
NeilBrown

diff --git a/fs/cachefiles/interface.c b/fs/cachefiles/interface.c
index 3f8a6f1a8fc3..a08250d244ea 100644
--- a/fs/cachefiles/interface.c
+++ b/fs/cachefiles/interface.c
@@ -436,7 +436,6 @@ static bool cachefiles_invalidate_cookie(struct fscache_cookie *cookie)
 				cachefiles_bury_object(volume->cache, object,
 						       fan, obj,
 						       FSCACHE_OBJECT_INVALIDATED);
-			end_removing(obj);
 		}
 		fput(old_file);
 	}
diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
index b97a40917a32..0104ac00485d 100644
--- a/fs/cachefiles/namei.c
+++ b/fs/cachefiles/namei.c
@@ -261,6 +261,7 @@ static int cachefiles_unlink(struct cachefiles_cache *cache,
  * - Directory backed objects are stuffed into the graveyard for userspace to
  *   delete
  * On entry dir must be locked.  It will be unlocked on exit.
+ * On entry there must be at least 2 refs on rep, one will be dropped on exit.
  */
 int cachefiles_bury_object(struct cachefiles_cache *cache,
 			   struct cachefiles_object *object,
@@ -275,12 +276,6 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
 
 	_enter(",'%pd','%pd'", dir, rep);
 
-	/* end_removing() will dput() @rep but we need to keep
-	 * a ref, so take one now.  This also stops the dentry
-	 * being negated when unlinked which we need.
-	 */
-	dget(rep);
-
 	if (rep->d_parent != dir) {
 		end_removing(rep);
 		_leave(" = -ESTALE");
@@ -650,7 +645,6 @@ bool cachefiles_look_up_object(struct cachefiles_object *object)
 			ret = cachefiles_bury_object(volume->cache, object,
 						     fan, de,
 						     FSCACHE_OBJECT_IS_WEIRD);
-		end_removing(de);
 		dput(dentry);
 		if (ret < 0)
 			return false;
diff --git a/fs/cachefiles/volume.c b/fs/cachefiles/volume.c
index ddf95ff5daf0..90ba926f488e 100644
--- a/fs/cachefiles/volume.c
+++ b/fs/cachefiles/volume.c
@@ -64,7 +64,6 @@ void cachefiles_acquire_volume(struct fscache_volume *vcookie)
 				cachefiles_bury_object(cache, NULL, cache->store,
 						       vdentry,
 						       FSCACHE_VOLUME_IS_WEIRD);
-			end_removing(vdentry);
 			cachefiles_put_directory(volume->dentry);
 			cond_resched();
 			goto retry;