| Message ID | CAOg9mSRocroyX=1xbEVsPbcxziow=cmpmH4jc2pn0syUx9CqnQ@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Oct 30, 2017 at 03:02:06PM -0400, Mike Marshall wrote: > I've been working with xfstests generic/003. It contains > numerous tests. > > For example, we were updating atime on a directory when a > new file was created in the directory. It "makes sense", > but it was easy to google "open posix" and see that it was > wrong: > > If O_CREAT is set and the file did not previously exist, upon > successful completion, open() shall mark for update the st_atime, > st_ctime, and st_mtime fields of the file and the st_ctime and > st_mtime fields of the parent directory. > > We fixed the above in the server. > > Another thing we've been doing wrong is to update atime > when a file is modified. > > Google "write posix" to the rescue again: > > Upon successful completion, where nbyte is greater than 0, write() > shall mark for update the st_ctime and st_mtime fields of the file, > and if the file is a regular file, the S_ISUID and S_ISGID bits > of the file mode may be cleared. > > This makes the test pass, and I don't see that it has any bad > side effects, does it seem OK to you all? > > diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c > index 336ecbf..009fce0 100644 > --- a/fs/orangefs/file.c > +++ b/fs/orangefs/file.c > @@ -384,7 +384,9 @@ static ssize_t do_readv_writev(enum > ORANGEFS_io_type type, struct file *file, > } else { > SetMtimeFlag(orangefs_inode); > inode->i_mtime = current_time(inode); > +/* > mark_inode_dirty_sync(inode); > +*/ That'll break dirty inode tracking. Your problem here is an orangefs issue: mark_inode_dirty_sync __mark_inode_dirty ->dirty_inode orangefs_dirty_inode SetAtimeFlag(orangefs_inode); i.e. every time the inode is dirtied for data, metadata or timestamps, orangefs is updating atime. If you need to do something whenever the kernel updates atime (or c/mtime), then implement ->update_time.... Cheers, Dave.
Hey Dave... Thanks for your help... we have an update_time patch, it got posted to fs-devel in the following thread if you have time to look (and everyone else too, I hope): [PATCH] orangefs: stop setting atime on inode dirty -Mike On Mon, Oct 30, 2017 at 5:54 PM, Dave Chinner <david@fromorbit.com> wrote: > On Mon, Oct 30, 2017 at 03:02:06PM -0400, Mike Marshall wrote: >> I've been working with xfstests generic/003. It contains >> numerous tests. >> >> For example, we were updating atime on a directory when a >> new file was created in the directory. It "makes sense", >> but it was easy to google "open posix" and see that it was >> wrong: >> >> If O_CREAT is set and the file did not previously exist, upon >> successful completion, open() shall mark for update the st_atime, >> st_ctime, and st_mtime fields of the file and the st_ctime and >> st_mtime fields of the parent directory. >> >> We fixed the above in the server. >> >> Another thing we've been doing wrong is to update atime >> when a file is modified. >> >> Google "write posix" to the rescue again: >> >> Upon successful completion, where nbyte is greater than 0, write() >> shall mark for update the st_ctime and st_mtime fields of the file, >> and if the file is a regular file, the S_ISUID and S_ISGID bits >> of the file mode may be cleared. >> >> This makes the test pass, and I don't see that it has any bad >> side effects, does it seem OK to you all? >> >> diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c >> index 336ecbf..009fce0 100644 >> --- a/fs/orangefs/file.c >> +++ b/fs/orangefs/file.c >> @@ -384,7 +384,9 @@ static ssize_t do_readv_writev(enum >> ORANGEFS_io_type type, struct file *file, >> } else { >> SetMtimeFlag(orangefs_inode); >> inode->i_mtime = current_time(inode); >> +/* >> mark_inode_dirty_sync(inode); >> +*/ > > That'll break dirty inode tracking. Your problem here is an > orangefs issue: > > mark_inode_dirty_sync > __mark_inode_dirty > ->dirty_inode > orangefs_dirty_inode > SetAtimeFlag(orangefs_inode); > > i.e. every time the inode is dirtied for data, metadata or > timestamps, orangefs is updating atime. > > If you need to do something whenever the kernel updates atime (or > c/mtime), then implement ->update_time.... > > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com
diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c index 336ecbf..009fce0 100644 --- a/fs/orangefs/file.c +++ b/fs/orangefs/file.c @@ -384,7 +384,9 @@ static ssize_t do_readv_writev(enum ORANGEFS_io_type type, struct file *file, } else { SetMtimeFlag(orangefs_inode); inode->i_mtime = current_time(inode); +/* mark_inode_dirty_sync(inode); +*/ } }