hverkuil, hey there!
I'm rebasing on reqv15 and cedrus-frame-test is failing with "Unable to queue media request: Device or resource busy"
at the same time, I'm getting some use-after-free on refcount_t
dmesg excerpt: http://leonov.paulk.fr/collins/~paulk/paste/index.php?paste=0ccbc0d7fadf7a29e9066ffb7f296776&raw=true
paulk-gagarine: does this happen with the first request you queue? What is in the failing request? (buffers, controls?)
I need a bit more info.
hverkuil, yes, it happens with the first request
I'm queuing the usual mpeg2 control and buffers
hverkuil, code is at https://github.com/free-electrons/cedrus-frame-test/tree/requests-api-v15
especially https://github.com/free-electrons/cedrus-frame-test/blob/requests-api-v15/v4l2.c#L369
I'll double check that everything's good on our driver side
paulk-gagarine: I think I see the problem. Using refcount_t for updating_count is probably the wrong choice.
I'll do some vim2m tests to try and reproduce and fix.
hverkuil, alright, thanks a lot :)
hverkuil, note that the media request queue error is probably unrelated
I get "cedrus 1c0e000.video-codec: request: unable to queue cedrus-frame-te:6, request in state updating"
in dmesg
paulk-gagarine: you have CONFIG_REFCOUNT_FULL on, which causes the refcount warnings. The first warning is harmless, the second is a lot more suspicious.
I do indeed!
mhh I can't really figure out why my request is in MEDIA_REQUEST_STATE_UPDATING
I don't ever see that state being set, actually
it's set in include/media/media-request.h: media_request_(un)lock_for_update
oh
thanks
so it seem linked with the refcount issue then
I think it might be better to move those functions to the C source. It's kind of unexpected that they are in the header.
well, I don't have strong opinions on that :)
I just tend to restrict git grep's scope to have it faster
but generally don't expect that it's a good think thing to do
paulk-gagarine: I think I see the bug:
in v4l2-ctrls.c, line 3594 you see:
                if (IS_ERR(obj))
                        return PTR_ERR(obj);
That should be:
                if (IS_ERR(obj)) {
                        media_request_unlock_for_update(req);
                        return PTR_ERR(obj);
                }
Can you check if that will solve your problem?
(besides the 'refcount_t: increment on 0; use-after-free.' warning, you'll still get that.
trying that right away hverkuil
hverkuil, same error
I don't think media_request_get_by_fd was failing
as a sidenote, commenting out the refcount bits does make it work
(not saying that it's a good thing to do, though ;)
paulk-gagarine: there is another bug in the same function at the end:
if (obj)
     media_request_unlock_for_update(obj->req);
Should be:
        if (obj) {
                media_request_unlock_for_update(obj->req);
                media_request_object_put(obj);
        }
paulk-gagarine: I've mailed you a patch fixing these issues and replacing refcount by a simple unsigned int.
Please test!
hverkuil, awesome, thanks!
Note that I am getting a "possible circular locking dependency detected" warning. I'm tracking that down now.
hverkuil, your patch does fix the issue :)
very nice
good to hear.
the lockdep warning is a false warning.
Not really sure how to fix this.