LWN: Comments on "A look at the 4.14 development cycle"

Realtek drivers

JanC_ — Thu, 02 Nov 2017 19:13:13 +0000

Ugh, I stumbled into that one too…

BTW, I'm currently using this version, which (for now) seems to work:
https://github.com/abperiasamy/rtl8812AU_8821AU_linux

Realtek drivers

scientes — Thu, 02 Nov 2017 02:28:01 +0000

I just ran into the Realtek driver madness[1], and will be returning the USB wifi adapter I bought (if I can). Does this mean that Realtek will actually clean up its rtl8812au driver anytime soon? The firmware for these is not just a blob, but a byte array: https://github.com/gnab/rtl8812au/blob/master/hal/OUTSRC/...

[1] https://bugzilla.kernel.org/show_bug.cgi?id=197675

A look at the 4.14 development cycle

kdave — Tue, 24 Oct 2017 12:42:00 +0000

$ git log v4.13..v4.14-rc6 | sed -n 's/^\s\+Reviewed-by:$[^<]*$<.*/\1/p' | sort | uniq -c | sort -rn | head -n30
123 Alex Deucher
95 Christoph Hellwig
72 Tim Sell
72 Johannes Thumshirn
70 Christian König
69 Thomas Gleixner
67 Daniel Vetter
65 David Sterba
64 Guenter Roeck
63 Darrick J. Wong
61 Geert Uytterhoeven
58 Oleg Drokin
58 Andrew Lunn
55 Laurent Pinchart
52 Dennis Dalessandro
46 Hannes Reinecke
44 Chris Wilson
43 Maarten Lankhorst
43 Florian Fainelli
42 David Binder
42 Andy Shevchenko
41 Sean Paul
40 Borislav Petkov
35 Jan Kara
35 Ido Schimmel
34 Mike Marciniszyn
34 Andreas Dilger
29 Simon Horman
29 Leon Romanovsky
29 Don Zickus

A look at the 4.14 development cycle

jani — Tue, 24 Oct 2017 08:46:03 +0000

Broken record: Please include Reviewed-by stats.

A look at the 4.14 development cycle

error27 — Sun, 22 Oct 2017 20:04:18 +0000

Most kernel devs think the (void) foo() format is ugly. You're often expected to know what things can't fail. Sometimes they can't fail in the sense that if they do the system won't boot.

To be honest the places, I have seen which use the (void) foo() format are often bad code where it should have been checked but the dev was too lazy to write error handling. Although possibly I am biased since I am looking at static checker warnings and spend most of my day looking at buggy staging code. It's like how a bail bondsman thinks everyone is a crook.

A look at the 4.14 development cycle

GustavoARSilva — Sun, 22 Oct 2017 18:51:35 +0000

I don't think I have ever sent you any patch and, it is obvious that you want to express your opinions on a very particular case. So don't generalize by including other people's names here.

Actually, only a third of the patches I sent during this development cycle were related to constification (the article is a bit misleading in this regard). So clearly I'm working on other stuff and I'm not doing the spam thing that you mention. So please, remove my name from your _spam_ list.

--
Gustavo A. R. Silva

A look at the 4.14 development cycle

abelloni — Sun, 22 Oct 2017 18:41:09 +0000

Well, most drivers don't care about the result of clk_enable or clk_prepare_enable because it will simply never fail on their platforms. The call form may have to be fixed but not as blindly and badly as it is done currently. I was ok with the "fix" being fixed. What I really don't want is the addition of unused strings.

A look at the 4.14 development cycle

kschendel — Sun, 22 Oct 2017 03:19:37 +0000

"Worse, some patches are taken by maintainers because they are flooded by hundreds of similar looking patches that seem to do the correct thing but are actually breaking drivers or making them bigger for no good reason."

I trust that you don't mean to imply that the driver in your example wasn't broken to start with? I don't advocate making things worse, of course, but if foo() can fail wth a return value, either the caller cares or it doesn't. If it cares, it has to check. If it doesn't, the only correct call form is (void) foo(), which the original code didn't do.

Failing to properly unwind state, and changing semantics by changing the point of call, are obviously not things that should be done in a patch that purports to fix errors. However, you seem to be asking for things to be left alone, rather than be fixed properly. I certainly hope I misunderstand your statement.

A look at the 4.14 development cycle

l1k — Sat, 21 Oct 2017 23:46:54 +0000

It's definitely a double-edged sword. I tried to raise this point at Kernel Recipes, see this video at 21m46s:
https://www.youtube.com/watch?v=y0__yxM0New#t=21m46s

The patch discussed in the video is http://git.kernel.org/linus/7dab5467647b : This driver contained a call to spi_get_drvdata(spi) before performing a null pointer check on spi. The patch moves the spi_get_drvdata() further down. However if spi is null, then we're missing an spi_set_drvdata() somewhere else, as e.g. in http://git.kernel.org/linus/0964e40947a6. Otherwise it's pointless to check for null and the check can be removed entirely. So the patch addresses the issue in a somewhat superficial way and doesn't check if there is a deeper issue here. And as it turns out, the patch was incomplete and had to be fixed up with http://git.kernel.org/linus/f00c2987bfdd. More diligence would be desirable to avoid such mistakes.

A look at the 4.14 development cycle

luya — Sat, 21 Oct 2017 17:50:24 +0000

One of improvement yet to be merged is a better handling of ECDT devices for laptops especially those from ASUS. https://patchwork.kernel.org/patch/9971557/

A look at the 4.14 development cycle

abelloni — Sat, 21 Oct 2017 15:28:53 +0000

As a maintainer, I see the patch spamming as done by Arvind, Bhumika and Gustavo as really harmful. They don't usually test their patches. They sometime even don't compile test them as seen in https://patchwork.kernel.org/patch/9758867/ for example. Often, comments from reviews are not taken into account and we never see a v2 of the patch.
Worse, some patches are taken by maintainers because they are flooded by hundreds of similar looking patches that seem to do the correct thing but are actually breaking drivers or making them bigger for no good reason. See https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/... later fixed by https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/... .