From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com ([192.55.52.93]:29702 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964919AbbI2N1V (ORCPT ); Tue, 29 Sep 2015 09:27:21 -0400 From: Jani Nikula To: Daniel Vetter , DRI Development Cc: Daniel Vetter , Intel Graphics Development , stable@vger.kernel.org, Jens Axboe , Daniel Vetter Subject: Re: [PATCH] drm: Fix locking for sysfs dpms file In-Reply-To: <1443513413-28873-1-git-send-email-daniel.vetter@ffwll.ch> References: <1443512228-10764-1-git-send-email-daniel.vetter@ffwll.ch> <1443513413-28873-1-git-send-email-daniel.vetter@ffwll.ch> Date: Tue, 29 Sep 2015 16:30:39 +0300 Message-ID: <87bnclic8w.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: stable-owner@vger.kernel.org List-ID: On Tue, 29 Sep 2015, Daniel Vetter wrote: > With atomic drivers we need to make sure that (at least in general) > property reads hold the right locks. But the legacy dpms property is > special and can be read locklessly. Since userspace loves to just > randomly look at that all the time (like with "status") do that. > > To make it clear that we play tricks use the READ_ONCE compiler > barrier (and also for paranoia). > > Note that there's not really anything bad going on since even with the > new atomic paths we eventually end up not chasing any pointers (and > hence possibly freed memory and other fun stuff). The locking WARNING > has been added in > > commit 88a48e297b3a3bac6022c03babfb038f1a886cea > Author: Rob Clark > Date: Thu Dec 18 16:01:50 2014 -0500 > > drm: add atomic properties > > but since drivers are converting not everyone will have seen this from > the start. > > Jens reported this and submitted a patch to just grab the > mode_config.connection_mutex, but we can do a bit better. > > v2: Remove unused variables I failed to git add for real. > Reference: http://mid.gmane.org/20150928194822.GA3930@kernel.dk > Reported-by: Jens Axboe > Cc: Jens Axboe > Cc: Rob Clark > Cc: stable@vger.kernel.org > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_sysfs.c | 12 +++--------- > 1 file changed, 3 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c > index f08873f6489c..615b7e667320 100644 > --- a/drivers/gpu/drm/drm_sysfs.c > +++ b/drivers/gpu/drm/drm_sysfs.c > @@ -230,18 +230,12 @@ static ssize_t dpms_show(struct device *device, > char *buf) > { > struct drm_connector *connector = to_drm_connector(device); > - struct drm_device *dev = connector->dev; > - uint64_t dpms_status; > - int ret; > + int dpms; > > - ret = drm_object_property_get_value(&connector->base, > - dev->mode_config.dpms_property, > - &dpms_status); > - if (ret) > - return 0; > + dpms = READ_ONCE(connector->dpms); > > return snprintf(buf, PAGE_SIZE, "%s\n", > - drm_get_dpms_name((int)dpms_status)); > + drm_get_dpms_name(dpms)); > } > > static ssize_t enabled_show(struct device *device, > -- > 2.5.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Jani Nikula, Intel Open Source Technology Center