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

[Matthew Brost <matthew.brost@intel.com>]

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

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
>