On 9/9/20 3:47 AM, Dmitry Osipenko wrote:
05.09.2020 13:34, Mikko Perttunen пишет:
+static int submit_process_bufs(struct drm_device *drm, struct gather_bo *bo,
+                              struct tegra_drm_job_data *job_data,
+                              struct tegra_drm_channel_ctx *ctx,
+                              struct drm_tegra_channel_submit *args,
+                              struct ww_acquire_ctx *acquire_ctx)
+{
+       struct drm_tegra_submit_buf __user *user_bufs_ptr =
+               u64_to_user_ptr(args->bufs_ptr);

If assignment makes line too long, then factor it out.

   struct drm_tegra_submit_buf __user *user_bufs_ptr;

   user_bufs_ptr = u64_to_user_ptr(args->bufs_ptr);

+       struct tegra_drm_mapping *mapping;
+       struct drm_tegra_submit_buf buf;
+       unsigned long copy_err;
+       int err;
+       u32 i;
+
+       job_data->used_mappings =
+               kcalloc(args->num_bufs, sizeof(*job_data->used_mappings), 
GFP_KERNEL);

The checkpatch should disallow this coding style. I'd write it as:

size_t size;

size = sizeof(*job_data->used_mappings);
job_data->used_mappings = kcalloc(args->num_bufs, size..);

I'll make these cleaner for next version.


+       if (!job_data->used_mappings)
+               return -ENOMEM;
+
+       for (i = 0; i < args->num_bufs; i++) {
+               copy_err = copy_from_user(&buf, user_bufs_ptr+i, sizeof(buf));

Whole array always should be copied at once. Please keep in mind that
each copy_from_user() has a cpu-time cost, there should maximum up to 2
copyings per job.


OK. BTW, do you have some reference/numbers for this or is it based on grate-driver experience?

Mikko
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to