Re: [PATCH] drm/xe/oa: Set stream->pollin in xe_oa_buffer_check_unlocked

[Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>]

On Wed, Jan 15, 2025 at 02:20:29PM -0800, Ashutosh Dixit wrote:
>We rely on stream->pollin to decide whether or not to block during
>poll/read calls. However, currently there are blocking read code paths
>which don't even set stream->pollin. The best place to consistently set
>stream->pollin for all code paths is therefore to set it in
>xe_oa_buffer_check_unlocked.
>
>Fixes: e936f885f1e9 ("drm/xe/oa/uapi: Expose OA stream fd")
>Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>

LGTM,

Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>

Thanks,
Umesh
>---
> drivers/gpu/drm/xe/xe_oa.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c
>index eeb96b5f49e2a..e27c1752aa57c 100644
>--- a/drivers/gpu/drm/xe/xe_oa.c
>+++ b/drivers/gpu/drm/xe/xe_oa.c
>@@ -237,7 +237,6 @@ static bool xe_oa_buffer_check_unlocked(struct xe_oa_stream *stream)
> 	u32 tail, hw_tail, partial_report_size, available;
> 	int report_size = stream->oa_buffer.format->size;
> 	unsigned long flags;
>-	bool pollin;
>
> 	spin_lock_irqsave(&stream->oa_buffer.ptr_lock, flags);
>
>@@ -282,11 +281,11 @@ static bool xe_oa_buffer_check_unlocked(struct xe_oa_stream *stream)
> 	stream->oa_buffer.tail = tail;
>
> 	available = xe_oa_circ_diff(stream, stream->oa_buffer.tail, stream->oa_buffer.head);
>-	pollin = available >= stream->wait_num_reports * report_size;
>+	stream->pollin = available >= stream->wait_num_reports * report_size;
>
> 	spin_unlock_irqrestore(&stream->oa_buffer.ptr_lock, flags);
>
>-	return pollin;
>+	return stream->pollin;
> }
>
> static enum hrtimer_restart xe_oa_poll_check_timer_cb(struct hrtimer *hrtimer)
>@@ -294,10 +293,8 @@ static enum hrtimer_restart xe_oa_poll_check_timer_cb(struct hrtimer *hrtimer)
> 	struct xe_oa_stream *stream =
> 		container_of(hrtimer, typeof(*stream), poll_check_timer);
>
>-	if (xe_oa_buffer_check_unlocked(stream)) {
>-		stream->pollin = true;
>+	if (xe_oa_buffer_check_unlocked(stream))
> 		wake_up(&stream->poll_wq);
>-	}
>
> 	hrtimer_forward_now(hrtimer, ns_to_ktime(stream->poll_period_ns));
>
>-- 
>2.47.1
>