On Wed, 2025-10-29 at 05:10 +0000, Al Viro wrote:
> 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?
Ack. Works for me.
Regards,
James
