* [PATCH v2 0/2] of: unittest: add overlay gpio test to catch gpio hog problem
From: frowand.list @ 2020-02-20 18:40 UTC (permalink / raw)
To: Rob Herring, Geert Uytterhoeven, pantelis.antoniou
Cc: devicetree, linux-kernel, Alan Tull
From: Frank Rowand <frank.rowand@sony.com>
Geert reports that gpio hog nodes are not properly processed when
the gpio hog node is added via an overlay reply and provides an
patch to fix the problem [1].
Add a unittest that shows the problem. Unittest will report "1 failed"
test before applying Geert's patch and "0 failed" after applying
Geert's patch.
I did not have a development system for which it would be easy to
experiment with applying an overlay containing a gpio hog, so I
instead created this unittest that uses a fake gpio node.
Some tests in the devicetree unittests result in printk messages
from the code being tested. It can be difficult to determine
whether the messages are the result of unittest or are potentially
reporting bugs that should be fixed. The most recent example of
a person asking whether to be concerned about these messages is [2].
Patch 2 adds annotations for all messages triggered by unittests,
except KERN_DEBUG messages. (KERN_DEBUG is a special case due to the
possible interaction of CONFIG_DYNAMIC_DEBUG.)
The annotations added in patch 2/2 add a small amount of verbosity
to the console output. I have created a proof of concept tool to
explore (1) how test harnesses could use the annotations and
(2) how to make the resulting console output easier to read and
understand as a human being. The tool 'of_unittest_expect' is
available at https://github.com/frowand/dt_tools
The format of the annotations is expected to change when unittests
are converted to use the kunit infrastructure when the broader
testing community has an opportunity to discuss the implementation
of annotations of test triggered messages.
[1] https://lore.kernel.org/r/20200220130149.26283-1-geert+renesas@glider.be
[2] https://lore.kernel.org/r/6021ac63-b5e0-ed3d-f964-7c6ef579cd68@huawei.com
Frank Rowand (2):
of: unittest: add overlay gpio test to catch gpio hog problem
of: unittest: annotate warnings triggered by unittest
drivers/of/unittest-data/Makefile | 8 +-
drivers/of/unittest-data/overlay_gpio_01.dts | 23 +
drivers/of/unittest-data/overlay_gpio_02a.dts | 16 +
drivers/of/unittest-data/overlay_gpio_02b.dts | 16 +
drivers/of/unittest-data/overlay_gpio_03.dts | 23 +
drivers/of/unittest-data/overlay_gpio_04a.dts | 16 +
drivers/of/unittest-data/overlay_gpio_04b.dts | 16 +
drivers/of/unittest.c | 628 ++++++++++++++++++++++++--
8 files changed, 715 insertions(+), 31 deletions(-)
create mode 100644 drivers/of/unittest-data/overlay_gpio_01.dts
create mode 100644 drivers/of/unittest-data/overlay_gpio_02a.dts
create mode 100644 drivers/of/unittest-data/overlay_gpio_02b.dts
create mode 100644 drivers/of/unittest-data/overlay_gpio_03.dts
create mode 100644 drivers/of/unittest-data/overlay_gpio_04a.dts
create mode 100644 drivers/of/unittest-data/overlay_gpio_04b.dts
--
Frank Rowand <frank.rowand@sony.com>
^ permalink raw reply
* [PATCH v2 1/2] of: unittest: add overlay gpio test to catch gpio hog problem
From: frowand.list @ 2020-02-20 18:40 UTC (permalink / raw)
To: Rob Herring, Geert Uytterhoeven, pantelis.antoniou
Cc: devicetree, linux-kernel, Alan Tull
In-Reply-To: <1582224021-12827-1-git-send-email-frowand.list@gmail.com>
From: Frank Rowand <frank.rowand@sony.com>
Geert reports that gpio hog nodes are not properly processed when
the gpio hog node is added via an overlay reply and provides an
RFC patch to fix the problem [1].
Add a unittest that shows the problem. Unittest will report "1 failed"
test before applying Geert's RFC patch and "0 failed" after applying
Geert's RFC patch.
[1] https://lore.kernel.org/linux-devicetree/20191230133852.5890-1-geert+renesas@glider.be/
Signed-off-by: Frank Rowand <frank.rowand@sony.com>
---
changes since v1:
- base on 5.6-rc1
- fixed node names in overlays
- removed unused fields from struct unittest_gpio_dev
- of_unittest_overlay_gpio() cleaned up comments
- of_unittest_overlay_gpio() moved saving global values into
probe_pass_count and chip_request_count more tightly around
test code expected to trigger changes in the global values
v1 of this patch incorrectly reported that it had made changes
since the RFC version, but it was mistakenly created from the
wrong branch.
There are checkpatch warnings.
- New files are in a directory already covered by MAINTAINERS
- The undocumented compatibles are restricted to use by unittest
and should not be documented under Documentation
- The printk() KERN_<LEVEL> warnings are false positives. The level
is supplied by a define parameter instead of a hard coded constant
- The lines over 80 characters are consistent with unittest.c style
This unittest was also valuable in that it allowed me to explore
possible issues related to the proposed solution to the gpio hog
problem.
drivers/of/unittest-data/Makefile | 8 +-
drivers/of/unittest-data/overlay_gpio_01.dts | 23 +++
drivers/of/unittest-data/overlay_gpio_02a.dts | 16 ++
drivers/of/unittest-data/overlay_gpio_02b.dts | 16 ++
drivers/of/unittest-data/overlay_gpio_03.dts | 23 +++
drivers/of/unittest-data/overlay_gpio_04a.dts | 16 ++
drivers/of/unittest-data/overlay_gpio_04b.dts | 16 ++
drivers/of/unittest.c | 253 ++++++++++++++++++++++++++
8 files changed, 370 insertions(+), 1 deletion(-)
create mode 100644 drivers/of/unittest-data/overlay_gpio_01.dts
create mode 100644 drivers/of/unittest-data/overlay_gpio_02a.dts
create mode 100644 drivers/of/unittest-data/overlay_gpio_02b.dts
create mode 100644 drivers/of/unittest-data/overlay_gpio_03.dts
create mode 100644 drivers/of/unittest-data/overlay_gpio_04a.dts
create mode 100644 drivers/of/unittest-data/overlay_gpio_04b.dts
diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
index 9b6807065827..009f4045c8e4 100644
--- a/drivers/of/unittest-data/Makefile
+++ b/drivers/of/unittest-data/Makefile
@@ -21,7 +21,13 @@ obj-$(CONFIG_OF_OVERLAY) += overlay.dtb.o \
overlay_bad_add_dup_prop.dtb.o \
overlay_bad_phandle.dtb.o \
overlay_bad_symbol.dtb.o \
- overlay_base.dtb.o
+ overlay_base.dtb.o \
+ overlay_gpio_01.dtb.o \
+ overlay_gpio_02a.dtb.o \
+ overlay_gpio_02b.dtb.o \
+ overlay_gpio_03.dtb.o \
+ overlay_gpio_04a.dtb.o \
+ overlay_gpio_04b.dtb.o
# enable creation of __symbols__ node
DTC_FLAGS_overlay += -@
diff --git a/drivers/of/unittest-data/overlay_gpio_01.dts b/drivers/of/unittest-data/overlay_gpio_01.dts
new file mode 100644
index 000000000000..699ff104ae10
--- /dev/null
+++ b/drivers/of/unittest-data/overlay_gpio_01.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+&unittest_test_bus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ gpio@0 {
+ compatible = "unittest-gpio";
+ reg = <0>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ ngpios = <2>;
+ gpio-line-names = "line-A", "line-B";
+
+ line-b {
+ gpio-hog;
+ gpios = <2 0>;
+ input;
+ line-name = "line-B-input";
+ };
+ };
+};
diff --git a/drivers/of/unittest-data/overlay_gpio_02a.dts b/drivers/of/unittest-data/overlay_gpio_02a.dts
new file mode 100644
index 000000000000..ec59aff6ed47
--- /dev/null
+++ b/drivers/of/unittest-data/overlay_gpio_02a.dts
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+&unittest_test_bus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ gpio@2 {
+ compatible = "unittest-gpio";
+ reg = <2>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ ngpios = <2>;
+ gpio-line-names = "line-A", "line-B";
+ };
+};
diff --git a/drivers/of/unittest-data/overlay_gpio_02b.dts b/drivers/of/unittest-data/overlay_gpio_02b.dts
new file mode 100644
index 000000000000..43ce111d41ce
--- /dev/null
+++ b/drivers/of/unittest-data/overlay_gpio_02b.dts
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+&unittest_test_bus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ gpio@2 {
+ line-a {
+ gpio-hog;
+ gpios = <1 0>;
+ input;
+ line-name = "line-A-input";
+ };
+ };
+};
diff --git a/drivers/of/unittest-data/overlay_gpio_03.dts b/drivers/of/unittest-data/overlay_gpio_03.dts
new file mode 100644
index 000000000000..6e0312340a1b
--- /dev/null
+++ b/drivers/of/unittest-data/overlay_gpio_03.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+&unittest_test_bus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ gpio@3 {
+ compatible = "unittest-gpio";
+ reg = <3>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ ngpios = <2>;
+ gpio-line-names = "line-A", "line-B", "line-C", "line-D";
+
+ line-d {
+ gpio-hog;
+ gpios = <4 0>;
+ input;
+ line-name = "line-D-input";
+ };
+ };
+};
diff --git a/drivers/of/unittest-data/overlay_gpio_04a.dts b/drivers/of/unittest-data/overlay_gpio_04a.dts
new file mode 100644
index 000000000000..7b1e04ebfa7a
--- /dev/null
+++ b/drivers/of/unittest-data/overlay_gpio_04a.dts
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+&unittest_test_bus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ gpio@4 {
+ compatible = "unittest-gpio";
+ reg = <4>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ ngpios = <2>;
+ gpio-line-names = "line-A", "line-B", "line-C", "line-D";
+ };
+};
diff --git a/drivers/of/unittest-data/overlay_gpio_04b.dts b/drivers/of/unittest-data/overlay_gpio_04b.dts
new file mode 100644
index 000000000000..a14e95c6699a
--- /dev/null
+++ b/drivers/of/unittest-data/overlay_gpio_04b.dts
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+&unittest_test_bus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ gpio@4 {
+ line-c {
+ gpio-hog;
+ gpios = <3 0>;
+ input;
+ line-name = "line-C-input";
+ };
+ };
+};
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 68b87587b2ef..6059bb363097 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -24,6 +24,7 @@
#include <linux/i2c.h>
#include <linux/i2c-mux.h>
+#include <linux/gpio/driver.h>
#include <linux/bitops.h>
@@ -46,6 +47,97 @@
failed; \
})
+/*
+ * Expected message may have a message level other than KERN_INFO.
+ * Print the expected message only if the current loglevel will allow
+ * the actual message to print.
+ */
+#define EXPECT_BEGIN(level, fmt, ...) \
+ printk(level pr_fmt("EXPECT \\ : ") fmt, ##__VA_ARGS__)
+
+#define EXPECT_END(level, fmt, ...) \
+ printk(level pr_fmt("EXPECT / : ") fmt, ##__VA_ARGS__)
+
+struct unittest_gpio_dev {
+ struct gpio_chip chip;
+};
+
+static int unittest_gpio_chip_request_count;
+static int unittest_gpio_probe_count;
+static int unittest_gpio_probe_pass_count;
+
+static int unittest_gpio_chip_request(struct gpio_chip *chip, unsigned int offset)
+{
+ unittest_gpio_chip_request_count++;
+
+ pr_debug("%s(): %s %d %d\n", __func__, chip->label, offset,
+ unittest_gpio_chip_request_count);
+ return 0;
+}
+
+static int unittest_gpio_probe(struct platform_device *pdev)
+{
+ struct unittest_gpio_dev *devptr;
+ int ret;
+
+ unittest_gpio_probe_count++;
+
+ devptr = kzalloc(sizeof(*devptr), GFP_KERNEL);
+ if (!devptr)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, devptr);
+
+ devptr->chip.of_node = pdev->dev.of_node;
+ devptr->chip.label = "of-unittest-gpio";
+ devptr->chip.base = -1; /* dynamic allocation */
+ devptr->chip.ngpio = 5;
+ devptr->chip.request = unittest_gpio_chip_request;
+
+ ret = gpiochip_add_data(&devptr->chip, NULL);
+
+ unittest(!ret,
+ "gpiochip_add_data() for node @%pOF failed, ret = %d\n", devptr->chip.of_node, ret);
+
+ if (!ret)
+ unittest_gpio_probe_pass_count++;
+ return ret;
+}
+
+static int unittest_gpio_remove(struct platform_device *pdev)
+{
+ struct unittest_gpio_dev *gdev = platform_get_drvdata(pdev);
+ struct device *dev = &pdev->dev;
+ struct device_node *np = pdev->dev.of_node;
+
+ dev_dbg(dev, "%s for node @%pOF\n", __func__, np);
+
+ if (!gdev)
+ return -EINVAL;
+
+ if (gdev->chip.base != -1)
+ gpiochip_remove(&gdev->chip);
+
+ platform_set_drvdata(pdev, NULL);
+ kfree(pdev);
+
+ return 0;
+}
+
+static const struct of_device_id unittest_gpio_id[] = {
+ { .compatible = "unittest-gpio", },
+ {}
+};
+
+static struct platform_driver unittest_gpio_driver = {
+ .probe = unittest_gpio_probe,
+ .remove = unittest_gpio_remove,
+ .driver = {
+ .name = "unittest-gpio",
+ .of_match_table = of_match_ptr(unittest_gpio_id),
+ },
+};
+
static void __init of_unittest_find_node_by_name(void)
{
struct device_node *np;
@@ -2183,6 +2275,153 @@ static inline void of_unittest_overlay_i2c_15(void) { }
#endif
+static void __init of_unittest_overlay_gpio(void)
+{
+ int chip_request_count;
+ int probe_pass_count;
+ int ret;
+
+ /*
+ * tests: apply overlays before registering driver
+ * Similar to installing a driver as a module, the
+ * driver is registered after applying the overlays.
+ *
+ * - apply overlay_gpio_01
+ * - apply overlay_gpio_02a
+ * - apply overlay_gpio_02b
+ * - register driver
+ *
+ * register driver will result in
+ * - probe and processing gpio hog for overlay_gpio_01
+ * - probe for overlay_gpio_02a
+ * - processing gpio for overlay_gpio_02b
+ */
+
+ probe_pass_count = unittest_gpio_probe_pass_count;
+ chip_request_count = unittest_gpio_chip_request_count;
+
+ /*
+ * overlay_gpio_01 contains gpio node and child gpio hog node
+ * overlay_gpio_02a contains gpio node
+ * overlay_gpio_02b contains child gpio hog node
+ */
+
+ unittest(overlay_data_apply("overlay_gpio_01", NULL),
+ "Adding overlay 'overlay_gpio_01' failed\n");
+
+ unittest(overlay_data_apply("overlay_gpio_02a", NULL),
+ "Adding overlay 'overlay_gpio_02a' failed\n");
+
+ unittest(overlay_data_apply("overlay_gpio_02b", NULL),
+ "Adding overlay 'overlay_gpio_02b' failed\n");
+
+ /*
+ * messages are the result of the probes, after the
+ * driver is registered
+ */
+
+ EXPECT_BEGIN(KERN_INFO,
+ "GPIO line <<int>> (line-B-input) hogged as input\n");
+
+ EXPECT_BEGIN(KERN_INFO,
+ "GPIO line <<int>> (line-A-input) hogged as input\n");
+
+ ret = platform_driver_register(&unittest_gpio_driver);
+ if (unittest(ret == 0, "could not register unittest gpio driver\n"))
+ return;
+
+ EXPECT_END(KERN_INFO,
+ "GPIO line <<int>> (line-A-input) hogged as input\n");
+ EXPECT_END(KERN_INFO,
+ "GPIO line <<int>> (line-B-input) hogged as input\n");
+
+ unittest(probe_pass_count + 2 == unittest_gpio_probe_pass_count,
+ "unittest_gpio_probe() failed or not called\n");
+
+ unittest(chip_request_count + 2 == unittest_gpio_chip_request_count,
+ "unittest_gpio_chip_request() called %d times (expected 1 time)\n",
+ unittest_gpio_chip_request_count - chip_request_count);
+
+ /*
+ * tests: apply overlays after registering driver
+ *
+ * Similar to a driver built-in to the kernel, the
+ * driver is registered before applying the overlays.
+ *
+ * overlay_gpio_03 contains gpio node and child gpio hog node
+ *
+ * - apply overlay_gpio_03
+ *
+ * apply overlay will result in
+ * - probe and processing gpio hog.
+ */
+
+ probe_pass_count = unittest_gpio_probe_pass_count;
+ chip_request_count = unittest_gpio_chip_request_count;
+
+ EXPECT_BEGIN(KERN_INFO,
+ "GPIO line <<int>> (line-D-input) hogged as input\n");
+
+ /* overlay_gpio_03 contains gpio node and child gpio hog node */
+
+ unittest(overlay_data_apply("overlay_gpio_03", NULL),
+ "Adding overlay 'overlay_gpio_03' failed\n");
+
+ EXPECT_END(KERN_INFO,
+ "GPIO line <<int>> (line-D-input) hogged as input\n");
+
+ unittest(probe_pass_count + 1 == unittest_gpio_probe_pass_count,
+ "unittest_gpio_probe() failed or not called\n");
+
+ unittest(chip_request_count + 1 == unittest_gpio_chip_request_count,
+ "unittest_gpio_chip_request() called %d times (expected 1 time)\n",
+ unittest_gpio_chip_request_count - chip_request_count);
+
+ /*
+ * overlay_gpio_04a contains gpio node
+ *
+ * - apply overlay_gpio_04a
+ *
+ * apply the overlay will result in
+ * - probe for overlay_gpio_04a
+ */
+
+ probe_pass_count = unittest_gpio_probe_pass_count;
+ chip_request_count = unittest_gpio_chip_request_count;
+
+ /* overlay_gpio_04a contains gpio node */
+
+ unittest(overlay_data_apply("overlay_gpio_04a", NULL),
+ "Adding overlay 'overlay_gpio_04a' failed\n");
+
+ unittest(probe_pass_count + 1 == unittest_gpio_probe_pass_count,
+ "unittest_gpio_probe() failed or not called\n");
+
+ /*
+ * overlay_gpio_04b contains child gpio hog node
+ *
+ * - apply overlay_gpio_04b
+ *
+ * apply the overlay will result in
+ * - processing gpio for overlay_gpio_04b
+ */
+
+ EXPECT_BEGIN(KERN_INFO,
+ "GPIO line <<int>> (line-C-input) hogged as input\n");
+
+ /* overlay_gpio_04b contains child gpio hog node */
+
+ unittest(overlay_data_apply("overlay_gpio_04b", NULL),
+ "Adding overlay 'overlay_gpio_04b' failed\n");
+
+ EXPECT_END(KERN_INFO,
+ "GPIO line <<int>> (line-C-input) hogged as input\n");
+
+ unittest(chip_request_count + 1 == unittest_gpio_chip_request_count,
+ "unittest_gpio_chip_request() called %d times (expected 1 time)\n",
+ unittest_gpio_chip_request_count - chip_request_count);
+}
+
static void __init of_unittest_overlay(void)
{
struct device_node *bus_np = NULL;
@@ -2242,6 +2481,8 @@ static void __init of_unittest_overlay(void)
of_unittest_overlay_i2c_cleanup();
#endif
+ of_unittest_overlay_gpio();
+
of_unittest_destroy_tracked_overlays();
out:
@@ -2295,6 +2536,12 @@ struct overlay_info {
OVERLAY_INFO_EXTERN(overlay_12);
OVERLAY_INFO_EXTERN(overlay_13);
OVERLAY_INFO_EXTERN(overlay_15);
+OVERLAY_INFO_EXTERN(overlay_gpio_01);
+OVERLAY_INFO_EXTERN(overlay_gpio_02a);
+OVERLAY_INFO_EXTERN(overlay_gpio_02b);
+OVERLAY_INFO_EXTERN(overlay_gpio_03);
+OVERLAY_INFO_EXTERN(overlay_gpio_04a);
+OVERLAY_INFO_EXTERN(overlay_gpio_04b);
OVERLAY_INFO_EXTERN(overlay_bad_add_dup_node);
OVERLAY_INFO_EXTERN(overlay_bad_add_dup_prop);
OVERLAY_INFO_EXTERN(overlay_bad_phandle);
@@ -2319,6 +2566,12 @@ struct overlay_info {
OVERLAY_INFO(overlay_12, 0),
OVERLAY_INFO(overlay_13, 0),
OVERLAY_INFO(overlay_15, 0),
+ OVERLAY_INFO(overlay_gpio_01, 0),
+ OVERLAY_INFO(overlay_gpio_02a, 0),
+ OVERLAY_INFO(overlay_gpio_02b, 0),
+ OVERLAY_INFO(overlay_gpio_03, 0),
+ OVERLAY_INFO(overlay_gpio_04a, 0),
+ OVERLAY_INFO(overlay_gpio_04b, 0),
OVERLAY_INFO(overlay_bad_add_dup_node, -EINVAL),
OVERLAY_INFO(overlay_bad_add_dup_prop, -EINVAL),
OVERLAY_INFO(overlay_bad_phandle, -EINVAL),
--
Frank Rowand <frank.rowand@sony.com>
^ permalink raw reply related
* [PATCH v2 2/2] of: unittest: annotate warnings triggered by unittest
From: frowand.list @ 2020-02-20 18:40 UTC (permalink / raw)
To: Rob Herring, Geert Uytterhoeven, pantelis.antoniou
Cc: devicetree, linux-kernel, Alan Tull
In-Reply-To: <1582224021-12827-1-git-send-email-frowand.list@gmail.com>
From: Frank Rowand <frank.rowand@sony.com>
Some tests in the devicetree unittests result in printk messages
from the code being tested. It can be difficult to determine
whether the messages are the result of unittest or are potentially
reporting bugs that should be fixed. The most recent example of
a person asking whether to be concerned about these messages is [1].
Add annotations for all messages triggered by unittests, except
KERN_DEBUG messages. (KERN_DEBUG is a special case due to the
possible interaction of CONFIG_DYNAMIC_DEBUG.)
The format of the annotations is expected to change when unittests
are converted to use the kunit infrastructure when the broader
testing community has an opportunity to discuss the implementation
of annotations of test triggered messages.
[1] https://lore.kernel.org/r/6021ac63-b5e0-ed3d-f964-7c6ef579cd68@huawei.com
Signed-off-by: Frank Rowand <frank.rowand@sony.com>
---
changes since v1:
- none
drivers/of/unittest.c | 375 ++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 345 insertions(+), 30 deletions(-)
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 6059bb363097..96ae8a762a9e 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -51,6 +51,9 @@
* Expected message may have a message level other than KERN_INFO.
* Print the expected message only if the current loglevel will allow
* the actual message to print.
+ *
+ * Do not use EXPECT_BEGIN() or EXPECT_END() for messages generated by
+ * pr_debug().
*/
#define EXPECT_BEGIN(level, fmt, ...) \
printk(level pr_fmt("EXPECT \\ : ") fmt, ##__VA_ARGS__)
@@ -536,29 +539,77 @@ static void __init of_unittest_parse_phandle_with_args(void)
/* Check for missing cells property */
memset(&args, 0, sizeof(args));
+
+ EXPECT_BEGIN(KERN_INFO,
+ "OF: /testcase-data/phandle-tests/consumer-a: could not get #phandle-cells-missing for /testcase-data/phandle-tests/provider1");
+
rc = of_parse_phandle_with_args(np, "phandle-list",
"#phandle-cells-missing", 0, &args);
+
+ EXPECT_END(KERN_INFO,
+ "OF: /testcase-data/phandle-tests/consumer-a: could not get #phandle-cells-missing for /testcase-data/phandle-tests/provider1");
+
unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
+
+ EXPECT_BEGIN(KERN_INFO,
+ "OF: /testcase-data/phandle-tests/consumer-a: could not get #phandle-cells-missing for /testcase-data/phandle-tests/provider1");
+
rc = of_count_phandle_with_args(np, "phandle-list",
"#phandle-cells-missing");
+
+ EXPECT_END(KERN_INFO,
+ "OF: /testcase-data/phandle-tests/consumer-a: could not get #phandle-cells-missing for /testcase-data/phandle-tests/provider1");
+
unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
/* Check for bad phandle in list */
memset(&args, 0, sizeof(args));
+
+ EXPECT_BEGIN(KERN_INFO,
+ "OF: /testcase-data/phandle-tests/consumer-a: could not find phandle");
+
rc = of_parse_phandle_with_args(np, "phandle-list-bad-phandle",
"#phandle-cells", 0, &args);
+
+ EXPECT_END(KERN_INFO,
+ "OF: /testcase-data/phandle-tests/consumer-a: could not find phandle");
+
unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
+
+ EXPECT_BEGIN(KERN_INFO,
+ "OF: /testcase-data/phandle-tests/consumer-a: could not find phandle");
+
rc = of_count_phandle_with_args(np, "phandle-list-bad-phandle",
"#phandle-cells");
+
+ EXPECT_END(KERN_INFO,
+ "OF: /testcase-data/phandle-tests/consumer-a: could not find phandle");
+
unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
/* Check for incorrectly formed argument list */
memset(&args, 0, sizeof(args));
+
+ EXPECT_BEGIN(KERN_INFO,
+ "OF: /testcase-data/phandle-tests/consumer-a: #phandle-cells = 3 found -1");
+
rc = of_parse_phandle_with_args(np, "phandle-list-bad-args",
"#phandle-cells", 1, &args);
+
+ EXPECT_END(KERN_INFO,
+ "OF: /testcase-data/phandle-tests/consumer-a: #phandle-cells = 3 found -1");
+
unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
+
+ EXPECT_BEGIN(KERN_INFO,
+ "OF: /testcase-data/phandle-tests/consumer-a: #phandle-cells = 3 found -1");
+
rc = of_count_phandle_with_args(np, "phandle-list-bad-args",
"#phandle-cells");
+
+ EXPECT_END(KERN_INFO,
+ "OF: /testcase-data/phandle-tests/consumer-a: #phandle-cells = 3 found -1");
+
unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
}
@@ -669,20 +720,41 @@ static void __init of_unittest_parse_phandle_with_args_map(void)
/* Check for missing cells,map,mask property */
memset(&args, 0, sizeof(args));
+
+ EXPECT_BEGIN(KERN_INFO,
+ "OF: /testcase-data/phandle-tests/consumer-b: could not get #phandle-missing-cells for /testcase-data/phandle-tests/provider1");
+
rc = of_parse_phandle_with_args_map(np, "phandle-list",
"phandle-missing", 0, &args);
+ EXPECT_END(KERN_INFO,
+ "OF: /testcase-data/phandle-tests/consumer-b: could not get #phandle-missing-cells for /testcase-data/phandle-tests/provider1");
+
unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
/* Check for bad phandle in list */
memset(&args, 0, sizeof(args));
+
+ EXPECT_BEGIN(KERN_INFO,
+ "OF: /testcase-data/phandle-tests/consumer-b: could not find phandle");
+
rc = of_parse_phandle_with_args_map(np, "phandle-list-bad-phandle",
"phandle", 0, &args);
+ EXPECT_END(KERN_INFO,
+ "OF: /testcase-data/phandle-tests/consumer-b: could not find phandle");
+
unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
/* Check for incorrectly formed argument list */
memset(&args, 0, sizeof(args));
+
+ EXPECT_BEGIN(KERN_INFO,
+ "OF: /testcase-data/phandle-tests/consumer-b: #phandle-cells = 2 found -1");
+
rc = of_parse_phandle_with_args_map(np, "phandle-list-bad-args",
"phandle", 1, &args);
+ EXPECT_END(KERN_INFO,
+ "OF: /testcase-data/phandle-tests/consumer-b: #phandle-cells = 2 found -1");
+
unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
}
@@ -1213,7 +1285,15 @@ static void __init of_unittest_platform_populate(void)
np = of_find_node_by_path("/testcase-data/testcase-device2");
pdev = of_find_device_by_node(np);
unittest(pdev, "device 2 creation failed\n");
+
+ EXPECT_BEGIN(KERN_INFO,
+ "platform testcase-data:testcase-device2: IRQ index 0 not found");
+
irq = platform_get_irq(pdev, 0);
+
+ EXPECT_END(KERN_INFO,
+ "platform testcase-data:testcase-device2: IRQ index 0 not found");
+
unittest(irq < 0 && irq != -EPROBE_DEFER,
"device parsing error failed - %d\n", irq);
}
@@ -1417,6 +1497,9 @@ static int __init unittest_data_add(void)
return 0;
}
+ EXPECT_BEGIN(KERN_INFO,
+ "Duplicate name in testcase-data, renamed to \"duplicate-name#1\"");
+
/* attach the sub-tree to live tree */
np = unittest_data_node->child;
while (np) {
@@ -1427,6 +1510,9 @@ static int __init unittest_data_add(void)
np = next;
}
+ EXPECT_END(KERN_INFO,
+ "Duplicate name in testcase-data, renamed to \"duplicate-name#1\"");
+
of_overlay_mutex_unlock();
return 0;
@@ -1769,8 +1855,18 @@ static int __init of_unittest_apply_revert_overlay_check(int overlay_nr,
/* test activation of device */
static void __init of_unittest_overlay_0(void)
{
+ int ret;
+
+ EXPECT_BEGIN(KERN_INFO,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data/overlay-node/test-bus/test-unittest0/status");
+
/* device should enable */
- if (of_unittest_apply_overlay_check(0, 0, 0, 1, PDEV_OVERLAY))
+ ret = of_unittest_apply_overlay_check(0, 0, 0, 1, PDEV_OVERLAY);
+
+ EXPECT_END(KERN_INFO,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data/overlay-node/test-bus/test-unittest0/status");
+
+ if (ret)
return;
unittest(1, "overlay test %d passed\n", 0);
@@ -1779,28 +1875,58 @@ static void __init of_unittest_overlay_0(void)
/* test deactivation of device */
static void __init of_unittest_overlay_1(void)
{
+ int ret;
+
+ EXPECT_BEGIN(KERN_INFO,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data/overlay-node/test-bus/test-unittest1/status");
+
/* device should disable */
- if (of_unittest_apply_overlay_check(1, 1, 1, 0, PDEV_OVERLAY))
+ ret = of_unittest_apply_overlay_check(1, 1, 1, 0, PDEV_OVERLAY);
+
+ EXPECT_END(KERN_INFO,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data/overlay-node/test-bus/test-unittest1/status");
+
+ if (ret)
return;
unittest(1, "overlay test %d passed\n", 1);
+
}
/* test activation of device */
static void __init of_unittest_overlay_2(void)
{
+ int ret;
+
+ EXPECT_BEGIN(KERN_INFO,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data/overlay-node/test-bus/test-unittest2/status");
+
/* device should enable */
- if (of_unittest_apply_overlay_check(2, 2, 0, 1, PDEV_OVERLAY))
- return;
+ ret = of_unittest_apply_overlay_check(2, 2, 0, 1, PDEV_OVERLAY);
+
+ EXPECT_END(KERN_INFO,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data/overlay-node/test-bus/test-unittest2/status");
+ if (ret)
+ return;
unittest(1, "overlay test %d passed\n", 2);
}
/* test deactivation of device */
static void __init of_unittest_overlay_3(void)
{
+ int ret;
+
+ EXPECT_BEGIN(KERN_INFO,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data/overlay-node/test-bus/test-unittest3/status");
+
/* device should disable */
- if (of_unittest_apply_overlay_check(3, 3, 1, 0, PDEV_OVERLAY))
+ ret = of_unittest_apply_overlay_check(3, 3, 1, 0, PDEV_OVERLAY);
+
+ EXPECT_END(KERN_INFO,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data/overlay-node/test-bus/test-unittest3/status");
+
+ if (ret)
return;
unittest(1, "overlay test %d passed\n", 3);
@@ -1819,8 +1945,18 @@ static void __init of_unittest_overlay_4(void)
/* test overlay apply/revert sequence */
static void __init of_unittest_overlay_5(void)
{
+ int ret;
+
+ EXPECT_BEGIN(KERN_INFO,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data/overlay-node/test-bus/test-unittest5/status");
+
/* device should disable */
- if (of_unittest_apply_revert_overlay_check(5, 5, 0, 1, PDEV_OVERLAY))
+ ret = of_unittest_apply_revert_overlay_check(5, 5, 0, 1, PDEV_OVERLAY);
+
+ EXPECT_END(KERN_INFO,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data/overlay-node/test-bus/test-unittest5/status");
+
+ if (ret)
return;
unittest(1, "overlay test %d passed\n", 5);
@@ -1834,6 +1970,8 @@ static void __init of_unittest_overlay_6(void)
int before = 0, after = 1;
const char *overlay_name;
+ int ret;
+
/* unittest device must be in before state */
for (i = 0; i < 2; i++) {
if (of_unittest_device_exists(unittest_nr + i, PDEV_OVERLAY)
@@ -1848,18 +1986,41 @@ static void __init of_unittest_overlay_6(void)
}
/* apply the overlays */
- for (i = 0; i < 2; i++) {
- overlay_name = overlay_name_from_nr(overlay_nr + i);
+ EXPECT_BEGIN(KERN_INFO,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data/overlay-node/test-bus/test-unittest6/status");
+
+ overlay_name = overlay_name_from_nr(overlay_nr + 0);
- if (!overlay_data_apply(overlay_name, &ovcs_id)) {
- unittest(0, "could not apply overlay \"%s\"\n",
- overlay_name);
+ ret = overlay_data_apply(overlay_name, &ovcs_id);
+
+ if (!ret) {
+ unittest(0, "could not apply overlay \"%s\"\n", overlay_name);
+ return;
+ }
+ ov_id[0] = ovcs_id;
+ of_unittest_track_overlay(ov_id[0]);
+
+ EXPECT_END(KERN_INFO,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data/overlay-node/test-bus/test-unittest6/status");
+
+ EXPECT_BEGIN(KERN_INFO,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data/overlay-node/test-bus/test-unittest7/status");
+
+ overlay_name = overlay_name_from_nr(overlay_nr + 1);
+
+ ret = overlay_data_apply(overlay_name, &ovcs_id);
+
+ if (!ret) {
+ unittest(0, "could not apply overlay \"%s\"\n", overlay_name);
return;
- }
- ov_id[i] = ovcs_id;
- of_unittest_track_overlay(ov_id[i]);
}
+ ov_id[1] = ovcs_id;
+ of_unittest_track_overlay(ov_id[1]);
+
+ EXPECT_END(KERN_INFO,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data/overlay-node/test-bus/test-unittest7/status");
+
for (i = 0; i < 2; i++) {
/* unittest device must be in after state */
@@ -1900,6 +2061,7 @@ static void __init of_unittest_overlay_6(void)
}
unittest(1, "overlay test %d passed\n", 6);
+
}
/* test overlay application in sequence */
@@ -1908,26 +2070,65 @@ static void __init of_unittest_overlay_8(void)
int i, ov_id[2], ovcs_id;
int overlay_nr = 8, unittest_nr = 8;
const char *overlay_name;
+ int ret;
/* we don't care about device state in this test */
+ EXPECT_BEGIN(KERN_INFO,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data/overlay-node/test-bus/test-unittest8/status");
+
+ overlay_name = overlay_name_from_nr(overlay_nr + 0);
+
+ ret = overlay_data_apply(overlay_name, &ovcs_id);
+ if (!ret)
+ unittest(0, "could not apply overlay \"%s\"\n", overlay_name);
+
+ EXPECT_END(KERN_INFO,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data/overlay-node/test-bus/test-unittest8/status");
+
+ if (!ret)
+ return;
+
+ ov_id[0] = ovcs_id;
+ of_unittest_track_overlay(ov_id[0]);
+
+ overlay_name = overlay_name_from_nr(overlay_nr + 1);
+
+ EXPECT_BEGIN(KERN_INFO,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data/overlay-node/test-bus/test-unittest8/property-foo");
+
/* apply the overlays */
- for (i = 0; i < 2; i++) {
+ ret = overlay_data_apply(overlay_name, &ovcs_id);
- overlay_name = overlay_name_from_nr(overlay_nr + i);
+ EXPECT_END(KERN_INFO,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data/overlay-node/test-bus/test-unittest8/property-foo");
- if (!overlay_data_apply(overlay_name, &ovcs_id)) {
- unittest(0, "could not apply overlay \"%s\"\n",
- overlay_name);
- return;
- }
- ov_id[i] = ovcs_id;
- of_unittest_track_overlay(ov_id[i]);
+ if (!ret) {
+ unittest(0, "could not apply overlay \"%s\"\n", overlay_name);
+ return;
}
+ ov_id[1] = ovcs_id;
+ of_unittest_track_overlay(ov_id[1]);
+
/* now try to remove first overlay (it should fail) */
ovcs_id = ov_id[0];
- if (!of_overlay_remove(&ovcs_id)) {
+
+ EXPECT_BEGIN(KERN_INFO,
+ "OF: overlay: node_overlaps_later_cs: #6 overlaps with #7 @/testcase-data/overlay-node/test-bus/test-unittest8");
+
+ EXPECT_BEGIN(KERN_INFO,
+ "OF: overlay: overlay #6 is not topmost");
+
+ ret = of_overlay_remove(&ovcs_id);
+
+ EXPECT_END(KERN_INFO,
+ "OF: overlay: overlay #6 is not topmost");
+
+ EXPECT_END(KERN_INFO,
+ "OF: overlay: node_overlaps_later_cs: #6 overlaps with #7 @/testcase-data/overlay-node/test-bus/test-unittest8");
+
+ if (!ret) {
unittest(0, "%s was destroyed @\"%s\"\n",
overlay_name_from_nr(overlay_nr + 0),
unittest_path(unittest_nr,
@@ -1959,6 +2160,7 @@ static void __init of_unittest_overlay_10(void)
/* device should disable */
ret = of_unittest_apply_overlay_check(10, 10, 0, 1, PDEV_OVERLAY);
+
if (unittest(ret == 0,
"overlay test %d failed; overlay application\n", 10))
return;
@@ -1982,6 +2184,7 @@ static void __init of_unittest_overlay_11(void)
/* device should disable */
ret = of_unittest_apply_revert_overlay_check(11, 11, 0, 1,
PDEV_OVERLAY);
+
unittest(ret == 0, "overlay test %d failed; overlay apply\n", 11);
}
@@ -2212,12 +2415,21 @@ static int of_unittest_overlay_i2c_init(void)
return ret;
ret = platform_driver_register(&unittest_i2c_bus_driver);
+
if (unittest(ret == 0,
"could not register unittest i2c bus driver\n"))
return ret;
#if IS_BUILTIN(CONFIG_I2C_MUX)
+
+ EXPECT_BEGIN(KERN_INFO,
+ "i2c i2c-1: Added multiplexed i2c bus 2");
+
ret = i2c_add_driver(&unittest_i2c_mux_driver);
+
+ EXPECT_END(KERN_INFO,
+ "i2c i2c-1: Added multiplexed i2c bus 2");
+
if (unittest(ret == 0,
"could not register unittest i2c mux driver\n"))
return ret;
@@ -2237,8 +2449,18 @@ static void of_unittest_overlay_i2c_cleanup(void)
static void __init of_unittest_overlay_i2c_12(void)
{
+ int ret;
+
/* device should enable */
- if (of_unittest_apply_overlay_check(12, 12, 0, 1, I2C_OVERLAY))
+ EXPECT_BEGIN(KERN_INFO,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data/overlay-node/test-bus/i2c-test-bus/test-unittest12/status");
+
+ ret = of_unittest_apply_overlay_check(12, 12, 0, 1, I2C_OVERLAY);
+
+ EXPECT_END(KERN_INFO,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data/overlay-node/test-bus/i2c-test-bus/test-unittest12/status");
+
+ if (ret)
return;
unittest(1, "overlay test %d passed\n", 12);
@@ -2247,8 +2469,18 @@ static void __init of_unittest_overlay_i2c_12(void)
/* test deactivation of device */
static void __init of_unittest_overlay_i2c_13(void)
{
+ int ret;
+
+ EXPECT_BEGIN(KERN_INFO,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data/overlay-node/test-bus/i2c-test-bus/test-unittest13/status");
+
/* device should disable */
- if (of_unittest_apply_overlay_check(13, 13, 1, 0, I2C_OVERLAY))
+ ret = of_unittest_apply_overlay_check(13, 13, 1, 0, I2C_OVERLAY);
+
+ EXPECT_END(KERN_INFO,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data/overlay-node/test-bus/i2c-test-bus/test-unittest13/status");
+
+ if (ret)
return;
unittest(1, "overlay test %d passed\n", 13);
@@ -2261,8 +2493,18 @@ static void of_unittest_overlay_i2c_14(void)
static void __init of_unittest_overlay_i2c_15(void)
{
+ int ret;
+
/* device should enable */
- if (of_unittest_apply_overlay_check(15, 15, 0, 1, I2C_OVERLAY))
+ EXPECT_BEGIN(KERN_INFO,
+ "i2c i2c-1: Added multiplexed i2c bus 3");
+
+ ret = of_unittest_apply_overlay_check(15, 15, 0, 1, I2C_OVERLAY);
+
+ EXPECT_END(KERN_INFO,
+ "i2c i2c-1: Added multiplexed i2c bus 3");
+
+ if (ret)
return;
unittest(1, "overlay test %d passed\n", 15);
@@ -2723,6 +2965,7 @@ static __init void of_unittest_overlay_high_level(void)
struct device_node *overlay_base_symbols;
struct device_node **pprev;
struct property *prop;
+ int ret;
if (!overlay_base_root) {
unittest(0, "overlay_base_root not initialized\n");
@@ -2837,15 +3080,86 @@ static __init void of_unittest_overlay_high_level(void)
/* now do the normal overlay usage test */
- unittest(overlay_data_apply("overlay", NULL),
- "Adding overlay 'overlay' failed\n");
+ EXPECT_BEGIN(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data-2/substation@100/status");
+ EXPECT_BEGIN(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data-2/fairway-1/status");
+ EXPECT_BEGIN(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data-2/fairway-1/ride@100/track@30/incline-up");
+ EXPECT_BEGIN(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data-2/fairway-1/ride@100/track@40/incline-up");
+ EXPECT_BEGIN(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data-2/lights@40000/status");
+ EXPECT_BEGIN(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data-2/lights@40000/color");
+ EXPECT_BEGIN(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data-2/lights@40000/rate");
+ EXPECT_BEGIN(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /__symbols__/hvac_2");
+ EXPECT_BEGIN(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /__symbols__/ride_200");
+ EXPECT_BEGIN(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /__symbols__/ride_200_left");
+ EXPECT_BEGIN(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /__symbols__/ride_200_right");
+
+ ret = overlay_data_apply("overlay", NULL);
+
+ EXPECT_END(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /__symbols__/ride_200_right");
+ EXPECT_END(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /__symbols__/ride_200_left");
+ EXPECT_END(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /__symbols__/ride_200");
+ EXPECT_END(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /__symbols__/hvac_2");
+ EXPECT_END(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data-2/lights@40000/rate");
+ EXPECT_END(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data-2/lights@40000/color");
+ EXPECT_END(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data-2/lights@40000/status");
+ EXPECT_END(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data-2/fairway-1/ride@100/track@40/incline-up");
+ EXPECT_END(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data-2/fairway-1/ride@100/track@30/incline-up");
+ EXPECT_END(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data-2/fairway-1/status");
+ EXPECT_END(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data-2/substation@100/status");
+
+ unittest(ret, "Adding overlay 'overlay' failed\n");
+
+ EXPECT_BEGIN(KERN_ERR,
+ "OF: overlay: ERROR: multiple fragments add and/or delete node /testcase-data-2/substation@100/motor-1/controller");
+ EXPECT_BEGIN(KERN_ERR,
+ "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/controller/name");
unittest(overlay_data_apply("overlay_bad_add_dup_node", NULL),
"Adding overlay 'overlay_bad_add_dup_node' failed\n");
+ EXPECT_END(KERN_ERR,
+ "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/controller/name");
+ EXPECT_END(KERN_ERR,
+ "OF: overlay: ERROR: multiple fragments add and/or delete node /testcase-data-2/substation@100/motor-1/controller");
+
+ EXPECT_BEGIN(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data-2/substation@100/motor-1/rpm_avail");
+ EXPECT_BEGIN(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data-2/substation@100/motor-1/rpm_avail");
+ EXPECT_BEGIN(KERN_ERR,
+ "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/rpm_avail");
+
unittest(overlay_data_apply("overlay_bad_add_dup_prop", NULL),
"Adding overlay 'overlay_bad_add_dup_prop' failed\n");
+ EXPECT_END(KERN_ERR,
+ "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/rpm_avail");
+ EXPECT_END(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data-2/substation@100/motor-1/rpm_avail");
+ EXPECT_END(KERN_ERR,
+ "OF: overlay: WARNING: memory leak will occur if overlay removed, property: /testcase-data-2/substation@100/motor-1/rpm_avail");
+
unittest(overlay_data_apply("overlay_bad_phandle", NULL),
"Adding overlay 'overlay_bad_phandle' failed\n");
@@ -2869,6 +3183,8 @@ static int __init of_unittest(void)
struct device_node *np;
int res;
+ pr_info("start of unittest - you will see error messages\n");
+
/* adding data for unittest */
if (IS_ENABLED(CONFIG_UML))
@@ -2887,7 +3203,6 @@ static int __init of_unittest(void)
}
of_node_put(np);
- pr_info("start of unittest - you will see error messages\n");
of_unittest_check_tree_linkage();
of_unittest_check_phandles();
of_unittest_find_node_by_name();
--
Frank Rowand <frank.rowand@sony.com>
^ permalink raw reply related
* [cifs:for-next 4/6] fs/cifs/smb2pdu.h:1616:1: error: expected identifier or '(' before '}' token
From: kbuild test robot @ 2020-02-20 18:41 UTC (permalink / raw)
To: Aurelien Aptel; +Cc: kbuild-all, linux-cifs, samba-technical, Steve French
[-- Attachment #1: Type: text/plain, Size: 2267 bytes --]
tree: git://git.samba.org/sfrench/cifs-2.6.git for-next
head: 93fc7de81fe90b31c3f99fb86dcc79649ef028b9
commit: 86eac7701750ff65108ecf8022514dbcedaa080d [4/6] cifs: rename posix create rsp
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 7.5.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 86eac7701750ff65108ecf8022514dbcedaa080d
# save the attached .config to linux build tree
GCC_VERSION=7.5.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
Note: the cifs/for-next HEAD 93fc7de81fe90b31c3f99fb86dcc79649ef028b9 builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
In file included from fs/cifs/cifsglob.h:32:0,
from fs/cifs/cifsfs.c:46:
>> fs/cifs/smb2pdu.h:1616:1: error: expected identifier or '(' before '}' token
};
^
vim +1616 fs/cifs/smb2pdu.h
730928c8f4be88 Ronnie Sahlberg 2018-08-08 1605
ab3459d8f0ef52 Steve French 2020-02-06 1606 /* equivalent of the contents of SMB3.1.1 POSIX open context response */
86eac7701750ff Aurelien Aptel 2020-02-08 1607 struct create_posix_rsp {
ab3459d8f0ef52 Steve French 2020-02-06 1608 __le32 nlink;
ab3459d8f0ef52 Steve French 2020-02-06 1609 __le32 reparse_tag;
ab3459d8f0ef52 Steve French 2020-02-06 1610 __le32 mode;
86eac7701750ff Aurelien Aptel 2020-02-08 1611 /*
86eac7701750ff Aurelien Aptel 2020-02-08 1612 * var sized owner SID
86eac7701750ff Aurelien Aptel 2020-02-08 1613 * var sized group SID
86eac7701750ff Aurelien Aptel 2020-02-08 1614 */
86eac7701750ff Aurelien Aptel 2020-02-08 1615 } __packed;
ab3459d8f0ef52 Steve French 2020-02-06 @1616 };
:::::: The code at line 1616 was first introduced by commit
:::::: ab3459d8f0ef52c38119ed58c4c29139efc7022c smb3: print warning once if posix context returned on open
:::::: TO: Steve French <stfrench@microsoft.com>
:::::: CC: Steve French <stfrench@microsoft.com>
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25773 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] spi: spidev: fix a max speed setting
From: Mark Brown @ 2020-02-20 18:42 UTC (permalink / raw)
To: Oleksandr Suvorov
Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Oleksandr Suvorov, Marcel Ziswiler, Igor Opanyuk,
Philippe Schenker
In-Reply-To: <20200220141143.3902922-3-oleksandr.suvorov-2KBjVHiyJgBBDgjK7y7TUQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 696 bytes --]
On Thu, Feb 20, 2020 at 02:11:51PM +0000, Oleksandr Suvorov wrote:
> SPI_IOC_WR_MAX_SPEED_HZ command always sets spi->max_speed_hz
> to the initial value come from DT.
This is intentional. It's doing a call to spi_setup() then restoring
the original value, the goal being just to run spi_setup() with the new
value - it's not really a good idea to change the maximum speed in the
first place.
> It leads to set a wrong max speed with IOCTL call.
In what way does it lead to the wrong speed being set?
> Fix the logic of a max speed assignment.
If the expectation is that the default speed should be changed for the
device this should be handled at the spidev level rather than in the
core.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v2] xfs_io/encrypt: support passing a keyring key to add_enckey
From: Darrick J. Wong @ 2020-02-20 18:42 UTC (permalink / raw)
To: Eric Biggers; +Cc: linux-xfs, fstests, linux-fscrypt, keyrings
In-Reply-To: <20200218214856.GA147283@gmail.com>
On Tue, Feb 18, 2020 at 01:48:57PM -0800, Eric Biggers wrote:
> On Mon, Feb 03, 2020 at 10:20:13AM -0800, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> >
> > Add a '-k' option to the 'add_enckey' xfs_io command to allow exercising
> > the key_id field that is being added to struct fscrypt_add_key_arg.
> >
> > This is needed for the corresponding test in xfstests.
> >
> > For more details, see the corresponding xfstests patches as well as
> > kernel commit 93edd392cad7 ("fscrypt: support passing a keyring key to
> > FS_IOC_ADD_ENCRYPTION_KEY").
> >
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > ---
> >
> > No changes since v1.
> >
> > This applies to the for-next branch of xfsprogs.
> >
> > configure.ac | 1 +
> > include/builddefs.in | 4 ++
> > io/encrypt.c | 90 +++++++++++++++++++++++++++++++------------
> > m4/package_libcdev.m4 | 21 ++++++++++
> > man/man8/xfs_io.8 | 10 +++--
> > 5 files changed, 98 insertions(+), 28 deletions(-)
> >
>
> Any comments on this patch? The corresponding xfstests patches were merged.
I didn't see any obvious bugs, though fwiw I'm not that familiar with
fscrypt. This looks like a pretty straightforward addition of a new
field to a kernel call structure and some other plumbing to fill out the
new field with CLI arguments / stdin.
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> - Eric
^ permalink raw reply
* Re: [PATCH 2/2] spi: spidev: fix a max speed setting
From: Mark Brown @ 2020-02-20 18:42 UTC (permalink / raw)
To: Oleksandr Suvorov
Cc: linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org,
Oleksandr Suvorov, Marcel Ziswiler, Igor Opanyuk,
Philippe Schenker
In-Reply-To: <20200220141143.3902922-3-oleksandr.suvorov@toradex.com>
[-- Attachment #1: Type: text/plain, Size: 696 bytes --]
On Thu, Feb 20, 2020 at 02:11:51PM +0000, Oleksandr Suvorov wrote:
> SPI_IOC_WR_MAX_SPEED_HZ command always sets spi->max_speed_hz
> to the initial value come from DT.
This is intentional. It's doing a call to spi_setup() then restoring
the original value, the goal being just to run spi_setup() with the new
value - it's not really a good idea to change the maximum speed in the
first place.
> It leads to set a wrong max speed with IOCTL call.
In what way does it lead to the wrong speed being set?
> Fix the logic of a max speed assignment.
If the expectation is that the default speed should be changed for the
device this should be handled at the spidev level rather than in the
core.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v2] xfs_io/encrypt: support passing a keyring key to add_enckey
From: Darrick J. Wong @ 2020-02-20 18:42 UTC (permalink / raw)
To: Eric Biggers; +Cc: linux-xfs, fstests, linux-fscrypt, keyrings
In-Reply-To: <20200218214856.GA147283@gmail.com>
On Tue, Feb 18, 2020 at 01:48:57PM -0800, Eric Biggers wrote:
> On Mon, Feb 03, 2020 at 10:20:13AM -0800, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> >
> > Add a '-k' option to the 'add_enckey' xfs_io command to allow exercising
> > the key_id field that is being added to struct fscrypt_add_key_arg.
> >
> > This is needed for the corresponding test in xfstests.
> >
> > For more details, see the corresponding xfstests patches as well as
> > kernel commit 93edd392cad7 ("fscrypt: support passing a keyring key to
> > FS_IOC_ADD_ENCRYPTION_KEY").
> >
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > ---
> >
> > No changes since v1.
> >
> > This applies to the for-next branch of xfsprogs.
> >
> > configure.ac | 1 +
> > include/builddefs.in | 4 ++
> > io/encrypt.c | 90 +++++++++++++++++++++++++++++++------------
> > m4/package_libcdev.m4 | 21 ++++++++++
> > man/man8/xfs_io.8 | 10 +++--
> > 5 files changed, 98 insertions(+), 28 deletions(-)
> >
>
> Any comments on this patch? The corresponding xfstests patches were merged.
I didn't see any obvious bugs, though fwiw I'm not that familiar with
fscrypt. This looks like a pretty straightforward addition of a new
field to a kernel call structure and some other plumbing to fill out the
new field with CLI arguments / stdin.
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> - Eric
^ permalink raw reply
* Re: [PATCH v2 1/4] drm/simple-kms: Add drm_simple_encoder_{init, create}()
From: Sam Ravnborg @ 2020-02-20 18:42 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: airlied, dri-devel, maarten.lankhorst, mripard, virtualization,
noralf, daniel, alexander.deucher, spice-devel, emil.velikov
In-Reply-To: <20200218084815.2137-2-tzimmermann@suse.de>
Hi Thomas.
On Tue, Feb 18, 2020 at 09:48:12AM +0100, Thomas Zimmermann wrote:
> This patch makes the internal encoder implementation of the simple
> KMS helpers available to drivers.
>
> These simple-encoder helpers initialize an encoder with an empty
> implementation. This covers the requirements of most of the existing
> DRM drivers. A call to drm_simple_encoder_create() allocates and
> initializes an encoder instance, a call to drm_simple_encoder_init()
> initializes a pre-allocated instance.
>
> v2:
> * move simple encoder to KMS helpers
> * remove name argument; simplifies implementation
> * don't allocate with devm_ interfaces; unsafe with DRM
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/gpu/drm/drm_simple_kms_helper.c | 83 ++++++++++++++++++++++++-
> include/drm/drm_simple_kms_helper.h | 7 +++
> 2 files changed, 87 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> index 15fb516ae2d8..745c2f34c42b 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -26,12 +26,90 @@
> * entity. Some flexibility for code reuse is provided through a separately
> * allocated &drm_connector object and supporting optional &drm_bridge
> * encoder drivers.
> + *
> + * Many drivers use an encoder with an empty implementation. Such encoders
> + * fulfill the minimum requirements of the display pipeline, but don't add
> + * additional functionality. The simple-encoder functions
> + * drm_simple_encoder_init() and drm_simple_encoder_create() provide an
> + * appropriate implementation.
This paragraph reads a bit strange to me - I read this as a
justification for addding a generic encoded that can be used by exisitg
drivers.
How about something like this:
Many drivers requires a very simple encoder that only fullfill
the minimum requirements of the display pipeline and do not add
any extra functionslity.
The simple-encoder functions drm_simple_encoder_init() and
drm_simple_encoder_create() provides an impålmentation of such
a simple encoder.
The simple encoder includes automatically release of resources.
And then leave it to the changelog to tell what should be done in
existing drivers.
> */
>
> -static const struct drm_encoder_funcs drm_simple_kms_encoder_funcs = {
> +static const struct drm_encoder_funcs drm_simple_encoder_funcs_cleanup = {
> .destroy = drm_encoder_cleanup,
> };
>
> +/**
> + * drm_simple_encoder_init - Initialize a preallocated encoder
> + * @dev: drm device
> + * @funcs: callbacks for this encoder
> + * @encoder_type: user visible type of the encoder
> + *
> + * Initialises a preallocated encoder that has no further functionality. The
> + * encoder will be released automatically. Settings for possible CRTC and
> + * clones are left to their initial values. The encoder will be cleaned up
> + * automatically as part of the mode-setting cleanup.
> + *
> + * Also see drm_simple_encoder_create().
s/Also see/See also/??
> + *
> + * Returns:
> + * Zero on success, error code on failure.
> + */
> +int drm_simple_encoder_init(struct drm_device *dev,
> + struct drm_encoder *encoder,
> + int encoder_type)
> +{
> + return drm_encoder_init(dev, encoder,
> + &drm_simple_encoder_funcs_cleanup,
> + encoder_type, NULL);
> +}
> +EXPORT_SYMBOL(drm_simple_encoder_init);
> +
> +static void drm_encoder_destroy(struct drm_encoder *encoder)
> +{
> + drm_encoder_cleanup(encoder);
> + kfree(encoder);
> +}
> +
> +static const struct drm_encoder_funcs drm_simple_encoder_funcs_destroy = {
> + .destroy = drm_encoder_destroy,
> +};
> +
> +/**
> + * drm_simple_encoder_create - Allocate and initialize an encoder
> + * @dev: drm device
> + * @encoder_type: user visible type of the encoder
> + *
> + * Allocates and initialises an encoder that has no further functionality. The
> + * encoder will be destroyed automatically as part of the mode-setting cleanup.
> + *
> + * See drm_simple_encoder_init() for more information.
> + *
> + * Returns:
> + * The encoder on success, a pointer-encoder error code on failure.
pointer-encoded?
> + */
> +struct drm_encoder *drm_simple_encoder_create(struct drm_device *dev,
> + int encoder_type)
> +{
> + struct drm_encoder *encoder;
> + int ret;
> +
> + encoder = kzalloc(sizeof(*encoder), GFP_KERNEL);
> + if (!encoder)
> + return ERR_PTR(-ENOMEM);
> + ret = drm_encoder_init(dev, encoder,
> + &drm_simple_encoder_funcs_destroy,
> + encoder_type, NULL);
> + if (ret)
> + goto err_kfree;
> +
> + return encoder;
> +
> +err_kfree:
> + kfree(encoder);
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL(drm_simple_encoder_create);
> +
> static enum drm_mode_status
> drm_simple_kms_crtc_mode_valid(struct drm_crtc *crtc,
> const struct drm_display_mode *mode)
> @@ -288,8 +366,7 @@ int drm_simple_display_pipe_init(struct drm_device *dev,
> return ret;
>
> encoder->possible_crtcs = drm_crtc_mask(crtc);
> - ret = drm_encoder_init(dev, encoder, &drm_simple_kms_encoder_funcs,
> - DRM_MODE_ENCODER_NONE, NULL);
> + ret = drm_simple_encoder_init(dev, encoder, DRM_MODE_ENCODER_NONE);
> if (ret || !connector)
> return ret;
>
> diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
> index e253ba7bea9d..54d5066d90c7 100644
> --- a/include/drm/drm_simple_kms_helper.h
> +++ b/include/drm/drm_simple_kms_helper.h
> @@ -181,4 +181,11 @@ int drm_simple_display_pipe_init(struct drm_device *dev,
> const uint64_t *format_modifiers,
> struct drm_connector *connector);
>
> +int drm_simple_encoder_init(struct drm_device *dev,
> + struct drm_encoder *encoder,
> + int encoder_type);
> +
> +struct drm_encoder *drm_simple_encoder_create(struct drm_device *dev,
> + int encoder_type);
> +
> #endif /* __LINUX_DRM_SIMPLE_KMS_HELPER_H */
Sam
^ permalink raw reply
* Re: [RFC PATCH] iommu/iova: Add a best-fit algorithm
From: Robin Murphy @ 2020-02-20 18:42 UTC (permalink / raw)
To: isaacm; +Cc: iommu, linux-kernel, kernel-team, pratikp, Liam Mark
In-Reply-To: <7239ddd532e94a4371289f3be23c66a3@codeaurora.org>
On 20/02/2020 6:38 am, isaacm@codeaurora.org wrote:
> On 2020-02-17 08:03, Robin Murphy wrote:
>> On 14/02/2020 11:06 pm, Isaac J. Manjarres wrote:
>>> From: Liam Mark <lmark@codeaurora.org>
>>>
>>> Using the best-fit algorithm, instead of the first-fit
>>> algorithm, may reduce fragmentation when allocating
>>> IOVAs.
>>
>> What kind of pathological allocation patterns make that a serious
>> problem? Is there any scope for simply changing the order of things in
>> the callers? Do these drivers also run under other DMA API backends
>> (e.g. 32-bit Arm)?
>>
> The usecases where the IOVA space has been fragmented have
> non-deterministic allocation
> patterns, and thus, it's not feasible to change the allocation order to
> avoid fragmenting
> the IOVA space.
What about combining smaller buffers into larger individual allocations;
any scope for that sort of thing? Certainly if you're consistently
allocating small things less than PAGE_SIZE then DMA pools would be
useful to avoid wanton memory wastage in general.
> From what we've observed, the usecases involve allocations of two types of
> buffers: one type of buffer between 1 KB to 4 MB in size, and another
> type of
> buffer between 1 KB to 400 MB in size.
>
> The pathological scenarios seem to arise when there are
> many (100+) randomly distributed non-power of two allocations, which in
> some cases leaves
> behind holes of up to 100+ MB in the IOVA space.
>
> Here are some examples that show the state of the IOVA space under which
> failure to
> allocate an IOVA was observed:
>
> Instance 1:
> Currently mapped total size : ~1.3GB
> Free space available : ~2GB
> Map for ~162MB fails.
> Max contiguous space available : < 162MB
>
> Instance 2:
> Currently mapped total size : ~950MB
> Free space available : ~2.3GB
> Map for ~320MB fails.
> Max contiguous space available : ~189MB
>
> Instance 3:
> Currently mapped total size : ~1.2GB
> Free space available : ~2.7GB
> Map for ~162MB fails.
> Max contiguous space available : <162MB
>
> We are still in the process of collecting data with the best-fit
> algorithm enabled
> to provide some numbers to show that it results in less IOVA space
> fragmentation.
Thanks for those examples, and I'd definitely like to see the
comparative figures. To dig a bit further, at the point where things
start failing, where are the cached nodes pointing? IIRC there is still
a pathological condition where empty space between limit_pfn and
cached32_node gets 'lost' if nothing in between is freed, so the bigger
the range of allocation sizes, the worse the effect, e.g.:
(considering an empty domain, pfn 0 *not* reserved, 32-bit limit_pfn)
alloc 4K, succeeds, cached32_node now at 4G-4K
alloc 2G, succeeds, cached32_node now at 0
alloc 4K, fails despite almost 2G contiguous free space within limit_pfn
(and max32_alloc_size==1 now fast-forwards *any* further allocation
attempt to failure)
If you're falling foul of this case (I was never sure how realistic a
problem it would be in practice), there are at least a couple of much
less invasive tweaks I can think of that would be worth exploring.
> To answer your question about whether if this driver run under other DMA
> API backends:
> yes, such as 32 bit ARM.
OK, that's what I suspected :)
AFAICS arch/arm's __alloc_iova() is also a first-fit algorithm, so if
you get better behaviour there then it would suggest that this aspect
isn't really the most important issue. Certainly, the fact that the
"best fit" logic here also happens to ignore the cached nodes does start
drawing me back to the point above.
Robin.
^ permalink raw reply
* Re: [RFC PATCH] iommu/iova: Add a best-fit algorithm
From: Robin Murphy @ 2020-02-20 18:42 UTC (permalink / raw)
To: isaacm; +Cc: pratikp, iommu, kernel-team, linux-kernel, Liam Mark
In-Reply-To: <7239ddd532e94a4371289f3be23c66a3@codeaurora.org>
On 20/02/2020 6:38 am, isaacm@codeaurora.org wrote:
> On 2020-02-17 08:03, Robin Murphy wrote:
>> On 14/02/2020 11:06 pm, Isaac J. Manjarres wrote:
>>> From: Liam Mark <lmark@codeaurora.org>
>>>
>>> Using the best-fit algorithm, instead of the first-fit
>>> algorithm, may reduce fragmentation when allocating
>>> IOVAs.
>>
>> What kind of pathological allocation patterns make that a serious
>> problem? Is there any scope for simply changing the order of things in
>> the callers? Do these drivers also run under other DMA API backends
>> (e.g. 32-bit Arm)?
>>
> The usecases where the IOVA space has been fragmented have
> non-deterministic allocation
> patterns, and thus, it's not feasible to change the allocation order to
> avoid fragmenting
> the IOVA space.
What about combining smaller buffers into larger individual allocations;
any scope for that sort of thing? Certainly if you're consistently
allocating small things less than PAGE_SIZE then DMA pools would be
useful to avoid wanton memory wastage in general.
> From what we've observed, the usecases involve allocations of two types of
> buffers: one type of buffer between 1 KB to 4 MB in size, and another
> type of
> buffer between 1 KB to 400 MB in size.
>
> The pathological scenarios seem to arise when there are
> many (100+) randomly distributed non-power of two allocations, which in
> some cases leaves
> behind holes of up to 100+ MB in the IOVA space.
>
> Here are some examples that show the state of the IOVA space under which
> failure to
> allocate an IOVA was observed:
>
> Instance 1:
> Currently mapped total size : ~1.3GB
> Free space available : ~2GB
> Map for ~162MB fails.
> Max contiguous space available : < 162MB
>
> Instance 2:
> Currently mapped total size : ~950MB
> Free space available : ~2.3GB
> Map for ~320MB fails.
> Max contiguous space available : ~189MB
>
> Instance 3:
> Currently mapped total size : ~1.2GB
> Free space available : ~2.7GB
> Map for ~162MB fails.
> Max contiguous space available : <162MB
>
> We are still in the process of collecting data with the best-fit
> algorithm enabled
> to provide some numbers to show that it results in less IOVA space
> fragmentation.
Thanks for those examples, and I'd definitely like to see the
comparative figures. To dig a bit further, at the point where things
start failing, where are the cached nodes pointing? IIRC there is still
a pathological condition where empty space between limit_pfn and
cached32_node gets 'lost' if nothing in between is freed, so the bigger
the range of allocation sizes, the worse the effect, e.g.:
(considering an empty domain, pfn 0 *not* reserved, 32-bit limit_pfn)
alloc 4K, succeeds, cached32_node now at 4G-4K
alloc 2G, succeeds, cached32_node now at 0
alloc 4K, fails despite almost 2G contiguous free space within limit_pfn
(and max32_alloc_size==1 now fast-forwards *any* further allocation
attempt to failure)
If you're falling foul of this case (I was never sure how realistic a
problem it would be in practice), there are at least a couple of much
less invasive tweaks I can think of that would be worth exploring.
> To answer your question about whether if this driver run under other DMA
> API backends:
> yes, such as 32 bit ARM.
OK, that's what I suspected :)
AFAICS arch/arm's __alloc_iova() is also a first-fit algorithm, so if
you get better behaviour there then it would suggest that this aspect
isn't really the most important issue. Certainly, the fact that the
"best fit" logic here also happens to ignore the cached nodes does start
drawing me back to the point above.
Robin.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply
* Re: [PATCH v2 1/4] drm/simple-kms: Add drm_simple_encoder_{init, create}()
From: Sam Ravnborg @ 2020-02-20 18:42 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: airlied, dri-devel, virtualization, kraxel, alexander.deucher,
spice-devel, emil.velikov
In-Reply-To: <20200218084815.2137-2-tzimmermann@suse.de>
Hi Thomas.
On Tue, Feb 18, 2020 at 09:48:12AM +0100, Thomas Zimmermann wrote:
> This patch makes the internal encoder implementation of the simple
> KMS helpers available to drivers.
>
> These simple-encoder helpers initialize an encoder with an empty
> implementation. This covers the requirements of most of the existing
> DRM drivers. A call to drm_simple_encoder_create() allocates and
> initializes an encoder instance, a call to drm_simple_encoder_init()
> initializes a pre-allocated instance.
>
> v2:
> * move simple encoder to KMS helpers
> * remove name argument; simplifies implementation
> * don't allocate with devm_ interfaces; unsafe with DRM
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/gpu/drm/drm_simple_kms_helper.c | 83 ++++++++++++++++++++++++-
> include/drm/drm_simple_kms_helper.h | 7 +++
> 2 files changed, 87 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> index 15fb516ae2d8..745c2f34c42b 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -26,12 +26,90 @@
> * entity. Some flexibility for code reuse is provided through a separately
> * allocated &drm_connector object and supporting optional &drm_bridge
> * encoder drivers.
> + *
> + * Many drivers use an encoder with an empty implementation. Such encoders
> + * fulfill the minimum requirements of the display pipeline, but don't add
> + * additional functionality. The simple-encoder functions
> + * drm_simple_encoder_init() and drm_simple_encoder_create() provide an
> + * appropriate implementation.
This paragraph reads a bit strange to me - I read this as a
justification for addding a generic encoded that can be used by exisitg
drivers.
How about something like this:
Many drivers requires a very simple encoder that only fullfill
the minimum requirements of the display pipeline and do not add
any extra functionslity.
The simple-encoder functions drm_simple_encoder_init() and
drm_simple_encoder_create() provides an impålmentation of such
a simple encoder.
The simple encoder includes automatically release of resources.
And then leave it to the changelog to tell what should be done in
existing drivers.
> */
>
> -static const struct drm_encoder_funcs drm_simple_kms_encoder_funcs = {
> +static const struct drm_encoder_funcs drm_simple_encoder_funcs_cleanup = {
> .destroy = drm_encoder_cleanup,
> };
>
> +/**
> + * drm_simple_encoder_init - Initialize a preallocated encoder
> + * @dev: drm device
> + * @funcs: callbacks for this encoder
> + * @encoder_type: user visible type of the encoder
> + *
> + * Initialises a preallocated encoder that has no further functionality. The
> + * encoder will be released automatically. Settings for possible CRTC and
> + * clones are left to their initial values. The encoder will be cleaned up
> + * automatically as part of the mode-setting cleanup.
> + *
> + * Also see drm_simple_encoder_create().
s/Also see/See also/??
> + *
> + * Returns:
> + * Zero on success, error code on failure.
> + */
> +int drm_simple_encoder_init(struct drm_device *dev,
> + struct drm_encoder *encoder,
> + int encoder_type)
> +{
> + return drm_encoder_init(dev, encoder,
> + &drm_simple_encoder_funcs_cleanup,
> + encoder_type, NULL);
> +}
> +EXPORT_SYMBOL(drm_simple_encoder_init);
> +
> +static void drm_encoder_destroy(struct drm_encoder *encoder)
> +{
> + drm_encoder_cleanup(encoder);
> + kfree(encoder);
> +}
> +
> +static const struct drm_encoder_funcs drm_simple_encoder_funcs_destroy = {
> + .destroy = drm_encoder_destroy,
> +};
> +
> +/**
> + * drm_simple_encoder_create - Allocate and initialize an encoder
> + * @dev: drm device
> + * @encoder_type: user visible type of the encoder
> + *
> + * Allocates and initialises an encoder that has no further functionality. The
> + * encoder will be destroyed automatically as part of the mode-setting cleanup.
> + *
> + * See drm_simple_encoder_init() for more information.
> + *
> + * Returns:
> + * The encoder on success, a pointer-encoder error code on failure.
pointer-encoded?
> + */
> +struct drm_encoder *drm_simple_encoder_create(struct drm_device *dev,
> + int encoder_type)
> +{
> + struct drm_encoder *encoder;
> + int ret;
> +
> + encoder = kzalloc(sizeof(*encoder), GFP_KERNEL);
> + if (!encoder)
> + return ERR_PTR(-ENOMEM);
> + ret = drm_encoder_init(dev, encoder,
> + &drm_simple_encoder_funcs_destroy,
> + encoder_type, NULL);
> + if (ret)
> + goto err_kfree;
> +
> + return encoder;
> +
> +err_kfree:
> + kfree(encoder);
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL(drm_simple_encoder_create);
> +
> static enum drm_mode_status
> drm_simple_kms_crtc_mode_valid(struct drm_crtc *crtc,
> const struct drm_display_mode *mode)
> @@ -288,8 +366,7 @@ int drm_simple_display_pipe_init(struct drm_device *dev,
> return ret;
>
> encoder->possible_crtcs = drm_crtc_mask(crtc);
> - ret = drm_encoder_init(dev, encoder, &drm_simple_kms_encoder_funcs,
> - DRM_MODE_ENCODER_NONE, NULL);
> + ret = drm_simple_encoder_init(dev, encoder, DRM_MODE_ENCODER_NONE);
> if (ret || !connector)
> return ret;
>
> diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
> index e253ba7bea9d..54d5066d90c7 100644
> --- a/include/drm/drm_simple_kms_helper.h
> +++ b/include/drm/drm_simple_kms_helper.h
> @@ -181,4 +181,11 @@ int drm_simple_display_pipe_init(struct drm_device *dev,
> const uint64_t *format_modifiers,
> struct drm_connector *connector);
>
> +int drm_simple_encoder_init(struct drm_device *dev,
> + struct drm_encoder *encoder,
> + int encoder_type);
> +
> +struct drm_encoder *drm_simple_encoder_create(struct drm_device *dev,
> + int encoder_type);
> +
> #endif /* __LINUX_DRM_SIMPLE_KMS_HELPER_H */
Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v17 4/9] mm: Introduce Reported pages
From: Alexander Duyck @ 2020-02-20 18:44 UTC (permalink / raw)
To: Mel Gorman, Alexander Duyck
Cc: kvm, david, mst, linux-kernel, linux-mm, akpm, yang.zhang.wz,
pagupta, konrad.wilk, nitesh, riel, willy, lcapitulino,
dave.hansen, wei.w.wang, aarcange, pbonzini, dan.j.williams,
mhocko, vbabka, osalvador
In-Reply-To: <20200219145511.GS3466@techsingularity.net>
On Wed, 2020-02-19 at 14:55 +0000, Mel Gorman wrote:
> On Tue, Feb 11, 2020 at 02:46:35PM -0800, Alexander Duyck wrote:
> > diff --git a/mm/page_reporting.c b/mm/page_reporting.c
> > new file mode 100644
> > index 000000000000..1047c6872d4f
> > --- /dev/null
> > +++ b/mm/page_reporting.c
> > @@ -0,0 +1,319 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include <linux/mm.h>
> > +#include <linux/mmzone.h>
> > +#include <linux/page_reporting.h>
> > +#include <linux/gfp.h>
> > +#include <linux/export.h>
> > +#include <linux/delay.h>
> > +#include <linux/scatterlist.h>
> > +
> > +#include "page_reporting.h"
> > +#include "internal.h"
> > +
> > +#define PAGE_REPORTING_DELAY (2 * HZ)
>
> I assume there is nothing special about 2 seconds other than "do some
> progress every so often".
Yes, nothing special. I played around with a few different values. I just
settled on 2 seconds as I figured with that and 1/16 of the list per pass
it came out to about 30 seconds which I felt is about the right time for a
fully utilized system to settle back to the inactive state.
> >
> > +static int
> > +page_reporting_cycle(struct page_reporting_dev_info *prdev, struct zone *zone,
> > + unsigned int order, unsigned int mt,
> > + struct scatterlist *sgl, unsigned int *offset)
> > +{
> > + struct free_area *area = &zone->free_area[order];
> > + struct list_head *list = &area->free_list[mt];
> > + unsigned int page_len = PAGE_SIZE << order;
> > + struct page *page, *next;
> > + int err = 0;
> > +
> > + /*
> > + * Perform early check, if free area is empty there is
> > + * nothing to process so we can skip this free_list.
> > + */
> > + if (list_empty(list))
> > + return err;
> > +
> > + spin_lock_irq(&zone->lock);
> > +
> > + /* loop through free list adding unreported pages to sg list */
> > + list_for_each_entry_safe(page, next, list, lru) {
> > + /* We are going to skip over the reported pages. */
> > + if (PageReported(page))
> > + continue;
> > +
> > + /* Attempt to pull page from list */
> > + if (!__isolate_free_page(page, order))
> > + break;
> > +
>
> Might want to note that you are breaking because the only reason to fail
> the isolation is that watermarks are not met and we are likely under
> memory pressure. It's not a big issue.
>
> However, while I think this is correct, it's hard to follow. This loop can
> be broken out of with pages still on the scatter gather list. The current
> flow guarantees that err will not be set at this point so the caller
> cleans it up so we always drain the list either here or in the caller.
I can probably submit a follow-up patch to update the comments. The reason
for not returning an error is because I didn't consider it an error that
we encountered the watermark and were not able to pull any more pages.
Instead I considered that the "stop" point for this pass and have it just
exit out of the loop and flush the data.
At the start of the next pass we will check against the low watermark
instead of the minimum watermark and if that check fails we will simply
stop reporting pages for the zone until additional pages are freed.
I can probably also update the description for page_reporting_cycle since
it may not be clear that the output for this is a partially filled in-
progress scatterlist so we always have to reporting any remaining entries
at the end of processing a given zone. It might make more sense if I move
the bits related to "leftover" in page_reporting_process_zone into their
own function.
> While I think it works, it's a bit fragile. I recommend putting a comment
> above this noting why it's safe and put a VM_WARN_ON_ONCE(err) before the
> break in case someone tries to change this in a years time and does not
> spot that the flow to reach page_reporting_drain *somewhere* is critical.
I assume this isn't about this section, but the section below?
> > + /* Add page to scatter list */
> > + --(*offset);
> > + sg_set_page(&sgl[*offset], page, page_len, 0);
> > +
> > + /* If scatterlist isn't full grab more pages */
> > + if (*offset)
> > + continue;
> > +
> > + /* release lock before waiting on report processing */
> > + spin_unlock_irq(&zone->lock);
> > +
> > + /* begin processing pages in local list */
> > + err = prdev->report(prdev, sgl, PAGE_REPORTING_CAPACITY);
> > +
So one thing I can do is probably add a comment here as well to more
thoroughly explain the reason why we wait to call the break until we are
in the block below.
> > + /* reset offset since the full list was reported */
> > + *offset = PAGE_REPORTING_CAPACITY;
> > +
> > + /* reacquire zone lock and resume processing */
> > + spin_lock_irq(&zone->lock);
> > +
> > + /* flush reported pages from the sg list */
> > + page_reporting_drain(prdev, sgl, PAGE_REPORTING_CAPACITY, !err);
> > +
> > + /*
> > + * Reset next to first entry, the old next isn't valid
> > + * since we dropped the lock to report the pages
> > + */
> > + next = list_first_entry(list, struct page, lru);
> > +
> > + /* exit on error */
> > + if (err)
> > + break;
And I assume you meant to add the VM_WARN_ON_ONCE here? The statement
above wouldn't make much sense since err would always be 0.
> > + }
> > +
> > + spin_unlock_irq(&zone->lock);
> > +
> > + return err;
> > +}
>
> I complained about the use of zone lock before but in this version, I
> think I'm ok with it. The lock is held for the free list manipulations
> which is what it's for. The state management with atomics seems
> reasonable.
>
> Otherwise I think this is ok and I think the implementation right. Of
> great importance to me was the allocator fast paths but they seem to be
> adequately protected by a static branch so
>
> Acked-by: Mel Gorman <mgorman@techsingularity.net>
>
> The ack applies regardless of whether you decide to document and
> defensively protect page_reporting_cycle against losing pages on the
> scatter/gather list but I do recommend it.
Thanks for reviewing this. I appreciate the feedback.
- Alex
^ permalink raw reply
* Re: [PATCH] ecryptfs: add mount option for specifying cipher driver.
From: Brian Kubisiak @ 2020-02-20 18:44 UTC (permalink / raw)
To: Tyler Hicks; +Cc: ecryptfs
In-Reply-To: <20200219163050.GA354535@elm>
> Have you looked into the possibility of increasing the priority of the
> implementation that you prefer on your SoC?
Yes, this should definitely be done, but I don't think it solves
the underlying issue. There are tradeoffs involved between the
security engine and the CPU implementation, and determining which
is "best" is dependent on what it is being used for. So I could
set the priority based on what I want for eCryptfs, but this also
affects every other consumer of the crypto API.
> I don't think allowing users to specify a cipher driver is a good idea.
> eCryptfs has always assumed that the crypto subsystem knows best about
> the ideal implementation of "cbc(aes)" and I believe that this is how
> the crypto subsystem expects eCryptfs to make use of their API.
I don't think this is true though. The crypto subsystem aims to
provide a sane default (ie whichever is the higher priority), but
allows overriding this choice if it would pick incorrectly. Since
it is making the choice with incomplete information (the crypto
subsystem can't know what you are using it for, so it doesn't
know which implementation is best), it makes sense that it could
be overridden from userspace. For example, the AF_ALG interface
to the crypto subsystem passes 'salg_name' directly from
userspace to allow this if needed.
I'd like to have this same flexibility in eCryptfs so I can
change which crypto implementation is used without affecting
other parts of the system.
> In addition to the design objection above, I'm worried about users
> shooting themselves in the foot with this mount option. For example,
> "ecryptfs_cipher_driver=ecb_aes_aesni" and
> "ecryptfs_cipher_driver=xts_aes_aesni" are accepted. eCryptfs is only
> implemented to operated in a (modified) CBC mode and letting users force
> their way into using anything else is dangerous/insecure.
I should probably also be checking that the requested driver
provides the correct algorithm, but haven't looked too closely
into that.
Brian
^ permalink raw reply
* Re: [PATCH 3/3] COLO: Optimize memory back-up process
From: Dr. David Alan Gilbert @ 2020-02-20 18:24 UTC (permalink / raw)
To: Hailiang Zhang; +Cc: danielcho, chen.zhang, qemu-devel, quintela
In-Reply-To: <20200217012049.22988-4-zhang.zhanghailiang@huawei.com>
* Hailiang Zhang (zhang.zhanghailiang@huawei.com) wrote:
> This patch will reduce the downtime of VM for the initial process,
> Privously, we copied all these memory in preparing stage of COLO
> while we need to stop VM, which is a time-consuming process.
> Here we optimize it by a trick, back-up every page while in migration
> process while COLO is enabled, though it affects the speed of the
> migration, but it obviously reduce the downtime of back-up all SVM'S
> memory in COLO preparing stage.
>
> Signed-off-by: Hailiang Zhang <zhang.zhanghailiang@huawei.com>
OK, I think this is right, but it took me quite a while to understand,
I think one of the comments below might not be right:
> ---
> migration/colo.c | 3 +++
> migration/ram.c | 35 +++++++++++++++++++++++++++--------
> migration/ram.h | 1 +
> 3 files changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/migration/colo.c b/migration/colo.c
> index d30c6bc4ad..febf010571 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -26,6 +26,7 @@
> #include "qemu/main-loop.h"
> #include "qemu/rcu.h"
> #include "migration/failover.h"
> +#include "migration/ram.h"
> #ifdef CONFIG_REPLICATION
> #include "replication.h"
> #endif
> @@ -906,6 +907,8 @@ void *colo_process_incoming_thread(void *opaque)
> */
> qemu_file_set_blocking(mis->from_src_file, true);
>
> + colo_incoming_start_dirty_log();
> +
> bioc = qio_channel_buffer_new(COLO_BUFFER_BASE_SIZE);
> fb = qemu_fopen_channel_input(QIO_CHANNEL(bioc));
> object_unref(OBJECT(bioc));
> diff --git a/migration/ram.c b/migration/ram.c
> index ed23ed1c7c..24a8aa3527 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2986,7 +2986,6 @@ int colo_init_ram_cache(void)
> }
> return -errno;
> }
> - memcpy(block->colo_cache, block->host, block->used_length);
> }
> }
>
> @@ -3005,12 +3004,16 @@ int colo_init_ram_cache(void)
> bitmap_set(block->bmap, 0, pages);
> }
> }
> +
> + return 0;
> +}
> +
> +void colo_incoming_start_dirty_log(void)
> +{
> ram_state = g_new0(RAMState, 1);
> ram_state->migration_dirty_pages = 0;
> qemu_mutex_init(&ram_state->bitmap_mutex);
> memory_global_dirty_log_start();
> -
> - return 0;
> }
>
> /* It is need to hold the global lock to call this helper */
> @@ -3348,7 +3351,7 @@ static int ram_load_precopy(QEMUFile *f)
>
> while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) {
> ram_addr_t addr, total_ram_bytes;
> - void *host = NULL;
> + void *host = NULL, *host_bak = NULL;
> uint8_t ch;
>
> /*
> @@ -3378,13 +3381,26 @@ static int ram_load_precopy(QEMUFile *f)
> if (flags & (RAM_SAVE_FLAG_ZERO | RAM_SAVE_FLAG_PAGE |
> RAM_SAVE_FLAG_COMPRESS_PAGE | RAM_SAVE_FLAG_XBZRLE)) {
> RAMBlock *block = ram_block_from_stream(f, flags);
> -
> /*
> - * After going into COLO, we should load the Page into colo_cache.
> + * After going into COLO, we should load the Page into colo_cache
> + * NOTE: We need to keep a copy of SVM's ram in colo_cache.
> + * Privously, we copied all these memory in preparing stage of COLO
> + * while we need to stop VM, which is a time-consuming process.
> + * Here we optimize it by a trick, back-up every page while in
> + * migration process while COLO is enabled, though it affects the
> + * speed of the migration, but it obviously reduce the downtime of
> + * back-up all SVM'S memory in COLO preparing stage.
> */
> - if (migration_incoming_in_colo_state()) {
> + if (migration_incoming_colo_enabled()) {
> host = colo_cache_from_block_offset(block, addr);
> - } else {
> + /*
> + * After going into COLO, load the Page into colo_cache.
> + */
> + if (!migration_incoming_in_colo_state()) {
> + host_bak = host;
> + }
> + }
> + if (!migration_incoming_in_colo_state()) {
> host = host_from_ram_block_offset(block, addr);
So this works out as quite complicated:
a) In normal migration we do the last one and just set:
host = host_from_ram_block_offset(block, addr);
host_bak = NULL
b) At the start, when colo_enabled, but !in_colo_state
host = colo_cache
host_bak = host
host = host_from_ram_block_offset
c) in_colo_state
host = colo_cache
host_bak = NULL
(b) is pretty confusing, setting host twice; can't we tidy that up?
Also, that last comment 'After going into COLO' I think is really
'Before COLO state, copy from ram into cache'
Dave
> }
> if (!host) {
> @@ -3506,6 +3522,9 @@ static int ram_load_precopy(QEMUFile *f)
> if (!ret) {
> ret = qemu_file_get_error(f);
> }
> + if (!ret && host_bak && host) {
> + memcpy(host_bak, host, TARGET_PAGE_SIZE);
> + }
> }
>
> ret |= wait_for_decompress_done();
> diff --git a/migration/ram.h b/migration/ram.h
> index a553d40751..5ceaff7cb4 100644
> --- a/migration/ram.h
> +++ b/migration/ram.h
> @@ -66,5 +66,6 @@ int ram_dirty_bitmap_reload(MigrationState *s, RAMBlock *rb);
> /* ram cache */
> int colo_init_ram_cache(void);
> void colo_release_ram_cache(void);
> +void colo_incoming_start_dirty_log(void);
>
> #endif
> --
> 2.21.0
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply
* Re: [PATCH] ASoC: tas2562: Add support for digital volume control
From: Mark Brown @ 2020-02-20 18:45 UTC (permalink / raw)
To: Dan Murphy; +Cc: lgirdwood, perex, tiwai, alsa-devel, linux-kernel
In-Reply-To: <20200220172721.10547-1-dmurphy@ti.com>
[-- Attachment #1: Type: text/plain, Size: 585 bytes --]
On Thu, Feb 20, 2020 at 11:27:21AM -0600, Dan Murphy wrote:
> + /* Set the Digital volume to -110dB */
> + ret = snd_soc_component_write(component, TAS2562_DVC_CFG4, 0x00);
> + if (ret)
> + return ret;
> + ret = snd_soc_component_write(component, TAS2562_DVC_CFG3, 0x00);
> + if (ret)
> + return ret;
> + ret = snd_soc_component_write(component, TAS2562_DVC_CFG2, 0x0d);
> + if (ret)
> + return ret;
> + ret = snd_soc_component_write(component, TAS2562_DVC_CFG1, 0x43);
> + if (ret)
> + return ret;
Is there a reason not to use the chip default here? Otherwise this
looks good.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] mm: memcontrol: asynchronous reclaim for memory.high
From: Tejun Heo @ 2020-02-20 18:45 UTC (permalink / raw)
To: Daniel Jordan
Cc: Johannes Weiner, Michal Hocko, Andrew Morton, Roman Gushchin,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, kernel-team-b10kYP2dOMg,
Peter Zijlstra
In-Reply-To: <20200220182326.ubcjycaubgykiy6e-S51bK0XF4qpuJJETbFA3a0B3C2bhBk7L0E9HWUfgJXw@public.gmane.org>
Hello,
On Thu, Feb 20, 2020 at 01:23:26PM -0500, Daniel Jordan wrote:
> The amount of work wouldn't seem to matter as long as the kernel thread stays
> in the cgroup and lives long enough. There's only the one-time cost of
> attaching it when it's forked. That seems doable for unbound workqueues (the
> async reclaim), but may not be for the network packets.
The setup cost can be lazy optimized but it'd still have to bounce the
tiny pieces of work to different threads instead of processing them in
one fell swoop from the same context, which most likely is gonna be
untenably expensive.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH] mm: memcontrol: asynchronous reclaim for memory.high
From: Tejun Heo @ 2020-02-20 18:45 UTC (permalink / raw)
To: Daniel Jordan
Cc: Johannes Weiner, Michal Hocko, Andrew Morton, Roman Gushchin,
linux-mm, cgroups, linux-kernel, kernel-team, Peter Zijlstra
In-Reply-To: <20200220182326.ubcjycaubgykiy6e@ca-dmjordan1.us.oracle.com>
Hello,
On Thu, Feb 20, 2020 at 01:23:26PM -0500, Daniel Jordan wrote:
> The amount of work wouldn't seem to matter as long as the kernel thread stays
> in the cgroup and lives long enough. There's only the one-time cost of
> attaching it when it's forked. That seems doable for unbound workqueues (the
> async reclaim), but may not be for the network packets.
The setup cost can be lazy optimized but it'd still have to bounce the
tiny pieces of work to different threads instead of processing them in
one fell swoop from the same context, which most likely is gonna be
untenably expensive.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH] sound: Replace zero-length array with flexible-array member
From: Mark Brown @ 2020-02-20 18:45 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Jaroslav Kysela, Takashi Iwai, Liam Girdwood, Clemens Ladisch,
Takashi Sakamoto, Cezary Rojewski, Pierre-Louis Bossart, Jie Yang,
alsa-devel, linux-kernel
In-Reply-To: <20200220132420.GA29765@embeddedor>
[-- Attachment #1: Type: text/plain, Size: 349 bytes --]
On Thu, Feb 20, 2020 at 07:24:20AM -0600, Gustavo A. R. Silva wrote:
> The current codebase makes use of the zero-length array language
> extension to the C90 standard, but the preferred mechanism to declare
> variable-length types such as these ones is a flexible array member[1][2],
> introduced in C99:
Acked-by: Mark Brown <broonie@kernel.org>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] ASoC: tas2562: Add support for digital volume control
From: Mark Brown @ 2020-02-20 18:45 UTC (permalink / raw)
To: Dan Murphy; +Cc: linux-kernel, alsa-devel, tiwai, lgirdwood
In-Reply-To: <20200220172721.10547-1-dmurphy@ti.com>
[-- Attachment #1: Type: text/plain, Size: 585 bytes --]
On Thu, Feb 20, 2020 at 11:27:21AM -0600, Dan Murphy wrote:
> + /* Set the Digital volume to -110dB */
> + ret = snd_soc_component_write(component, TAS2562_DVC_CFG4, 0x00);
> + if (ret)
> + return ret;
> + ret = snd_soc_component_write(component, TAS2562_DVC_CFG3, 0x00);
> + if (ret)
> + return ret;
> + ret = snd_soc_component_write(component, TAS2562_DVC_CFG2, 0x0d);
> + if (ret)
> + return ret;
> + ret = snd_soc_component_write(component, TAS2562_DVC_CFG1, 0x43);
> + if (ret)
> + return ret;
Is there a reason not to use the chip default here? Otherwise this
looks good.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v6 1/2] mm: Add MREMAP_DONTUNMAP to mremap().
From: Brian Geffon @ 2020-02-20 18:45 UTC (permalink / raw)
To: Minchan Kim
Cc: Andrew Morton, Michael S . Tsirkin, Arnd Bergmann, LKML, linux-mm,
Linux API, Andy Lutomirski, Will Deacon, Andrea Arcangeli,
Sonny Rao, Joel Fernandes, Yu Zhao, Jesse Barnes, Florian Weimer,
Kirill A . Shutemov
In-Reply-To: <CADyq12zUEq9kcyuR_Qm9MrU1ii-+9n8T2hK6QNzj=kH5zn0VrA@mail.gmail.com>
Sorry I should clarify that this is the behavior with MREMAP_FIXED is
used, and to expand on that, it would potentially even have unmapped
the region at the destination address and then fail in vma_to_resize
too, so I hope that explains why that check landed there. But should
these situations be considered a bug?
Brian
On Thu, Feb 20, 2020 at 10:36 AM Brian Geffon <bgeffon@google.com> wrote:
>
> Hi Minchan,
>
> > And here we got error if the addr is in non-anonymous-private vma so the
> > syscall will fail but old vma is gone? I guess it's not your intention?
>
> This is exactly what happens today in several situations, because
> vma_to_resize is called unconditionally. For example if the old vma
> has VM_HUGETLB and old_len < new_len it would have unmapped a portion
> and then in vma_to_resize returned -EINVAL, similarly when old_len = 0
> with a non-sharable mapping it will have called do_munmap only to fail
> in vma_to_resize, if the vma has VM_DONTEXPAND set and you shrink the
> size with old_len < new_len it would return -EFAULT after having done
> the unmap on the decreased portion. So I followed the pattern to keep
> the change simple and maintain consistency with existing behavior.
>
> But with that being said, Kirill made the point that resizing a VMA
> while also using MREMAP_DONTUNMAP doesn't have any clear use case and
> I agree with that, I'm unable to think of a situation where you'd want
> to resize a VMA and use MREMAP_DONTUNMAP. So I'm tempted to mail a new
> version which returns -EINVAL if old_len != new_len that would resolve
> this concern here as nothing would be unmapped ever at the old
> position add it would clean up the change to very few lines of code.
>
> What do you think?
>
> Thank you for taking the time to review.
>
> Brian
^ permalink raw reply
* Support for early wakeup in DRM
From: jsanka @ 2020-02-20 18:45 UTC (permalink / raw)
To: 'Sean Paul', 'Rob Clark', dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 1177 bytes --]
Hello All,
I am seeking recommendations for DRM compatible methods of updating the HW
other than frame commit path. When exiting idle/runtime_suspend, the driver
votes for a bunch of resources including power/clk/bandwidth as a part of
first commit handling. This usually adds a few millisecond delay before
processing the frame. The requirement is to find possible ways to reduce
this delay by providing an early intimation to the framework to "prepare"
the HW by voting for the resources and keep the HW ready to process an
imminent frame commit. Especially in performance oriented Automotive world,
these delays are very time critical and we are working on ways to mitigate
them.
DRM framework converges all the parameters affecting the HW in terms of DRM
properties in a single COMMIT call. To address the above issue, we need a
parallel channel which should allow the framework to make necessary changes
to the HW without violating the master access privileges.
Before resorting to custom downstream ways, I want to check with the
community for folks who might have encountered and resolved such issues.
Thanks and Regards,
Jeykumar S
Qualcomm Inc.
[-- Attachment #1.2: Type: text/html, Size: 3473 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH] sound: Replace zero-length array with flexible-array member
From: Mark Brown @ 2020-02-20 18:45 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Pierre-Louis Bossart, Cezary Rojewski, alsa-devel, Jie Yang,
Takashi Iwai, Clemens Ladisch, Liam Girdwood, linux-kernel
In-Reply-To: <20200220132420.GA29765@embeddedor>
[-- Attachment #1: Type: text/plain, Size: 349 bytes --]
On Thu, Feb 20, 2020 at 07:24:20AM -0600, Gustavo A. R. Silva wrote:
> The current codebase makes use of the zero-length array language
> extension to the C90 standard, but the preferred mechanism to declare
> variable-length types such as these ones is a flexible array member[1][2],
> introduced in C99:
Acked-by: Mark Brown <broonie@kernel.org>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [Intel-gfx] [PATCH 09/12] drm: Shrink drm_display_mode timings
From: Ville Syrjälä @ 2020-02-20 18:47 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, dri-devel
In-Reply-To: <20200220181908.GX2363188@phenom.ffwll.local>
On Thu, Feb 20, 2020 at 07:19:08PM +0100, Daniel Vetter wrote:
> On Wed, Feb 19, 2020 at 10:35:41PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Store the timings (apart from the clock) as u16. The uapi mode
> > struct already uses u16 for everything so using something bigger
> > internally doesn't really help us.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Makes sense I guess. This could mean some implicit pointer math is now no
> longer auto-upgraded to big enough integers though ...
u16 promotes to int. So can't really see how this would go wrong. Well,
unless someone is using these to store some larger intermediate values.
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> > ---
> > drivers/gpu/drm/drm_modes.c | 7 ------
> > include/drm/drm_modes.h | 46 ++++++++++++++++++-------------------
> > 2 files changed, 23 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > index 0e7c9ba241c4..cc9fc52f9f7c 100644
> > --- a/drivers/gpu/drm/drm_modes.c
> > +++ b/drivers/gpu/drm/drm_modes.c
> > @@ -1917,13 +1917,6 @@ EXPORT_SYMBOL(drm_mode_create_from_cmdline_mode);
> > void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out,
> > const struct drm_display_mode *in)
> > {
> > - WARN(in->hdisplay > USHRT_MAX || in->hsync_start > USHRT_MAX ||
> > - in->hsync_end > USHRT_MAX || in->htotal > USHRT_MAX ||
> > - in->hskew > USHRT_MAX || in->vdisplay > USHRT_MAX ||
> > - in->vsync_start > USHRT_MAX || in->vsync_end > USHRT_MAX ||
> > - in->vtotal > USHRT_MAX || in->vscan > USHRT_MAX,
> > - "timing values too large for mode info\n");
> > -
> > out->clock = in->clock;
> > out->hdisplay = in->hdisplay;
> > out->hsync_start = in->hsync_start;
> > diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> > index b28c0234fcd7..b585074945b5 100644
> > --- a/include/drm/drm_modes.h
> > +++ b/include/drm/drm_modes.h
> > @@ -278,16 +278,16 @@ struct drm_display_mode {
> > * Pixel clock in kHz.
> > */
> > int clock; /* in kHz */
> > - int hdisplay;
> > - int hsync_start;
> > - int hsync_end;
> > - int htotal;
> > - int hskew;
> > - int vdisplay;
> > - int vsync_start;
> > - int vsync_end;
> > - int vtotal;
> > - int vscan;
> > + u16 hdisplay;
> > + u16 hsync_start;
> > + u16 hsync_end;
> > + u16 htotal;
> > + u16 hskew;
> > + u16 vdisplay;
> > + u16 vsync_start;
> > + u16 vsync_end;
> > + u16 vtotal;
> > + u16 vscan;
> > /**
> > * @flags:
> > *
> > @@ -356,19 +356,19 @@ struct drm_display_mode {
> > * difference is exactly a factor of 10.
> > */
> > int crtc_clock;
> > - int crtc_hdisplay;
> > - int crtc_hblank_start;
> > - int crtc_hblank_end;
> > - int crtc_hsync_start;
> > - int crtc_hsync_end;
> > - int crtc_htotal;
> > - int crtc_hskew;
> > - int crtc_vdisplay;
> > - int crtc_vblank_start;
> > - int crtc_vblank_end;
> > - int crtc_vsync_start;
> > - int crtc_vsync_end;
> > - int crtc_vtotal;
> > + u16 crtc_hdisplay;
> > + u16 crtc_hblank_start;
> > + u16 crtc_hblank_end;
> > + u16 crtc_hsync_start;
> > + u16 crtc_hsync_end;
> > + u16 crtc_htotal;
> > + u16 crtc_hskew;
> > + u16 crtc_vdisplay;
> > + u16 crtc_vblank_start;
> > + u16 crtc_vblank_end;
> > + u16 crtc_vsync_start;
> > + u16 crtc_vsync_end;
> > + u16 crtc_vtotal;
> >
> > /**
> > * @private_flags:
> > --
> > 2.24.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [Intel-gfx] [PATCH 09/12] drm: Shrink drm_display_mode timings
From: Ville Syrjälä @ 2020-02-20 18:47 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, dri-devel
In-Reply-To: <20200220181908.GX2363188@phenom.ffwll.local>
On Thu, Feb 20, 2020 at 07:19:08PM +0100, Daniel Vetter wrote:
> On Wed, Feb 19, 2020 at 10:35:41PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Store the timings (apart from the clock) as u16. The uapi mode
> > struct already uses u16 for everything so using something bigger
> > internally doesn't really help us.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Makes sense I guess. This could mean some implicit pointer math is now no
> longer auto-upgraded to big enough integers though ...
u16 promotes to int. So can't really see how this would go wrong. Well,
unless someone is using these to store some larger intermediate values.
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> > ---
> > drivers/gpu/drm/drm_modes.c | 7 ------
> > include/drm/drm_modes.h | 46 ++++++++++++++++++-------------------
> > 2 files changed, 23 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > index 0e7c9ba241c4..cc9fc52f9f7c 100644
> > --- a/drivers/gpu/drm/drm_modes.c
> > +++ b/drivers/gpu/drm/drm_modes.c
> > @@ -1917,13 +1917,6 @@ EXPORT_SYMBOL(drm_mode_create_from_cmdline_mode);
> > void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out,
> > const struct drm_display_mode *in)
> > {
> > - WARN(in->hdisplay > USHRT_MAX || in->hsync_start > USHRT_MAX ||
> > - in->hsync_end > USHRT_MAX || in->htotal > USHRT_MAX ||
> > - in->hskew > USHRT_MAX || in->vdisplay > USHRT_MAX ||
> > - in->vsync_start > USHRT_MAX || in->vsync_end > USHRT_MAX ||
> > - in->vtotal > USHRT_MAX || in->vscan > USHRT_MAX,
> > - "timing values too large for mode info\n");
> > -
> > out->clock = in->clock;
> > out->hdisplay = in->hdisplay;
> > out->hsync_start = in->hsync_start;
> > diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> > index b28c0234fcd7..b585074945b5 100644
> > --- a/include/drm/drm_modes.h
> > +++ b/include/drm/drm_modes.h
> > @@ -278,16 +278,16 @@ struct drm_display_mode {
> > * Pixel clock in kHz.
> > */
> > int clock; /* in kHz */
> > - int hdisplay;
> > - int hsync_start;
> > - int hsync_end;
> > - int htotal;
> > - int hskew;
> > - int vdisplay;
> > - int vsync_start;
> > - int vsync_end;
> > - int vtotal;
> > - int vscan;
> > + u16 hdisplay;
> > + u16 hsync_start;
> > + u16 hsync_end;
> > + u16 htotal;
> > + u16 hskew;
> > + u16 vdisplay;
> > + u16 vsync_start;
> > + u16 vsync_end;
> > + u16 vtotal;
> > + u16 vscan;
> > /**
> > * @flags:
> > *
> > @@ -356,19 +356,19 @@ struct drm_display_mode {
> > * difference is exactly a factor of 10.
> > */
> > int crtc_clock;
> > - int crtc_hdisplay;
> > - int crtc_hblank_start;
> > - int crtc_hblank_end;
> > - int crtc_hsync_start;
> > - int crtc_hsync_end;
> > - int crtc_htotal;
> > - int crtc_hskew;
> > - int crtc_vdisplay;
> > - int crtc_vblank_start;
> > - int crtc_vblank_end;
> > - int crtc_vsync_start;
> > - int crtc_vsync_end;
> > - int crtc_vtotal;
> > + u16 crtc_hdisplay;
> > + u16 crtc_hblank_start;
> > + u16 crtc_hblank_end;
> > + u16 crtc_hsync_start;
> > + u16 crtc_hsync_end;
> > + u16 crtc_htotal;
> > + u16 crtc_hskew;
> > + u16 crtc_vdisplay;
> > + u16 crtc_vblank_start;
> > + u16 crtc_vblank_end;
> > + u16 crtc_vsync_start;
> > + u16 crtc_vsync_end;
> > + u16 crtc_vtotal;
> >
> > /**
> > * @private_flags:
> > --
> > 2.24.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
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.