diff mbox series

Fix oops in NFS

Message ID 2229136.1703246451@warthog.procyon.org.uk (mailing list archive)
State New
Headers show
Series Fix oops in NFS | expand

Commit Message

David Howells Dec. 22, 2023, noon UTC 
David Howells <dhowells@redhat.com> wrote:

> A better way, though, is to move the call to nfs_netfs_inode_init()
> and give it a flag to say whether or not we want the facility.

Okay, I think I'll fold in the attached change.

David
---

Comments

Matthew Wilcox Jan. 5, 2024, 4:52 a.m. UTC | #1 
On Fri, Dec 22, 2023 at 12:00:51PM +0000, David Howells wrote:
> David Howells <dhowells@redhat.com> wrote:
> 
> > A better way, though, is to move the call to nfs_netfs_inode_init()
> > and give it a flag to say whether or not we want the facility.
> 
> Okay, I think I'll fold in the attached change.

This commit (100ccd18bb41 in linux-next 20240104) is bad for me.  After
it, running xfstests gives me first a bunch of errors along these lines:

00004 depmod: ERROR: failed to load symbols from /lib/modules/6.7.0-rc7-00037-g100ccd18bb41/kernel/fs/gfs2/gfs2.ko: Exec format error
00004 depmod: ERROR: failed to load symbols from /lib/modules/6.7.0-rc7-00037-g100ccd18bb41/kernel/fs/zonefs/zonefs.ko: Exec format error
00004 depmod: ERROR: failed to load symbols from /lib/modules/6.7.0-rc7-00037-g100ccd18bb41/kernel/security/keys/encrypted-keys/encrypted-keys.ko: Exec format error

and then later:

00016 generic/001       run fstests generic/001 at 2024-01-05 04:50:46
00017 [not run] this test requires a valid $TEST_DEV
00017 generic/002       run fstests generic/002 at 2024-01-05 04:50:46
00017 [not run] this test requires a valid $TEST_DEV
00017 generic/003       run fstests generic/003 at 2024-01-05 04:50:47
00018 [not run] this test requires a valid $SCRATCH_DEV
...

so I think that's page cache corruption of some kind.
David Howells Jan. 5, 2024, 10:12 a.m. UTC | #2 
Matthew Wilcox <willy@infradead.org> wrote:

> This commit (100ccd18bb41 in linux-next 20240104) is bad for me.  After
> it, running xfstests gives me first a bunch of errors along these lines:
> 
> 00004 depmod: ERROR: failed to load symbols from /lib/modules/6.7.0-rc7-00037-g100ccd18bb41/kernel/fs/gfs2/gfs2.ko: Exec format error
> 00004 depmod: ERROR: failed to load symbols from /lib/modules/6.7.0-rc7-00037-g100ccd18bb41/kernel/fs/zonefs/zonefs.ko: Exec format error
> 00004 depmod: ERROR: failed to load symbols from /lib/modules/6.7.0-rc7-00037-g100ccd18bb41/kernel/security/keys/encrypted-keys/encrypted-keys.ko: Exec format error
> 
> and then later:
> 
> 00016 generic/001       run fstests generic/001 at 2024-01-05 04:50:46
> 00017 [not run] this test requires a valid $TEST_DEV
> 00017 generic/002       run fstests generic/002 at 2024-01-05 04:50:46
> 00017 [not run] this test requires a valid $TEST_DEV
> 00017 generic/003       run fstests generic/003 at 2024-01-05 04:50:47
> 00018 [not run] this test requires a valid $SCRATCH_DEV
> ...
> 
> so I think that's page cache corruption of some kind.

Is that being run on NFS?  Is /lib on NFS?

David
David Howells Jan. 5, 2024, 11:48 a.m. UTC | #3 
Do you have CONFIG_NFS_FSCACHE set?  Are you using a cache?

David
Matthew Wilcox Jan. 5, 2024, 1:17 p.m. UTC | #4 
On Fri, Jan 05, 2024 at 10:12:55AM +0000, David Howells wrote:
> Matthew Wilcox <willy@infradead.org> wrote:
> 
> > This commit (100ccd18bb41 in linux-next 20240104) is bad for me.  After
> > it, running xfstests gives me first a bunch of errors along these lines:
> > 
> > 00004 depmod: ERROR: failed to load symbols from /lib/modules/6.7.0-rc7-00037-g100ccd18bb41/kernel/fs/gfs2/gfs2.ko: Exec format error
> > 00004 depmod: ERROR: failed to load symbols from /lib/modules/6.7.0-rc7-00037-g100ccd18bb41/kernel/fs/zonefs/zonefs.ko: Exec format error
> > 00004 depmod: ERROR: failed to load symbols from /lib/modules/6.7.0-rc7-00037-g100ccd18bb41/kernel/security/keys/encrypted-keys/encrypted-keys.ko: Exec format error
> > 
> > and then later:
> > 
> > 00016 generic/001       run fstests generic/001 at 2024-01-05 04:50:46
> > 00017 [not run] this test requires a valid $TEST_DEV
> > 00017 generic/002       run fstests generic/002 at 2024-01-05 04:50:46
> > 00017 [not run] this test requires a valid $TEST_DEV
> > 00017 generic/003       run fstests generic/003 at 2024-01-05 04:50:47
> > 00018 [not run] this test requires a valid $SCRATCH_DEV
> > ...
> > 
> > so I think that's page cache corruption of some kind.
> 
> Is that being run on NFS?  Is /lib on NFS?

No NFS involvement; this is supposed to be an XFS test ...

/dev/sda on / type ext4 (rw,relatime)
host on /host type 9p (rw,relatime,access=client,trans=virtio)
/dev/sdb on /mnt/test type xfs (rw,relatime,attr2,inode64,logbufs=8,logbsize=32k,noquota)

CONFIG_NETFS_SUPPORT=y
# CONFIG_NETFS_STATS is not set
# CONFIG_FSCACHE is not set
CONFIG_NETWORK_FILESYSTEMS=y
# CONFIG_NFS_FS is not set
# CONFIG_NFSD is not set
# CONFIG_CEPH_FS is not set
# CONFIG_CIFS is not set
# CONFIG_SMB_SERVER is not set
# CONFIG_CODA_FS is not set
# CONFIG_AFS_FS is not set
CONFIG_9P_FS=y
# CONFIG_9P_FS_POSIX_ACL is not set
# CONFIG_9P_FS_SECURITY is not set
CONFIG_NLS=y
David Howells Jan. 5, 2024, 2:33 p.m. UTC | #5 
Matthew Wilcox <willy@infradead.org> wrote:

> This commit (100ccd18bb41 in linux-next 20240104) is bad for me.  After
> it, running xfstests gives me first a bunch of errors along these lines:

This may be related to a patch that is in linux-next 20240105, but not
20240104 ("9p: Fix initialisation of netfs_inode for 9p").

David
diff mbox series

Patch

diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 55345753ae8d..b66466e97459 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -249,7 +249,7 @@  void v9fs_free_inode(struct inode *inode)
 static void v9fs_set_netfs_context(struct inode *inode)
 {
 	struct v9fs_inode *v9inode = V9FS_I(inode);
-	netfs_inode_init(&v9inode->netfs, &v9fs_req_ops);
+	netfs_inode_init(&v9inode->netfs, &v9fs_req_ops, true);
 }
 
 int v9fs_init_inode(struct v9fs_session_info *v9ses,
diff --git a/fs/afs/dynroot.c b/fs/afs/dynroot.c
index 1f656005018e..9c517269ff95 100644
--- a/fs/afs/dynroot.c
+++ b/fs/afs/dynroot.c
@@ -76,7 +76,7 @@  struct inode *afs_iget_pseudo_dir(struct super_block *sb, bool root)
 	/* there shouldn't be an existing inode */
 	BUG_ON(!(inode->i_state & I_NEW));
 
-	netfs_inode_init(&vnode->netfs, NULL);
+	netfs_inode_init(&vnode->netfs, NULL, false);
 	inode->i_size		= 0;
 	inode->i_mode		= S_IFDIR | S_IRUGO | S_IXUGO;
 	if (root) {
diff --git a/fs/afs/inode.c b/fs/afs/inode.c
index 2b44a342b4a1..381521e9e118 100644
--- a/fs/afs/inode.c
+++ b/fs/afs/inode.c
@@ -58,7 +58,7 @@  static noinline void dump_vnode(struct afs_vnode *vnode, struct afs_vnode *paren
  */
 static void afs_set_netfs_context(struct afs_vnode *vnode)
 {
-	netfs_inode_init(&vnode->netfs, &afs_req_ops);
+	netfs_inode_init(&vnode->netfs, &afs_req_ops, true);
 }
 
 /*
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index 3149d79a9dbe..0c25d326afc4 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -574,7 +574,7 @@  struct inode *ceph_alloc_inode(struct super_block *sb)
 	doutc(fsc->client, "%p\n", &ci->netfs.inode);
 
 	/* Set parameters for the netfs library */
-	netfs_inode_init(&ci->netfs, &ceph_netfs_ops);
+	netfs_inode_init(&ci->netfs, &ceph_netfs_ops, false);
 
 	spin_lock_init(&ci->i_ceph_lock);
 
diff --git a/fs/nfs/fscache.h b/fs/nfs/fscache.h
index 5407ab8c8783..e3cb4923316b 100644
--- a/fs/nfs/fscache.h
+++ b/fs/nfs/fscache.h
@@ -80,7 +80,7 @@  static inline void nfs_netfs_put(struct nfs_netfs_io_data *netfs)
 }
 static inline void nfs_netfs_inode_init(struct nfs_inode *nfsi)
 {
-	netfs_inode_init(&nfsi->netfs, &nfs_netfs_ops);
+	netfs_inode_init(&nfsi->netfs, &nfs_netfs_ops, false);
 }
 extern void nfs_netfs_initiate_read(struct nfs_pgio_header *hdr);
 extern void nfs_netfs_read_completion(struct nfs_pgio_header *hdr);
diff --git a/include/linux/netfs.h b/include/linux/netfs.h
index a5374218efe4..06a03dd1aff1 100644
--- a/include/linux/netfs.h
+++ b/include/linux/netfs.h
@@ -456,22 +456,27 @@  static inline struct netfs_inode *netfs_inode(struct inode *inode)
  * netfs_inode_init - Initialise a netfslib inode context
  * @ctx: The netfs inode to initialise
  * @ops: The netfs's operations list
+ * @use_zero_point: True to use the zero_point read optimisation
  *
  * Initialise the netfs library context struct.  This is expected to follow on
  * directly from the VFS inode struct.
  */
 static inline void netfs_inode_init(struct netfs_inode *ctx,
-				    const struct netfs_request_ops *ops)
+				    const struct netfs_request_ops *ops,
+				    bool use_zero_point)
 {
 	ctx->ops = ops;
 	ctx->remote_i_size = i_size_read(&ctx->inode);
-	ctx->zero_point = ctx->remote_i_size;
+	ctx->zero_point = LLONG_MAX;
 	ctx->flags = 0;
 #if IS_ENABLED(CONFIG_FSCACHE)
 	ctx->cache = NULL;
 #endif
 	/* ->releasepage() drives zero_point */
-	mapping_set_release_always(ctx->inode.i_mapping);
+	if (use_zero_point) {
+		ctx->zero_point = ctx->remote_i_size;
+		mapping_set_release_always(ctx->inode.i_mapping);
+	}
 }
 
 /**