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

Ghimiray, Himal Prasad
2026-04-06T04:46:29Z

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
>>