On 02/07/18 12:56, Tomasz Figa wrote:
> Hi Hans,
>
> On Mon, Jun 4, 2018 at 8:47 PM Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
> [snip]
>> +static void media_request_object_release(struct kref *kref)
>> +{
>> + struct media_request_object *obj =
>> + container_of(kref, struct media_request_object, kref);
>> + struct media_request *req = obj->req;
>> +
>> + if (req)
>> + media_request_object_unbind(obj);
>
> Is it possible and fine to have a request bound here?
> media_request_clean() seems to explicitly unbind before releasing and
> this function would be only called if last reference is put, so maybe
> we should actually WARN_ON(req)?
I agree. It used to be that req could be non-NULL for valid reasons in
previous versions of the series (at least, I think so), but this is no
longer the case in the current code. Adding a WARN_ON makes a lot of
sense.
>
>> + obj->ops->release(obj);
>> +}
> [snip]
>> @@ -87,7 +104,12 @@ struct media_device_ops {
>> * @enable_source: Enable Source Handler function pointer
>> * @disable_source: Disable Source Handler function pointer
>> *
>> + * @req_queue_mutex: Serialise validating and queueing requests in order to
>> + * guarantee exclusive access to the state of the device on
>> + * the tip of the request queue.
>> * @ops: Operation handler callbacks
>> + * @req_queue_mutex: Serialise the MEDIA_REQUEST_IOC_QUEUE ioctl w.r.t.
>> + * other operations that stop or start streaming.
>
> Merge conflict? req_queue_mutex is documented twice.
Thanks, I'll remove that.
Regards,
Hans
>
> Best regards,
> Tomasz
>
