Re: [PATCH v3 5/6] xfs: add per-AG writeback workqueue infrastructure

[Kundan Kumar <kundan.kumar@samsung.com>]

On 1/30/2026 3:51 AM, Darrick J. Wong wrote:
> On Fri, Jan 16, 2026 at 03:38:17PM +0530, Kundan Kumar wrote:
>> Introduce per-AG writeback worker infrastructure at mount time.
>> This patch adds initialization and teardown only, without changing
>> writeback behavior.
>>
>> Signed-off-by: Kundan Kumar <kundan.kumar@samsung.com>
>> Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
>> ---
>>   fs/xfs/xfs_aops.c  | 79 ++++++++++++++++++++++++++++++++++++++++++++++
>>   fs/xfs/xfs_aops.h  |  3 ++
>>   fs/xfs/xfs_mount.c |  2 ++
>>   fs/xfs/xfs_mount.h | 10 ++++++
>>   fs/xfs/xfs_super.c |  2 ++
>>   5 files changed, 96 insertions(+)
>>
>> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
>> index a26f79815533..9d5b65922cd2 100644
>> --- a/fs/xfs/xfs_aops.c
>> +++ b/fs/xfs/xfs_aops.c
>> @@ -23,6 +23,23 @@
>>   #include "xfs_zone_alloc.h"
>>   #include "xfs_rtgroup.h"
>>   
>> +#define XFS_AG_TASK_POOL_MIN 1024
>> +
>> +struct xfs_ag_wb_task {
>> +	struct list_head list;
>> +	struct xfs_inode *ip;
>> +	struct writeback_control wbc;
>> +	xfs_agnumber_t agno;
>> +};
>> +
>> +struct xfs_ag_wb {
>> +	struct delayed_work ag_work;
>> +	spinlock_t lock;
>> +	struct list_head task_list;
>> +	xfs_agnumber_t agno;
>> +	struct xfs_mount *mp;
>> +};
> 
> Help me understand the data structures here ... for each AG there's an
> xfs_ag_wb object which can be run as a delayed workqueue item.  This
> xfs_ag_wb is the head of a list of xfs_ag_wb_task items?
> 
> In turn, each xfs_ag_wb_task list item points to an inode and
> (redundantly?) the agnumber?  So in effect each AG has a kworker that
> can say "do all of this file's pagecache writeback for all dirty folios
> tagged with the same agnumber"?
> 
> <shrug> It's hard to tell with no comments about how these two pieces of
> data relate to each other.
> 
> --D
> 

Yes, that’s the intent. struct xfs_ag_wb is per-AG state and owns the
delayed_work that runs the AG worker on m_ag_wq. Each xfs_ag_wb
maintains a queue of pending work items that request "process inode X 
for AG Y".

struct xfs_ag_wb_task is the queued request, it carries the target inode
plus a snapshot of wbc.

When the AG worker runs, it drains its task list and for each inode does 
a one-pass scan of the mapping, filtering folios tagged for that AG and
submitting IO for those folios only.

>>   struct xfs_writepage_ctx {
>>   	struct iomap_writepage_ctx ctx;
>>   	unsigned int		data_seq;
>> @@ -666,6 +683,68 @@ static const struct iomap_writeback_ops xfs_zoned_writeback_ops = {
>>   	.writeback_submit	= xfs_zoned_writeback_submit,
>>   };
>>   
>> +void
>> +xfs_init_ag_writeback(struct xfs_mount *mp)
>> +{
>> +	xfs_agnumber_t agno;
>> +
>> +	mp->m_ag_wq = alloc_workqueue("xfs-ag-wb", WQ_UNBOUND | WQ_MEM_RECLAIM,
>> +				      0);
>> +	if (!mp->m_ag_wq)
>> +		return;
>> +
>> +	mp->m_ag_wb = kcalloc(mp->m_sb.sb_agcount,
>> +				sizeof(struct xfs_ag_wb),
>> +				GFP_KERNEL);
>> +
>> +	if (!mp->m_ag_wb) {
>> +		destroy_workqueue(mp->m_ag_wq);
>> +		mp->m_ag_wq = NULL;
>> +		return;
>> +	}
>> +
>> +	for (agno = 0; agno < mp->m_sb.sb_agcount; agno++) {
>> +		struct xfs_ag_wb *awb = &mp->m_ag_wb[agno];
>> +
>> +		spin_lock_init(&awb->lock);
>> +		INIT_LIST_HEAD(&awb->task_list);
>> +		awb->agno = agno;
>> +		awb->mp = mp;
> 
> Can't you stuff this information in struct xfs_perag instead of asking
> for a potentially huge allocation?
> 

I can switch the xfs_ag_wb fields (lock/list/work/agno/mp) into
struct xfs_perag and initialize them when perags are set up.