linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Jeff Layton <jlayton@kernel.org>
Cc: Jan Kara <jack@suse.cz>, Alexander Viro <viro@zeniv.linux.org.uk>,
	Christian Brauner <brauner@kernel.org>,
	linux-mm@kvack.org, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	kernel test robot <oliver.sang@intel.com>
Subject: Re: [PATCH 1/2] fs: initialize inode->__i_ctime to the epoch
Date: Fri, 8 Sep 2023 14:10:15 +0200	[thread overview]
Message-ID: <20230908121015.k2xmkbiw7dljj24g@quack3> (raw)
In-Reply-To: <0716e97eadc834ac4be97af5d6bbab82c5dc4ac9.camel@kernel.org>

On Fri 08-09-23 07:41:45, Jeff Layton wrote:
> On Fri, 2023-09-08 at 12:42 +0200, Jan Kara wrote:
> > On Thu 07-09-23 12:33:47, Jeff Layton wrote:
> > > With the advent of multigrain timestamps, we use inode_set_ctime_current
> > > to set the ctime, which can skip updating if the existing ctime appears
> > > to be in the future. Because we don't initialize this field at
> > > allocation time, that could prevent the ctime from being initialized
> > > properly when the inode is instantiated.
> > > 
> > > Always initialize the ctime field to the epoch so that the filesystem
> > > can set the timestamps properly later.
> > > 
> > > Reported-by: kernel test robot <oliver.sang@intel.com>
> > > Closes: https://lore.kernel.org/oe-lkp/202309071017.a64aca5e-oliver.sang@intel.com
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > 
> > Looks good but don't you need the same treatment to atime after your patch
> > 2/2?
> > 
> > 
> 
> I don't think so. Most filesystems are doing something along the lines
> of this when allocating a new inode:
> 
>     inode->i_atime = inode->i_mtime = inode_set_ctime_current(inode);
> 
> ...and I think they pretty much all have to initialize i_atime properly,
> since someone could stat the inode before an atime update occurs.

Ah, right. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> > > ---
> > >  fs/inode.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/fs/inode.c b/fs/inode.c
> > > index 35fd688168c5..54237f4242ff 100644
> > > --- a/fs/inode.c
> > > +++ b/fs/inode.c
> > > @@ -168,6 +168,8 @@ int inode_init_always(struct super_block *sb, struct inode *inode)
> > >  	inode->i_fop = &no_open_fops;
> > >  	inode->i_ino = 0;
> > >  	inode->__i_nlink = 1;
> > > +	inode->__i_ctime.tv_sec = 0;
> > > +	inode->__i_ctime.tv_nsec = 0;
> > >  	inode->i_opflags = 0;
> > >  	if (sb->s_xattr)
> > >  		inode->i_opflags |= IOP_XATTR;
> > > 
> > > -- 
> > > 2.41.0
> > > 
> 
> -- 
> Jeff Layton <jlayton@kernel.org>
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR


  reply	other threads:[~2023-09-08 12:10 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-07 16:33 [PATCH 0/2] fs: fixes for multigrain ctime code Jeff Layton
2023-09-07 16:33 ` [PATCH 1/2] fs: initialize inode->__i_ctime to the epoch Jeff Layton
2023-09-08 10:42   ` Jan Kara
2023-09-08 11:41     ` Jeff Layton
2023-09-08 12:10       ` Jan Kara [this message]
2023-09-07 16:33 ` [PATCH 2/2] fs: don't update the atime if existing atime is newer than "now" Jeff Layton
2023-09-08 12:11   ` Jan Kara
2023-09-08 12:10 ` [PATCH 0/2] fs: fixes for multigrain ctime code Christian Brauner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230908121015.k2xmkbiw7dljj24g@quack3 \
    --to=jack@suse.cz \
    --cc=brauner@kernel.org \
    --cc=jlayton@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=oliver.sang@intel.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox