[PATCH 0/2] xfs: move & sync up xfs_trans_inode with userspace

[PATCH 0/2] xfs: move & sync up xfs_trans_inode with userspace

[Eric Sandeen]

userspace moved xfs_trans_inode.c to libxfs after doing enough mangling
to share it, so do the same in the kernel now.

Add the CRTIME handling as well, since userspace has that, even though the
kernel doesn't use it for now.

-Eric

[PATCH 1/2] xfs: move xfs_trans_inode.c to libxfs/

[Eric Sandeen]

Userspace now has an identical xfs_trans_inode.c which it has already
moved to libxfs/ so do the same move for kernelspace.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
index b74a47169297..06b68b6115bc 100644
--- a/fs/xfs/Makefile
+++ b/fs/xfs/Makefile
@@ -49,6 +49,7 @@ xfs-y				+= $(addprefix libxfs/, \
 				   xfs_refcount_btree.o \
 				   xfs_sb.o \
 				   xfs_symlink_remote.o \
+				   xfs_trans_inode.o \
 				   xfs_trans_resv.o \
 				   xfs_types.o \
 				   )
@@ -107,8 +108,7 @@ xfs-y				+= xfs_log.o \
 				   xfs_rmap_item.o \
 				   xfs_log_recover.o \
 				   xfs_trans_ail.o \
-				   xfs_trans_buf.o \
-				   xfs_trans_inode.o
+				   xfs_trans_buf.o
 
 # optional features
 xfs-$(CONFIG_XFS_QUOTA)		+= xfs_dquot.o \
diff --git a/fs/xfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c
similarity index 100%
rename from fs/xfs/xfs_trans_inode.c
rename to fs/xfs/libxfs/xfs_trans_inode.c

Re: [PATCH 1/2] xfs: move xfs_trans_inode.c to libxfs/

[Brian Foster]

On Fri, Jul 12, 2019 at 12:44:48PM -0500, Eric Sandeen wrote:
> Userspace now has an identical xfs_trans_inode.c which it has already
> moved to libxfs/ so do the same move for kernelspace.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

> 
> diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
> index b74a47169297..06b68b6115bc 100644
> --- a/fs/xfs/Makefile
> +++ b/fs/xfs/Makefile
> @@ -49,6 +49,7 @@ xfs-y				+= $(addprefix libxfs/, \
>  				   xfs_refcount_btree.o \
>  				   xfs_sb.o \
>  				   xfs_symlink_remote.o \
> +				   xfs_trans_inode.o \
>  				   xfs_trans_resv.o \
>  				   xfs_types.o \
>  				   )
> @@ -107,8 +108,7 @@ xfs-y				+= xfs_log.o \
>  				   xfs_rmap_item.o \
>  				   xfs_log_recover.o \
>  				   xfs_trans_ail.o \
> -				   xfs_trans_buf.o \
> -				   xfs_trans_inode.o
> +				   xfs_trans_buf.o
>  
>  # optional features
>  xfs-$(CONFIG_XFS_QUOTA)		+= xfs_dquot.o \
> diff --git a/fs/xfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c
> similarity index 100%
> rename from fs/xfs/xfs_trans_inode.c
> rename to fs/xfs/libxfs/xfs_trans_inode.c
> 

[PATCH 2/2] xfs: sync up xfs_trans_inode with userspace

[Eric Sandeen]

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;
+	}
 }
 
 /*

Re: [PATCH 2/2] xfs: sync up xfs_trans_inode with userspace

[Brian Foster]

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>

>  }
>  
>  /*
> 
> 

Re: [PATCH 2/2] xfs: sync up xfs_trans_inode with userspace

[Darrick J. Wong]

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>
> 
> >  }
> >  
> >  /*
> > 
> > 

Re: [PATCH 2/2] xfs: sync up xfs_trans_inode with userspace

[Dave Chinner]

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.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 2/2] xfs: sync up xfs_trans_inode with userspace

[Darrick J. Wong]

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

Re: [PATCH 2/2] xfs: sync up xfs_trans_inode with userspace

[Eric Sandeen]

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

Re: [PATCH 2/2] xfs: sync up xfs_trans_inode with userspace

[Eric Sandeen]

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.
>