From patchwork Tue Dec 13 23:08:10 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 9473333 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 65B1B60476 for ; Tue, 13 Dec 2016 23:09:17 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 57B7F28285 for ; Tue, 13 Dec 2016 23:09:17 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4C96828403; Tue, 13 Dec 2016 23:09:17 +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=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 9DA2228285 for ; Tue, 13 Dec 2016 23:09:13 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2C7EE6E6DC; Tue, 13 Dec 2016 23:08:47 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-wj0-x242.google.com (mail-wj0-x242.google.com [IPv6:2a00:1450:400c:c01::242]) by gabe.freedesktop.org (Postfix) with ESMTPS id 6B8BA6E6C7 for ; Tue, 13 Dec 2016 23:08:33 +0000 (UTC) Received: by mail-wj0-x242.google.com with SMTP id xy5so507936wjc.1 for ; Tue, 13 Dec 2016 15:08:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=h4OTDgvZdmsuTkQaf9Zr4deMQLoXNgUK2DcBB0T9FBE=; b=AgSwomCp6xGNnW+kLvoptlzgaubvo7TPiAtKnPUA7sqL4jx679Pebrvgwz5AA/szR0 BOq2vrRP/sYH1+9ZRluVMupVA5k4njjQJgayTOX7cTRpLsYuiq/9hkopgDzDaAq1Zfsn boyI5zG9uZPwcd1LFJGKOhSB+tRa4jVsGnDkc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=h4OTDgvZdmsuTkQaf9Zr4deMQLoXNgUK2DcBB0T9FBE=; b=SX6bavZaIy0zKlo+moEmcj3ttLl0N5lYUDWdpmZYUwxotQN1hOMciK9AeC/xVySjpD lRGhHE6dz6lGsAgNN5myDwiHpwlhQ28JemMpaK6RixXeiT5yjmMfIGS/i+3uGFvyVccg TJOdVUEWlPEE4MHzC518G26EeD0bwbb/QDss1KOmmaxFex9i6V2KVNKdsHjGOfXt7rEo EkKL0q2kbwA5wmkPm9jnJlN6Jz9fYi1ETpEz341uZIlKMbP7I6+fGTISb4/4ROA0F7AD Sv6J2DyGlTEXr4cS9icOTJMUcVOQlTgn/3hmICDTnj7IqnIvUEVGOw4LSKO/ds+AVOcO E2wQ== X-Gm-Message-State: AKaTC03auVCLJDL70UQQKMn0oDGnD/9+rgEG+GFMr0eQ5ZsCi/o662jYy4v4oWvhEg50tQ== X-Received: by 10.28.199.71 with SMTP id x68mr4355559wmf.34.1481670511354; Tue, 13 Dec 2016 15:08:31 -0800 (PST) Received: from phenom.ffwll.local ([2a02:168:56b5:0:ac27:b86c:7764:9429]) by smtp.gmail.com with ESMTPSA id ke6sm64350375wjb.21.2016.12.13.15.08.30 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 13 Dec 2016 15:08:30 -0800 (PST) From: Daniel Vetter To: DRI Development Subject: [PATCH 09/13] drm: Tighten locking in drm_mode_getconnector Date: Wed, 14 Dec 2016 00:08:10 +0100 Message-Id: <20161213230814.19598-10-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20161213230814.19598-1-daniel.vetter@ffwll.ch> References: <20161213230814.19598-1-daniel.vetter@ffwll.ch> Cc: Daniel Vetter , Intel Graphics Development , Daniel Vetter X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP - Modeset state needs mode_config->connection mutex, that covers figuring out the encoder, and reading properties (since in the atomic case those need to look at connector->state). - Don't hold any locks for stuff that's invariant (i.e. possible connectors). - Same for connector lookup and unref, those don't need any locks. - And finally the probe stuff is only protected by mode_config->mutex. While at it updated the kerneldoc for these fields in drm_connector and add docs explaining what's protected by which locks. Signed-off-by: Daniel Vetter Reviewed-by: Sean Paul --- drivers/gpu/drm/drm_connector.c | 92 ++++++++++++++++++++--------------------- include/drm/drm_connector.h | 23 +++++++++-- 2 files changed, 63 insertions(+), 52 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 0d4728704099..44b556d5d40c 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1160,43 +1160,65 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, memset(&u_mode, 0, sizeof(struct drm_mode_modeinfo)); - mutex_lock(&dev->mode_config.mutex); - connector = drm_connector_lookup(dev, out_resp->connector_id); - if (!connector) { - ret = -ENOENT; - goto out_unlock; - } + if (!connector) + return -ENOENT; + + drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); + encoder = drm_connector_get_encoder(connector); + if (encoder) + out_resp->encoder_id = encoder->base.id; + else + out_resp->encoder_id = 0; + + ret = drm_mode_object_get_properties(&connector->base, file_priv->atomic, + (uint32_t __user *)(unsigned long)(out_resp->props_ptr), + (uint64_t __user *)(unsigned long)(out_resp->prop_values_ptr), + &out_resp->count_props); + drm_modeset_unlock(&dev->mode_config.connection_mutex); + if (ret) + goto out_unref; for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) if (connector->encoder_ids[i] != 0) encoders_count++; + if ((out_resp->count_encoders >= encoders_count) && encoders_count) { + copied = 0; + encoder_ptr = (uint32_t __user *)(unsigned long)(out_resp->encoders_ptr); + for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { + if (connector->encoder_ids[i] != 0) { + if (put_user(connector->encoder_ids[i], + encoder_ptr + copied)) { + ret = -EFAULT; + goto out_unref; + } + copied++; + } + } + } + out_resp->count_encoders = encoders_count; + + out_resp->connector_id = connector->base.id; + out_resp->connector_type = connector->connector_type; + out_resp->connector_type_id = connector->connector_type_id; + + mutex_lock(&dev->mode_config.mutex); if (out_resp->count_modes == 0) { connector->funcs->fill_modes(connector, dev->mode_config.max_width, dev->mode_config.max_height); } - /* delayed so we get modes regardless of pre-fill_modes state */ - list_for_each_entry(mode, &connector->modes, head) - if (drm_mode_expose_to_userspace(mode, file_priv)) - mode_count++; - - out_resp->connector_id = connector->base.id; - out_resp->connector_type = connector->connector_type; - out_resp->connector_type_id = connector->connector_type_id; out_resp->mm_width = connector->display_info.width_mm; out_resp->mm_height = connector->display_info.height_mm; out_resp->subpixel = connector->display_info.subpixel_order; out_resp->connection = connector->status; - drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); - encoder = drm_connector_get_encoder(connector); - if (encoder) - out_resp->encoder_id = encoder->base.id; - else - out_resp->encoder_id = 0; + /* delayed so we get modes regardless of pre-fill_modes state */ + list_for_each_entry(mode, &connector->modes, head) + if (drm_mode_expose_to_userspace(mode, file_priv)) + mode_count++; /* * This ioctl is called twice, once to determine how much space is @@ -1219,36 +1241,10 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, } } out_resp->count_modes = mode_count; - - ret = drm_mode_object_get_properties(&connector->base, file_priv->atomic, - (uint32_t __user *)(unsigned long)(out_resp->props_ptr), - (uint64_t __user *)(unsigned long)(out_resp->prop_values_ptr), - &out_resp->count_props); - if (ret) - goto out; - - if ((out_resp->count_encoders >= encoders_count) && encoders_count) { - copied = 0; - encoder_ptr = (uint32_t __user *)(unsigned long)(out_resp->encoders_ptr); - for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { - if (connector->encoder_ids[i] != 0) { - if (put_user(connector->encoder_ids[i], - encoder_ptr + copied)) { - ret = -EFAULT; - goto out; - } - copied++; - } - } - } - out_resp->count_encoders = encoders_count; - out: - drm_modeset_unlock(&dev->mode_config.connection_mutex); - - drm_connector_unreference(connector); -out_unlock: mutex_unlock(&dev->mode_config.mutex); +out_unref: + drm_connector_unreference(connector); return ret; } diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index a24559ef8bb7..9af4141165d3 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -559,9 +559,6 @@ struct drm_cmdline_mode { * @interlace_allowed: can this connector handle interlaced modes? * @doublescan_allowed: can this connector handle doublescan? * @stereo_allowed: can this connector handle stereo modes? - * @modes: modes available on this connector (from fill_modes() + user) - * @status: one of the drm_connector_status enums (connected, not, or unknown) - * @probed_modes: list of modes derived directly from the display * @funcs: connector control functions * @edid_blob_ptr: DRM property containing EDID if present * @properties: property tracking for this connector @@ -631,11 +628,27 @@ struct drm_connector { * Protected by @mutex. */ bool registered; + + /** + * @modes: + * Modes available on this connector (from fill_modes() + user). + * Protected by dev->mode_config.mutex. + */ struct list_head modes; /* list of modes on this connector */ + /** + * @status: + * One of the drm_connector_status enums (connected, not, or unknown). + * Protected by dev->mode_config.mutex. + */ enum drm_connector_status status; - /* these are modes added by probing with DDC or the BIOS */ + /** + * @probed_modes: + * These are modes added by probing with DDC or the BIOS, before + * filtering is applied. Used by the probe helpers.Protected by + * dev->mode_config.mutex. + */ struct list_head probed_modes; /** @@ -644,6 +657,8 @@ struct drm_connector { * flat panels in embedded systems, the driver should initialize the * display_info.width_mm and display_info.height_mm fields with the * physical size of the display. + * + * Protected by dev->mode_config.mutex. */ struct drm_display_info display_info; const struct drm_connector_funcs *funcs;