[PATCH 0/3] Introduce Xen fault injection facility

[PATCH 0/3] Introduce Xen fault injection facility

[Stanislav Kinsburskii]

This series adds a facility, which can be used to instrument Xen code with
fault injections.
It is based "Fault injection capabilities infrastructure" described here:
- Documentation/fault-injection/fault-injection.txt

First patch adds a generic facility to use anywhere in Xen.
When using it all the fault injection user land control directories (if
any) will appear here:
- /sys/kernel/debug/xen/fault_inject/

To distinguish with generic (or core) Xen fault injections, next two
patches add additional directories to the root path above for blkback and
netback drivers respectively:
- /sys/kernel/debug/xen/fault_inject/xen-blkback/
- /sys/kernel/debug/xen/fault_inject/xen-netback/

---

Stanislav Kinsburskii (3):
      xen: add generic fault injection facility
      xen netback: add fault injection facility
      xen blkback: add fault injection facility


 arch/x86/xen/Kconfig                   |    7 ++
 arch/x86/xen/Makefile                  |    1 
 arch/x86/xen/fault_inject.c            |  109 +++++++++++++++++++++++++++++
 drivers/block/Kconfig                  |    7 ++
 drivers/block/xen-blkback/Makefile     |    1 
 drivers/block/xen-blkback/blkback.c    |    9 ++
 drivers/block/xen-blkback/blkback_fi.c |  116 +++++++++++++++++++++++++++++++
 drivers/block/xen-blkback/blkback_fi.h |   37 ++++++++++
 drivers/block/xen-blkback/common.h     |    3 +
 drivers/block/xen-blkback/xenbus.c     |    5 +
 drivers/net/Kconfig                    |    8 ++
 drivers/net/xen-netback/Makefile       |    1 
 drivers/net/xen-netback/common.h       |    3 +
 drivers/net/xen-netback/netback.c      |    3 +
 drivers/net/xen-netback/netback_fi.c   |  119 ++++++++++++++++++++++++++++++++
 drivers/net/xen-netback/netback_fi.h   |   35 +++++++++
 drivers/net/xen-netback/xenbus.c       |    6 ++
 include/xen/fault_inject.h             |   45 ++++++++++++
 18 files changed, 515 insertions(+)
 create mode 100644 arch/x86/xen/fault_inject.c
 create mode 100644 drivers/block/xen-blkback/blkback_fi.c
 create mode 100644 drivers/block/xen-blkback/blkback_fi.h
 create mode 100644 drivers/net/xen-netback/netback_fi.c
 create mode 100644 drivers/net/xen-netback/netback_fi.h
 create mode 100644 include/xen/fault_inject.h
Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

[PATCH 1/3] xen: add generic fault injection facility

[Stanislav Kinsburskii]

The overall idea of this patch is to add a generic fault injection facility
to Xen, which later can be used in various places by different Xen parts.

Core implementation ideas:

- The facility build is controlled by boolean config
  CONFIG_XEN_FAULT_INJECTION option ("N" by default).

- All fault injection logic is located in an optionally compiled separated
  file.

- Fault injection attribute and control directory creation and destruction
  are wrapped with helpers, producing and accepting a pointer to an opaque
  object thus making all the rest of code independent on fault injection
  engine.

When enabled Xen root fault injection directory appears:

- /sys/kernel/debug/xen/fault_inject/

The falicity provides the following helpers (exported to be accessible in
modules):

- xen_fi_add(name) - adds fault injection control directory "name" to Xen
  root fault injection directory

- xen_fi_dir_create(name) - allows to create a subdirectory "name" in Xen
  root fault injection directory.

- xen_fi_dir_add(dir, name) - adds fault injection control directory "name"
  to directory "dir"

- xen_should_fail(fi) - check whether fi hav to fail.

Signed-off-by: Stanislav Kinsburskii <staskins@amazon.com>
CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Juergen Gross <jgross@suse.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: x86@kernel.org
CC: xen-devel@lists.xenproject.org
CC: linux-kernel@vger.kernel.org
CC: Stanislav Kinsburskii <staskins@amazon.com>
CC: David Woodhouse <dwmw@amazon.co.uk>
---
 arch/x86/xen/Kconfig        |    7 +++
 arch/x86/xen/Makefile       |    1 
 arch/x86/xen/fault_inject.c |  109 +++++++++++++++++++++++++++++++++++++++++++
 include/xen/fault_inject.h  |   45 ++++++++++++++++++
 4 files changed, 162 insertions(+)
 create mode 100644 arch/x86/xen/fault_inject.c
 create mode 100644 include/xen/fault_inject.h

diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
index c1f98f3..483fc16 100644
--- a/arch/x86/xen/Kconfig
+++ b/arch/x86/xen/Kconfig
@@ -77,3 +77,10 @@ config XEN_PVH
 	bool "Support for running as a PVH guest"
 	depends on XEN && XEN_PVHVM && ACPI
 	def_bool n
+
+config XEN_FAULT_INJECTION
+	bool "Enable Xen fault injection"
+	depends on FAULT_INJECTION_DEBUG_FS
+	default n
+	help
+	  Enable Xen fault injection facility
diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
index d83cb54..3158fe1 100644
--- a/arch/x86/xen/Makefile
+++ b/arch/x86/xen/Makefile
@@ -34,3 +34,4 @@ obj-$(CONFIG_XEN_DOM0)		+= vga.o
 obj-$(CONFIG_SWIOTLB_XEN)	+= pci-swiotlb-xen.o
 obj-$(CONFIG_XEN_EFI)		+= efi.o
 obj-$(CONFIG_XEN_PVH)	 	+= xen-pvh.o
+obj-$(CONFIG_XEN_FAULT_INJECTION)	+= fault_inject.o
diff --git a/arch/x86/xen/fault_inject.c b/arch/x86/xen/fault_inject.c
new file mode 100644
index 0000000..ecf0f7c
--- /dev/null
+++ b/arch/x86/xen/fault_inject.c
@@ -0,0 +1,109 @@
+/*
+ * Fauit injection interface for Xen virtual block devices
+ *
+ * Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation; or, when distributed
+ * separately from the Linux kernel or incorporated into other
+ * software packages, subject to the following license:
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this source file (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy, modify,
+ * merge, publish, distribute, sublicense, and/or sell copies of the Software,
+ * and to permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include <linux/slab.h>
+#include <linux/fault-inject.h>
+
+#include <xen/fault_inject.h>
+
+#include "debugfs.h"
+
+static struct dentry *d_fi_debug;
+
+static DECLARE_FAULT_ATTR(template_attr);
+
+struct xen_fi {
+	struct dentry		*dir;
+	struct fault_attr	attr;
+};
+
+struct xen_fi *xen_fi_dir_add(struct dentry *parent, const char *name)
+{
+	struct xen_fi *fi;
+	struct dentry *dir;
+
+	fi = kzalloc(sizeof(*fi), GFP_KERNEL);
+	if (!fi)
+		return ERR_PTR(-ENOMEM);
+
+	memcpy(&fi->attr, &template_attr, sizeof(template_attr));
+
+	dir = fault_create_debugfs_attr(name, parent, &fi->attr);
+	if (IS_ERR(dir)) {
+		kfree(fi);
+		return (struct xen_fi *)dir;
+	}
+
+	fi->dir = dir;
+
+	return fi;
+}
+EXPORT_SYMBOL(xen_fi_dir_add);
+
+struct xen_fi *xen_fi_add(const char *name)
+{
+	return xen_fi_dir_add(d_fi_debug, name);
+}
+
+void xen_fi_del(struct xen_fi *fi)
+{
+	debugfs_remove_recursive(fi->dir);
+	kfree(fi);
+}
+EXPORT_SYMBOL(xen_fi_del);
+
+bool xen_should_fail(struct xen_fi *fi)
+{
+	return should_fail(&fi->attr, 0);
+}
+EXPORT_SYMBOL(xen_should_fail);
+
+struct dentry *xen_fi_dir_create(const char *name)
+{
+	return debugfs_create_dir(name, d_fi_debug);
+}
+EXPORT_SYMBOL(xen_fi_dir_create);
+
+static int __init xen_fi_init(void)
+{
+	struct dentry *d_xen;
+
+	d_xen = xen_init_debugfs();
+	if (!d_xen)
+		return -ENOMEM;
+
+	d_fi_debug = debugfs_create_dir("fault_inject", d_xen);
+	if (d_fi_debug == NULL)
+		return -ENOMEM;
+
+	return 0;
+}
+
+fs_initcall(xen_fi_init);
diff --git a/include/xen/fault_inject.h b/include/xen/fault_inject.h
new file mode 100644
index 0000000..e2bf14a
--- /dev/null
+++ b/include/xen/fault_inject.h
@@ -0,0 +1,45 @@
+#ifndef _XEN_FAULT_INJECT_H
+#define _XEN_FAULT_INJECT_H
+
+struct dentry;
+struct xen_fi;
+
+#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
+
+struct dentry *xen_fi_dir_create(const char *name);
+
+struct xen_fi *xen_fi_dir_add(struct dentry *parent, const char *name);
+struct xen_fi *xen_fi_add(const char *name);
+void xen_fi_del(struct xen_fi *fi);
+
+bool xen_should_fail(struct xen_fi *fi);
+
+#else
+
+static inline struct dentry *xen_fi_dir_create(const char *name)
+{
+	return NULL;
+}
+
+static inline struct xen_fi *xen_fi_dir_add(struct dentry *parent, const char *name)
+{
+	return NULL;
+}
+
+static inline struct xen_fi *xen_fi_add(const char *name)
+{
+	return NULL;
+}
+
+static inline void xen_fi_del(struct xen_fi *fi) { }
+
+static inline bool xen_should_fail(struct xen_fi *fi)
+{
+	return false;
+}
+
+#endif /* CONFIG_FAULT_INJECTION_DEBUG_FS */
+
+#endif /* _XEN_FAULT_INJECT_H */
+
+

Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

[PATCH 2/3] xen netback: add fault injection facility

[Stanislav Kinsburskii]

This patch adds wrapper helpers around generic Xen fault inject facility.
The major reason is to keep all the module fault injection directories
in a dedicated subdirectory instead of Xen fault inject root.

IOW, when using these helpers, per-device and named by device name fault
injection control directories will appear under the following directory:
- /sys/kernel/debug/xen/fault_inject/xen-netback/
instead of:
- /sys/kernel/debug/xen/fault_inject/

Signed-off-by: Stanislav Kinsburskii <staskins@amazon.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Paul Durrant <paul.durrant@citrix.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: Matteo Croce <mcroce@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Daniel Borkmann <daniel@iogearbox.net>
CC: Gerard Garcia <ggarcia@deic.uab.cat>
CC: David Ahern <dsa@cumulusnetworks.com>
CC: Juergen Gross <jgross@suse.com>
CC: Amir Levy <amir.jer.levy@intel.com>
CC: Jakub Kicinski <jakub.kicinski@netronome.com>
CC: linux-kernel@vger.kernel.org
CC: netdev@vger.kernel.org
CC: xen-devel@lists.xenproject.org
CC: Stanislav Kinsburskii <staskins@amazon.com>
CC: David Woodhouse <dwmw@amazon.co.uk>
---
 drivers/net/Kconfig                  |    8 ++
 drivers/net/xen-netback/Makefile     |    1 
 drivers/net/xen-netback/common.h     |    3 +
 drivers/net/xen-netback/netback.c    |    3 +
 drivers/net/xen-netback/netback_fi.c |  119 ++++++++++++++++++++++++++++++++++
 drivers/net/xen-netback/netback_fi.h |   35 ++++++++++
 drivers/net/xen-netback/xenbus.c     |    6 ++
 7 files changed, 175 insertions(+)
 create mode 100644 drivers/net/xen-netback/netback_fi.c
 create mode 100644 drivers/net/xen-netback/netback_fi.h

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 8918466..5cc9acd 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -465,6 +465,14 @@ config XEN_NETDEV_BACKEND
 	  compile this driver as a module, chose M here: the module
 	  will be called xen-netback.
 
+config XEN_NETDEV_BACKEND_FAULT_INJECTION
+	  bool "Xen net-device backend driver fault injection"
+	  depends on XEN_NETDEV_BACKEND
+	  depends on XEN_FAULT_INJECTION
+	  default n
+	  help
+	    Allow to inject errors to Xen backend network driver
+
 config VMXNET3
 	tristate "VMware VMXNET3 ethernet driver"
 	depends on PCI && INET
diff --git a/drivers/net/xen-netback/Makefile b/drivers/net/xen-netback/Makefile
index d49798a..28abcdc 100644
--- a/drivers/net/xen-netback/Makefile
+++ b/drivers/net/xen-netback/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_XEN_NETDEV_BACKEND) := xen-netback.o
 
 xen-netback-y := netback.o xenbus.o interface.o hash.o rx.o
+xen-netback-$(CONFIG_XEN_NETDEV_BACKEND_FAULT_INJECTION) += netback_fi.o
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index a46a1e9..30d676d 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -286,6 +286,9 @@ struct xenvif {
 
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *xenvif_dbg_root;
+#ifdef CONFIG_XEN_NETDEV_BACKEND_FAULT_INJECTION
+	void *fi_info;
+#endif
 #endif
 
 	struct xen_netif_ctrl_back_ring ctrl;
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index a27daa2..ecc416e 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -33,6 +33,7 @@
  */
 
 #include "common.h"
+#include "netback_fi.h"
 
 #include <linux/kthread.h>
 #include <linux/if_vlan.h>
@@ -1649,6 +1650,7 @@ static int __init netback_init(void)
 			PTR_ERR(xen_netback_dbg_root));
 #endif /* CONFIG_DEBUG_FS */
 
+	(void) xen_netbk_fi_init();
 	return 0;
 
 failed_init:
@@ -1659,6 +1661,7 @@ module_init(netback_init);
 
 static void __exit netback_fini(void)
 {
+	xen_netbk_fi_fini();
 #ifdef CONFIG_DEBUG_FS
 	if (!IS_ERR_OR_NULL(xen_netback_dbg_root))
 		debugfs_remove_recursive(xen_netback_dbg_root);
diff --git a/drivers/net/xen-netback/netback_fi.c b/drivers/net/xen-netback/netback_fi.c
new file mode 100644
index 0000000..47541d0
--- /dev/null
+++ b/drivers/net/xen-netback/netback_fi.c
@@ -0,0 +1,119 @@
+/*
+ * Fault injection interface for Xen backend network driver
+ *
+ * Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation; or, when distributed
+ * separately from the Linux kernel or incorporated into other
+ * software packages, subject to the following license:
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this source file (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy, modify,
+ * merge, publish, distribute, sublicense, and/or sell copies of the Software,
+ * and to permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "common.h"
+
+#include <linux/debugfs.h>
+
+#include <xen/fault_inject.h>
+#include "netback_fi.h"
+
+static struct dentry *vif_fi_dir;
+
+static const char *xenvif_fi_names[] = {
+};
+
+struct xenvif_fi {
+	struct dentry *dir;
+	struct xen_fi *faults[XENVIF_FI_MAX];
+};
+
+int xen_netbk_fi_init(void)
+{
+	vif_fi_dir = xen_fi_dir_create("xen-netback");
+	if (!vif_fi_dir)
+		return -ENOMEM;
+	return 0;
+}
+
+void xen_netbk_fi_fini(void)
+{
+	debugfs_remove_recursive(vif_fi_dir);
+}
+
+void xenvif_fi_fini(struct xenvif *vif)
+{
+	struct xenvif_fi *vfi = vif->fi_info;
+	int fi;
+
+	if (!vif->fi_info)
+		return;
+
+	vif->fi_info = NULL;
+
+	for (fi = 0; fi < XENVIF_FI_MAX; fi++)
+		xen_fi_del(vfi->faults[fi]);
+	debugfs_remove_recursive(vfi->dir);
+	kfree(vfi);
+}
+
+int xenvif_fi_init(struct xenvif *vif)
+{
+	struct dentry *parent;
+	struct xenvif_fi *vfi;
+	int fi, err = -ENOMEM;
+
+	parent = vif_fi_dir;
+	if (!parent)
+		return -ENOMEM;
+
+	vfi = kmalloc(sizeof(*vfi), GFP_KERNEL);
+	if (!vfi)
+		return -ENOMEM;
+
+	vfi->dir = debugfs_create_dir(vif->dev->name, parent);
+	if (!vfi->dir)
+		goto err_dir;
+
+	for (fi = 0; fi < XENVIF_FI_MAX; fi++) {
+		vfi->faults[fi] = xen_fi_dir_add(vfi->dir,
+				xenvif_fi_names[fi]);
+		if (!vfi->faults[fi])
+			goto err_fault;
+	}
+
+	vif->fi_info = vfi;
+	return 0;
+
+err_fault:
+	for (; fi > 0; fi--)
+		xen_fi_del(vfi->faults[fi]);
+	debugfs_remove_recursive(vfi->dir);
+err_dir:
+	kfree(vfi);
+	return err;
+}
+
+bool xenvif_should_fail(struct xenvif *vif, xenvif_fi_t type)
+{
+	struct xenvif_fi *vfi = vif->fi_info;
+
+	return xen_should_fail(vfi->faults[type]);
+}
diff --git a/drivers/net/xen-netback/netback_fi.h b/drivers/net/xen-netback/netback_fi.h
new file mode 100644
index 0000000..895c6a6
--- /dev/null
+++ b/drivers/net/xen-netback/netback_fi.h
@@ -0,0 +1,35 @@
+#ifndef _XEN_NETBACK_FI_H
+#define _XEN_NETBACK_FI_H
+
+struct xen_fi;
+
+typedef enum {
+	XENVIF_FI_MAX
+} xenvif_fi_t;
+
+#ifdef CONFIG_XEN_NETDEV_BACKEND_FAULT_INJECTION
+
+int xen_netbk_fi_init(void);
+void xen_netbk_fi_fini(void);
+
+void xenvif_fi_fini(struct xenvif *vif);
+int xenvif_fi_init(struct xenvif *vif);
+
+bool xenvif_should_fail(struct xenvif *vif, xenvif_fi_t type);
+
+#else
+
+static inline int xen_netbk_fi_init(void) { return 0; }
+static inline void xen_netbk_fi_fini(void) { }
+
+static inline void xenvif_fi_fini(struct xenvif *vif) { }
+static inline int xenvif_fi_init(struct xenvif *vif) { return 0; }
+
+static inline bool xenvif_should_fail(struct xenvif *vif, xenvif_fi_t type)
+{
+	return false;
+}
+
+#endif /* CONFIG_XEN_NETDEV_BACKEND_FAULT_INJECTION */
+
+#endif /* _XEN_NETBACK_FI_H */
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index e1aef25..c775ee0 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -21,6 +21,7 @@
 #include "common.h"
 #include <linux/vmalloc.h>
 #include <linux/rtnetlink.h>
+#include "netback_fi.h"
 
 struct backend_info {
 	struct xenbus_device *dev;
@@ -502,6 +503,7 @@ static void backend_disconnect(struct backend_info *be)
 #ifdef CONFIG_DEBUG_FS
 		xenvif_debugfs_delif(vif);
 #endif /* CONFIG_DEBUG_FS */
+		xenvif_fi_fini(vif);
 		xenvif_disconnect_data(vif);
 
 		/* At this point some of the handlers may still be active
@@ -1024,6 +1026,10 @@ static void connect(struct backend_info *be)
 		}
 	}
 
+	err = xenvif_fi_init(be->vif);
+	if (err)
+		goto err;
+
 #ifdef CONFIG_DEBUG_FS
 	xenvif_debugfs_addif(be->vif);
 #endif /* CONFIG_DEBUG_FS */

Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

[PATCH 3/3] xen blkback: add fault injection facility

[Stanislav Kinsburskii]

This patch adds wrapper helpers around generic Xen fault inject
facility.
The major reason is to keep all the module fault injection directories
in a dedicated subdirectory instead of Xen fault inject root.

IOW, when using these helpers, per-device and named by device name
fault injection control directories will appear under the following
directory:
- /sys/kernel/debug/xen/fault_inject/xen-blkback/
instead of:
- /sys/kernel/debug/xen/fault_inject/

Signed-off-by: Stanislav Kinsburskii <staskins@amazon.com>
CC: Jens Axboe <axboe@kernel.dk>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: "Roger Pau Monné" <roger.pau@citrix.com>
CC: linux-kernel@vger.kernel.org
CC: linux-block@vger.kernel.org
CC: xen-devel@lists.xenproject.org
CC: Stanislav Kinsburskii <staskins@amazon.com>
CC: David Woodhouse <dwmw@amazon.co.uk>
---
 drivers/block/Kconfig                  |    7 ++
 drivers/block/xen-blkback/Makefile     |    1 
 drivers/block/xen-blkback/blkback.c    |    9 ++
 drivers/block/xen-blkback/blkback_fi.c |  116 ++++++++++++++++++++++++++++++++
 drivers/block/xen-blkback/blkback_fi.h |   37 ++++++++++
 drivers/block/xen-blkback/common.h     |    3 +
 drivers/block/xen-blkback/xenbus.c     |    5 +
 7 files changed, 178 insertions(+)
 create mode 100644 drivers/block/xen-blkback/blkback_fi.c
 create mode 100644 drivers/block/xen-blkback/blkback_fi.h

diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index ad9b687..0ce05fe 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -436,6 +436,13 @@ config XEN_BLKDEV_BACKEND
 	  compile this driver as a module, chose M here: the module
 	  will be called xen-blkback.
 
+config XEN_BLKDEV_BACKEND_FAULT_INJECTION
+	  bool "Xen block-device backend driver fault injection"
+	  depends on XEN_BLKDEV_BACKEND
+	  depends on XEN_FAULT_INJECTION
+	  default n
+	  help
+	    Allow to inject errors to Xen backend block driver
 
 config VIRTIO_BLK
 	tristate "Virtio block driver"
diff --git a/drivers/block/xen-blkback/Makefile b/drivers/block/xen-blkback/Makefile
index e491c1b..035d134 100644
--- a/drivers/block/xen-blkback/Makefile
+++ b/drivers/block/xen-blkback/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_XEN_BLKDEV_BACKEND) := xen-blkback.o
 
 xen-blkback-y	:= blkback.o xenbus.o
+xen-blkback-$(CONFIG_XEN_BLKDEV_BACKEND_FAULT_INJECTION) += blkback_fi.o
diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
index 987d665..2933bec 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -51,6 +51,7 @@
 #include <xen/balloon.h>
 #include <xen/grant_table.h>
 #include "common.h"
+#include "blkback_fi.h"
 
 /*
  * Maximum number of unused free pages to keep in the internal buffer.
@@ -1491,11 +1492,19 @@ static int __init xen_blkif_init(void)
 	if (rc)
 		goto failed_init;
 
+	(void) xen_blkbk_fi_init();
  failed_init:
 	return rc;
 }
 
 module_init(xen_blkif_init);
 
+static void __exit xen_blkif_fini(void)
+{
+	xen_blkbk_fi_fini();
+}
+
+module_exit(xen_blkif_fini);
+
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_ALIAS("xen-backend:vbd");
diff --git a/drivers/block/xen-blkback/blkback_fi.c b/drivers/block/xen-blkback/blkback_fi.c
new file mode 100644
index 0000000..b7abaea
--- /dev/null
+++ b/drivers/block/xen-blkback/blkback_fi.c
@@ -0,0 +1,116 @@
+/*
+ * Fault injection interface for Xen virtual block devices
+ *
+ * Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation; or, when distributed
+ * separately from the Linux kernel or incorporated into other
+ * software packages, subject to the following license:
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this source file (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy, modify,
+ * merge, publish, distribute, sublicense, and/or sell copies of the Software,
+ * and to permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include <linux/slab.h>
+#include <linux/debugfs.h>
+
+#include <xen/fault_inject.h>
+
+#include "blkback_fi.h"
+#include "common.h"
+
+static struct dentry *blkif_fi_dir;
+
+static const char *xen_blkif_fi_names[XENBLKIF_FI_MAX] = {
+};
+
+struct xen_blkif_fi {
+	struct dentry *dir;
+	struct xen_fi *faults[XENBLKIF_FI_MAX];
+};
+
+int xen_blkbk_fi_init(void)
+{
+	blkif_fi_dir = xen_fi_dir_create("xen-blkback");
+	if (!blkif_fi_dir)
+		return -ENOMEM;
+	return 0;
+}
+
+void xen_blkbk_fi_fini(void)
+{
+	debugfs_remove_recursive(blkif_fi_dir);
+}
+
+void xen_blkif_fi_fini(struct xen_blkif *blkif)
+{
+	struct xen_blkif_fi *bfi = blkif->fi_info;
+	int fi;
+
+	if (!blkif->fi_info)
+		return;
+
+	blkif->fi_info = NULL;
+
+	for (fi = 0; fi < XENBLKIF_FI_MAX; fi++)
+		xen_fi_del(bfi->faults[fi]);
+	debugfs_remove_recursive(bfi->dir);
+	kfree(bfi);
+}
+
+int xen_blkif_fi_init(struct xen_blkif *blkif)
+{
+	struct xen_blkif_fi *bfi;
+	int fi, err = -ENOMEM;
+
+	bfi = kmalloc(sizeof(*bfi), GFP_KERNEL);
+	if (!bfi)
+		return -ENOMEM;
+
+	bfi->dir = debugfs_create_dir(dev_name(&blkif->be->dev->dev),
+				      blkif_fi_dir);
+	if (!bfi->dir)
+		goto err_dir;
+
+	for (fi = 0; fi < XENBLKIF_FI_MAX; fi++) {
+		bfi->faults[fi] = xen_fi_dir_add(bfi->dir,
+						 xen_blkif_fi_names[fi]);
+		if (!bfi->faults[fi])
+			goto err_fault;
+	}
+
+	blkif->fi_info = bfi;
+	return 0;
+
+err_fault:
+	for (; fi > 0; fi--)
+		xen_fi_del(bfi->faults[fi]);
+	debugfs_remove_recursive(bfi->dir);
+err_dir:
+	kfree(bfi);
+	return err;
+}
+
+bool xenblkif_should_fail(struct xen_blkif *blkif, xen_blkbk_fi_t type)
+{
+	struct xen_blkif_fi *bfi = blkif->fi_info;
+
+	return xen_should_fail(bfi->faults[type]);
+}
diff --git a/drivers/block/xen-blkback/blkback_fi.h b/drivers/block/xen-blkback/blkback_fi.h
new file mode 100644
index 0000000..4e29850
--- /dev/null
+++ b/drivers/block/xen-blkback/blkback_fi.h
@@ -0,0 +1,37 @@
+#ifndef _XEN_BLKBACK_FI_H
+#define _XEN_BLKBACK_FI_H
+
+struct xen_fi;
+struct xen_blkif;
+
+typedef enum {
+	XENBLKIF_FI_MAX
+} xen_blkbk_fi_t;
+
+#ifdef CONFIG_XEN_BLKDEV_BACKEND_FAULT_INJECTION
+
+void xen_blkbk_fi_fini(void);
+int xen_blkbk_fi_init(void);
+
+void xen_blkif_fi_fini(struct xen_blkif *blkif);
+int xen_blkif_fi_init(struct xen_blkif *blkif);
+
+bool xenblkif_should_fail(struct xen_blkif *blkif, xen_blkbk_fi_t type);
+
+#else
+
+static inline void xen_blkbk_fi_fini(void) { }
+static inline int xen_blkbk_fi_init(void) { return 0; }
+
+static inline void xen_blkif_fi_fini(struct xen_blkif *blkif) { }
+static inline int xen_blkif_fi_init(struct xen_blkif *blkif) { return 0; }
+
+static inline bool xenblkif_should_fail(struct xen_blkif *blkif, xen_blkbk_fi_t type)
+{
+	return false;
+}
+
+#endif /* CONFIG_XEN_BLKDEV_BACKEND_FAULT_INJECTION */
+
+#endif /* _XEN_BLKBACK_FI_H */
+
diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h
index ecb35fe..6d12d4d 100644
--- a/drivers/block/xen-blkback/common.h
+++ b/drivers/block/xen-blkback/common.h
@@ -329,6 +329,9 @@ struct xen_blkif {
 	/* All rings for this device. */
 	struct xen_blkif_ring	*rings;
 	unsigned int		nr_rings;
+#ifdef CONFIG_XEN_BLKDEV_BACKEND_FAULT_INJECTION
+	void			*fi_info;
+#endif
 };
 
 struct seg_buf {
diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index 21c1be1..9931fc8 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -22,6 +22,7 @@
 #include <xen/events.h>
 #include <xen/grant_table.h>
 #include "common.h"
+#include "blkback_fi.h"
 
 /* On the XenBus the max length of 'ring-ref%u'. */
 #define RINGREF_NAME_LEN (20)
@@ -246,6 +247,8 @@ static int xen_blkif_disconnect(struct xen_blkif *blkif)
 	unsigned int j, r;
 	bool busy = false;
 
+	xen_blkif_fi_fini(blkif);
+
 	for (r = 0; r < blkif->nr_rings; r++) {
 		struct xen_blkif_ring *ring = &blkif->rings[r];
 		unsigned int i = 0;
@@ -998,6 +1001,8 @@ static int read_per_ring_refs(struct xen_blkif_ring *ring, const char *dir)
 		return err;
 	}
 
+	(void) xen_blkif_fi_init(blkif);
+
 	return 0;
 
 fail:

Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

Re: [PATCH 1/3] xen: add generic fault injection facility

[Juergen Gross]

On 20/04/18 12:47, Stanislav Kinsburskii wrote:
> The overall idea of this patch is to add a generic fault injection facility
> to Xen, which later can be used in various places by different Xen parts.
> 
> Core implementation ideas:
> 
> - The facility build is controlled by boolean config
>   CONFIG_XEN_FAULT_INJECTION option ("N" by default).
> 
> - All fault injection logic is located in an optionally compiled separated
>   file.
> 
> - Fault injection attribute and control directory creation and destruction
>   are wrapped with helpers, producing and accepting a pointer to an opaque
>   object thus making all the rest of code independent on fault injection
>   engine.
> 
> When enabled Xen root fault injection directory appears:
> 
> - /sys/kernel/debug/xen/fault_inject/
> 
> The falicity provides the following helpers (exported to be accessible in
> modules):
> 
> - xen_fi_add(name) - adds fault injection control directory "name" to Xen
>   root fault injection directory
> 
> - xen_fi_dir_create(name) - allows to create a subdirectory "name" in Xen
>   root fault injection directory.
> 
> - xen_fi_dir_add(dir, name) - adds fault injection control directory "name"
>   to directory "dir"
> 
> - xen_should_fail(fi) - check whether fi hav to fail.
> 
> Signed-off-by: Stanislav Kinsburskii <staskins@amazon.com>
> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> CC: Juergen Gross <jgross@suse.com>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: Ingo Molnar <mingo@redhat.com>
> CC: "H. Peter Anvin" <hpa@zytor.com>
> CC: x86@kernel.org
> CC: xen-devel@lists.xenproject.org
> CC: linux-kernel@vger.kernel.org
> CC: Stanislav Kinsburskii <staskins@amazon.com>
> CC: David Woodhouse <dwmw@amazon.co.uk>
> ---
>  arch/x86/xen/Kconfig        |    7 +++
>  arch/x86/xen/Makefile       |    1 
>  arch/x86/xen/fault_inject.c |  109 +++++++++++++++++++++++++++++++++++++++++++
>  include/xen/fault_inject.h  |   45 ++++++++++++++++++
>  4 files changed, 162 insertions(+)
>  create mode 100644 arch/x86/xen/fault_inject.c
>  create mode 100644 include/xen/fault_inject.h
> 
> diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
> index c1f98f3..483fc16 100644
> --- a/arch/x86/xen/Kconfig
> +++ b/arch/x86/xen/Kconfig
> @@ -77,3 +77,10 @@ config XEN_PVH
>  	bool "Support for running as a PVH guest"
>  	depends on XEN && XEN_PVHVM && ACPI
>  	def_bool n
> +
> +config XEN_FAULT_INJECTION
> +	bool "Enable Xen fault injection"
> +	depends on FAULT_INJECTION_DEBUG_FS
> +	default n
> +	help
> +	  Enable Xen fault injection facility

Why for x86 only? I'd rather add this under drivers/xen

> diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
> index d83cb54..3158fe1 100644
> --- a/arch/x86/xen/Makefile
> +++ b/arch/x86/xen/Makefile
> @@ -34,3 +34,4 @@ obj-$(CONFIG_XEN_DOM0)		+= vga.o
>  obj-$(CONFIG_SWIOTLB_XEN)	+= pci-swiotlb-xen.o
>  obj-$(CONFIG_XEN_EFI)		+= efi.o
>  obj-$(CONFIG_XEN_PVH)	 	+= xen-pvh.o
> +obj-$(CONFIG_XEN_FAULT_INJECTION)	+= fault_inject.o
> diff --git a/arch/x86/xen/fault_inject.c b/arch/x86/xen/fault_inject.c
> new file mode 100644
> index 0000000..ecf0f7c
> --- /dev/null
> +++ b/arch/x86/xen/fault_inject.c
> @@ -0,0 +1,109 @@
> +/*
> + * Fauit injection interface for Xen virtual block devices
> + *
> + * Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation; or, when distributed
> + * separately from the Linux kernel or incorporated into other
> + * software packages, subject to the following license:

Please use the appropriate SPDX header instead of the full GPL2
boilerplate.


Juergen

Re: [PATCH 2/3] xen netback: add fault injection facility

[Juergen Gross]

On 20/04/18 12:47, Stanislav Kinsburskii wrote:
> This patch adds wrapper helpers around generic Xen fault inject facility.
> The major reason is to keep all the module fault injection directories
> in a dedicated subdirectory instead of Xen fault inject root.
> 
> IOW, when using these helpers, per-device and named by device name fault
> injection control directories will appear under the following directory:
> - /sys/kernel/debug/xen/fault_inject/xen-netback/
> instead of:
> - /sys/kernel/debug/xen/fault_inject/
> 
> Signed-off-by: Stanislav Kinsburskii <staskins@amazon.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Paul Durrant <paul.durrant@citrix.com>
> CC: "David S. Miller" <davem@davemloft.net>
> CC: Matteo Croce <mcroce@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Daniel Borkmann <daniel@iogearbox.net>
> CC: Gerard Garcia <ggarcia@deic.uab.cat>
> CC: David Ahern <dsa@cumulusnetworks.com>
> CC: Juergen Gross <jgross@suse.com>
> CC: Amir Levy <amir.jer.levy@intel.com>
> CC: Jakub Kicinski <jakub.kicinski@netronome.com>
> CC: linux-kernel@vger.kernel.org
> CC: netdev@vger.kernel.org
> CC: xen-devel@lists.xenproject.org
> CC: Stanislav Kinsburskii <staskins@amazon.com>
> CC: David Woodhouse <dwmw@amazon.co.uk>
> ---
>  drivers/net/Kconfig                  |    8 ++
>  drivers/net/xen-netback/Makefile     |    1 
>  drivers/net/xen-netback/common.h     |    3 +
>  drivers/net/xen-netback/netback.c    |    3 +
>  drivers/net/xen-netback/netback_fi.c |  119 ++++++++++++++++++++++++++++++++++
>  drivers/net/xen-netback/netback_fi.h |   35 ++++++++++
>  drivers/net/xen-netback/xenbus.c     |    6 ++
>  7 files changed, 175 insertions(+)
>  create mode 100644 drivers/net/xen-netback/netback_fi.c
>  create mode 100644 drivers/net/xen-netback/netback_fi.h
> 
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 8918466..5cc9acd 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -465,6 +465,14 @@ config XEN_NETDEV_BACKEND
>  	  compile this driver as a module, chose M here: the module
>  	  will be called xen-netback.
>  
> +config XEN_NETDEV_BACKEND_FAULT_INJECTION
> +	  bool "Xen net-device backend driver fault injection"
> +	  depends on XEN_NETDEV_BACKEND
> +	  depends on XEN_FAULT_INJECTION
> +	  default n
> +	  help
> +	    Allow to inject errors to Xen backend network driver
> +
>  config VMXNET3
>  	tristate "VMware VMXNET3 ethernet driver"
>  	depends on PCI && INET
> diff --git a/drivers/net/xen-netback/Makefile b/drivers/net/xen-netback/Makefile
> index d49798a..28abcdc 100644
> --- a/drivers/net/xen-netback/Makefile
> +++ b/drivers/net/xen-netback/Makefile
> @@ -1,3 +1,4 @@
>  obj-$(CONFIG_XEN_NETDEV_BACKEND) := xen-netback.o
>  
>  xen-netback-y := netback.o xenbus.o interface.o hash.o rx.o
> +xen-netback-$(CONFIG_XEN_NETDEV_BACKEND_FAULT_INJECTION) += netback_fi.o
> diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
> index a46a1e9..30d676d 100644
> --- a/drivers/net/xen-netback/common.h
> +++ b/drivers/net/xen-netback/common.h
> @@ -286,6 +286,9 @@ struct xenvif {
>  
>  #ifdef CONFIG_DEBUG_FS
>  	struct dentry *xenvif_dbg_root;
> +#ifdef CONFIG_XEN_NETDEV_BACKEND_FAULT_INJECTION
> +	void *fi_info;
> +#endif
>  #endif
>  
>  	struct xen_netif_ctrl_back_ring ctrl;
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index a27daa2..ecc416e 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -33,6 +33,7 @@
>   */
>  
>  #include "common.h"
> +#include "netback_fi.h"
>  
>  #include <linux/kthread.h>
>  #include <linux/if_vlan.h>
> @@ -1649,6 +1650,7 @@ static int __init netback_init(void)
>  			PTR_ERR(xen_netback_dbg_root));
>  #endif /* CONFIG_DEBUG_FS */
>  
> +	(void) xen_netbk_fi_init();

This is the only usage of xen_netbk_fi_init(). Why don't you make it
return void from the beginning?

>  	return 0;
>  
>  failed_init:
> @@ -1659,6 +1661,7 @@ module_init(netback_init);
>  
>  static void __exit netback_fini(void)
>  {
> +	xen_netbk_fi_fini();
>  #ifdef CONFIG_DEBUG_FS
>  	if (!IS_ERR_OR_NULL(xen_netback_dbg_root))
>  		debugfs_remove_recursive(xen_netback_dbg_root);
> diff --git a/drivers/net/xen-netback/netback_fi.c b/drivers/net/xen-netback/netback_fi.c
> new file mode 100644
> index 0000000..47541d0
> --- /dev/null
> +++ b/drivers/net/xen-netback/netback_fi.c
> @@ -0,0 +1,119 @@
> +/*
> + * Fault injection interface for Xen backend network driver
> + *
> + * Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation; or, when distributed
> + * separately from the Linux kernel or incorporated into other
> + * software packages, subject to the following license:

SPDX again.

> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this source file (the "Software"), to deal in the Software without
> + * restriction, including without limitation the rights to use, copy, modify,
> + * merge, publish, distribute, sublicense, and/or sell copies of the Software,
> + * and to permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#include "common.h"
> +
> +#include <linux/debugfs.h>
> +
> +#include <xen/fault_inject.h>
> +#include "netback_fi.h"
> +
> +static struct dentry *vif_fi_dir;
> +
> +static const char *xenvif_fi_names[] = {
> +};
> +
> +struct xenvif_fi {
> +	struct dentry *dir;
> +	struct xen_fi *faults[XENVIF_FI_MAX];
> +};
> +
> +int xen_netbk_fi_init(void)
> +{
> +	vif_fi_dir = xen_fi_dir_create("xen-netback");
> +	if (!vif_fi_dir)
> +		return -ENOMEM;
> +	return 0;
> +}
> +
> +void xen_netbk_fi_fini(void)
> +{
> +	debugfs_remove_recursive(vif_fi_dir);
> +}
> +
> +void xenvif_fi_fini(struct xenvif *vif)
> +{
> +	struct xenvif_fi *vfi = vif->fi_info;
> +	int fi;
> +
> +	if (!vif->fi_info)
> +		return;
> +
> +	vif->fi_info = NULL;
> +
> +	for (fi = 0; fi < XENVIF_FI_MAX; fi++)
> +		xen_fi_del(vfi->faults[fi]);
> +	debugfs_remove_recursive(vfi->dir);
> +	kfree(vfi);
> +}
> +
> +int xenvif_fi_init(struct xenvif *vif)
> +{
> +	struct dentry *parent;
> +	struct xenvif_fi *vfi;
> +	int fi, err = -ENOMEM;
> +
> +	parent = vif_fi_dir;
> +	if (!parent)
> +		return -ENOMEM;
> +
> +	vfi = kmalloc(sizeof(*vfi), GFP_KERNEL);
> +	if (!vfi)
> +		return -ENOMEM;
> +
> +	vfi->dir = debugfs_create_dir(vif->dev->name, parent);
> +	if (!vfi->dir)
> +		goto err_dir;
> +
> +	for (fi = 0; fi < XENVIF_FI_MAX; fi++) {
> +		vfi->faults[fi] = xen_fi_dir_add(vfi->dir,
> +				xenvif_fi_names[fi]);

How does this work? xenvif_fi_names[] is an empty array and this is the
only reference to it. Who is allocating the memory for that array?

> +		if (!vfi->faults[fi])
> +			goto err_fault;
> +	}
> +
> +	vif->fi_info = vfi;
> +	return 0;
> +
> +err_fault:
> +	for (; fi > 0; fi--)
> +		xen_fi_del(vfi->faults[fi]);

What about vfi->faults[0] ?

> +	debugfs_remove_recursive(vfi->dir);
> +err_dir:
> +	kfree(vfi);
> +	return err;
> +}
> +
> +bool xenvif_should_fail(struct xenvif *vif, xenvif_fi_t type)
> +{
> +	struct xenvif_fi *vfi = vif->fi_info;
> +
> +	return xen_should_fail(vfi->faults[type]);
> +}
> diff --git a/drivers/net/xen-netback/netback_fi.h b/drivers/net/xen-netback/netback_fi.h
> new file mode 100644
> index 0000000..895c6a6
> --- /dev/null
> +++ b/drivers/net/xen-netback/netback_fi.h
> @@ -0,0 +1,35 @@
> +#ifndef _XEN_NETBACK_FI_H
> +#define _XEN_NETBACK_FI_H
> +
> +struct xen_fi;

Why?

> +
> +typedef enum {
> +	XENVIF_FI_MAX
> +} xenvif_fi_t;

It would have helped if you had added some users of the stuff you are
adding here. This enum just looks weird this way.

> +
> +#ifdef CONFIG_XEN_NETDEV_BACKEND_FAULT_INJECTION
> +
> +int xen_netbk_fi_init(void);
> +void xen_netbk_fi_fini(void);
> +
> +void xenvif_fi_fini(struct xenvif *vif);
> +int xenvif_fi_init(struct xenvif *vif);
> +
> +bool xenvif_should_fail(struct xenvif *vif, xenvif_fi_t type);
> +
> +#else
> +
> +static inline int xen_netbk_fi_init(void) { return 0; }
> +static inline void xen_netbk_fi_fini(void) { }
> +
> +static inline void xenvif_fi_fini(struct xenvif *vif) { }
> +static inline int xenvif_fi_init(struct xenvif *vif) { return 0; }
> +
> +static inline bool xenvif_should_fail(struct xenvif *vif, xenvif_fi_t type)
> +{
> +	return false;
> +}
> +
> +#endif /* CONFIG_XEN_NETDEV_BACKEND_FAULT_INJECTION */
> +
> +#endif /* _XEN_NETBACK_FI_H */
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
> index e1aef25..c775ee0 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -21,6 +21,7 @@
>  #include "common.h"
>  #include <linux/vmalloc.h>
>  #include <linux/rtnetlink.h>
> +#include "netback_fi.h"
>  
>  struct backend_info {
>  	struct xenbus_device *dev;
> @@ -502,6 +503,7 @@ static void backend_disconnect(struct backend_info *be)
>  #ifdef CONFIG_DEBUG_FS
>  		xenvif_debugfs_delif(vif);
>  #endif /* CONFIG_DEBUG_FS */
> +		xenvif_fi_fini(vif);
>  		xenvif_disconnect_data(vif);
>  
>  		/* At this point some of the handlers may still be active
> @@ -1024,6 +1026,10 @@ static void connect(struct backend_info *be)
>  		}
>  	}
>  
> +	err = xenvif_fi_init(be->vif);
> +	if (err)
> +		goto err;
> +
>  #ifdef CONFIG_DEBUG_FS
>  	xenvif_debugfs_addif(be->vif);
>  #endif /* CONFIG_DEBUG_FS */
>

Without any user of that infrastructure I really can't say whether I
want this.


Juergen

Re: [PATCH 3/3] xen blkback: add fault injection facility

[Juergen Gross]

On 20/04/18 12:47, Stanislav Kinsburskii wrote:
> This patch adds wrapper helpers around generic Xen fault inject
> facility.
> The major reason is to keep all the module fault injection directories
> in a dedicated subdirectory instead of Xen fault inject root.
> 
> IOW, when using these helpers, per-device and named by device name
> fault injection control directories will appear under the following
> directory:
> - /sys/kernel/debug/xen/fault_inject/xen-blkback/
> instead of:
> - /sys/kernel/debug/xen/fault_inject/
> 
> Signed-off-by: Stanislav Kinsburskii <staskins@amazon.com>
> CC: Jens Axboe <axboe@kernel.dk>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> CC: "Roger Pau Monné" <roger.pau@citrix.com>
> CC: linux-kernel@vger.kernel.org
> CC: linux-block@vger.kernel.org
> CC: xen-devel@lists.xenproject.org
> CC: Stanislav Kinsburskii <staskins@amazon.com>
> CC: David Woodhouse <dwmw@amazon.co.uk>

This is an exact copy of the netback patch apart from the names.

I don't like adding multiple copies of the same coding to the tree.


Juergen

Re: [PATCH 1/3] xen: add generic fault injection facility

[staskins]

On 04/20/18 12:59, Juergen Gross wrote:
> On 20/04/18 12:47, Stanislav Kinsburskii wrote:
>> diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
>> index c1f98f3..483fc16 100644
>> --- a/arch/x86/xen/Kconfig
>> +++ b/arch/x86/xen/Kconfig
>> @@ -77,3 +77,10 @@ config XEN_PVH
>>   	bool "Support for running as a PVH guest"
>>   	depends on XEN && XEN_PVHVM && ACPI
>>   	def_bool n
>> +
>> +config XEN_FAULT_INJECTION
>> +	bool "Enable Xen fault injection"
>> +	depends on FAULT_INJECTION_DEBUG_FS
>> +	default n
>> +	help
>> +	  Enable Xen fault injection facility
> Why for x86 only? I'd rather add this under drivers/xen

Sure.

>
>> diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
>> index d83cb54..3158fe1 100644
>> --- a/arch/x86/xen/Makefile
>> +++ b/arch/x86/xen/Makefile
>> @@ -34,3 +34,4 @@ obj-$(CONFIG_XEN_DOM0)		+= vga.o
>>   obj-$(CONFIG_SWIOTLB_XEN)	+= pci-swiotlb-xen.o
>>   obj-$(CONFIG_XEN_EFI)		+= efi.o
>>   obj-$(CONFIG_XEN_PVH)	 	+= xen-pvh.o
>> +obj-$(CONFIG_XEN_FAULT_INJECTION)	+= fault_inject.o
>> diff --git a/arch/x86/xen/fault_inject.c b/arch/x86/xen/fault_inject.c
>> new file mode 100644
>> index 0000000..ecf0f7c
>> --- /dev/null
>> +++ b/arch/x86/xen/fault_inject.c
>> @@ -0,0 +1,109 @@
>> +/*
>> + * Fauit injection interface for Xen virtual block devices
>> + *
>> + * Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License version 2
>> + * as published by the Free Software Foundation; or, when distributed
>> + * separately from the Linux kernel or incorporated into other
>> + * software packages, subject to the following license:
> Please use the appropriate SPDX header instead of the full GPL2
> boilerplate.

Ditto.

>
>
> Juergen
>

Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

Re: [PATCH 2/3] xen netback: add fault injection facility

[staskins]

On 04/20/18 13:25, Juergen Gross wrote:
> On 20/04/18 12:47, Stanislav Kinsburskii wrote:
>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
>> index 8918466..5cc9acd 100644
>> --- a/drivers/net/Kconfig
>> +++ b/drivers/net/Kconfig
>> @@ -465,6 +465,14 @@ config XEN_NETDEV_BACKEND
>>   	  compile this driver as a module, chose M here: the module
>>   	  will be called xen-netback.
>>   
>> +config XEN_NETDEV_BACKEND_FAULT_INJECTION
>> +	  bool "Xen net-device backend driver fault injection"
>> +	  depends on XEN_NETDEV_BACKEND
>> +	  depends on XEN_FAULT_INJECTION
>> +	  default n
>> +	  help
>> +	    Allow to inject errors to Xen backend network driver
>> +
>>   config VMXNET3
>>   	tristate "VMware VMXNET3 ethernet driver"
>>   	depends on PCI && INET
>> diff --git a/drivers/net/xen-netback/Makefile b/drivers/net/xen-netback/Makefile
>> index d49798a..28abcdc 100644
>> --- a/drivers/net/xen-netback/Makefile
>> +++ b/drivers/net/xen-netback/Makefile
>> @@ -1,3 +1,4 @@
>>   obj-$(CONFIG_XEN_NETDEV_BACKEND) := xen-netback.o
>>   
>>   xen-netback-y := netback.o xenbus.o interface.o hash.o rx.o
>> +xen-netback-$(CONFIG_XEN_NETDEV_BACKEND_FAULT_INJECTION) += netback_fi.o
>> diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
>> index a46a1e9..30d676d 100644
>> --- a/drivers/net/xen-netback/common.h
>> +++ b/drivers/net/xen-netback/common.h
>> @@ -286,6 +286,9 @@ struct xenvif {
>>   
>>   #ifdef CONFIG_DEBUG_FS
>>   	struct dentry *xenvif_dbg_root;
>> +#ifdef CONFIG_XEN_NETDEV_BACKEND_FAULT_INJECTION
>> +	void *fi_info;
>> +#endif
>>   #endif
>>   
>>   	struct xen_netif_ctrl_back_ring ctrl;
>> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
>> index a27daa2..ecc416e 100644
>> --- a/drivers/net/xen-netback/netback.c
>> +++ b/drivers/net/xen-netback/netback.c
>> @@ -33,6 +33,7 @@
>>    */
>>   
>>   #include "common.h"
>> +#include "netback_fi.h"
>>   
>>   #include <linux/kthread.h>
>>   #include <linux/if_vlan.h>
>> @@ -1649,6 +1650,7 @@ static int __init netback_init(void)
>>   			PTR_ERR(xen_netback_dbg_root));
>>   #endif /* CONFIG_DEBUG_FS */
>>   
>> +	(void) xen_netbk_fi_init();
> This is the only usage of xen_netbk_fi_init(). Why don't you make it
> return void from the beginning?

Well, I could do so, of course.
My intention was to treat this as an error. But then it doesn't 
correlate to ignored debugfs directory creation error above.

>>   	return 0;
>>   
>>   failed_init:
>> @@ -1659,6 +1661,7 @@ module_init(netback_init);
>>   
>>   static void __exit netback_fini(void)
>>   {
>> +	xen_netbk_fi_fini();
>>   #ifdef CONFIG_DEBUG_FS
>>   	if (!IS_ERR_OR_NULL(xen_netback_dbg_root))
>>   		debugfs_remove_recursive(xen_netback_dbg_root);
>> diff --git a/drivers/net/xen-netback/netback_fi.c b/drivers/net/xen-netback/netback_fi.c
>> new file mode 100644
>> index 0000000..47541d0
>> --- /dev/null
>> +++ b/drivers/net/xen-netback/netback_fi.c
>> @@ -0,0 +1,119 @@
>> +/*
>> + * Fault injection interface for Xen backend network driver
>> + *
>> + * Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License version 2
>> + * as published by the Free Software Foundation; or, when distributed
>> + * separately from the Linux kernel or incorporated into other
>> + * software packages, subject to the following license:
> SPDX again.

Will fix.

>
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a copy
>> + * of this source file (the "Software"), to deal in the Software without
>> + * restriction, including without limitation the rights to use, copy, modify,
>> + * merge, publish, distribute, sublicense, and/or sell copies of the Software,
>> + * and to permit persons to whom the Software is furnished to do so, subject to
>> + * the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
>> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + */
>> +
>> +#include "common.h"
>> +
>> +#include <linux/debugfs.h>
>> +
>> +#include <xen/fault_inject.h>
>> +#include "netback_fi.h"
>> +
>> +static struct dentry *vif_fi_dir;
>> +
>> +static const char *xenvif_fi_names[] = {
>> +};
>> +
>> +struct xenvif_fi {
>> +	struct dentry *dir;
>> +	struct xen_fi *faults[XENVIF_FI_MAX];
>> +};
>> +
>> +int xen_netbk_fi_init(void)
>> +{
>> +	vif_fi_dir = xen_fi_dir_create("xen-netback");
>> +	if (!vif_fi_dir)
>> +		return -ENOMEM;
>> +	return 0;
>> +}
>> +
>> +void xen_netbk_fi_fini(void)
>> +{
>> +	debugfs_remove_recursive(vif_fi_dir);
>> +}
>> +
>> +void xenvif_fi_fini(struct xenvif *vif)
>> +{
>> +	struct xenvif_fi *vfi = vif->fi_info;
>> +	int fi;
>> +
>> +	if (!vif->fi_info)
>> +		return;
>> +
>> +	vif->fi_info = NULL;
>> +
>> +	for (fi = 0; fi < XENVIF_FI_MAX; fi++)
>> +		xen_fi_del(vfi->faults[fi]);
>> +	debugfs_remove_recursive(vfi->dir);
>> +	kfree(vfi);
>> +}
>> +
>> +int xenvif_fi_init(struct xenvif *vif)
>> +{
>> +	struct dentry *parent;
>> +	struct xenvif_fi *vfi;
>> +	int fi, err = -ENOMEM;
>> +
>> +	parent = vif_fi_dir;
>> +	if (!parent)
>> +		return -ENOMEM;
>> +
>> +	vfi = kmalloc(sizeof(*vfi), GFP_KERNEL);
>> +	if (!vfi)
>> +		return -ENOMEM;
>> +
>> +	vfi->dir = debugfs_create_dir(vif->dev->name, parent);
>> +	if (!vfi->dir)
>> +		goto err_dir;
>> +
>> +	for (fi = 0; fi < XENVIF_FI_MAX; fi++) {
>> +		vfi->faults[fi] = xen_fi_dir_add(vfi->dir,
>> +				xenvif_fi_names[fi]);
> How does this work? xenvif_fi_names[] is an empty array and this is the
> only reference to it. Who is allocating the memory for that array?

Well, it works in the way one adds a var to enum (which is used as a key 
later) and a corresponding string into the array (which is used as a 
name for the fault directory in sysfs).

>> +		if (!vfi->faults[fi])
>> +			goto err_fault;
>> +	}
>> +
>> +	vif->fi_info = vfi;
>> +	return 0;
>> +
>> +err_fault:
>> +	for (; fi > 0; fi--)
>> +		xen_fi_del(vfi->faults[fi]);
> What about vfi->faults[0] ?

Thanks! Will fix.


>> +	debugfs_remove_recursive(vfi->dir);
>> +err_dir:
>> +	kfree(vfi);
>> +	return err;
>> +}
>> +
>> +bool xenvif_should_fail(struct xenvif *vif, xenvif_fi_t type)
>> +{
>> +	struct xenvif_fi *vfi = vif->fi_info;
>> +
>> +	return xen_should_fail(vfi->faults[type]);
>> +}
>> diff --git a/drivers/net/xen-netback/netback_fi.h b/drivers/net/xen-netback/netback_fi.h
>> new file mode 100644
>> index 0000000..895c6a6
>> --- /dev/null
>> +++ b/drivers/net/xen-netback/netback_fi.h
>> @@ -0,0 +1,35 @@
>> +#ifndef _XEN_NETBACK_FI_H
>> +#define _XEN_NETBACK_FI_H
>> +
>> +struct xen_fi;
> Why?

Ditto.

>> +
>> +typedef enum {
>> +	XENVIF_FI_MAX
>> +} xenvif_fi_t;
> It would have helped if you had added some users of the stuff you are
> adding here. This enum just looks weird this way.
>
it in pla
Yeah... Probably I should mark this thing as a RFC.

>> +
>> +#ifdef CONFIG_XEN_NETDEV_BACKEND_FAULT_INJECTION
>> +
>> +int xen_netbk_fi_init(void);
>> +void xen_netbk_fi_fini(void);
>> +
>> +void xenvif_fi_fini(struct xenvif *vif);
>> +int xenvif_fi_init(struct xenvif *vif);
>> +
>> +bool xenvif_should_fail(struct xenvif *vif, xenvif_fi_t type);
>> +
>> +#else
>> +
>> +static inline int xen_netbk_fi_init(void) { return 0; }
>> +static inline void xen_netbk_fi_fini(void) { }
>> +
>> +static inline void xenvif_fi_fini(struct xenvif *vif) { }
>> +static inline int xenvif_fi_init(struct xenvif *vif) { return 0; }
>> +
>> +static inline bool xenvif_should_fail(struct xenvif *vif, xenvif_fi_t type)
>> +{
>> +	return false;
>> +}
>> +
>> +#endif /* CONFIG_XEN_NETDEV_BACKEND_FAULT_INJECTION */
>> +
>> +#endif /* _XEN_NETBACK_FI_H */
>> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
>> index e1aef25..c775ee0 100644
>> --- a/drivers/net/xen-netback/xenbus.c
>> +++ b/drivers/net/xen-netback/xenbus.c
>> @@ -21,6 +21,7 @@
>>   #include "common.h"
>>   #include <linux/vmalloc.h>
>>   #include <linux/rtnetlink.h>
>> +#include "netback_fi.h"
>>   
>>   struct backend_info {
>>   	struct xenbus_device *dev;
>> @@ -502,6 +503,7 @@ static void backend_disconnect(struct backend_info *be)
>>   #ifdef CONFIG_DEBUG_FS
>>   		xenvif_debugfs_delif(vif);
>>   #endif /* CONFIG_DEBUG_FS */
>> +		xenvif_fi_fini(vif);
>>   		xenvif_disconnect_data(vif);
>>   
>>   		/* At this point some of the handlers may still be active
>> @@ -1024,6 +1026,10 @@ static void connect(struct backend_info *be)
>>   		}
>>   	}
>>   
>> +	err = xenvif_fi_init(be->vif);
>> +	if (err)
>> +		goto err;
>> +
>>   #ifdef CONFIG_DEBUG_FS
>>   	xenvif_debugfs_addif(be->vif);
>>   #endif /* CONFIG_DEBUG_FS */
>>
> Without any user of that infrastructure I really can't say whether I
> want this.
>

The code we are using this faults for is not yet sent (we have it in plans).
Probably I'll send it once again after this code using it is sent.
Thanks anyway!

> Juergen
>

Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

Re: [PATCH 3/3] xen blkback: add fault injection facility

[staskins]

On 04/20/18 13:28, Juergen Gross wrote:
> On 20/04/18 12:47, Stanislav Kinsburskii wrote:
>> This patch adds wrapper helpers around generic Xen fault inject
>> facility.
>> The major reason is to keep all the module fault injection directories
>> in a dedicated subdirectory instead of Xen fault inject root.
>>
>> IOW, when using these helpers, per-device and named by device name
>> fault injection control directories will appear under the following
>> directory:
>> - /sys/kernel/debug/xen/fault_inject/xen-blkback/
>> instead of:
>> - /sys/kernel/debug/xen/fault_inject/
>>
>> Signed-off-by: Stanislav Kinsburskii <staskins@amazon.com>
>> CC: Jens Axboe <axboe@kernel.dk>
>> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> CC: "Roger Pau Monné" <roger.pau@citrix.com>
>> CC: linux-kernel@vger.kernel.org
>> CC: linux-block@vger.kernel.org
>> CC: xen-devel@lists.xenproject.org
>> CC: Stanislav Kinsburskii <staskins@amazon.com>
>> CC: David Woodhouse <dwmw@amazon.co.uk>
> This is an exact copy of the netback patch apart from the names.
>
> I don't like adding multiple copies of the same coding to the tree.

Sure, will dedupplicate this.

>
> Juergen
>

Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

Re: [PATCH 2/3] xen netback: add fault injection facility

[Juergen Gross]

On 20/04/18 14:52, staskins@amazon.com wrote:
> On 04/20/18 13:25, Juergen Gross wrote:
>> On 20/04/18 12:47, Stanislav Kinsburskii wrote:
>>> +    for (fi = 0; fi < XENVIF_FI_MAX; fi++) {
>>> +        vfi->faults[fi] = xen_fi_dir_add(vfi->dir,
>>> +                xenvif_fi_names[fi]);
>> How does this work? xenvif_fi_names[] is an empty array and this is the
>> only reference to it. Who is allocating the memory for that array?
> 
> Well, it works in the way one adds a var to enum (which is used as a key
> later) and a corresponding string into the array (which is used as a
> name for the fault directory in sysfs).

Then you should size the array via XENVIF_FI_MAX.


Juergen

Re: [PATCH 2/3] xen netback: add fault injection facility

[staskins]

On 04/20/18 15:00, Juergen Gross wrote:
> On 20/04/18 14:52, staskins@amazon.com wrote:
>> On 04/20/18 13:25, Juergen Gross wrote:
>>> On 20/04/18 12:47, Stanislav Kinsburskii wrote:
>>>> +    for (fi = 0; fi < XENVIF_FI_MAX; fi++) {
>>>> +        vfi->faults[fi] = xen_fi_dir_add(vfi->dir,
>>>> +                xenvif_fi_names[fi]);
>>> How does this work? xenvif_fi_names[] is an empty array and this is the
>>> only reference to it. Who is allocating the memory for that array?
>> Well, it works in the way one adds a var to enum (which is used as a key
>> later) and a corresponding string into the array (which is used as a
>> name for the fault directory in sysfs).
> Then you should size the array via XENVIF_FI_MAX.

Makes sense.
Thanks!

Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

Re: [PATCH 3/3] xen blkback: add fault injection facility

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2017 bytes --]

Hi Stanislav,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]
[also build test ERROR on v4.17-rc1 next-20180420]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Stanislav-Kinsburskii/Introduce-Xen-fault-injection-facility/20180422-201946
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/block//xen-blkback/blkback_fi.c: In function 'xen_blkif_fi_init':
>> drivers/block//xen-blkback/blkback_fi.c:87:51: error: dereferencing pointer to incomplete type 'struct backend_info'
     bfi->dir = debugfs_create_dir(dev_name(&blkif->be->dev->dev),
                                                      ^~

vim +87 drivers/block//xen-blkback/blkback_fi.c

    77	
    78	int xen_blkif_fi_init(struct xen_blkif *blkif)
    79	{
    80		struct xen_blkif_fi *bfi;
    81		int fi, err = -ENOMEM;
    82	
    83		bfi = kmalloc(sizeof(*bfi), GFP_KERNEL);
    84		if (!bfi)
    85			return -ENOMEM;
    86	
  > 87		bfi->dir = debugfs_create_dir(dev_name(&blkif->be->dev->dev),
    88					      blkif_fi_dir);
    89		if (!bfi->dir)
    90			goto err_dir;
    91	
    92		for (fi = 0; fi < XENBLKIF_FI_MAX; fi++) {
    93			bfi->faults[fi] = xen_fi_dir_add(bfi->dir,
    94							 xen_blkif_fi_names[fi]);
    95			if (!bfi->faults[fi])
    96				goto err_fault;
    97		}
    98	
    99		blkif->fi_info = bfi;
   100		return 0;
   101	
   102	err_fault:
   103		for (; fi > 0; fi--)
   104			xen_fi_del(bfi->faults[fi]);
   105		debugfs_remove_recursive(bfi->dir);
   106	err_dir:
   107		kfree(bfi);
   108		return err;
   109	}
   110	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 63017 bytes --]

Re: [PATCH 2/3] xen netback: add fault injection facility

[Wei Liu]

On Fri, Apr 20, 2018 at 10:47:31AM +0000, Stanislav Kinsburskii wrote:
>  
>  #include <linux/kthread.h>
>  #include <linux/if_vlan.h>
> @@ -1649,6 +1650,7 @@ static int __init netback_init(void)
>  			PTR_ERR(xen_netback_dbg_root));
>  #endif /* CONFIG_DEBUG_FS */
>  
> +	(void) xen_netbk_fi_init();

If you care about the return value, please propagate it to
netback_init's caller. Otherwise you can just make the function return
void.

> +
> +int xenvif_fi_init(struct xenvif *vif)
> +{
> +	struct dentry *parent;
> +	struct xenvif_fi *vfi;
> +	int fi, err = -ENOMEM;
> +
> +	parent = vif_fi_dir;
> +	if (!parent)
> +		return -ENOMEM;
> +
> +	vfi = kmalloc(sizeof(*vfi), GFP_KERNEL);
> +	if (!vfi)
> +		return -ENOMEM;
> +
> +	vfi->dir = debugfs_create_dir(vif->dev->name, parent);
> +	if (!vfi->dir)
> +		goto err_dir;
> +
> +	for (fi = 0; fi < XENVIF_FI_MAX; fi++) {
> +		vfi->faults[fi] = xen_fi_dir_add(vfi->dir,
> +				xenvif_fi_names[fi]);
> +		if (!vfi->faults[fi])
> +			goto err_fault;
> +	}
> +
> +	vif->fi_info = vfi;
> +	return 0;
> +
> +err_fault:
> +	for (; fi > 0; fi--)

fi >= 0

Wei.