Сделаем код чище: Что можно исправить в ядре Linux

Сделаем код чище: Что можно исправить в ядре Linux

660

Наверное почти все желали бы испытать что-то поменять в ядре Linux к лучшему, но не знают с чего же начать. Я желаю обрисовать несколько заморочек, поправить которые под силу каждому , и на примере показать путь от нахождения трудности до опубликования её исправления в перечне рассылки. По ходу повествования читатель познакомится с некими вспомогательными утилитами.

Из года в год, из драйвера в драйвер кочуют одни и те же элементарные трудности, нередко связанные с незнанием каких-то обычных практик, утилит либо расширений, имеющихся в ядре Linux.

Вот лаконичный перечень такового рода заморочек:

опечатки и описки в документации и коментариях
собственная реализация вывода содержимого структур либо их полей на экран
собственная реализация алгоритмов, которые уже предоставлены в библиотеке ядра
определение имеющихся констант и типов данных

Опечатки и описки

Опечатки и описки в документации и комментах — дело не редкое. Один человек, а конкретно Lucas De Marchi, разработал специальную утилиту codespell, чтоб отлавливать такие опечатки.

Нижеследующий пример всё о для себя скажет сам.

Клонируем ядро:
mkdir ~/devel
cd ~/devel
git clone git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
cd ~/devel/linux-next
Обратите внимание, что работать мы будем с самым свежайшим, то есть с деревом linux-next.

Совершаем тестовый пуск codespell:
$ codespell.py drivers/staging/unisys
drivers/staging/unisys/include/guestlinuxdebug.h:138: doesnt ==> doesn’t
Сейчас исправляем:
$ codespell.py -w -i 3 drivers/staging/unisys
* doesnt show, so we
doesnt ==> doesn’t (Y/n) y
FIXED: drivers/staging/unisys/include/guestlinuxdebug.h
Смотрим, что вышло:
— a/drivers/staging/unisys/include/guestlinuxdebug.h
+++ b/drivers/staging/unisys/include/guestlinuxdebug.h
@@ -135,7 +135,7 @@ enum event_pc { /* POSTCODE event identifier tuples */
#define POSTCODE_SEVERITY_ERR DIAG_SEVERITY_ERR
#define POSTCODE_SEVERITY_WARNING DIAG_SEVERITY_WARNING
#define POSTCODE_SEVERITY_INFO DIAG_SEVERITY_PRINT /* TODO-> Info currently
— * doesnt show, so we
+ * doesn’t show, so we
* set info=warning */
/* example call of POSTCODE_LINUX_2(VISOR_CHIPSET_PC, POSTCODE_SEVERITY_ERR);
* Please also note that the resulting postcode is in hex, so if you are

Собственная реализация вывода

Не столько неувязка, сколько улучшение читаемости кода и микрооптимизация: нередко существенное уменьшение расходуемой памяти на стеке при вызове функции, уменьшение количества передаваемых спецификаторов в vsnprintf().

Ранее я обрисовал специаные расширения спецификатора %p в ядре, сейчас очередь за применением приобретенных познаний.

В качестве простоты возьмём шаблон
Он дозволяет отыскивать передачу пары б через стек, которую можно заменить расширением %*ph[CDN]. %02x[-: ]%02x[-: ]%02x.

Поищем в коде:
$ git grep -n -i -e ‘%02x[-: ]%02x[-: ]%02x’ drivers/staging/unisys

drivers/staging/unisys/virtpci/virtpci.c:1313: "[%d:%d] VNic:%02x:%02x:%02x:%02x:%02x:%02x num_rcv_bufs:%d mtu:%d",

Что будем делать дальше, я обрисую на примере ниже. А пока перебегаем к последующим проблемным местам.

Собственная реализация алгоритмов

Вот, возьмём, к примеру, drivers/staging/fbtft/fbtft-bus.c, строчки 99-100:
for (i = 0; i < pad; i++)
*buf++ = 0x000;
pad определена как u16 и может быть в спектре от 0 до 3, то есть от 0 до 6 б. Как мы знаем, memset() жутко оптимизированная функция, в особенности на малых размерах. Почему не применить?
Либо ещё пример из того же драйвера, а конкретно drivers/staging/fbtft/fbtft-core.c, строчки 1091-1096:
/* make debug message */
msg[0] = ‘’;
for (j = 0; j < i; j++) {
snprintf(str, 128, " %02X", buf[j]);
strcat(msg, str);
}
Вот не знали люди, что в ядре есть bin2hex(), не говоря уже о том, что strcat() совсем излишний — snprintf() добавляет терминирующий ‘’.

Попытайтесь модифицировать без помощи других.
Кто-то уже увидел как можно ещё упростить?

Определение имеющихся констант и типов данных

Возвравщаясь к драйверу unisys, запустим такую команду:
$ git grep -n MAX_MACADDR_LEN drivers/staging/unisys/
drivers/staging/unisys/common-spar/include/channels/iochannel.h:190:#ifndef MAX_MACADDR_LEN
drivers/staging/unisys/common-spar/include/channels/iochannel.h:191:#define MAX_MACADDR_LEN 6 /* number of bytes…
drivers/staging/unisys/common-spar/include/channels/iochannel.h:192:#endif
…и так дальше…

Уверен, мейнтенеры с радостью воспримут от вас патч, заменяющий их определение обычным ядерным. Но стоит отметить, что константа длины MAC адреса давным издавна определена в ядре как ETH_ALEN.

Пример исправления

Перебегаем плавненько к практике. Выше мы отыскали место, где при выводе пары б употребляется передача каждого из их через стек.

Раз мы поглядим в код, то увидим последующее:
str_pos += scnprintf(vbuf + str_pos, len — str_pos,
"[%d:%d] VNic:%02x:%02x:%02x:%02x:%02x:%02x num_rcv_bufs:%d mtu:%d",
tmpvpcidev->bus_no,
tmpvpcidev->device_no,
tmpvpcidev->net.mac_addr[0],
tmpvpcidev->net.mac_addr[1],
tmpvpcidev->net.mac_addr[2],
tmpvpcidev->net.mac_addr[3],
tmpvpcidev->net.mac_addr[4],
tmpvpcidev->net.mac_addr[5],
tmpvpcidev->net.num_rcv_bufs,
tmpvpcidev->net.mtu);
Что ж, мы с лёгкостью можем применять особое расширение спецификатора — %pM. А это оказывается кто-то так выводит MAC адресок!

Давайте заменим и поглядим на итог:
— a/drivers/staging/unisys/virtpci/virtpci.c
+++ b/drivers/staging/unisys/virtpci/virtpci.c
@@ -1310,15 +1310,10 @@ static ssize_t info_debugfs_read(struct file *file, char __user *buf,
tmpvpcidev->scsi.max.cmd_per_lun);
} else {
str_pos += scnprintf(vbuf + str_pos, len — str_pos,
— "[%d:%d] VNic:%02x:%02x:%02x:%02x:%02x:%02x num_rcv_bufs:%d mtu:%d",
+ "[%d:%d] VNic:%pM num_rcv_bufs:%d mtu:%d",
tmpvpcidev->bus_no,
tmpvpcidev->device_no,
— tmpvpcidev->net.mac_addr[0],
— tmpvpcidev->net.mac_addr[1],
— tmpvpcidev->net.mac_addr[2],
— tmpvpcidev->net.mac_addr[3],
— tmpvpcidev->net.mac_addr[4],
— tmpvpcidev->net.mac_addr[5],
+ tmpvpcidev->net.mac_addr,
tmpvpcidev->net.num_rcv_bufs,
tmpvpcidev->net.mtu);
}
Тщательно я не буду обрисовывать как это делается, только укажу, что будет нужно включить драйвер в конфигурации с помощью опций: Вроде бы хорошо — на 5 строк и переменных на стеке меньше. Стоит всё же откомпилировать итог.
CONFIG_STAGING=y
CONFIG_UNISYSPAR=y
CONFIG_UNISYS_VIRTPCI=m

Сохраняем наше изменение в дереве с помощью git commit -a -s и форматируем в виде патча.
$ git format-patch HEAD~1
0001-staging-unisys-print-MAC-address-via-pM.patch
Дальше, воспользуемся восхитительным скриптом get_maintainter.pl, чтоб выяснить кого нужно информировать индивидуально.
$ scripts/get_maintainer.pl —git-min-percent=67 —nor —norolestats 00*
Benjamin Romer <benjamin.romer@unisys.com>
David Kershner <david.kershner@unisys.com>
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
sparmaintainer@unisys.com
devel@driverdev.osuosl.org
linux-kernel@vger.kernel.org
Отправляем наш патч по адресам:
$ git send-email —cc-cmd ‘scripts/get_maintainer.pl —git-min-percent=67 —nor —nol —norolestats’ 00*
0001-staging-unisys-print-MAC-address-via-pM.patch
Who should the emails be sent to (if any)? devel@driverdev.osuosl.org, sparmaintainer@unisys.com
Message-ID to be used as In-Reply-To for the first email (if any)? …
Send this email? ([y]es|[n]o|[q]uit|[a]ll): y
Вот и письмо. habrahabr.ru