Re: [RFC 01/15] drm/xe: Add xe_usm_queue generic USM circular buffer

["Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>]

On 02-04-2026 02:58, Matthew Brost wrote:
> On Wed, Mar 18, 2026 at 01:14:42PM +0530, Himal Prasad Ghimiray wrote:
>> Introduce struct xe_usm_queue, a generic lock-protected circular FIFO
>> used by USM consumers (page fault, access counter) to receive fixed-size
>> entries from IRQ context and process them via a work_struct.
>>
>> Provide three inline helpers in xe_usm_queue.h:
>>    xe_usm_queue_full()  - CIRC_SPACE check (caller holds lock)
>>    xe_usm_queue_pop()   - dequeue one entry (acquires lock internally)
>>    xe_usm_queue_push()  - enqueue one entry (caller holds lock)
>>
>> Suggested-by: Matthew Brost <matthew.brost@intel.com>
>> Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> 
> I think this is useful, but right after I suggested it, I changed the
> semantics of the page-fault circular buffer to be a single buffer [1]
> that multiple workers pull from. This eliminates HoQ blocking (e.g.,
> five access events are pushed; the first one takes a really long time,
> but the other four are much faster—yet the fifth event gets stuck behind
> the long-running one). So maybe start with a single-queue +
> multiple-workers-pulling design here? This likely matters considerably
> less than the page-fault cases, but overall I think it’s a better
> structure.
> 
> Also, maybe the xe_usm_queue can be reused by page faults, but the
> management of that queue actually gets more convoluted in [2] to
> implement the page-fault cache that greatly speeds up storms of faults.
> So maybe drop the second patch in the series for now to avoid conflicts,
> and if we can converge on a shared xe_usm_queue implementation later,
> great..

Sure will change this implementation to support 1 queue : N worker, and 
will drop patch 2.

Thanks

> 
> Matt
> 
> [1] https://patchwork.freedesktop.org/patch/707293/?series=162167&rev=4
> [2] https://patchwork.freedesktop.org/patch/707300/?series=162167&rev=4
> 
>> ---
>>   drivers/gpu/drm/xe/xe_usm_queue.h | 100 ++++++++++++++++++++++++++++++
>>   1 file changed, 100 insertions(+)
>>   create mode 100644 drivers/gpu/drm/xe/xe_usm_queue.h
>>
>> diff --git a/drivers/gpu/drm/xe/xe_usm_queue.h b/drivers/gpu/drm/xe/xe_usm_queue.h
>> new file mode 100644
>> index 000000000000..5dc2d692d8bb
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_usm_queue.h
>> @@ -0,0 +1,100 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2026 Intel Corporation
>> + */
>> +
>> +#ifndef _XE_USM_QUEUE_H_
>> +#define _XE_USM_QUEUE_H_
>> +
>> +#include <linux/circ_buf.h>
>> +#include <linux/spinlock.h>
>> +#include <linux/workqueue.h>
>> +
>> +/**
>> + * struct xe_usm_queue - Generic USM circular FIFO queue
>> + *
>> + * A lock-protected circular buffer used by both the page fault consumer and
>> + * the access counter consumer. Producers push fixed-size entries from IRQ
>> + * context; a work_struct processes them asynchronously.
>> + */
>> +struct xe_usm_queue {
>> +	/**
>> +	 * @data: Raw byte buffer backing the queue, protected by @lock
>> +	 */
>> +	void *data;
>> +	/** @size: Total size of @data in bytes */
>> +	u32 size;
>> +	/** @head: Write cursor in bytes, moved by producer, protected by @lock */
>> +	u32 head;
>> +	/** @tail: Read cursor in bytes, moved by consumer, protected by @lock */
>> +	u32 tail;
>> +	/** @lock: Protects the queue head/tail */
>> +	spinlock_t lock;
>> +	/** @worker: Work item scheduled to drain the queue */
>> +	struct work_struct worker;
>> +};
>> +
>> +/**
>> + * xe_usm_queue_full - Check whether the queue has no room for one more entry
>> + * @q: queue
>> + * @size: exact size of one entry in bytes (sizeof the entry struct)
>> + *
>> + * Must be called with @q->lock held.
>> + *
>> + * Return: true if the queue is full
>> + */
>> +static inline bool xe_usm_queue_full(struct xe_usm_queue *q, u32 size)
>> +{
>> +	u32 entry_size = roundup_pow_of_two(size);
>> +
>> +	lockdep_assert_held(&q->lock);
>> +
>> +	return CIRC_SPACE(q->head, q->tail, q->size) <= entry_size;
>> +}
>> +/**
>> + * xe_usm_queue_pop - Pop one entry from the queue into @out
>> + * @q: queue
>> + * @out: destination buffer, must be at least @size bytes
>> + * @size: exact size of one entry in bytes (sizeof the entry struct)
>> + *
>> + * Acquires @q->lock internally with spin_lock_irq().
>> + *
>> + * Return: true if an entry was dequeued, false if the queue was empty
>> + */
>> +static inline bool xe_usm_queue_pop(struct xe_usm_queue *q, void *out,
>> +				    u32 size)
>> +{
>> +	u32 entry_size = roundup_pow_of_two(size);
>> +	bool found = false;
>> +
>> +	spin_lock_irq(&q->lock);
>> +	if (q->tail != q->head) {
>> +		memcpy(out, q->data + q->tail, size);
>> +		q->tail = (q->tail + entry_size) % q->size;
>> +		found = true;
>> +	}
>> +	spin_unlock_irq(&q->lock);
>> +
>> +	return found;
>> +}
>> +
>> +/**
>> + * xe_usm_queue_push - Push one entry into the queue
>> + * @q: queue
>> + * @in: source buffer, must be at least @size bytes
>> + * @size: size of one entry of in bytes
>> + *
>> + * Must be called with @q->lock held.
>> + * Caller must check xe_usm_queue_full() before calling.
>> + */
>> +static inline void xe_usm_queue_push(struct xe_usm_queue *q, const void *in,
>> +				     u32 size)
>> +{
>> +	u32 entry_size = roundup_pow_of_two(size);
>> +
>> +	lockdep_assert_held(&q->lock);
>> +
>> +	memcpy(q->data + q->head, in, size);
>> +	q->head = (q->head + entry_size) % q->size;
>> +}
>> +#endif
>> -- 
>> 2.34.1
>>