2023-06-21 14:56:30

by Jeff Layton

Subject: [PATCH 03/79] s390: switch to new ctime accessors

In later patches, we're going to change how the ctime.tv_nsec field is
utilized. Switch to using accessor functions instead of raw accesses of
inode->i_ctime.

Signed-off-by: Jeff Layton <[email protected]>
---
arch/s390/hypfs/inode.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/s390/hypfs/inode.c b/arch/s390/hypfs/inode.c
index ee919bfc8186..30fa336ec63e 100644
--- a/arch/s390/hypfs/inode.c
+++ b/arch/s390/hypfs/inode.c
@@ -53,7 +53,7 @@ static void hypfs_update_update(struct super_block *sb)
struct inode *inode = d_inode(sb_info->update_file);

sb_info->last_update = ktime_get_seconds();
- inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode);
+ inode->i_atime = inode->i_mtime = inode_ctime_set_current(inode);
}

/* directory tree removal functions */
@@ -101,7 +101,7 @@ static struct inode *hypfs_make_inode(struct super_block *sb, umode_t mode)
ret->i_mode = mode;
ret->i_uid = hypfs_info->uid;
ret->i_gid = hypfs_info->gid;
- ret->i_atime = ret->i_mtime = ret->i_ctime = current_time(ret);
+ ret->i_atime = ret->i_mtime = inode_ctime_set_current(ret);
if (S_ISDIR(mode))
set_nlink(ret, 2);
}
--
2.41.0

2023-06-21 16:47:22

by Jan Kara

Subject: Re: [PATCH 03/79] s390: switch to new ctime accessors

On Wed 21-06-23 10:45:16, Jeff Layton wrote:
> In later patches, we're going to change how the ctime.tv_nsec field is
> utilized. Switch to using accessor functions instead of raw accesses of
> inode->i_ctime.
>
> Signed-off-by: Jeff Layton <[email protected]>

Looks good to me. Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

Honza

> ---
> arch/s390/hypfs/inode.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/s390/hypfs/inode.c b/arch/s390/hypfs/inode.c
> index ee919bfc8186..30fa336ec63e 100644
> --- a/arch/s390/hypfs/inode.c
> +++ b/arch/s390/hypfs/inode.c
> @@ -53,7 +53,7 @@ static void hypfs_update_update(struct super_block *sb)
> struct inode *inode = d_inode(sb_info->update_file);
>
> sb_info->last_update = ktime_get_seconds();
> - inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode);
> + inode->i_atime = inode->i_mtime = inode_ctime_set_current(inode);
> }
>
> /* directory tree removal functions */
> @@ -101,7 +101,7 @@ static struct inode *hypfs_make_inode(struct super_block *sb, umode_t mode)
> ret->i_mode = mode;
> ret->i_uid = hypfs_info->uid;
> ret->i_gid = hypfs_info->gid;
> - ret->i_atime = ret->i_mtime = ret->i_ctime = current_time(ret);
> + ret->i_atime = ret->i_mtime = inode_ctime_set_current(ret);
> if (S_ISDIR(mode))
> set_nlink(ret, 2);
> }
> --
> 2.41.0
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2023-06-22 17:53:29

by Jeff Layton

Subject: Re: [PATCH 03/79] s390: switch to new ctime accessors

On Thu, 2023-06-22 at 19:35 +0200, Alexander Gordeev wrote:
> On Wed, Jun 21, 2023 at 10:45:16AM -0400, Jeff Layton wrote:
>
> Hi Jeff,
> > In later patches, we're going to change how the ctime.tv_nsec field is
> > utilized. Switch to using accessor functions instead of raw accesses of
> > inode->i_ctime.
> >
> > Signed-off-by: Jeff Layton <[email protected]>
> > ---
> > arch/s390/hypfs/inode.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/s390/hypfs/inode.c b/arch/s390/hypfs/inode.c
> > index ee919bfc8186..30fa336ec63e 100644
> > --- a/arch/s390/hypfs/inode.c
> > +++ b/arch/s390/hypfs/inode.c
> > @@ -53,7 +53,7 @@ static void hypfs_update_update(struct super_block *sb)
> > struct inode *inode = d_inode(sb_info->update_file);
> >
> > sb_info->last_update = ktime_get_seconds();
> > - inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode);
> > + inode->i_atime = inode->i_mtime = inode_ctime_set_current(inode);
> > }
> >
> > /* directory tree removal functions */
> > @@ -101,7 +101,7 @@ static struct inode *hypfs_make_inode(struct super_block *sb, umode_t mode)
> > ret->i_mode = mode;
> > ret->i_uid = hypfs_info->uid;
> > ret->i_gid = hypfs_info->gid;
> > - ret->i_atime = ret->i_mtime = ret->i_ctime = current_time(ret);
> > + ret->i_atime = ret->i_mtime = inode_ctime_set_current(ret);
> > if (S_ISDIR(mode))
> > set_nlink(ret, 2);
> > }
>
> I guess, inode_set_ctime() called from inode_ctime_set_current()
> updates i_ctime and is part of some other series?
>

No, that gets added in patch #1 of this series.

You should have gotten cc'ed on that one, though the postings to vger
mailing lists of patches 1, 2, and 79 bounced because the mail header
length on those was >8k.

--
Jeff Layton <[email protected]>

2023-06-22 17:53:59

by Alexander Gordeev

Subject: Re: [PATCH 03/79] s390: switch to new ctime accessors

On Wed, Jun 21, 2023 at 10:45:16AM -0400, Jeff Layton wrote:

Hi Jeff,
> In later patches, we're going to change how the ctime.tv_nsec field is
> utilized. Switch to using accessor functions instead of raw accesses of
> inode->i_ctime.
>
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> arch/s390/hypfs/inode.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/s390/hypfs/inode.c b/arch/s390/hypfs/inode.c
> index ee919bfc8186..30fa336ec63e 100644
> --- a/arch/s390/hypfs/inode.c
> +++ b/arch/s390/hypfs/inode.c
> @@ -53,7 +53,7 @@ static void hypfs_update_update(struct super_block *sb)
> struct inode *inode = d_inode(sb_info->update_file);
>
> sb_info->last_update = ktime_get_seconds();
> - inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode);
> + inode->i_atime = inode->i_mtime = inode_ctime_set_current(inode);
> }
>
> /* directory tree removal functions */
> @@ -101,7 +101,7 @@ static struct inode *hypfs_make_inode(struct super_block *sb, umode_t mode)
> ret->i_mode = mode;
> ret->i_uid = hypfs_info->uid;
> ret->i_gid = hypfs_info->gid;
> - ret->i_atime = ret->i_mtime = ret->i_ctime = current_time(ret);
> + ret->i_atime = ret->i_mtime = inode_ctime_set_current(ret);
> if (S_ISDIR(mode))
> set_nlink(ret, 2);
> }

I guess, inode_set_ctime() called from inode_ctime_set_current()
updates i_ctime and is part of some other series?

Thanks!

2023-06-22 18:43:37

by Alexander Gordeev

Subject: Re: [PATCH 03/79] s390: switch to new ctime accessors

On Thu, Jun 22, 2023 at 01:51:33PM -0400, Jeff Layton wrote:
> On Thu, 2023-06-22 at 19:35 +0200, Alexander Gordeev wrote:
> > On Wed, Jun 21, 2023 at 10:45:16AM -0400, Jeff Layton wrote:
> >
> > Hi Jeff,
> > > In later patches, we're going to change how the ctime.tv_nsec field is
> > > utilized. Switch to using accessor functions instead of raw accesses of
> > > inode->i_ctime.
> > >
> > > Signed-off-by: Jeff Layton <[email protected]>
> > > ---
> > > arch/s390/hypfs/inode.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/s390/hypfs/inode.c b/arch/s390/hypfs/inode.c
> > > index ee919bfc8186..30fa336ec63e 100644
> > > --- a/arch/s390/hypfs/inode.c
> > > +++ b/arch/s390/hypfs/inode.c
> > > @@ -53,7 +53,7 @@ static void hypfs_update_update(struct super_block *sb)
> > > struct inode *inode = d_inode(sb_info->update_file);
> > >
> > > sb_info->last_update = ktime_get_seconds();
> > > - inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode);
> > > + inode->i_atime = inode->i_mtime = inode_ctime_set_current(inode);
> > > }
> > >
> > > /* directory tree removal functions */
> > > @@ -101,7 +101,7 @@ static struct inode *hypfs_make_inode(struct super_block *sb, umode_t mode)
> > > ret->i_mode = mode;
> > > ret->i_uid = hypfs_info->uid;
> > > ret->i_gid = hypfs_info->gid;
> > > - ret->i_atime = ret->i_mtime = ret->i_ctime = current_time(ret);
> > > + ret->i_atime = ret->i_mtime = inode_ctime_set_current(ret);
> > > if (S_ISDIR(mode))
> > > set_nlink(ret, 2);
> > > }
> >
> > I guess, inode_set_ctime() called from inode_ctime_set_current()
> > updates i_ctime and is part of some other series?
> >
>
> No, that gets added in patch #1 of this series.
>
> You should have gotten cc'ed on that one, though the postings to vger
> mailing lists of patches 1, 2, and 79 bounced because the mail header
> length on those was >8k.

I actually received #1, if we're talking about this one:
https://lore.kernel.org/all/[email protected]/

I see inode_set_ctime() gets called, but nowere defined.

> Jeff Layton <[email protected]>

2023-06-22 19:43:42

by Jeff Layton

Subject: Re: [PATCH 03/79] s390: switch to new ctime accessors

On Thu, 2023-06-22 at 20:22 +0200, Alexander Gordeev wrote:
> On Thu, Jun 22, 2023 at 01:51:33PM -0400, Jeff Layton wrote:
> > On Thu, 2023-06-22 at 19:35 +0200, Alexander Gordeev wrote:
> > > On Wed, Jun 21, 2023 at 10:45:16AM -0400, Jeff Layton wrote:
> > >
> > > Hi Jeff,
> > > > In later patches, we're going to change how the ctime.tv_nsec field is
> > > > utilized. Switch to using accessor functions instead of raw accesses of
> > > > inode->i_ctime.
> > > >
> > > > Signed-off-by: Jeff Layton <[email protected]>
> > > > ---
> > > > arch/s390/hypfs/inode.c | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/arch/s390/hypfs/inode.c b/arch/s390/hypfs/inode.c
> > > > index ee919bfc8186..30fa336ec63e 100644
> > > > --- a/arch/s390/hypfs/inode.c
> > > > +++ b/arch/s390/hypfs/inode.c
> > > > @@ -53,7 +53,7 @@ static void hypfs_update_update(struct super_block *sb)
> > > > struct inode *inode = d_inode(sb_info->update_file);
> > > >
> > > > sb_info->last_update = ktime_get_seconds();
> > > > - inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode);
> > > > + inode->i_atime = inode->i_mtime = inode_ctime_set_current(inode);
> > > > }
> > > >
> > > > /* directory tree removal functions */
> > > > @@ -101,7 +101,7 @@ static struct inode *hypfs_make_inode(struct super_block *sb, umode_t mode)
> > > > ret->i_mode = mode;
> > > > ret->i_uid = hypfs_info->uid;
> > > > ret->i_gid = hypfs_info->gid;
> > > > - ret->i_atime = ret->i_mtime = ret->i_ctime = current_time(ret);
> > > > + ret->i_atime = ret->i_mtime = inode_ctime_set_current(ret);
> > > > if (S_ISDIR(mode))
> > > > set_nlink(ret, 2);
> > > > }
> > >
> > > I guess, inode_set_ctime() called from inode_ctime_set_current()
> > > updates i_ctime and is part of some other series?
> > >
> >
> > No, that gets added in patch #1 of this series.
> >
> > You should have gotten cc'ed on that one, though the postings to vger
> > mailing lists of patches 1, 2, and 79 bounced because the mail header
> > length on those was >8k.
>
> I actually received #1, if we're talking about this one:
> https://lore.kernel.org/all/[email protected]/
>
> I see inode_set_ctime() gets called, but nowere defined.
>
>

That's a bug -- that should be calling inode_ctime_set() instead. It
gets fixed up in a later patch in the series, but that should be fixed.

I'm already respinning this now with an updated coccinelle patch. I'll
make sure that's correct on the next posting.

Thanks,
--
Jeff Layton <[email protected]>