LINUX.ORG.RU
ФорумTalks

Быдлокод в ядре линукса (файл drivers/tty/serial/samsung.c)


1

3

Не каждый додумается до такого:

static struct s3c24xx_uart_port s3c24xx_serial_ports[CONFIG_SERIAL_SAMSUNG_UARTS] = {
	[0] = {
		.port = {
			.lock		= __SPIN_LOCK_UNLOCKED(s3c24xx_serial_ports[0].port.lock),
			.iotype		= UPIO_MEM,
			.uartclk	= 0,
			.fifosize	= 16,
			.ops		= &s3c24xx_serial_ops,
			.flags		= UPF_BOOT_AUTOCONF,
			.line		= 0,
		}
	},
	[1] = {
		.port = {
			.lock		= __SPIN_LOCK_UNLOCKED(s3c24xx_serial_ports[1].port.lock),
			.iotype		= UPIO_MEM,
			.uartclk	= 0,
			.fifosize	= 16,
			.ops		= &s3c24xx_serial_ops,
			.flags		= UPF_BOOT_AUTOCONF,
			.line		= 1,
		}
	},
#if CONFIG_SERIAL_SAMSUNG_UARTS > 2

	[2] = {
		.port = {
			.lock		= __SPIN_LOCK_UNLOCKED(s3c24xx_serial_ports[2].port.lock),
			.iotype		= UPIO_MEM,
			.uartclk	= 0,
			.fifosize	= 16,
			.ops		= &s3c24xx_serial_ops,
			.flags		= UPF_BOOT_AUTOCONF,
			.line		= 2,
		}
	},
#endif
#if CONFIG_SERIAL_SAMSUNG_UARTS > 3
	[3] = {
		.port = {
			.lock		= __SPIN_LOCK_UNLOCKED(s3c24xx_serial_ports[3].port.lock),
			.iotype		= UPIO_MEM,
			.uartclk	= 0,
			.fifosize	= 16,
			.ops		= &s3c24xx_serial_ops,
			.flags		= UPF_BOOT_AUTOCONF,
			.line		= 3,
		}
	}
#endif
};

★★★★★
Ответ на: комментарий от tailgunner

Для случая драйвера я не вижу проблемы…

Значит тебе ещё рано давать другим советы.

Есть всего три места размещения структуры: на стеке, на куче, в статическом сегменте. Если вдруг тебе надо рассказывать, почему такие вещи только дурак попрет на стек, и чем чревато размещение на куче, ты не готов быть системным разработчиком.

baka-kun ★★★★★
()
Ответ на: комментарий от baka-kun

Есть всего три места размещения структуры: на стеке, на куче, в статическом сегменте.

Эээ, а как относится способ _инциализации_ к _аллокации_? Аллоцироваться оно может все также в статическом сегменте, к способу инициализации это никак не относится.

theos ★★★
()
Ответ на: комментарий от baka-kun

Значит тебе ещё рано давать другим советы.

В общем, да. Насколько я могу судить, мой совет не каждый поймет.

Если вдруг тебе надо рассказывать, почему такие вещи только дурак попрет на стек

Мне надо рассказать, почему ты подумал о размещении на стеке или в куче. Хотя... нет, не надо.

tailgunner ★★★★★
()

что такое ядро линукса?

doctorx ★★★★
()
Ответ на: комментарий от border-radius

...которые между тем отлично работают.

IPR ★★★★★
()
Ответ на: комментарий от theos

Эээ, а как относится способ _инциализации_ к _аллокации_?

Статический сегмент будет инициализирован в момент загрузки из бинарника, он уже есть.

Аллоцироваться оно может все также в статическом сегменте

То есть вместо загрузки готового предлагается создать в статическом сегменте дополнительные константы, а при запуске выполнять код по их перекладыванию из одних кусков памяти в другие? А что, память и процессоры нынче дешевы…

к способу инициализации это никак не относится

Там чуть раньше маллок предлагался, ага… Хотя согласен, от tailgunner я такой глупости не слышал, только предложение увеличить потребление памяти мертвым грузом, который никогда не будет использоваться после загрузки.

PS. И это если не говорить о платформах, в которых ядерный статический сегмент недоступен на запись.

baka-kun ★★★★★
()
Ответ на: комментарий от baka-kun

А что, память и процессоры нынче дешевы…

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

только предложение увеличить потребление памяти

Какое увеличение памяти? Добавление на стэк в инициализации переменной-счетчика? Ты это в здравом уме пишешь?

theos ★★★
()
Ответ на: комментарий от baka-kun

То есть вместо загрузки готового предлагается создать в статическом сегменте дополнительные константы

Кем предлагается-то? Я говорил об очевидной вещи - статически инициализируется первый элемент массива, а в остальные копируются данные из первого + делается специфическая инициализация (2 поля, насколько я могу судить).

И это если не говорить о платформах, в которых ядерный статический сегмент недоступен на запись.

У сабжевого драйвера в этом сегменте спинлоки.

tailgunner ★★★★★
()
Ответ на: комментарий от theos

Какое увеличение памяти? Добавление на стэк в инициализации переменной-счетчика?

Эту мелочь на стеке даже не считаем, а вот несколько немаленьких const char *name, например, придется создавать в рантайме. Это препроцессору легко развернуть __SPIN_LOCK_UNLOCKED(s3c24xx_serial_ports[2].port.lock), чтобы название лока стало текстовой константой, а «стрелку в задней полусфере» без интроспекции придется туго.

Ведь нам же по условию необходимо создать абсолютно идентичную структуру, да?

baka-kun ★★★★★
()
Ответ на: комментарий от baka-kun

Статический сегмент будет инициализирован в момент загрузки из бинарника, он уже есть.

Заполнение нулями это не инициализация.

true_admin ★★★★★
()
Ответ на: комментарий от baka-kun

а вот несколько немаленьких const char *name, например, придется создавать в рантайме.

Учитывая что это нужно для отладки, это не имеет особого значение. Ну и конечно в рантайме ничего выделять не нужно, но работы больше конечно по сравнению с чистым препроцессором:

static char locks_names[CONFIG_SERIAL_SAMSUNG_UARTS][30];
....
for (int i = 0; i < CONFIG_SERIAL_SAMSUNG_UARTS; i++) {
  strcpy(locks_names[i], "my_samsung_lock_i");
  locks_names[i][16] = '0' + i;
  ...
}

theos ★★★
()
Ответ на: комментарий от theos

И по твоему ЭТО красиво? И валяющиеся мертвым грузом куски locks_names[CONFIG_SERIAL_SAMSUNG_UARTS][30], «my_samsung_lock_i» тоже? А если в железке портов будет больше девяти?

И, конечно, тебе удобнее видеть в логах и/или дебагере «some_shit_from_nowhere», чем «s3c24xx_serial_ports[3].port.lock»… А и правда, чего заботиться об этих разработчиках? Простому юзеру похер на удобство отладки и читаемость кода.

PS. Я более чем уверен, что в данном случае не нужно алгоритмическое программирование, если достаточно блочного копирования. :)

baka-kun ★★★★★
()
Последнее исправление: baka-kun (всего исправлений: 1)
Ответ на: комментарий от KRoN73

Samsung все же южнокорейская компания, а азиатские компании редко прибегают к услугам индусов.
По работе (относительно игр) знаю, что узкоглазые генерят вероятно даже больший говнокод, особенно если речь о C/C++.
Сколько кодеров сошло с ума в попытках разобрать код очередного корейского высера мне даже представить страшно.

winddos ★★★
()
Ответ на: комментарий от winddos

Samsung все же южнокорейская компания

Ну, я так подумал, что это был сарказм. Сарказмом же и ответил.

А так, первый коммит, связанный с этим файлом от Greg Kroah-Hartman. Но писал, походу, не он, просто перенёс: «tty: move drivers/serial/ to drivers/tty/serial/»

Это в git не отслеживается перенос файлов или он тупо грохнул в одном месте и добавил в другом?

KRoN73 ★★★★★
()
Ответ на: комментарий от KRoN73

Вот, где этот гениальный код зарождался:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=0...

А корни его идут с первой версии, того же автора — в самом первом варианте уже всё в том же духе:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=history;f=driv...

Я так понимаю, что вот страница героя дня:
http://www.fluff.org/ben/linux/

:)

KRoN73 ★★★★★
()

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

prot ★★
()
Ответ на: комментарий от Alve

Я бы на Вашем месте подумал что-то вроде «а может быть есть некоторая причина, по которой ЭТО сделано именно ТАК»

Любитель поиска скрытого смысла?

Siado ★★★★★
()
Ответ на: комментарий от baka-kun

Это препроцессору легко развернуть __SPIN_LOCK_UNLOCKED(s3c24xx_serial_ports[2].port.lock), чтобы название лока стало текстовой константой, а «стрелку в задней полусфере» без интроспекции придется туго.

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

tailgunner ★★★★★
()
Последнее исправление: tailgunner (всего исправлений: 1)
Ответ на: комментарий от ttnl

да, а потом тебе потребуется что-нибудь слегка изменить в настройках одного из портов...

Будь мужиком, пошли в lkml патч, ткни их носом в «быдлокод» :)

sergej ★★★★★
()
Ответ на: комментарий от Siado

Нет. Программист. Дело в том, часто бывает такая ситуация: код выглядит по-дебильному, но имеет какое-то побочное свойство, которое программист решил использовать. Например, иногда НЕ делать операции в цикле, а сделать несколько однотипных вещей друг за другом равноценно оптимизации по скорости выполнения. Или вот, в Z80 например использовался хак - если написать подряд две команды типа push a; pop a, вроде как тупость и говнокод - положили регистр в стек, вытащили из стека, ничего не изменилось, а процессор выполнил какую-то там операцию, которая обычно занимает больше тактов (не совсем точно помню, это было сто лет назад). Кроме того, иногда бывает и такая ситуация - код нарочно делают как можно более развёрнутым, если в команде есть джуниоры, а тимлид хочет код, лишённый побочных эффектов, какие бывают в «умных» однострочниках «крутых» девелоперов. В общем, это долго объяснять и без примеров всё равно непонятно.

Alve ★★★★★
()
Ответ на: комментарий от sergej

Будь мужиком, пошли в lkml патч, ткни их носом в «быдлокод» :)

после чего в толксах появится новая тема про Торвальдса и его высказывания

Harald ★★★★★
()
Ответ на: комментарий от Alve

Или вот, в Z80 например использовался хак - если написать подряд две команды типа push a; pop a

Такого не помню, но точно помню, что за'push'ивание экрана нулями производилось значительно быстрее ldir'а

redgremlin ★★★★★
()

а что вас удивляет ???

есть конкретная серия железок, от 2-х до 4-х каналов и надо дать дескрипторы каждому каналу..дали, наиболее быстрым и понятным всем способом.

Там вот выше по треду кому-то нужна jvm чтобы подавить странные позывы:)

зы. ребятки из samsung сделали красиво и правильно. Надо учится на подобном коде

MKuznetsov ★★★★★
()
Ответ на: комментарий от baka-kun

И по твоему ЭТО красиво?

Я и не говорил о красоте.

И валяющиеся мертвым грузом куски locks_names[CONFIG_SERIAL_SAMSUNG_UARTS][30],

Так а строки не будут бесплатными полюбому, что одна строка, что так

или дебагере «some_shit_from_nowhere», чем «s3c24xx_serial_ports[3].port.lock

Конечно мне будет удобнее видеть строчку которую я лично задал, а не абстрактное выражение.

если достаточно блочного копирования.

Да никто и не спорит что лучше с макросами, но и ручками в цикле разница не такая большая, и уж о производительности речь точно не идет.

theos ★★★
()
Ответ на: комментарий от tailgunner

Неаккуратненько, но не смертельно…

То есть ты предлагаешь быдлокодить? Заменить корректную и удобную статическую инициализацию на кусок быдлокода с потерей функциональности ради чего? «Моему чувству прекрасного претит эта портянка, сэр», так?

baka-kun ★★★★★
()
Ответ на: комментарий от theos

Я и не говорил о красоте.

Напомню, тема началась с того, что абсолютно корректную, простую и понятную статическую инициализацию структуры на этапе компиляции обозвали быдлокодом.

строки не будут бесплатными полюбому, что одна строка, что так

Инициализировать известные при компиляции статические данные на этапе выполнения, да ещё с потерей функциональности, не говоря о расходе десятков лишних байт в .data и .code на однократно используемый мусор — вот настоящий быдлокодинг.

ручками в цикле разница не такая большая, и уж о производительности речь точно не идет

Речь идет об удобстве обслуживания и разработки. Как бы ты аккуратно и красиво не написал свой цикл, он не будет настолько же понятным и аккуратным.

baka-kun ★★★★★
()
Ответ на: комментарий от baka-kun

расходе десятков лишних байт

Вот пот этому то линукс и торомозит!!!

Инициализировать известные при компиляции статические данные на этапе выполнения, да ещё с потерей функциональности

Где потеря?

Речь идет об удобстве обслуживания и разработки.

Конечно, копипаст 5 раз это гораздо майнтабельнее, даа

theos ★★★
()
Ответ на: комментарий от true_admin

Там речь о том, что .bss при запуске инициализируют нулями, поэтому предлагается всё аналогичное int i = 0; заменить просто на int i;, чтобы пренести из .data в .bss, чтобы тупо не увеличивать размер бинарника.

baka-kun ★★★★★
()
Ответ на: комментарий от theos

Вот пот этому то линукс и торомозит!!!

Не только, там есть проблемы by design. ;)

Где потеря?

Как это «где»? А отладка с «some_random_shit» вместо нормальных идентификаторов уже не считается за регрессию? А абсолютно необоснованное сжирание ядерной памяти?

копипаст 5 раз это гораздо майнтабельнее

Три. Да. Существенно. Не нравится, добавь на ещё один define, как уже предлагалось. Декларативное заполнение структуры данных в данном случае куда нагляднее императивного процедурного программирования.

baka-kun ★★★★★
()
Ответ на: комментарий от Tark

>на яве сабж потребовал бы всего одной фабрики

Ага, фабрики по производству ОЗУ.

это точно в квотезы ))

nanoolinux ★★★★
()
Ответ на: комментарий от baka-kun

А отладка с «some_random_shit»

Чувак, ты вообще включайся, уже который раз лажаешь. Ты в строку можешь захреначить что хочешь, и ровно то выражение что подставит макрос тоже.

необоснованное сжирание ядерной памяти?

Пара десятков байт это не ОЛОЛО СЖИРАНИЕ ПАМЯТИ

Три. Да. Существенно.

И еще раз, прекрати лажать, там 4ыре раза, и еще ифдефы на случаи 3 и 4, офигенно локаничено и понятно, про которые ты конечно тоже забыл.

theos ★★★
()
Ответ на: комментарий от theos

Ты в строку можешь захреначить что хочешь, и ровно то выражение что подставит макрос тоже.

Вроде про удобство (а особенно нужность) создания этой строки в рантайме мы уже говорили?

Пара десятков байт это не ОЛОЛО СЖИРАНИЕ ПАМЯТИ

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

там 4ыре раза

Там четыре порта. Копипасты — три. Действительно лаконично и понятно.

baka-kun ★★★★★
()
Ответ на: комментарий от baka-kun

Там речь о том, что .bss при запуске инициализируют нулями

И я об этом же. Кроме обнуления bss ничего не происходит и ненулевые поля нуждаются в ручном указании начальных значений. Что мы и видем в этой структуре. Куда её не засунь нужно «ручное» заполнение полей.

чтобы пренести из .data в .bss, чтобы тупо не увеличивать размер бинарника.

как это сказывается на размере бинарника?

true_admin ★★★★★
()
Ответ на: комментарий от baka-kun

Вроде про удобство (а особенно нужность) создания этой строки в рантайме мы уже говорили?

Ты меня утомил.

Вот именно от такого подхода ядро и пухнет.

Ты мне сейчас напоминаешь одних моих знакомых, которые «оптимизируют» на спичках, в одном месте, и полностью забивают на собственно те места которые и тормозят.

theos ★★★
()
Ответ на: комментарий от true_admin

Что мы и видем в этой структуре.

Эта структура заполняется на этапе компиляции, а не выполнения. Она в готовом виде грузится в память, никакого кода для этого при запуске не выполняется.

как это сказывается на размере бинарника?

При загрузке выделяется нужное место под .bss и заполняется нулями, в бинарнике он не хранится, а .data читается уже заполненным из файла. Именно там лежат все твои строковые константы, например, как и прочие статически инициализированные объекты.

baka-kun ★★★★★
()
Ответ на: комментарий от theos

Ты меня утомил.

Спасибо за комплимент.

«оптимизируют» на спичках

Я вроде не оптимизирую, а как бы призываю не заниматься усложнением и «деоптимизацией» уже готового, нет?

baka-kun ★★★★★
()
Ответ на: комментарий от baka-kun

«деоптимизацией» уже готового, нет?

Да нет никакой пессимизации, с точки зрения производительности они эквивалентны, особенно если ты решишь прочитать условие задачи и увидеть что это дравйвер ввода-вывода, что значит 30ок присвоений в инициализации там вообще не играыет роли, а значит вопрос _только_ майнтайненса и читаемости. И в этом смысле мы уже с тейлганером оба еще на той странице согласились что вариант с дефайном просто эстетически приятнее (по причине близости задания константы к ее значениям), и потому цикл обсуждался просто как потенциальная возможность.

theos ★★★
()
Ответ на: комментарий от baka-kun

То есть ты предлагаешь быдлокодить?

Ты как-то странно воспринимаешь все мои слова.

Заменить корректную и удобную статическую инициализацию на кусок быдлокода

Статическая инициализация в приведенном ТС виде «корректна и удобна»? Вопросов больше нет.

tailgunner ★★★★★
()
Ответ на: комментарий от baka-kun

Она в готовом виде грузится в память

ух ты, а я не знал :(

ЗЫ голосую за вариант кода с макросом потому что он 1) короче и нагляднее 2) never repeat yourself 3) меньше проблем при копипасте т.к. при добавлении новых элементов в массив можно забыть поправить какое-нить поле.

true_admin ★★★★★
()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.