Bug 6940

Исследовал проблему - гипотеза о том, что writepages не успевает снять блокировку со страницы кеша при закрытии, не подтвердилась. Видимо, это какой-то внутренний VFS процесс. Создал скрипт, воспроизводящий проблему, и поправил патч. Отправил в linux-cifs@ рассылку.

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

Патч из рассылки отозвал.

Глубже исследовал проблему. Выяснил, что в ядре есть buddy page allocator, который периодически захватывает страницы памяти (увеличивает количество ссылок на страницу) и, поэтому, invalidate_remote_inode не всегда срабатывает, так как там идёт ещё и проверка на количество ссылок.

Что же касается invalidate_inode_pages2, то там удаляются и такие страницы. Видимо, это сделано специально, так как в комментариях к функции invalidate_complete_page2, которая вызывается для каждой страницы айнода в этом случае, так и написано, что она не оставляет страницы, если даже на них есть ссылки из вызова shrink_page_list (который использует buddy page allocator) или они временно находятся в векторах страниц.

Таким образом, использование invalidate_inode_pages2 оправданно в данном случае.

Далее займусь правкой патча, и его пробросом с апстрим.

Подготовил и отправил патч в апстрим. Пробросил в ветку для 35 ядра http://git.etersoft.ru/people/piastry/packages/?p=etercifs-sources.git;a=shortlog;h=refs/heads/v2.6.35. Протестировал с помощью RECT, проблема себя не воспроизвелась.

Патч получил Reviewed-by тэг. Жду принятия в апстрим и закрываю багу.

Отправил скрипт, воспроизводящий проблему. Вопрос о принятии патча в ядро до выхода 2.6.38 пока открыт.

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

В linux-fsdevel патч, но так и не получил комментариев. Жду решения от апстрим о дальнейшей судьбе патча.

Обсудил текущий статус патча с разработчиками. Высказалась идея о том, чтобы не просто выдавать сообщение об ошибке при невозможности обнулить кэш, а возвращать её пользователю. Если дальше возражений не последует, то буду править патч.

После продолжительного обсуждения решено было всё же не возвращать ошибку, а просто пометить кэш снова невалидным. Отправил патч в рассылку.

Патч в апстрим: http://git.kernel.org/?p=linux/kernel/git/sfrench/cifs-2.6.git;a=commitdiff;h=d0d4695b41e0f30c63687235871b5c9d9d86fa6f

Надо разобраться с логикой возвращаемого значения функции cifs_invalidate_mapping.

Исследовал данную проблему. Поделил вызовы, использующие cifs_invalidate на две группы:
1) вызовы которой должны реагировать на ошибку -EBUSY и возвращать её: cifs_d_revalidate, cifs_file_aio_read (будущая реализация), cifs_file_strict_mmap.
2) вызовы которой не должны обращать на данную ошибку внимания: cifs_getattrs, cifs_lseek, cifs_fsync.

Написал об этом разработчиками. Жду их комментарии.

Получил одобрение по поводу своего предложения. Далее займусь реализацией.

Создал патч, потестировал и отправил:
http://article.gmane.org/gmane.linux.kernel.cifs/2832.

Получил комментарии, написал свои соображения.

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

Выяснилось, что не стоит обращать внимание на -EBUSY в fsync вызове - далее буду править патч и тестировать заново.

Поправил, потестировал и отправил в рассылку новую версию патча:
http://article.gmane.org/gmane.linux.kernel.cifs/2874.

В апстрим пересмотрел своё мнение по поводу данного патча - патч в данном своём виде не имеет смысла. Появилась идея разбить функцию cifs_invalidate_mapping, и вызывать непосредственно invalidate_inode_pages2 и filemap_write_and_wait там, где это нужно.

Начал работу над патчем:
http://git.etersoft.ru/people/piastry/packages/?p=cifs-2.6.git;a=commitdiff;h=2b416980d89aa0c69a308a85192a06206389b3d2
Далее добавлю функцию launder_page.

Добавил функцию launder_page и потестировал код. Отправил в рассылку:
http://permalink.gmane.org/gmane.linux.kernel.cifs/2973
http://permalink.gmane.org/gmane.linux.kernel.cifs/2972

Согласно полученному комментарию, переделал первый патч и отправил.

Поправил второй патч согласно полученным комментариям и отправил в рассылку.

Провёл тестирование производительности первого патча с помощью dbench (результаты в норме - производительность не ухудшилась) и продолжил обсуждение, начатое в http://bugs.etersoft.ru/show_bug.cgi?id=5342#c33 по второму патчу. Возник вопрос о необходимости делать flush в getattr - выяснили, что это мало влияет на логику работы (так как данные в слушае отсутствия оплока сразу сбрасываются на сервер и ситуация, когда flush затронет какие-то данные маловероятна). Но так как мы всё равно проходимся по страницам кэша, возможно падение производительности, что и собираюсь выяснить далее.

Наблюдал не большое падение производительности (около процента). Всё же ввиду маловероятности данной ситуации и отсутствия реальной необходимости сбрасывать каждый раз данные на сервер, который итак уже сбрасываются вне зависимости от нашего решения в getattr, убрал flush. По той же причине убрал и в llseek. Отправил новую версию патча в рассылку:
http://permalink.gmane.org/gmane.linux.kernel.cifs/3113

В связи с тем, что реализация writepages в cifs будет изменена на асинхронную, встаёт вопрос, чтобы заранее предугадать эту ситуацию в нашем патче. Поэтому, я предложил использовать функцию filemap_fdatawait, которая будет ожидать окончания сброса данных на сервер. Так как это нашло подтверждение, то далее займусь правкой патча.

Поправил патч, протестировал и отправил в рассылку:
https://patchwork.kernel.org/patch/729101/

Протестировал последний патч с помощью dbench. Наблюдал одинаковые результаты с наличием вызова filemap_fdatawait в getattr и llseek и без него. В общем, патч увеличивает производительность на ~5%. Написал об этом в рассылку.

Патч получил Reviewed-by.

Теперь оба патча приняты в for-next ветку (второй: http://bugs.etersoft.ru/show_bug.cgi?id=6517#c34). Бага решена.