xfs: move & sync up xfs_trans_inode with userspace

PATCH In review Eric Sandeen 5 participants 10 messages Jul 12, 2019 – Jul 16, 2019 XFS filesystem

Share & tools

Eric Sandeen

author

2019-07-12 17:43 UTC #0 lore raw

cover xfs: move & sync up xfs_trans_inode with userspace

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

Eric Sandeen

author

2019-07-12 17:44 UTC #1 lore raw

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

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 <[email protected]>

---

fs/xfs/Makefile +2 -2

index b74a47169297..06b68b6115bc 100644
@@ -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 \

fs/xfs/libxfs/xfs_trans_inode.c renamed from fs/xfs/xfs_trans_inode.c +0 -0

similarity index 100%
rename from fs/xfs/xfs_trans_inode.c
rename to fs/xfs/libxfs/xfs_trans_inode.c

Brian Foster

Reviewed-by on v1 1/2

2019-07-15 11:30 UTC #2 lore raw

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

Quoted 5 lines · 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 <[email protected]>
---

Reviewed-by Brian Foster <[email protected]>

Quoted 28 lines

fs/xfs/Makefile +2 -2

index b74a47169297..06b68b6115bc 100644
@@ -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 \

fs/xfs/libxfs/xfs_trans_inode.c renamed from fs/xfs/xfs_trans_inode.c +0 -0

similarity index 100%
rename from fs/xfs/xfs_trans_inode.c
rename to fs/xfs/libxfs/xfs_trans_inode.c

Eric Sandeen

author

2019-07-12 17:46 UTC #3 lore raw

PATCH 2/2 xfs: sync up xfs_trans_inode with userspace

In reply to 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 <[email protected]>

---

fs/xfs/libxfs/xfs_trans_inode.c +4 -0

index 93d14e47269d..a9ad90926b87 100644
@@ -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;
+	}
 }
 
 /*

Brian Foster

Reviewed-by on v1 2/2

2019-07-15 11:30 UTC #4 lore raw

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

Quoted 11 lines · 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 <[email protected]>
---
fs/xfs/libxfs/xfs_trans_inode.c +0 -0
index 93d14e47269d..a9ad90926b87 100644

fs/xfs/libxfs/xfs_trans_inode.c xfs_trans_ichgtime( · line 72

Quoted 7 lines

fs/xfs/libxfs/xfs_trans_inode.c +4 -0

@@ -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 <[email protected]>

Quoted 5 lines

fs/xfs/libxfs/xfs_trans_inode.c +0 -0
 }
 
 /*

Darrick J. Wong

2019-07-15 15:10 UTC #5 lore raw

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

Quoted 7 lines · Brian Foster

Quoted 6 lines · 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 <[email protected]>
---

Quoted 15 lines

fs/xfs/libxfs/xfs_trans_inode.c xfs_trans_ichgtime( · line 72
Quoted 12 lines
fs/xfs/libxfs/xfs_trans_inode.c +4 -0
index 93d14e47269d..a9ad90926b87 100644
@@ -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

Quoted 7 lines

Reviewed-by Brian Foster <[email protected]>
Quoted 5 lines
fs/xfs/libxfs/xfs_trans_inode.c +0 -0
 }
 
 /*

Dave Chinner

2019-07-15 22:23 UTC #6 lore raw

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

In reply to Eric Sandeen

Quoted 11 lines · 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 <[email protected]>
---
fs/xfs/libxfs/xfs_trans_inode.c +0 -0
index 93d14e47269d..a9ad90926b87 100644

fs/xfs/libxfs/xfs_trans_inode.c xfs_trans_ichgtime( · line 72

Quoted 7 lines

fs/xfs/libxfs/xfs_trans_inode.c +4 -0

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

Signature · 2 lines

Dave Chinner
[email protected]

Darrick J. Wong

2019-07-16 00:03 UTC #7 lore raw

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

Quoted 22 lines · Dave Chinner

Quoted 11 lines · 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 <[email protected]>
---
fs/xfs/libxfs/xfs_trans_inode.c +0 -0
index 93d14e47269d..a9ad90926b87 100644
fs/xfs/libxfs/xfs_trans_inode.c xfs_trans_ichgtime( · line 72
Quoted 7 lines
fs/xfs/libxfs/xfs_trans_inode.c +4 -0
@@ -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.

Quoted 4 lines

	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

Quoted 6 lines

Cheers,
Dave.
Signature · 2 lines
Dave Chinner
[email protected]

Eric Sandeen

2019-07-16 00:06 UTC #8 lore raw

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

Quoted 26 lines · Darrick J. Wong

Quoted 25 lines · Dave Chinner
Quoted 11 lines · 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 <[email protected]>
---
fs/xfs/libxfs/xfs_trans_inode.c +0 -0
index 93d14e47269d..a9ad90926b87 100644
fs/xfs/libxfs/xfs_trans_inode.c xfs_trans_ichgtime( · line 72
Quoted 7 lines
fs/xfs/libxfs/xfs_trans_inode.c +4 -0
@@ -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;

Quoted 6 lines

Quoted 1 line
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

Eric Sandeen

2019-07-16 00:05 UTC #9 lore raw

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

In reply to Dave Chinner

Quoted 22 lines · Dave Chinner

Quoted 11 lines · 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 <[email protected]>
---
fs/xfs/libxfs/xfs_trans_inode.c +0 -0
index 93d14e47269d..a9ad90926b87 100644
fs/xfs/libxfs/xfs_trans_inode.c xfs_trans_ichgtime( · line 72
Quoted 7 lines
fs/xfs/libxfs/xfs_trans_inode.c +4 -0
@@ -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.

Quoted 2 lines

	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 :(

Quoted 6 lines

And convert userspace over to the same :P
Cheers,
Dave.