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

Наверняка многие хотели бы попробовать что-то изменить в ядре 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 в ядре, теперь очередь за применением полученных знаний.


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

Поищем в коде:

$ 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] = '\0';
  for (j = 0; j < i; j++) {
      snprintf(str, 128, " %02X", buf[j]);
      strcat(msg, str);
  }

Вот не знали люди, что в ядре есть bin2hex(), не говоря уже о том, что strcat() совершенно лишний — snprintf() добавляет терминирующий '\0'.

Попробуйте модифицировать самостоятельно.

Кто-то уже увидел как можно ещё упростить?

На самом деле буфер нужен для вывода дампа в шестнадцатиричном виде, поэтому удаляем цикл, переменную msg и заменяем это всё либо спецификатором %*ph с передаваемой длиной i, либо вызовом print_hex_bytes().

Обратите внимание далее по коду есть подобное, можно за компанию и его оптимизировать: строки 1192-1202.

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

Возвращаясь к драйверу 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);

А это оказывается кто-то так выводит MAC адрес! Что ж, мы с лёгкостью можем использовать специальное расширение спецификатора — %pM.

Давайте заменим и посмотрим на результат:

--- 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 <[e-mail]>
David Kershner <[e-mail]>
Greg Kroah-Hartman <[e-mail]>
[e-mail]
[e-mail]
[e-mail]

Отправляем наш патч по адресам:

$ 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)? [e-mail], [e-mail]
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

Вот и письмецо.

UPDATE: Уже в ядре: 9a836c0a6310e6e9.