Bug 7334

Summary: Реализация механизма кэширования блокировок в CIFS
Product: CIFS@Etersoft Reporter: Pavel Shilovsky <piastry>
Component: блокировки файлов и доступAssignee: Pavel Shilovsky <piastry>
Status: CLOSED FIXED QA Contact: Vitaly Lipatov <lav>
Severity: normal    
Priority: P2 CC: amorozov, lav, sin
Version: не указана   
Target Milestone: ---   
Hardware: PC   
OS: All   
Whiteboard:
Заявки RT: Связано с:
Дата напоминания:
Bug Depends on: 7685    
Bug Blocks: 3043, 6765    
Deadline: 2011-11-14   

Description Pavel Shilovsky 2011-05-27 13:41:44 MSK
Согласно http://bugs.etersoft.ru/show_bug.cgi?id=7302#c16 и обсуждению с lav@ решено вынести в отдельную багу.
Comment 1 Pavel Shilovsky 2011-09-16 11:18:53 MSK
Реализовал механизм кэширования блокировок на основе старых разработок (из баги #5442):

http://git.etersoft.ru/people/piastry/packages/?p=cifs-2.6.git;a=shortlog;h=refs/heads/etersoft-lock-cache

Далее планируется провести более подробное тестирование данного функционала.
Comment 2 Pavel Shilovsky 2011-09-22 20:31:11 MSK
Переосмыслил дизайн. Нашёл ошибки связанные с расширением и data races. Переписал реализацию и провёл первоначальное тестирование. Далее требуется провести более подробное синтетическое тестирование.
Comment 3 Pavel Shilovsky 2011-09-23 20:48:51 MSK
Разобрался как хранятся posix блокировки в vfs и использовал данный мезанизм для реализации кэширования блокировок в стиле posix.
http://git.etersoft.ru/people/piastry/packages/?p=cifs-2.6.git;a=shortlog;h=refs/heads/etersoft-lock-cache

Провёл тестирование реализованного функционала (mandatory и posix варианты) с потощью RECT  и Connectathon Test suite соответственно с Samba-3.6.0-alt1 с опциями kernel oplocks = no и level2 oplocks = yes.
http://git.etersoft.ru/people/piastry/packages/?p=cifs-2.6.git;a=shortlog;h=refs/heads/v3.0-lock-cache

В ходе работ обнаружил ошибку в выборе исходников ядра пакетом etercifs, что поправил (войдёт в следующую версию пакета).
http://git.etersoft.ru/people/piastry/packages/?p=etercifs.git;a=shortlog;h=refs/heads/master

Далее можно собирать тестовую версию пакета etercifs и тестировать с WINE.
Comment 4 Pavel Shilovsky 2011-09-24 23:12:09 MSK
Просмотрел код и обнаружил некоторые недочёты в сематике и стиле. Провёл рефакторинг кода. Добавил новый функионал по быстрому снятию блокировок при закрытии (запросы отправляются партиями).

Закоммитил изменения в новую ветку lock-cache-to-kernel (http://git.etersoft.ru/people/piastry/packages/?p=cifs-2.6.git;a=shortlog;h=refs/heads/lock-cache-to-kernel). Далее займусь портированием патчей в v3.0-lock-cache и более тщательным тестированием.
Comment 5 Pavel Shilovsky 2011-09-26 21:33:35 MSK
Портировал патчи из lock-cache-to-kernel. Изучал различие в поведении mandatory и posix блокировок. Обнаружил ошибки и недочёты, поправил стиль кода.

Создал реализацию, которая полностью удовлетворяет mandatory семантике (текущая не является таковой). Так же поднял в рассылке linux-cifs тему о статусе данного кода в апстрим.

Итого сейчас имею две актуальные ветки:
v3.0-lock-cache
v3.0-lock-cache-fullmand

Протетированно с помощью RECT (mandatory блокировки) Connectathon Test suite (posix блокировки).
Comment 6 Pavel Shilovsky 2011-09-26 23:07:34 MSK
Сделал некоторые изменения в описании патчей. Исследовал работу вызова unlock  в связке WINE+CIFS - текущая реализация некорректно работает с пересекающимися блокировками. По результатам обсуждения в devel@, создал ещё две ветки etersoft-fullmand и v3.0-fullmand с исправлениями для вызова unlock.

Собрал две сборки 5.1.0 ( v3.0-fullmand ) и 5.1.1 ( v3.0-lock-cache-fullmand ) для тестирования.
Comment 8 Pavel Shilovsky 2011-10-07 23:44:39 MSK
Исправил ошибку из http://bugs.etersoft.ru/show_bug.cgi?id=7685#c4, поправил стиль кода, протестировал с помощью RECT обе ветки (c fullmand и без). 

По результатам обсуждения в рассылке отправил туда патчи без fullmand (так как режим fullmand не соответствует изначальной цели, преследуемой в оригинальном cifs.ko).
Comment 9 Pavel Shilovsky 2011-10-14 14:13:20 MSK
(В ответ на comment #8)
...
> По результатам обсуждения в рассылке отправил туда патчи без fullmand
Первые два патча в аптрим (http://bugs.etersoft.ru/show_bug.cgi?id=6517#c54).
Comment 10 Pavel Shilovsky 2011-10-22 15:35:45 MSK
В третьем патче обнаружена ошибка залипания блокировки. Поправил 3 и 5 патчи, протестировал и отправил рассылку исправленные версии.
Comment 11 Pavel Shilovsky 2011-10-24 23:57:40 MSK
Остальные патчи из данной серии приняты в апстрим
Comment 12 Pavel Shilovsky 2011-10-26 12:27:49 MSK
Тестирование нового функционала прошло успешно: http://bugs.etersoft.ru/show_bug.cgi?id=7685
Comment 13 Pavel Shilovsky 2011-10-26 12:38:09 MSK
Закрываю.
Comment 14 Pavel Shilovsky 2011-10-26 22:55:04 MSK
Обнаружил некоторые недостатки в патчах, принятых в апстрим.
Comment 15 Pavel Shilovsky 2011-10-26 23:56:49 MSK
Создал и протестировал три патча, исправляющие вышеуказанные недостатки. Отправил в рассылку.
Comment 16 Pavel Shilovsky 2011-10-27 13:44:02 MSK
Включил вышеописанные патчи в основной код etercifs. Выпустил 5.1.3 сборку для тестирования.
Comment 18 Pavel Shilovsky 2011-10-27 17:13:26 MSK
Обсудил с amorozov@ ситуацию с разблокированием.

Если у нас есть код вида:

fd = open(filename)
pid = fork()
if (pid == 0) {
  lock(fd)
} else {
  sleep(10)
  lock(fd)
}

то блокировка поставленная в дочернем процессе не снимется в текущей реализации.

Таким образом, любое приложение, запущенное вайн-приложением и установившее блокировки на унаследованный файл не сможет их снять, если не укажет снятие явно (вызовом unlock с тем же смещением и длиной блокировки) - то, есть завершение процесса, снятие блокировок со всей области файла(от 0 до бесконечности) работать не будет.

В связи с этим, решили откатить данное изменения для CIFS и WINE и добавить в последний предупреждающие сообщения при наличии пересекающихся блокировок на чтение от одного пида на одном файловом дескрипторе и нескольких файловых дескрипторах одного файла.

Собрал новую сборку 5.1.4 с учётом вышеописанного, исключив из неё патч fullmand (протестировал с помощью RECT и Connectathon test suite).
Comment 19 Pavel Shilovsky 2011-10-29 17:22:56 MSK
(В ответ на comment #15)
> Создал и протестировал три патча, исправляющие вышеуказанные недостатки.
> Отправил в рассылку.

Последний из трёх патчей принят в аптрим.

Переосмыслил первый, разбил второй на два и отправил в рассылку три новых патча.
Comment 20 Pavel Shilovsky 2011-11-07 16:23:47 MSK
(В ответ на comment #19)
> Переосмыслил первый, разбил второй на два и отправил в рассылку три новых
> патча.

Все три патча приняты в апстрим.

Создал патч с подробными описаними 4-х основных реализованных функий (test и add/set для mandatory и posix вариантов), логика работы которых нуждается в описании.
Comment 21 Pavel Shilovsky 2011-11-09 11:04:31 MSK
(В ответ на comment #20)
> Создал патч с подробными описаними 4-х основных реализованных функий (test и
> add/set для mandatory и posix вариантов), логика работы которых нуждается в
> описании.

Патч в апстрим.
Comment 22 Pavel Shilovsky 2011-11-09 11:40:05 MSK
(В ответ на comment #18)
> Собрал новую сборку 5.1.4 с учётом вышеописанного, исключив из неё патч
> fullmand (протестировал с помощью RECT и Connectathon test suite).

Тестирование сборки 5.1.4 прошло успешно: http://bugs.etersoft.ru/show_bug.cgi?id=7685#c23. Данный код войдёт в следующий релиз 5.2.0. Задача решена.
Comment 23 Александр Морозов 2011-11-09 17:18:40 MSK
> В связи с этим, решили откатить данное изменения для CIFS и WINE и добавить в
> последний предупреждающие сообщения при наличии пересекающихся блокировок на
> чтение от одного пида на одном файловом дескрипторе и нескольких файловых
> дескрипторах одного файла.

Изменение убрал. Предупреждение пока не добавлял. Не очень понятно, относится оно только к CIFS или к остальным ФС тоже. Какая проблема возникает при установке пересекающихся блокировок на чтение?
Comment 24 Pavel Shilovsky 2011-11-09 22:31:49 MSK
(В ответ на comment #23)
> Изменение убрал. Предупреждение пока не добавлял. Не очень понятно, относится
> оно только к CIFS или к остальным ФС тоже. Какая проблема возникает при
> установке пересекающихся блокировок на чтение?

При наличии пересекающихся блокировок в связке CIFS+WINE возникают проблемы, связанные с тем, что CIFS не позволяет разбивать существующую блокировку. То есть вариант для POSIX файловых систем, когда у нас стоят блокировки (0,10) и (5,15), и мы, чтобы снять только первую, отправляем запрос снять (0,5), для CIFS не проходит из-за ограничений протокола.

Исправить это, не сломав принципы функционирования VFS, пока непонятно как. Поэтому, предлагаю в случае наличия пересекающихся блокировок от одного процесса (пусть и с нескольких файловых дескрипторов) выводить предупреждающее сообщение в случае CIFS.
Comment 25 Vitaly Lipatov 2012-02-25 18:52:08 MSK
Не очень понял, чем закончилось. Мы добавили предупреждение при пересекающихся блокировках?
Comment 26 Pavel Shilovsky 2012-02-27 10:38:50 MSK
Да, добавили.
Comment 27 Vitaly Lipatov 2012-08-12 22:29:31 MSK
(В ответ на comment #24)
...
> есть вариант для POSIX файловых систем, когда у нас стоят блокировки (0,10) и
> (5,15), и мы, чтобы снять только первую, отправляем запрос снять (0,5), для
> CIFS не проходит из-за ограничений протокола.
> 
> Исправить это, не сломав принципы функционирования VFS, пока непонятно как.
А нельзя снимать блокировку и ставить новую? Или ставить новую и снимать старую?
Возможно это стоит обсудить. Как-то же должна решаться задача совместимости с POSIX.

С другой стороны, если это Wine написан так, что использует подход разбиения блокировки, быть может стоит его код пересмотреть?
Comment 28 Pavel Shilovsky 2012-08-13 11:51:52 MSK
(В ответ на comment #27)
> (В ответ на comment #24)
> ...
> > есть вариант для POSIX файловых систем, когда у нас стоят блокировки (0,10) и
> > (5,15), и мы, чтобы снять только первую, отправляем запрос снять (0,5), для
> > CIFS не проходит из-за ограничений протокола.
> > 
> > Исправить это, не сломав принципы функционирования VFS, пока непонятно как.
> А нельзя снимать блокировку и ставить новую? Или ставить новую и снимать
> старую?
> Возможно это стоит обсудить. Как-то же должна решаться задача совместимости с
> POSIX.

Это будет неатомарно, поэтому смысла особого нет.

> 
> С другой стороны, если это Wine написан так, что использует подход разбиения
> блокировки, быть может стоит его код пересмотреть?

Wine написан основываясь на принципах функционирования VFS. Правильнее сделать так, как уже обсуждалось ранее: добавить в VFS специальный под-вызов FCNTL, который будет обрабатывать логику работы блокировок в стиле Windows. Но это отдельная большая задача.