Re: Subject: v5.15 backport - 5e672cd69f0a xfs: non-blocking inodegc pushes

[Chris Dunlop <chris@onthe.net.au>]

On Thu, Jul 13, 2023 at 10:31:04AM +1000, Chris Dunlop wrote:
> On Wed, Jul 12, 2023 at 12:26:54PM +0300, Amir Goldstein wrote:
>> On Wed, Jul 12, 2023 at 5:37 AM Chris Dunlop <chris@onthe.net.au> wrote:
>>>
>>> Request for backport to v5.15:
>>>
>>> 5e672cd69f0a xfs: non-blocking inodegc pushes
>>
>> This is not the subject of above commit, it was the
>> subject of the cover letter:
>> https://www.spinics.net/lists/linux-xfs/msg61813.html
>>
>> containing the following upstream commits:
>> 7cf2b0f9611b xfs: bound maximum wait time for inodegc work
>> 5e672cd69f0a xfs: introduce xfs_inodegc_push()
> ...
>> Leah has already queued these two patches for 5.15 backport,
>> but she is now on sick leave, so that was not done yet.
>>
>> We did however, identify a few more inodegc fixes from 6.4,
>> which also fix a bug in one of the two commits above:
>>
>> 03e0add80f4c xfs: explicitly specify cpu when forcing inodegc delayed
>> work to run immediately
>>  Fixes: 7cf2b0f9611b ("xfs: bound maximum wait time for inodegc work")
>> b37c4c8339cd xfs: check that per-cpu inodegc workers actually run on that cpu
>> 2254a7396a0c xfs: fix xfs_inodegc_stop racing with mod_delayed_work
>>  Fixes: 6191cf3ad59f ("xfs: flush inodegc workqueue tasks before cancel")
>>
>> 6191cf3ad59f ("xfs: flush inodegc workqueue tasks before cancel") has already
>> been applied to 5.15.y.
>>
>> stable tree rules require that the above fixes from 6.4 be applied to 6.1.y
>> before 5.15.y, so I have already tested them and they are ready to be posted.
>> I wanted to wait a bit after the 6.4.0 release to make sure that we did not pull
>> the blanket too much to the other side, because as the reports from Chris
>> demonstrate, the inodegc fixes had some unexpected outcomes.
>
> Just to clarify: whilst I updated to 6.1 specifically to gain access 
> to the non-blocking inodegc commits, there's no evidence those inodegc 
> commits had anything at all to do with my issue on 6.1, and Dave's 
> sense is that they probably weren't involved.
>
> That said, those "Fixes:" commits that haven't yet made it to 6.1 look 
> at least a little suspicious to my naive eye.

Hmmm.... in particular, this fix:

2254a7396a0c xfs: fix xfs_inodegc_stop racing with mod_delayed_work
  Fixes: 6191cf3ad59f ("xfs: flush inodegc workqueue tasks before cancel")

mentions:

----
For this to trip, we must have a thread draining the inodedgc workqueue
and a second thread trying to queue inodegc work to that workqueue.
This can happen if freezing or a ro remount race with reclaim poking our
faux inodegc shrinker and another thread dropping an unlinked O_RDONLY
file:
----

I'm indeed periodically freezing these filesystems to take snapshots.

So that could possibly be the source of my problem, although my kernel log 
does not have the WARN_ON_ONCE() mentioned in the patch.

Cheers,

Chris