Bug 6633

Summary: Анализ патчей в cifs-upstream
Product: CIFS@Etersoft Reporter: Pavel Shilovsky <piastry>
Component: ДолжностныеAssignee: BUGS@Etersoft <bugs>
Status: DEFERRED --- QA Contact:
Severity: minor    
Priority: P4 CC: lav, sin
Version: не указана   
Target Milestone: ---   
Hardware: PC   
OS: All   
Whiteboard:
Заявки RT: Связано с:
Дата напоминания: onFridays

Description Pavel Shilovsky 2010-12-14 10:39:40 MSK
Помимо того, как наши патчи требуют анализа с кого-то другого, чтобы быть принятыми, так и от нас желательно участвовать в анализе сторонних патчей. Это поможет нам изначально повлиять на изменение интересующих нас мест кода и заручиться поддержкой других разработчиков при продвижении своих патчей.
Comment 1 Pavel Shilovsky 2010-12-14 10:45:30 MSK
Провёл анализ патчей направленных на исправлении логики состояния соединения cifs клиента. Идея состоит в том, что клиент не будет устанавливать изначально забитые таймауты под конкретный запрос, а будет ждать ответ бесконечно и определять, что сервер недоступен по специальному "пинг" запросу каждые 30 секунд. Если сервер не отвечает на три запроса подряд, то мы считаем сервер недоступным и переустанавливаем соединение.

Так же изменения коснулись стиля кода - много мест поправлено и исправлены потенциальные ошибки (например, добавлен повтор запроса на запись при ответе EAGAIN в момент флуша в синронном режиме.
Comment 2 Pavel Shilovsky 2010-12-14 17:59:38 MSK
Один из вышеописанных патчей вызвал дискуссию на наличие races в кода удаления запросов из очереди. Выяснили, что на данный момент проблемы не существует, но существует возможность её появления в дальнейшем.
Comment 3 Vitaly Lipatov 2010-12-17 16:30:59 MSK
Если есть возможность, и это не затруднительно, было бы хорошо видеть ссылки на коммиты.
Comment 4 Pavel Shilovsky 2010-12-18 14:35:23 MSK
Да, конечно. Патч, описанный выше - https://patchwork.kernel.org/patch/398802/.
Так же можно посмотреть остальные 13 патчей (правда после Jeff Layton добавил поддержку NT_CANCEL) и теперь это группа из 18 патчей.
Comment 5 Pavel Shilovsky 2010-12-22 21:02:23 MSK
http://www.spinics.net/lists/linux-cifs/msg02082.html
Провёл анализ новой версии патчей, относящихся к транспортному уровню. Начало по ссылке выше.
Comment 6 Pavel Shilovsky 2010-12-27 19:00:13 MSK
Анализ нескольких патчей из вышеописанной группы (try #4): http://article.gmane.org/gmane.linux.kernel.cifs/2162
Comment 7 Pavel Shilovsky 2010-12-30 17:08:48 MSK
http://permalink.gmane.org/gmane.linux.kernel.cifs/2204
Comment 8 Pavel Shilovsky 2010-12-30 17:09:28 MSK
Не проставилось время в посте выше.
Comment 9 Pavel Shilovsky 2010-12-30 18:33:23 MSK
http://comments.gmane.org/gmane.linux.kernel.cifs/2206
Comment 10 Pavel Shilovsky 2011-01-12 00:04:27 MSK
Провёл анализ данного патча - добавление параметра пространства имён сети при установке соединения.

https://patchwork.kernel.org/patch/472231/

Проблема описана вот тут:
https://patchwork.kernel.org/patch/470221/

Синхронизировался с веткой master из апстрима.
Comment 11 Pavel Shilovsky 2011-01-18 22:58:33 MSK
Провёл анализ патча:
https://patchwork.kernel.org/patch/459311/
Comment 12 Pavel Shilovsky 2011-01-19 17:20:58 MSK
Провёл анализ патчей http://thread.gmane.org/gmane.linux.kernel.cifs/2308.
Comment 13 Pavel Shilovsky 2011-01-27 18:41:54 MSK
Провёл анализ следующих патчей:
https://patchwork.kernel.org/patch/512001/ (всё ок)
https://patchwork.kernel.org/patch/512011/ (тут автор сам его отозвал, так что жду следующую версию)
Comment 14 Pavel Shilovsky 2011-03-28 16:24:31 MSK
Провёл анализ и сделал замечания по патчу: https://patchwork.kernel.org/patch/639541/
Comment 15 Pavel Shilovsky 2011-04-04 23:18:17 MSK
Просмотрел серию патчей, добавляющих возможность асинхронной работы с записью. Ссылка на первый из них: https://patchwork.kernel.org/patch/683711/.
Comment 16 Pavel Shilovsky 2011-04-12 14:13:11 MSK
(В ответ на comment #15)
> Просмотрел серию патчей, добавляющих возможность асинхронной работы с записью.
> Ссылка на первый из них: https://patchwork.kernel.org/patch/683711/.

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

Выяснилось, что при работе с Windows 7 сервером на LAN при записи гигабайтного файла получается почти 20% пророст в скорости.  Стоит заметить, что установка Windows 7 по умолчанию не смогла обработать такое количество одновременно приходящих запросов записи по CIFS. После исследования выяснилось, что данное поведение исправляет установка большего MaxWorkItems в регистре (я выставил 4096).

Написал об этом в рассылку и так же приложил некоторые незначительные замечания по одному из патчей.
Comment 17 Pavel Shilovsky 2011-04-15 13:07:14 MSK
Проанализировал и провёл тестирование второй версии патчей. Скорость увеличилась незначительно (около 0.5%), но возможно ситуация изменится при переходе от LAN к WAN.
Comment 18 Pavel Shilovsky 2011-05-04 14:41:31 MSK
Провёл ревью третьей версии серии патчей для поддержки асинхронной записи.
Comment 19 Pavel Shilovsky 2011-05-24 18:12:13 MSK
Начал тестирование четвёртой серии патчей для асинхронной записи. Ядро выдаёт kernel panic или зависает. После выяснилось, что проблема не в патчах, а в текущем master.  Далее буду выяснять какой из патчей принёс ошибку.
Comment 20 Pavel Shilovsky 2011-05-25 15:10:37 MSK
После исправленние вышеописанной ошибки, провёл тестирование серии патчей для асинхронной записи. Выявил увеличение производительности записи на ~15 процентов, по сравнению с последовательным вариантом.
Comment 21 Pavel Shilovsky 2011-08-27 22:52:50 MSK
Провёл предварительный анализ двух серий патчей:
1) Реорганизация cifs_demultiplex_thread кода
http://article.gmane.org/gmane.linux.kernel.cifs/4064
2) Добавление возможности асинхронного чтения
http://article.gmane.org/gmane.linux.kernel.cifs/4082

Далее планирую изучить их более тщательно и провести тестирование (по второй серии как только выйдёт следующая версия - разработчик сам отозвал текущую).
Comment 22 Pavel Shilovsky 2011-10-11 14:53:59 MSK
Провёл анализ и тестирование кода асинхронного чтения построенного поверх патчей реорганизации кода demultiplex_thread. Результаты для чтения на 100 Mbit LAN c сервером Windows 7 были близки к результатам для записи - около 11.5 MB/s против текущих 7 MB/s.
Comment 23 Pavel Shilovsky 2011-11-08 13:02:14 MSK
Провёл анализ патча "Add support for flock over a cifs mount".
Comment 24 Pavel Shilovsky 2011-11-09 23:11:08 MSK
Провёл анализ патча "cifs: lower default wsize when unix extensions are not used", который исправляет поведение записи на некоторых нестандартных CIFS серверах, выставив по умолчанию размер записываемых данных в одно сообщение 65536.
Comment 25 Pavel Shilovsky 2011-12-01 16:51:26 MSK
Проанализоровал патч "RFC: Attach locks to cifsFileInfo instead of cifsInodeInfo", который решает проблему вызовы unlock: снимаются блокировки со всех файловых дескрипторов процесса, а не с дескриптора, вызвавшего unlock. Высказал свои замечания в рассылке.
Comment 26 Pavel Shilovsky 2012-02-21 12:20:08 MSK
Провёл анализ серии из 8 патчей, конвертирующих работу записи в режимах direct и strictcache (без оплока на запись) в асинхронный режим.
Comment 27 Pavel Shilovsky 2012-04-27 16:29:29 MSK
Провёл анализ серии из 4-х патчей: cifs: pave way for change to "strictcache" by default.
Comment 28 Pavel Shilovsky 2012-09-06 16:23:41 MSK
Провёл анализ трёх патчей в рассылке.
Comment 29 Pavel Shilovsky 2012-10-30 23:19:34 MSK
Изучал проблему (https://bugzilla.redhat.com/show_bug.cgi?id=867344) и соответствующие ей патчи в рассылке.
Comment 30 Pavel Shilovsky 2012-11-21 19:37:30 MSK
Провёл анализ 6-и патчей в рассылке.
Comment 31 Pavel Shilovsky 2012-12-10 18:24:05 MSK
Провёл анализ 4 патчей.
Comment 32 Pavel Shilovsky 2013-03-06 19:47:04 MSK
Провёл анализ патчей в апстрим.
Comment 33 Pavel Shilovsky 2013-05-28 10:41:06 MSK
Провёл анализ набора из 19 патчей, который улучшают алгоритм выбора параметров аутентификации и подписи.
Comment 34 Pavel Shilovsky 2013-11-12 09:43:23 MSK
Провёл анализ 2 патчей.
Comment 35 Pavel Shilovsky 2013-11-19 21:51:43 MSK
Провёл анализ патча "cifs: Allow mounts for paths for restricted intermediate paths", исправляющего монтирование путей с отсутствием прав доступа в середине.
Comment 36 Pavel Shilovsky 2013-11-19 23:10:14 MSK
(В ответ на comment #35)
> Провёл анализ патча "cifs: Allow mounts for paths for restricted
> intermediate paths", исправляющего монтирование путей с отсутствием прав
> доступа в середине.

Протестировал патч, проблема не исправлена.
Comment 37 Pavel Shilovsky 2013-11-20 10:52:05 MSK
Провёл анализ патча "[CIFS] Validate Negotiate Info".
Comment 38 Pavel Shilovsky 2014-02-07 20:33:35 MSK
Провёл анализ патчей:
[PATCH] cifs: use a flexarray in cifs_writedata
[PATCH] cifs: clean up page array when uncached write send fails
Comment 39 Pavel Shilovsky 2014-02-17 10:38:27 MSK
[PATCH] cifs: ensure that uncached writes handle unmapped areas correctly (см. http://bugs.etersoft.ru/show_bug.cgi?id=7142#c34)
[PATCH] cifs: sanity check length of data to send before sending
Comment 40 Pavel Shilovsky 2014-03-13 10:16:59 MSK
cifs: Wait for writebacks to complete before attempting write.
Comment 41 Pavel Shilovsky 2014-03-21 12:58:17 MSK
Разбирался с проблемой (патчем) дэдлока в коде оплоков и ошибкой при использовании cache=loose.
Comment 42 Vitaly Lipatov 2014-09-11 18:45:51 MSK
Откладываем задачи, к которым не обращались более 100 дней.