Re: [PATCH v2 06/50] primitives for maintaining persisitency

Al Viro
2025-10-29T05:10:59Z

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:
> [...]
> > +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?