Re: [RFC PATCH 2/4] DRM: Add support of AI Processor Unit (APU)

From: Hillf Danton <hdanton@sina.com>
To: Alexandre Bailon <abailon@baylibre.com>
Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 2/4] DRM: Add support of AI Processor Unit (APU)
Date: Sun, 19 Sep 2021 11:19:24 +0800	[thread overview]
Message-ID: <20210919031924.4426-1-hdanton@sina.com> (raw)
In-Reply-To: <20210917125945.620097-3-abailon@baylibre.com>

On Fri, 17 Sep 2021 14:59:43 +0200 Alexandre Bailon wrote:
> +static DEFINE_IDA(req_ida);
> +static LIST_HEAD(complete_node);

I see accesses to complete_node in apu_drm_callback(), apu_job_timedout()
and ioctl_gem_dequeue() without working out the serialization to avoid
list corruption. Can you add a comment to specify it?

> +
> +int apu_drm_callback(struct apu_core *apu_core, void *data, int len)
> +{
> +	struct apu_request *apu_req, *tmp;
> +	struct apu_dev_request *hdr = data;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&apu_core->ctx_lock, flags);
> +	list_for_each_entry_safe(apu_req, tmp, &apu_core->requests, node) {
> +		struct apu_job *job = apu_req->job;
> +
> +		if (job && hdr->id == job->id) {
> +			kref_get(&job->refcount);
> +			job->result = hdr->result;
> +			if (job->size_out)
> +				memcpy(job->data_out, hdr->data + job->size_in,
> +				       min(job->size_out, hdr->size_out));
> +			job->size_out = hdr->size_out;
> +			list_add(&job->node, &complete_node);
> +			list_del(&apu_req->node);
> +			ida_simple_remove(&req_ida, hdr->id);
> +			kfree(apu_req);
> +			drm_send_event(job->apu_drm->drm,
> +				       &job->event->pending_event);
> +			dma_fence_signal_locked(job->done_fence);
> +		}
> +	}
> +	spin_unlock_irqrestore(&apu_core->ctx_lock, flags);
> +
> +	return 0;
> +}