Сделаем код чище: Что можно исправить в ядре 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'.Попробуйте модифицировать самостоятельно.Кто-то уже увидел как можно ещё упростить?
Определение существующих констант и типов данных Возвравщаясь к драйверу 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=yCONFIG_UNISYSPAR=yCONFIG_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
