[LU-1118] Push file_update_time() into .page_mkwrite

• Status: Resolved
• Project: Lustre
• Fix Version/s: Lustre 2.9.0
• Type: Improvement
• Priority: Minor
• Reporter: Yang Sheng
• Assignee: Yang Sheng
• Resolution: Fixed

Description

Please add a check for this so that it isn't forgotten. This isn't something that will cause a compile error, so it could easily be missed when porting to a new kernel.

Cheers, Andreas

Begin forwarded message:

> From: Jan Kara <[e-mail]>
> Date: 16 February, 2012 6:46:08 MST
> To: LKML <[e-mail]>
> Cc: [e-mail], [e-mail], Andrew Morton <[e-mail]>, Eric Sandeen <[e-mail]>, Dave Chinner <[e-mail]>, Jan Kara <[e-mail]>, Peter Zijlstra <[e-mail]>, Ingo Molnar <[e-mail]>, Paul Mackerras <[e-mail]>, Arnaldo Carvalho de Melo <[e-mail]>, Jaya Kumar <[e-mail]>, Sage Weil <[e-mail]>, [e-mail], Steve French <[e-mail]>, [e-mail], Eric Van Hensbergen <[e-mail]>, Ron Minnich <[e-mail]>, Latchesar Ionkov <[e-mail]>, [e-mail], Miklos Szeredi <[e-mail]>, [e-mail], Steven Whitehouse <[e-mail]>, [e-mail], Greg Kroah-Hartman <[e-mail]>, Trond Myklebust <[e-mail]>, [e-mail]
> Subject: [PATCH 00/11] Push file_update_time() into .page_mkwrite
>
> Hello,
>
> to provide reliable support for filesystem freezing, filesystems need to have
> complete control over when metadata is changed. In particular,
> file_update_time() calls from page fault code make it impossible for
> filesystems to prevent inodes from being dirtied while the filesystem is
> frozen.
>
> To fix the issue, this patch set changes page fault code to call
> file_update_time() only when ->page_mkwrite() callback is not provided. If the
> callback is provided, it is the responsibility of the filesystem to perform
> update of i_mtime / i_ctime if needed. We also push file_update_time() call
> to all existing ->page_mkwrite() implementations if the time update does not
> obviously happen by other means. If you know your filesystem does not need
> update of modification times in ->page_mkwrite() handler, please speak up and
> I'll drop the patch for your filesystem.
>
> As a side note, an alternative would be to remove call of file_update_time()
> from page fault code altogether and require all filesystems needing it to do
> that in their ->page_mkwrite() implementation. That is certainly possible
> although maybe slightly inefficient and would require auditting 100+
> vm_operations_structs shake.
>
> If I get acks on these patches, Andrew, would you be willing to take these
> patches?
>
> Honza
>

Comments

[Yang Sheng]

On 2012-02-17, at 1:34 PM, Jinshan Xiong wrote:
> What if we call file_update_time() in ll_page_mkwrite() anyway. So in older kernels, it will be called twice, is there any side-effect for this?
There is some overhead to doing this, due to getting the kernel timestamp repeatedly. It also locks the inode to increment the version, and marks
the inode dirty again.

Better to make a configure check (if this is possible), otherwise a kernel version check. If some vendor backports this patch then it will call this twice, but at least it will avoid overhead in most cases.

[Gerrit Updater]

Yang Sheng ([e-mail]) uploaded a new patch: http://review.whamcloud.com/18683
Subject: LU-1118 llite: Invoke file_update_time in page_mkwrite
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: af14f5840bc994468bf4e947f30698d7195b178e

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch http://review.whamcloud.com/18683/
Subject: LU-1118 llite: Invoke file_update_time in page_mkwrite
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: f28278c8cb55ce78ba0e6a180ce2b5daef1d8bea

[Yang Sheng]

Patch landed, So close this ticket.