On Tue, 29 Sep 2015, Daniel Vetter <daniel.vetter@xxxxxxxx> 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 <robdclark@xxxxxxxxx> > 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@xxxxxxxxx > Reported-by: Jens Axboe <axboe@xxxxxx> > Cc: Jens Axboe <axboe@xxxxxx> > Cc: Rob Clark <robdclark@xxxxxxxxx> > Cc: stable@xxxxxxxxxxxxxxx > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Jani Nikula, Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html