Thread (72 messages) 72 messages, 22 authors, 2020-12-08

Re: [PATCH 000/141] Fix fall-through warnings for Clang

From: Miguel Ojeda <hidden>
Date: 2020-11-25 00:32:53
Also in: amd-gfx, bridge, ceph-devel, dm-devel, dri-devel, intel-gfx, intel-wired-lan, keyrings, linux-acpi, linux-arm-msm, linux-block, linux-can, linux-cifs, linux-crypto, linux-ext4, linux-fbdev, linux-gpio, linux-hardening, linux-hwmon, linux-i3c, linux-ide, linux-iio, linux-integrity, linux-media, linux-mediatek, linux-mm, linux-mmc, linux-nfs, linux-rdma, linux-renesas-soc, linux-scsi, linux-sctp, linux-security-module, linux-usb, linux-watchdog, linux-wireless, lkml, netdev, netfilter-devel, op-tee, selinux, target-devel, virtualization, xen-devel
Subsystem: drm drivers, drm drivers and misc gpu patches, the rest · Maintainers: David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Linus Torvalds

On Mon, Nov 23, 2020 at 9:38 PM James Bottomley
[off-list ref] wrote:
So you think a one line patch should take one minute to produce ... I
really don't think that's grounded in reality.
No, I have not said that. Please don't put words in my mouth (again).

I have said *authoring* lines of *this* kind takes a minute per line.
Specifically: lines fixing the fallthrough warning mechanically and
repeatedly where the compiler tells you to, and doing so full-time for
a month.

For instance, take the following one from Gustavo. Are you really
saying it takes 12 minutes (your number) to write that `break;`?
diff --git a/drivers/gpu/drm/via/via_irq.c b/drivers/gpu/drm/via/via_irq.c
index 24cc445169e2..a3e0fb5b8671 100644
--- a/drivers/gpu/drm/via/via_irq.c
+++ b/drivers/gpu/drm/via/via_irq.c
@@ -364,6 +364,7 @@ int via_wait_irq(struct drm_device *dev, void
*data, struct drm_file *file_priv)
                irqwait->request.sequence +=
                        atomic_read(&cur_irq->irq_received);
                irqwait->request.type &= ~_DRM_VBLANK_RELATIVE;
+               break;
        case VIA_IRQ_ABSOLUTE:
                break;
        default:
 I suppose a one line
patch only takes a minute to merge with b4 if no-one reviews or tests
it, but that's not really desirable.
I have not said that either. I said reviewing and merging those are
noise compared to any complex patch. Testing should be done by the
author comparing codegen.
Part of what I'm trying to measure is the "and useful" bit because
that's not a given.
It is useful since it makes intent clear. It also catches actual bugs,
which is even more valuable.
Well, you know, subsystems are very different in terms of the amount of
patches a maintainer has to process per release cycle of the kernel.
If a maintainer is close to capacity, additional patches, however
trivial, become a problem.  If a maintainer has spare cycles, trivial
patches may look easy.
First of all, voluntary maintainers choose their own workload.
Furthermore, we already measure capacity in the `MAINTAINERS` file:
maintainers can state they can only handle a few patches. Finally, if
someone does not have time for a trivial patch, they are very unlikely
to have any time to review big ones.
You seem to be saying that because you find it easy to merge trivial
patches, everyone should.
Again, I have not said anything of the sort.

Cheers,
Miguel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help