On Sun, Jan 05, 2020 at 10:35:56PM +0200, Nikolay Borisov wrote:
> > + /*
> > + * The following is to fix a potential -1 discrepenancy that I'm not
> > + * sure how to reproduce. But given that this is the only place that
> > + * utilizes these numbers and this is only called by from
> > + * btrfs_finish_extent_commit() which is synchronized, we can correct
> > + * here.
> > + */
> > + if (discardable_extents < 0)
> > + atomic_add(-discardable_extents,
> > + &discard_ctl->discardable_extents);
> > +
> > + if (discardable_bytes < 0)
> > + atomic64_add(-discardable_bytes,
> > + &discard_ctl->discardable_bytes);
> > +
> > + if (discardable_extents <= 0) {
> > + spin_unlock(&discard_ctl->lock);
> > + return;
> > + }
>
> Perhaps a WARN_ON for each of those conditions? AFAIU this is papering
> over a real issue which is still not fully diagnosed, no? In this case
> if someone hits it in the wild they could come back with some stack traces?
I don't think the stacktrace itself would help us, the call chain will
be always the same. We need a reproducer for it and random user reports
would likely not help either, besides that the issue happens. Some sort
of developer-only warning would be desirable though.
