| Message ID | 20231006184922.252188-16-aalbersh@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fs-verity support for XFS | expand |
On Fri, Oct 06, 2023 at 08:49:09PM +0200, Andrey Albershteyn wrote: > As noted by Dave there are two problems with using fs-verity's > workqueue in XFS: > > 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. > > 2. The fsverity workqueue is global - it creates a cross-filesystem > contention point. > > This patch adds per-filesystem, per-cpu workqueue for fsverity > work. If we ever want to implement compression and/or fscrypt, can we use this pread workqueue for that too? Sounds good to me... Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com> > --- > fs/xfs/xfs_mount.h | 1 + > fs/xfs/xfs_super.c | 9 +++++++++ > 2 files changed, 10 insertions(+) > > diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h > index d19cca099bc3..3d77844b255e 100644 > --- a/fs/xfs/xfs_mount.h > +++ b/fs/xfs/xfs_mount.h > @@ -109,6 +109,7 @@ typedef struct xfs_mount { > struct xfs_mru_cache *m_filestream; /* per-mount filestream data */ > struct workqueue_struct *m_buf_workqueue; > struct workqueue_struct *m_unwritten_workqueue; > + struct workqueue_struct *m_postread_workqueue; > struct workqueue_struct *m_reclaim_workqueue; > struct workqueue_struct *m_sync_workqueue; > struct workqueue_struct *m_blockgc_wq; > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > index 819a3568b28f..5e1ec5978176 100644 > --- a/fs/xfs/xfs_super.c > +++ b/fs/xfs/xfs_super.c > @@ -554,6 +554,12 @@ xfs_init_mount_workqueues( > if (!mp->m_unwritten_workqueue) > goto out_destroy_buf; > > + mp->m_postread_workqueue = alloc_workqueue("xfs-pread/%s", > + XFS_WQFLAGS(WQ_FREEZABLE | WQ_MEM_RECLAIM), > + 0, mp->m_super->s_id); > + if (!mp->m_postread_workqueue) > + goto out_destroy_postread; > + > mp->m_reclaim_workqueue = alloc_workqueue("xfs-reclaim/%s", > XFS_WQFLAGS(WQ_FREEZABLE | WQ_MEM_RECLAIM), > 0, mp->m_super->s_id); > @@ -587,6 +593,8 @@ xfs_init_mount_workqueues( > destroy_workqueue(mp->m_reclaim_workqueue); > out_destroy_unwritten: > destroy_workqueue(mp->m_unwritten_workqueue); > +out_destroy_postread: > + destroy_workqueue(mp->m_postread_workqueue); > out_destroy_buf: > destroy_workqueue(mp->m_buf_workqueue); > out: > @@ -602,6 +610,7 @@ xfs_destroy_mount_workqueues( > destroy_workqueue(mp->m_inodegc_wq); > destroy_workqueue(mp->m_reclaim_workqueue); > destroy_workqueue(mp->m_unwritten_workqueue); > + destroy_workqueue(mp->m_postread_workqueue); > destroy_workqueue(mp->m_buf_workqueue); > } > > -- > 2.40.1 >
On 2023-10-11 11:55:58, Darrick J. Wong wrote: > On Fri, Oct 06, 2023 at 08:49:09PM +0200, Andrey Albershteyn wrote: > > As noted by Dave there are two problems with using fs-verity's > > workqueue in XFS: > > > > 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. > > > > 2. The fsverity workqueue is global - it creates a cross-filesystem > > contention point. > > > > This patch adds per-filesystem, per-cpu workqueue for fsverity > > work. > > If we ever want to implement compression and/or fscrypt, can we use this > pread workqueue for that too? I think yes. > Sounds good to me... > Reviewed-by: Darrick J. Wong <djwong@kernel.org> Thanks!
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h index d19cca099bc3..3d77844b255e 100644 --- a/fs/xfs/xfs_mount.h +++ b/fs/xfs/xfs_mount.h @@ -109,6 +109,7 @@ typedef struct xfs_mount { struct xfs_mru_cache *m_filestream; /* per-mount filestream data */ struct workqueue_struct *m_buf_workqueue; struct workqueue_struct *m_unwritten_workqueue; + struct workqueue_struct *m_postread_workqueue; struct workqueue_struct *m_reclaim_workqueue; struct workqueue_struct *m_sync_workqueue; struct workqueue_struct *m_blockgc_wq; diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c index 819a3568b28f..5e1ec5978176 100644 --- a/fs/xfs/xfs_super.c +++ b/fs/xfs/xfs_super.c @@ -554,6 +554,12 @@ xfs_init_mount_workqueues( if (!mp->m_unwritten_workqueue) goto out_destroy_buf; + mp->m_postread_workqueue = alloc_workqueue("xfs-pread/%s", + XFS_WQFLAGS(WQ_FREEZABLE | WQ_MEM_RECLAIM), + 0, mp->m_super->s_id); + if (!mp->m_postread_workqueue) + goto out_destroy_postread; + mp->m_reclaim_workqueue = alloc_workqueue("xfs-reclaim/%s", XFS_WQFLAGS(WQ_FREEZABLE | WQ_MEM_RECLAIM), 0, mp->m_super->s_id); @@ -587,6 +593,8 @@ xfs_init_mount_workqueues( destroy_workqueue(mp->m_reclaim_workqueue); out_destroy_unwritten: destroy_workqueue(mp->m_unwritten_workqueue); +out_destroy_postread: + destroy_workqueue(mp->m_postread_workqueue); out_destroy_buf: destroy_workqueue(mp->m_buf_workqueue); out: @@ -602,6 +610,7 @@ xfs_destroy_mount_workqueues( destroy_workqueue(mp->m_inodegc_wq); destroy_workqueue(mp->m_reclaim_workqueue); destroy_workqueue(mp->m_unwritten_workqueue); + destroy_workqueue(mp->m_postread_workqueue); destroy_workqueue(mp->m_buf_workqueue); }
As noted by Dave there are two problems with using fs-verity's workqueue in XFS: 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. 2. The fsverity workqueue is global - it creates a cross-filesystem contention point. This patch adds per-filesystem, per-cpu workqueue for fsverity work. Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com> --- fs/xfs/xfs_mount.h | 1 + fs/xfs/xfs_super.c | 9 +++++++++ 2 files changed, 10 insertions(+)