From patchwork Wed Nov 28 08:37:42 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans Verkuil X-Patchwork-Id: 10702055 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id C2E6F14D6 for ; Wed, 28 Nov 2018 08:37:56 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B11932C5B9 for ; Wed, 28 Nov 2018 08:37:56 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A59E12C976; Wed, 28 Nov 2018 08:37:56 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 373092CD21 for ; Wed, 28 Nov 2018 08:37:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727845AbeK1Tin (ORCPT ); Wed, 28 Nov 2018 14:38:43 -0500 Received: from lb3-smtp-cloud8.xs4all.net ([194.109.24.29]:50984 "EHLO lb3-smtp-cloud8.xs4all.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727476AbeK1Tim (ORCPT ); Wed, 28 Nov 2018 14:38:42 -0500 Received: from tschai.fritz.box ([212.251.195.8]) by smtp-cloud8.xs4all.net with ESMTPA id RvLvgzI1KEPjORvLzgfqaJ; Wed, 28 Nov 2018 09:37:51 +0100 From: hverkuil-cisco@xs4all.nl To: linux-media@vger.kernel.org Cc: Sakari Ailus , Tomasz Figa , Paul Kocialkowski Subject: [PATCH for v4.20 0/5] vb2 fixes (mostly request API related) Date: Wed, 28 Nov 2018 09:37:42 +0100 Message-Id: <20181128083747.18530-1-hverkuil-cisco@xs4all.nl> X-Mailer: git-send-email 2.19.1 MIME-Version: 1.0 X-CMAE-Envelope: MS4wfJ9+wY4LMrw43qnC2A+BkbGO7tu35R7kqOc1eQNaZOLO1TE1tjWe34KvsMz65fsmGQQJcbmTDUINx8Y8EbF7cGVO8I5flZuwVRCK1ycvrvHKYXoBwCmt jdQJwL6mZFg4ksfbpgmoWB7QqgIQ+2575kFAdlcTLPw4cWnV3PCfHvmSDA57tpu3rjGvkNWoUN9jCq7Z2TARzvy3z3MJWO8GAA0ehxS845MPUM20DN3znerw Y+cgQIlOruZpExXFqV3H2Q/AM9afLg48DtHfeMJIHM3J5M/+qyrDggnTR/zyWatDWsMnG7cM56cIUKX5NEPAsQ== Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Hans Verkuil While improving the v4l2-compliance tests I came across several vb2 problems. After modifying v4l2-compliance I was now able to use the vivid error injection feature to test what happens if VIDIOC_STREAMON fails and a following STREAMON succeeds. This generated patches 1/5 and 4/5+5/5. Patch 1/5 fixes an issue that was never noticed before since it didn't result in kernel oopses or warnings, but after the second STREAMON it won't call start_streaming anymore until you call REQBUFS(0) or close the device node. Patches 4 and 5 are request specific fixes for the same corner case: the code would unbind (in vb2) or complete (in vivid) a request if start_streaming fails, but it should just leave things as-is. The buffer is just put back in the queue, ready for the next attempt at STREAMON. Patch 2/5 was also discovered by v4l2-compliance: the request fd in v4l2_buffer should be ignored by VIDIOC_PREPARE_BUF, but it wasn't. Patch 3/5 fixes a nasty corner case: a buffer with associated request is queued, and then the request fd is closed by v4l2-compliance. When the driver calls vb2_buffer_done, the buffer request object is unbound, the object is put, and indirectly the associated request is put as well, and since that was the last references to the request the whole request is released, which requires the ability to call mutex_lock. But vb2_buffer_done is atomic (it can be called from interrupt context), so this shouldn't happen. vb2 now takes an extra refcount to the request on qbuf and releases it on dqbuf and at two other places where an internal dqbuf is done. Note that 'skip request checks for VIDIOC_PREPARE_BUF' is a duplicate of https://patchwork.linuxtv.org/patch/53171/, which is now marked as superseded. I've marked all these patches for 4.20, but I think it is also possible to apply them for 4.21 since the request API is only used by virtual drivers and a staging driver. Regards, Hans Hans Verkuil (5): vb2: don't call __vb2_queue_cancel if vb2_start_streaming failed vb2: skip request checks for VIDIOC_PREPARE_BUF vb2: keep a reference to the request until dqbuf vb2: don't unbind/put the object when going to state QUEUED vivid: drop v4l2_ctrl_request_complete() from start_streaming .../media/common/videobuf2/videobuf2-core.c | 44 +++++++++++++++---- .../media/common/videobuf2/videobuf2-v4l2.c | 11 +++-- drivers/media/platform/vivid/vivid-sdr-cap.c | 2 - drivers/media/platform/vivid/vivid-vbi-cap.c | 2 - drivers/media/platform/vivid/vivid-vbi-out.c | 2 - drivers/media/platform/vivid/vivid-vid-cap.c | 2 - drivers/media/platform/vivid/vivid-vid-out.c | 2 - include/media/videobuf2-core.h | 2 + 8 files changed, 44 insertions(+), 23 deletions(-) Acked-by: Sakari Ailus