Re: [PATCH RFC 04/29] fsverity: add per-sb workqueue for post read processing

Re: [PATCH RFC 04/29] fsverity: add per-sb workqueue for post read processing

[Christoph Hellwig]

On Mon, Jul 28, 2025 at 10:30:08PM +0200, Andrey Albershteyn wrote:
> From: Andrey Albershteyn <aalbersh@redhat.com>
> 
> For XFS, fsverity's global workqueue is not really suitable due to:
> 
> 1. High priority workqueues are used within XFS to ensure that data
>    IO completion cannot stall processing of journal IO completions.
>    Hence using a WQ_HIGHPRI workqueue directly in the user data IO
>    path is a potential filesystem livelock/deadlock vector.

Do they?  I though the whole point of WQ_HIGHPRI was that they'd
have separate rescue workers to avoid any global pool effects.

> 2. The fsverity workqueue is global - it creates a cross-filesystem
>    contention point.

How does this not affect the other file systems?

If the global workqueue is such an issue, maybe it should be addressed
in an initial series before the xfs support?

Re: [PATCH RFC 04/29] fsverity: add per-sb workqueue for post read processing

[Tejun Heo]

Hello,

On Mon, Aug 11, 2025 at 01:45:19PM +0200, Christoph Hellwig wrote:
> On Mon, Jul 28, 2025 at 10:30:08PM +0200, Andrey Albershteyn wrote:
> > From: Andrey Albershteyn <aalbersh@redhat.com>
> > 
> > For XFS, fsverity's global workqueue is not really suitable due to:
> > 
> > 1. High priority workqueues are used within XFS to ensure that data
> >    IO completion cannot stall processing of journal IO completions.
> >    Hence using a WQ_HIGHPRI workqueue directly in the user data IO
> >    path is a potential filesystem livelock/deadlock vector.
> 
> Do they?  I though the whole point of WQ_HIGHPRI was that they'd
> have separate rescue workers to avoid any global pool effects.

HIGHPRI and MEM_RECLAIM are orthogonal. HIGHPRI makes the workqueue use
worker pools with high priority, so all work items would execute at MIN_NICE
(-20). Hmm... actually, rescuer doesn't set priority according to the
workqueue's, which seems buggy.

Thanks.

-- 
tejun

Re: [PATCH RFC 04/29] fsverity: add per-sb workqueue for post read processing

[Christoph Hellwig]

> On Mon, Aug 11, 2025 at 01:45:19PM +0200, Christoph Hellwig wrote:
> > On Mon, Jul 28, 2025 at 10:30:08PM +0200, Andrey Albershteyn wrote:
> > > From: Andrey Albershteyn <aalbersh@redhat.com>
> > > 
> > > For XFS, fsverity's global workqueue is not really suitable due to:
> > > 
> > > 1. High priority workqueues are used within XFS to ensure that data
> > >    IO completion cannot stall processing of journal IO completions.
> > >    Hence using a WQ_HIGHPRI workqueue directly in the user data IO
> > >    path is a potential filesystem livelock/deadlock vector.
> > 
> > Do they?  I though the whole point of WQ_HIGHPRI was that they'd
> > have separate rescue workers to avoid any global pool effects.
> 
> HIGHPRI and MEM_RECLAIM are orthogonal. HIGHPRI makes the workqueue use
> worker pools with high priority, so all work items would execute at MIN_NICE
> (-20). Hmm... actually, rescuer doesn't set priority according to the
> workqueue's, which seems buggy.

Andrey (or others involved with previous versions):  is interference
with the log completion workqueue what you ran into?

Tejun, are you going to prepare a patch to fix the rescuer priority?

Re: [PATCH RFC 04/29] fsverity: add per-sb workqueue for post read processing

[Tejun Heo]

Hello,

On Tue, Aug 12, 2025 at 09:43:50AM +0200, Christoph Hellwig wrote:
...
> Andrey (or others involved with previous versions):  is interference
> with the log completion workqueue what you ran into?
> 
> Tejun, are you going to prepare a patch to fix the rescuer priority?

NVM, I was confused. All rescuers, regardless of the associated workqueue,
set their nice level to MIN_NICE. IIRC, the rationale was that by the time
rescuer triggers the queued work items already have experienced noticeable
latencies and that rescuer invocations would be pretty rare. I'd be
surprised if rescuer behavior is showing up as easily observable
interferences in most cases. The system should already be thrashing quite a
bit for rescuers to be active and whatever noise rescuer behavior might
cause should usually be drowned by other things.

Thanks.

-- 
tejun