| Message ID | 112d2e52-c96c-af83-7e53-5fca12448c76@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xfs: move & sync up xfs_trans_inode with userspace | expand |
On Fri, Jul 12, 2019 at 12:46:17PM -0500, Eric Sandeen wrote: > Add an XFS_ICHGTIME_CREATE case to xfs_trans_ichgtime() to keep in > sync with userspace. (Currently no kernel caller sends this flag.) > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > --- > > diff --git a/fs/xfs/libxfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c > index 93d14e47269d..a9ad90926b87 100644 > --- a/fs/xfs/libxfs/xfs_trans_inode.c > +++ b/fs/xfs/libxfs/xfs_trans_inode.c > @@ -66,6 +66,10 @@ xfs_trans_ichgtime( > inode->i_mtime = tv; > if (flags & XFS_ICHGTIME_CHG) > inode->i_ctime = tv; > + if (flags & XFS_ICHGTIME_CREATE) { > + ip->i_d.di_crtime.t_sec = (int32_t)tv.tv_sec; > + ip->i_d.di_crtime.t_nsec = (int32_t)tv.tv_nsec; > + } Could we add a "for libxfs" or some such one liner comment to this so long as this is unused in the kernel? With that: Reviewed-by: Brian Foster <bfoster@redhat.com> > } > > /* > >
On Mon, Jul 15, 2019 at 07:30:08AM -0400, Brian Foster wrote: > On Fri, Jul 12, 2019 at 12:46:17PM -0500, Eric Sandeen wrote: > > Add an XFS_ICHGTIME_CREATE case to xfs_trans_ichgtime() to keep in > > sync with userspace. (Currently no kernel caller sends this flag.) > > > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > > --- > > > > diff --git a/fs/xfs/libxfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c > > index 93d14e47269d..a9ad90926b87 100644 > > --- a/fs/xfs/libxfs/xfs_trans_inode.c > > +++ b/fs/xfs/libxfs/xfs_trans_inode.c > > @@ -66,6 +66,10 @@ xfs_trans_ichgtime( > > inode->i_mtime = tv; > > if (flags & XFS_ICHGTIME_CHG) > > inode->i_ctime = tv; > > + if (flags & XFS_ICHGTIME_CREATE) { > > + ip->i_d.di_crtime.t_sec = (int32_t)tv.tv_sec; > > + ip->i_d.di_crtime.t_nsec = (int32_t)tv.tv_nsec; > > + } > > Could we add a "for libxfs" or some such one liner comment to this so > long as this is unused in the kernel? With that: FWIW I was planning (5.4) to fix xfs_ialloc to use ICHGTIME_CREATE, so it won't be "for libxfs only" for long. :) --D > Reviewed-by: Brian Foster <bfoster@redhat.com> > > > } > > > > /* > > > >
On Fri, Jul 12, 2019 at 12:46:17PM -0500, Eric Sandeen wrote: > Add an XFS_ICHGTIME_CREATE case to xfs_trans_ichgtime() to keep in > sync with userspace. (Currently no kernel caller sends this flag.) > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > --- > > diff --git a/fs/xfs/libxfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c > index 93d14e47269d..a9ad90926b87 100644 > --- a/fs/xfs/libxfs/xfs_trans_inode.c > +++ b/fs/xfs/libxfs/xfs_trans_inode.c > @@ -66,6 +66,10 @@ xfs_trans_ichgtime( > inode->i_mtime = tv; > if (flags & XFS_ICHGTIME_CHG) > inode->i_ctime = tv; > + if (flags & XFS_ICHGTIME_CREATE) { > + ip->i_d.di_crtime.t_sec = (int32_t)tv.tv_sec; > + ip->i_d.di_crtime.t_nsec = (int32_t)tv.tv_nsec; > + } Please use the same format as the rest of the function. i.e. if (flags & XFS_ICHGTIME_CREATE) ip->i_d.di_crtime = tv; And convert userspace over to the same :P Cheers, Dave.
On Tue, Jul 16, 2019 at 08:22:09AM +1000, Dave Chinner wrote: > On Fri, Jul 12, 2019 at 12:46:17PM -0500, Eric Sandeen wrote: > > Add an XFS_ICHGTIME_CREATE case to xfs_trans_ichgtime() to keep in > > sync with userspace. (Currently no kernel caller sends this flag.) > > > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > > --- > > > > diff --git a/fs/xfs/libxfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c > > index 93d14e47269d..a9ad90926b87 100644 > > --- a/fs/xfs/libxfs/xfs_trans_inode.c > > +++ b/fs/xfs/libxfs/xfs_trans_inode.c > > @@ -66,6 +66,10 @@ xfs_trans_ichgtime( > > inode->i_mtime = tv; > > if (flags & XFS_ICHGTIME_CHG) > > inode->i_ctime = tv; > > + if (flags & XFS_ICHGTIME_CREATE) { > > + ip->i_d.di_crtime.t_sec = (int32_t)tv.tv_sec; > > + ip->i_d.di_crtime.t_nsec = (int32_t)tv.tv_nsec; > > + } > > Please use the same format as the rest of the function. i.e. > > if (flags & XFS_ICHGTIME_CREATE) > ip->i_d.di_crtime = tv; > > And convert userspace over to the same :P How about promoting crtime to struct inode while we're at it? (That said I think Eric was going for the quick resync now to keep kernel libxfs in sync with xfsprogs libxfs...) --D > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com
On 7/15/19 5:22 PM, Dave Chinner wrote: > On Fri, Jul 12, 2019 at 12:46:17PM -0500, Eric Sandeen wrote: >> Add an XFS_ICHGTIME_CREATE case to xfs_trans_ichgtime() to keep in >> sync with userspace. (Currently no kernel caller sends this flag.) >> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com> >> --- >> >> diff --git a/fs/xfs/libxfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c >> index 93d14e47269d..a9ad90926b87 100644 >> --- a/fs/xfs/libxfs/xfs_trans_inode.c >> +++ b/fs/xfs/libxfs/xfs_trans_inode.c >> @@ -66,6 +66,10 @@ xfs_trans_ichgtime( >> inode->i_mtime = tv; >> if (flags & XFS_ICHGTIME_CHG) >> inode->i_ctime = tv; >> + if (flags & XFS_ICHGTIME_CREATE) { >> + ip->i_d.di_crtime.t_sec = (int32_t)tv.tv_sec; >> + ip->i_d.di_crtime.t_nsec = (int32_t)tv.tv_nsec; >> + } > > Please use the same format as the rest of the function. i.e. > > if (flags & XFS_ICHGTIME_CREATE) > ip->i_d.di_crtime = tv; struct timespec64 tv; struct timespec64 i_atime; struct timespec64 i_mtime; struct timespec64 i_ctime; ... xfs_timestamp_t di_crtime; /* time created */ bzzzzt. will take a bit more work I think :( > And convert userspace over to the same :P > > Cheers, > > Dave. >
On 7/15/19 7:03 PM, Darrick J. Wong wrote: > On Tue, Jul 16, 2019 at 08:22:09AM +1000, Dave Chinner wrote: >> On Fri, Jul 12, 2019 at 12:46:17PM -0500, Eric Sandeen wrote: >>> Add an XFS_ICHGTIME_CREATE case to xfs_trans_ichgtime() to keep in >>> sync with userspace. (Currently no kernel caller sends this flag.) >>> >>> Signed-off-by: Eric Sandeen <sandeen@redhat.com> >>> --- >>> >>> diff --git a/fs/xfs/libxfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c >>> index 93d14e47269d..a9ad90926b87 100644 >>> --- a/fs/xfs/libxfs/xfs_trans_inode.c >>> +++ b/fs/xfs/libxfs/xfs_trans_inode.c >>> @@ -66,6 +66,10 @@ xfs_trans_ichgtime( >>> inode->i_mtime = tv; >>> if (flags & XFS_ICHGTIME_CHG) >>> inode->i_ctime = tv; >>> + if (flags & XFS_ICHGTIME_CREATE) { >>> + ip->i_d.di_crtime.t_sec = (int32_t)tv.tv_sec; >>> + ip->i_d.di_crtime.t_nsec = (int32_t)tv.tv_nsec; >>> + } >> >> Please use the same format as the rest of the function. i.e. >> >> if (flags & XFS_ICHGTIME_CREATE) >> ip->i_d.di_crtime = tv; >> >> And convert userspace over to the same :P > > How about promoting crtime to struct inode while we're at it? > > (That said I think Eric was going for the quick resync now to keep > kernel libxfs in sync with xfsprogs libxfs...) well, yeah but we don't have to pollute crap just to save 4 lines in a diff. I'm ok with moving the file and leaving this delta there for now if we need to work harder on those last 4 lines of diff ;) -Eric
diff --git a/fs/xfs/libxfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c index 93d14e47269d..a9ad90926b87 100644 --- a/fs/xfs/libxfs/xfs_trans_inode.c +++ b/fs/xfs/libxfs/xfs_trans_inode.c @@ -66,6 +66,10 @@ xfs_trans_ichgtime( inode->i_mtime = tv; if (flags & XFS_ICHGTIME_CHG) inode->i_ctime = tv; + if (flags & XFS_ICHGTIME_CREATE) { + ip->i_d.di_crtime.t_sec = (int32_t)tv.tv_sec; + ip->i_d.di_crtime.t_nsec = (int32_t)tv.tv_nsec; + } } /*
Add an XFS_ICHGTIME_CREATE case to xfs_trans_ichgtime() to keep in sync with userspace. (Currently no kernel caller sends this flag.) Signed-off-by: Eric Sandeen <sandeen@redhat.com> ---