[Intel-gfx] [PATCH 06/15] drm: Drop modeset_lock_all from the getproperty ioctl
Alex Deucher
alexdeucher at gmail.com
Thu Apr 13 20:03:16 UTC 2017
On Mon, Apr 3, 2017 at 4:32 AM, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> Properties, i.e. the struct drm_property specifying the type and value
> range of a property, not the instantiation on a given object, are
> invariant over the lifetime of a driver.
>
> Hence no locking at all is needed, we can just remove it.
>
> While at it give the function some love and simplify it, to get it
> under the 80 char limit:
> - Straighten the loops to reduce the nesting.
> - use u64_to_user_ptr casting helper
> - use put_user for fixed u64 copies.
>
> Note there's a small behavioural change in that we now copy parts of
> the values to userspace if the arrays are a bit too small. Since
> userspace will immediately retry anyway, this doesn't matter.
>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
This causes a segfault in our ddx:
https://bugs.freedesktop.org/show_bug.cgi?id=100673
Alex
> ---
> drivers/gpu/drm/drm_property.c | 72 +++++++++++++++++-------------------------
> 1 file changed, 29 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
> index b17959c3e099..3feef0659940 100644
> --- a/drivers/gpu/drm/drm_property.c
> +++ b/drivers/gpu/drm/drm_property.c
> @@ -442,8 +442,7 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
> struct drm_property *property;
> int enum_count = 0;
> int value_count = 0;
> - int ret = 0, i;
> - int copied;
> + int i, copied;
> struct drm_property_enum *prop_enum;
> struct drm_mode_property_enum __user *enum_ptr;
> uint64_t __user *values_ptr;
> @@ -451,55 +450,43 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
> if (!drm_core_check_feature(dev, DRIVER_MODESET))
> return -EINVAL;
>
> - drm_modeset_lock_all(dev);
> property = drm_property_find(dev, out_resp->prop_id);
> - if (!property) {
> - ret = -ENOENT;
> - goto done;
> - }
> -
> - if (drm_property_type_is(property, DRM_MODE_PROP_ENUM) ||
> - drm_property_type_is(property, DRM_MODE_PROP_BITMASK)) {
> - list_for_each_entry(prop_enum, &property->enum_list, head)
> - enum_count++;
> - }
> -
> - value_count = property->num_values;
> + if (!property)
> + return -ENOENT;
>
> strncpy(out_resp->name, property->name, DRM_PROP_NAME_LEN);
> out_resp->name[DRM_PROP_NAME_LEN-1] = 0;
> out_resp->flags = property->flags;
>
> - if ((out_resp->count_values >= value_count) && value_count) {
> - values_ptr = (uint64_t __user *)(unsigned long)out_resp->values_ptr;
> - for (i = 0; i < value_count; i++) {
> - if (copy_to_user(values_ptr + i, &property->values[i], sizeof(uint64_t))) {
> - ret = -EFAULT;
> - goto done;
> - }
> + value_count = property->num_values;
> + values_ptr = u64_to_user_ptr(out_resp->values_ptr);
> +
> + for (i = 0; i < value_count; i++) {
> + if (i < out_resp->count_values &&
> + put_user(property->values[i], values_ptr + i)) {
> + return -EFAULT;
> }
> }
> out_resp->count_values = value_count;
>
> + copied = 0;
> + enum_ptr = u64_to_user_ptr(out_resp->enum_blob_ptr);
> +
> if (drm_property_type_is(property, DRM_MODE_PROP_ENUM) ||
> - drm_property_type_is(property, DRM_MODE_PROP_BITMASK)) {
> - if ((out_resp->count_enum_blobs >= enum_count) && enum_count) {
> - copied = 0;
> - enum_ptr = (struct drm_mode_property_enum __user *)(unsigned long)out_resp->enum_blob_ptr;
> - list_for_each_entry(prop_enum, &property->enum_list, head) {
> -
> - if (copy_to_user(&enum_ptr[copied].value, &prop_enum->value, sizeof(uint64_t))) {
> - ret = -EFAULT;
> - goto done;
> - }
> -
> - if (copy_to_user(&enum_ptr[copied].name,
> - &prop_enum->name, DRM_PROP_NAME_LEN)) {
> - ret = -EFAULT;
> - goto done;
> - }
> - copied++;
> - }
> + drm_property_type_is(property, DRM_MODE_PROP_BITMASK)) {
> + list_for_each_entry(prop_enum, &property->enum_list, head) {
> + enum_count++;
> + if (out_resp->count_enum_blobs <= enum_count)
> + continue;
> +
> + if (copy_to_user(&enum_ptr[copied].value,
> + &prop_enum->value, sizeof(uint64_t)))
> + return -EFAULT;
> +
> + if (copy_to_user(&enum_ptr[copied].name,
> + &prop_enum->name, DRM_PROP_NAME_LEN))
> + return -EFAULT;
> + copied++;
> }
> out_resp->count_enum_blobs = enum_count;
> }
> @@ -514,9 +501,8 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
> */
> if (drm_property_type_is(property, DRM_MODE_PROP_BLOB))
> out_resp->count_enum_blobs = 0;
> -done:
> - drm_modeset_unlock_all(dev);
> - return ret;
> +
> + return 0;
> }
>
> static void drm_property_free_blob(struct kref *kref)
> --
> 2.11.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
More information about the Intel-gfx
mailing list