On Tue, Oct 28, 2025 at 08:38:00AM -0400, James Bottomley wrote:

On Tue, 2025-10-28 at 00:45 +0000, Al Viro wrote:
[...]

quoted

+void d_make_discardable(struct dentry *dentry)
+{
+	spin_lock(&dentry->d_lock);
+	dentry->d_flags &= ~DCACHE_PERSISTENT;
+	dentry->d_lockref.count--;
+	rcu_read_lock();
+	finish_dput(dentry);
+}
+EXPORT_SYMBOL(d_make_discardable);

I was going to ask why you don't have a WARN_ON if the dentry is not
persistent here.  Fortunately I read the next patch which gives the
explanation and saw that you do do this in patch 50.  For those of us
who have a very linear way of reading and responding to patches, it
would have been helpful to put a comment at the top saying something
like persistency will be checked when all callers are converted, which
you can replace in patch 50.

Point...  How about
void d_make_discardable(struct dentry *dentry)
{
	spin_lock(&dentry->d_lock);
	/*
	 * By the end of the series we'll add 
	 * WARN_ON(!(dentry->d_flags & DCACHE_PERSISTENT);
	 * here, but while object removal is done by a few common helpers,
	 * object creation tends to be open-coded (if nothing else, new inode
	 * needs to be set up), so adding a warning from the very beginning 
	 * would make for much messier patch series.  
	 */
	dentry->d_flags &= ~DCACHE_PERSISTENT;
	dentry->d_lockref.count--;
	rcu_read_lock();  
	finish_dput(dentry);
}

at that point of the series, with comment replaced with WARN_ON() in
#50?