Re: [RFC PATCH v2 17/17] WIP: drm/tegra: Implement new UAPI

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux