All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] xfs: fix a few realtime bugs
@ 2020-09-10  5:36 Darrick J. Wong
  2020-09-10  5:36 ` [PATCH 1/2] xfs: refactor inode flags propagation code Darrick J. Wong
  2020-09-10  5:36 ` [PATCH 2/2] xfs: don't propagate RTINHERIT -> REALTIME when there is no rtdev Darrick J. Wong
  0 siblings, 2 replies; 5+ messages in thread
From: Darrick J. Wong @ 2020-09-10  5:36 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs, hch

Hi all,

This series fixes some problems with inode flags being propagated
incorrectly when a directory has the rtinherit flag set but there is no
realtime device.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=realtime-bugs
---
 fs/xfs/xfs_inode.c |  114 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 66 insertions(+), 48 deletions(-)


^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH 1/2] xfs: refactor inode flags propagation code
  2020-09-10  5:36 [PATCH v2 0/2] xfs: fix a few realtime bugs Darrick J. Wong
@ 2020-09-10  5:36 ` Darrick J. Wong
  2020-09-10  5:42   ` Christoph Hellwig
  2020-09-10  5:36 ` [PATCH 2/2] xfs: don't propagate RTINHERIT -> REALTIME when there is no rtdev Darrick J. Wong
  1 sibling, 1 reply; 5+ messages in thread
From: Darrick J. Wong @ 2020-09-10  5:36 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs, hch

From: Darrick J. Wong <darrick.wong@oracle.com>

Hoist the code that propagates di_flags and di_flags2 from a parent to a
new child into separate functions.  No functional changes.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_inode.c |  113 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 65 insertions(+), 48 deletions(-)


diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index c06129cffba9..4c520cc10191 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -714,6 +714,67 @@ xfs_lookup(
 	return error;
 }
 
+/* Propagate di_flags from a parent inode to a child inode. */
+static void
+xfs_inode_propagate_flags(
+	struct xfs_inode	*ip,
+	const struct xfs_inode	*pip)
+{
+	unsigned int		di_flags = 0;
+	umode_t			mode = VFS_I(ip)->i_mode;
+
+	if (S_ISDIR(mode)) {
+		if (pip->i_d.di_flags & XFS_DIFLAG_RTINHERIT)
+			di_flags |= XFS_DIFLAG_RTINHERIT;
+		if (pip->i_d.di_flags & XFS_DIFLAG_EXTSZINHERIT) {
+			di_flags |= XFS_DIFLAG_EXTSZINHERIT;
+			ip->i_d.di_extsize = pip->i_d.di_extsize;
+		}
+		if (pip->i_d.di_flags & XFS_DIFLAG_PROJINHERIT)
+			di_flags |= XFS_DIFLAG_PROJINHERIT;
+	} else if (S_ISREG(mode)) {
+		if (pip->i_d.di_flags & XFS_DIFLAG_RTINHERIT)
+			di_flags |= XFS_DIFLAG_REALTIME;
+		if (pip->i_d.di_flags & XFS_DIFLAG_EXTSZINHERIT) {
+			di_flags |= XFS_DIFLAG_EXTSIZE;
+			ip->i_d.di_extsize = pip->i_d.di_extsize;
+		}
+	}
+	if ((pip->i_d.di_flags & XFS_DIFLAG_NOATIME) &&
+	    xfs_inherit_noatime)
+		di_flags |= XFS_DIFLAG_NOATIME;
+	if ((pip->i_d.di_flags & XFS_DIFLAG_NODUMP) &&
+	    xfs_inherit_nodump)
+		di_flags |= XFS_DIFLAG_NODUMP;
+	if ((pip->i_d.di_flags & XFS_DIFLAG_SYNC) &&
+	    xfs_inherit_sync)
+		di_flags |= XFS_DIFLAG_SYNC;
+	if ((pip->i_d.di_flags & XFS_DIFLAG_NOSYMLINKS) &&
+	    xfs_inherit_nosymlinks)
+		di_flags |= XFS_DIFLAG_NOSYMLINKS;
+	if ((pip->i_d.di_flags & XFS_DIFLAG_NODEFRAG) &&
+	    xfs_inherit_nodefrag)
+		di_flags |= XFS_DIFLAG_NODEFRAG;
+	if (pip->i_d.di_flags & XFS_DIFLAG_FILESTREAM)
+		di_flags |= XFS_DIFLAG_FILESTREAM;
+
+	ip->i_d.di_flags |= di_flags;
+}
+
+/* Propagate di_flags2 from a parent inode to a child inode. */
+static void
+xfs_inode_propagate_flags2(
+	struct xfs_inode	*ip,
+	const struct xfs_inode	*pip)
+{
+	if (pip->i_d.di_flags2 & XFS_DIFLAG2_COWEXTSIZE) {
+		ip->i_d.di_flags2 |= XFS_DIFLAG2_COWEXTSIZE;
+		ip->i_d.di_cowextsize = pip->i_d.di_cowextsize;
+	}
+	if (pip->i_d.di_flags2 & XFS_DIFLAG2_DAX)
+		ip->i_d.di_flags2 |= XFS_DIFLAG2_DAX;
+}
+
 /*
  * Allocate an inode on disk and return a copy of its in-core version.
  * The in-core inode is locked exclusively.  Set mode, nlink, and rdev
@@ -857,54 +918,10 @@ xfs_ialloc(
 		break;
 	case S_IFREG:
 	case S_IFDIR:
-		if (pip && (pip->i_d.di_flags & XFS_DIFLAG_ANY)) {
-			uint		di_flags = 0;
-
-			if (S_ISDIR(mode)) {
-				if (pip->i_d.di_flags & XFS_DIFLAG_RTINHERIT)
-					di_flags |= XFS_DIFLAG_RTINHERIT;
-				if (pip->i_d.di_flags & XFS_DIFLAG_EXTSZINHERIT) {
-					di_flags |= XFS_DIFLAG_EXTSZINHERIT;
-					ip->i_d.di_extsize = pip->i_d.di_extsize;
-				}
-				if (pip->i_d.di_flags & XFS_DIFLAG_PROJINHERIT)
-					di_flags |= XFS_DIFLAG_PROJINHERIT;
-			} else if (S_ISREG(mode)) {
-				if (pip->i_d.di_flags & XFS_DIFLAG_RTINHERIT)
-					di_flags |= XFS_DIFLAG_REALTIME;
-				if (pip->i_d.di_flags & XFS_DIFLAG_EXTSZINHERIT) {
-					di_flags |= XFS_DIFLAG_EXTSIZE;
-					ip->i_d.di_extsize = pip->i_d.di_extsize;
-				}
-			}
-			if ((pip->i_d.di_flags & XFS_DIFLAG_NOATIME) &&
-			    xfs_inherit_noatime)
-				di_flags |= XFS_DIFLAG_NOATIME;
-			if ((pip->i_d.di_flags & XFS_DIFLAG_NODUMP) &&
-			    xfs_inherit_nodump)
-				di_flags |= XFS_DIFLAG_NODUMP;
-			if ((pip->i_d.di_flags & XFS_DIFLAG_SYNC) &&
-			    xfs_inherit_sync)
-				di_flags |= XFS_DIFLAG_SYNC;
-			if ((pip->i_d.di_flags & XFS_DIFLAG_NOSYMLINKS) &&
-			    xfs_inherit_nosymlinks)
-				di_flags |= XFS_DIFLAG_NOSYMLINKS;
-			if ((pip->i_d.di_flags & XFS_DIFLAG_NODEFRAG) &&
-			    xfs_inherit_nodefrag)
-				di_flags |= XFS_DIFLAG_NODEFRAG;
-			if (pip->i_d.di_flags & XFS_DIFLAG_FILESTREAM)
-				di_flags |= XFS_DIFLAG_FILESTREAM;
-
-			ip->i_d.di_flags |= di_flags;
-		}
-		if (pip && (pip->i_d.di_flags2 & XFS_DIFLAG2_ANY)) {
-			if (pip->i_d.di_flags2 & XFS_DIFLAG2_COWEXTSIZE) {
-				ip->i_d.di_flags2 |= XFS_DIFLAG2_COWEXTSIZE;
-				ip->i_d.di_cowextsize = pip->i_d.di_cowextsize;
-			}
-			if (pip->i_d.di_flags2 & XFS_DIFLAG2_DAX)
-				ip->i_d.di_flags2 |= XFS_DIFLAG2_DAX;
-		}
+		if (pip && (pip->i_d.di_flags & XFS_DIFLAG_ANY))
+			xfs_inode_propagate_flags(ip, pip);
+		if (pip && (pip->i_d.di_flags2 & XFS_DIFLAG2_ANY))
+			xfs_inode_propagate_flags2(ip, pip);
 		/* FALLTHROUGH */
 	case S_IFLNK:
 		ip->i_df.if_format = XFS_DINODE_FMT_EXTENTS;


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH 2/2] xfs: don't propagate RTINHERIT -> REALTIME when there is no rtdev
  2020-09-10  5:36 [PATCH v2 0/2] xfs: fix a few realtime bugs Darrick J. Wong
  2020-09-10  5:36 ` [PATCH 1/2] xfs: refactor inode flags propagation code Darrick J. Wong
@ 2020-09-10  5:36 ` Darrick J. Wong
  2020-09-10  5:42   ` Christoph Hellwig
  1 sibling, 1 reply; 5+ messages in thread
From: Darrick J. Wong @ 2020-09-10  5:36 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs, hch

From: Darrick J. Wong <darrick.wong@oracle.com>

While running generic/042 with -drtinherit=1 set in MKFS_OPTIONS, I
observed that the kernel will gladly set the realtime flag on any file
created on the loopback filesystem even though that filesystem doesn't
actually have a realtime device attached.  This leads to verifier
failures and doesn't make any sense, so be smarter about this.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_inode.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 4c520cc10191..ab43ccb88ee7 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -733,7 +733,8 @@ xfs_inode_propagate_flags(
 		if (pip->i_d.di_flags & XFS_DIFLAG_PROJINHERIT)
 			di_flags |= XFS_DIFLAG_PROJINHERIT;
 	} else if (S_ISREG(mode)) {
-		if (pip->i_d.di_flags & XFS_DIFLAG_RTINHERIT)
+		if ((pip->i_d.di_flags & XFS_DIFLAG_RTINHERIT) &&
+		    xfs_sb_version_hasrealtime(&ip->i_mount->m_sb))
 			di_flags |= XFS_DIFLAG_REALTIME;
 		if (pip->i_d.di_flags & XFS_DIFLAG_EXTSZINHERIT) {
 			di_flags |= XFS_DIFLAG_EXTSIZE;


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] xfs: refactor inode flags propagation code
  2020-09-10  5:36 ` [PATCH 1/2] xfs: refactor inode flags propagation code Darrick J. Wong
@ 2020-09-10  5:42   ` Christoph Hellwig
  0 siblings, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2020-09-10  5:42 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs, hch

> +		if (pip && (pip->i_d.di_flags & XFS_DIFLAG_ANY))
> +			xfs_inode_propagate_flags(ip, pip);
> +		if (pip && (pip->i_d.di_flags2 & XFS_DIFLAG2_ANY))
> +			xfs_inode_propagate_flags2(ip, pip);

I'd further simplify this to:

		if (pip) {
			if (pip->i_d.di_flags & XFS_DIFLAG_ANY)
				xfs_inode_propagate_flags(ip, pip);
			if (pip->i_d.di_flags2 & XFS_DIFLAG2_ANY)
				xfs_inode_propagate_flags2(ip, pip);
		}

and maybe use "inherit" instead of "propagate"

But otherwise this looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] xfs: don't propagate RTINHERIT -> REALTIME when there is no rtdev
  2020-09-10  5:36 ` [PATCH 2/2] xfs: don't propagate RTINHERIT -> REALTIME when there is no rtdev Darrick J. Wong
@ 2020-09-10  5:42   ` Christoph Hellwig
  0 siblings, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2020-09-10  5:42 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs, hch

On Wed, Sep 09, 2020 at 10:36:57PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> While running generic/042 with -drtinherit=1 set in MKFS_OPTIONS, I
> observed that the kernel will gladly set the realtime flag on any file
> created on the loopback filesystem even though that filesystem doesn't
> actually have a realtime device attached.  This leads to verifier
> failures and doesn't make any sense, so be smarter about this.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-09-10  5:42 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-10  5:36 [PATCH v2 0/2] xfs: fix a few realtime bugs Darrick J. Wong
2020-09-10  5:36 ` [PATCH 1/2] xfs: refactor inode flags propagation code Darrick J. Wong
2020-09-10  5:42   ` Christoph Hellwig
2020-09-10  5:36 ` [PATCH 2/2] xfs: don't propagate RTINHERIT -> REALTIME when there is no rtdev Darrick J. Wong
2020-09-10  5:42   ` Christoph Hellwig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.