On Mon, Apr 11, 2022 at 07:02:19PM +0000, Dexuan Cui wrote:
From: Saurabh Singh Sengar ssengar@linux.microsoft.com Sent: Monday, April 11, 2022 12:56 AM
...
- if (fb->pitches[0] * fb->height > hv->fb_size)
- if (fb->pitches[0] * fb->height > hv->fb_size) {
drm_err(&hv->dev, "hv->hdev, fb size requested by process %sfor %d X %d (pitch %d) is greater than allocated size %ld\n",
Should we use drm_err_ratelimited() instead of drm_err()?
The error is not produced in good cases, for every resolution change which is violating this error should print once.
Thanks for the clarification! Then drm_err() looks good to me.
I suggest having it print every time some application tries to violate the policy set at boot time. If we use ratelimit many resolutions error change will be suppressed. Please let me know your thoughts.
The line exceeds 80 chars.
At first I tried braking the line to respect 80 character boundary, but checkpatch.pl reported it as a problem. And these new lines are suggested by checkpatch.pl itself. Looks the recent rule realted to 80 charachters are changed. Ref : ...
Good to know! Thanks for sharing the link!
FYI, the default max_line_length in scripts/checkpatch.pl is 100 now: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
"80-chars" is still preferred, but isn't a hard limit. I also noticed "never break user-visible strings such as printk messages ", so yes you're correct. It's perfectly fine to have a not-too-long string that exceeds 80 chars.
Good information ! thank you for digging this.
current->comm, fb->width, fb->height, fb->pitches[0], hv->fb_size);- }
Maybe we can use the below: drm_err_ratelimited(&hv->dev, "%s: requested %dX%d (pitch %d) " "exceeds fb_size %ld\n", current->comm, fb->width, fb->height, fb->pitches[0], hv->fb_size);
Note: the first chars of last 3 lines should align with the "&" in the same column. Please run "scripts/checkpatch.pl" against the patch.
I have tested checkpatch.pl before sending, for the current patch there is no problem from checkpatch.pl
The line is 138-char long, which seems a little longer to me :-) IMO we can make it shorter, e.g. be removing the part "hv->hdev as the "drm_err(&hv->dev," already tells us which device it's.
Ok, will make it shorter.
BTW, if we run the script with --strict, it reports the below:
# scripts/checkpatch.pl --strict 0001-drm-hyperv-Added-error-message-for-fb-size-greater-t.patch CHECK: Alignment should match open parenthesis #28: FILE: drivers/gpu/drm/hyperv/hyperv_drm_modeset.c:127:
drm_err(&hv->dev, "hv->hdev, fb size requested by process %s for %d X %d (pitch %d) is greater than allocated size %ld\n",current->comm, fb->width, fb->height, fb->pitches[0], hv->fb_size);
Sure, will fix this.