* [PATCH 01/11] poller: Give pollers a name
2020-04-22 7:54 [PATCH v4 00/11] Protect code from pollers Sascha Hauer
@ 2020-04-22 7:54 ` Sascha Hauer
2020-04-22 7:54 ` [PATCH 02/11] poller: Add a poller command Sascha Hauer
` (9 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Sascha Hauer @ 2020-04-22 7:54 UTC (permalink / raw)
To: Barebox List; +Cc: Edmund Henniges, Daniel Glöckner
It helps debugging when pollers have a name, so give them one.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
common/poller.c | 8 +++++---
common/ratp/ratp.c | 2 +-
drivers/input/gpio_keys.c | 2 +-
drivers/input/imx_keypad.c | 2 +-
drivers/input/input.c | 2 +-
drivers/input/qt1070.c | 2 +-
drivers/input/twl6030_pwrbtn.c | 2 +-
drivers/led/core.c | 2 +-
drivers/usb/gadget/udc-core.c | 2 +-
drivers/watchdog/wd_core.c | 2 +-
include/poller.h | 5 +++--
11 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/common/poller.c b/common/poller.c
index 32795b641f..b1a2122f91 100644
--- a/common/poller.c
+++ b/common/poller.c
@@ -16,11 +16,12 @@
static LIST_HEAD(poller_list);
static int poller_active;
-int poller_register(struct poller_struct *poller)
+int poller_register(struct poller_struct *poller, const char *name)
{
if (poller->registered)
return -EBUSY;
+ poller->name = xstrdup(name);
list_add_tail(&poller->list, &poller_list);
poller->registered = 1;
@@ -35,6 +36,7 @@ int poller_unregister(struct poller_struct *poller)
list_del(&poller->list);
poller->registered = 0;
+ free(poller->name);
return 0;
}
@@ -92,12 +94,12 @@ int poller_call_async(struct poller_async *pa, uint64_t delay_ns,
return 0;
}
-int poller_async_register(struct poller_async *pa)
+int poller_async_register(struct poller_async *pa, const char *name)
{
pa->poller.func = poller_async_callback;
pa->active = 0;
- return poller_register(&pa->poller);
+ return poller_register(&pa->poller, name);
}
int poller_async_unregister(struct poller_async *pa)
diff --git a/common/ratp/ratp.c b/common/ratp/ratp.c
index e84ad22167..9625a31b18 100644
--- a/common/ratp/ratp.c
+++ b/common/ratp/ratp.c
@@ -454,7 +454,7 @@ int barebox_ratp(struct console_device *cdev)
if (ret < 0)
goto out;
- ret = poller_register(&ctx->poller);
+ ret = poller_register(&ctx->poller, "ratp");
if (ret)
goto out1;
diff --git a/drivers/input/gpio_keys.c b/drivers/input/gpio_keys.c
index 38c0f11535..11d598c402 100644
--- a/drivers/input/gpio_keys.c
+++ b/drivers/input/gpio_keys.c
@@ -166,7 +166,7 @@ static int __init gpio_keys_probe(struct device_d *dev)
if (ret)
return ret;
- ret = poller_register(&gk->poller);
+ ret = poller_register(&gk->poller, dev_name(dev));
if (ret)
return ret;
diff --git a/drivers/input/imx_keypad.c b/drivers/input/imx_keypad.c
index 44ff9b7856..6757fac72b 100644
--- a/drivers/input/imx_keypad.c
+++ b/drivers/input/imx_keypad.c
@@ -410,7 +410,7 @@ static int __init imx_keypad_probe(struct device_d *dev)
keypad->poller.func = imx_keypad_check_for_events;
- ret = poller_register(&keypad->poller);
+ ret = poller_register(&keypad->poller, dev_name(dev));
if (ret)
return ret;
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 1e8f6e178e..bcc8667417 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -201,7 +201,7 @@ static int input_init(void)
ic->fifo = kfifo_alloc(32);
ic->notifier.notify = input_console_notify;
input_register_notfier(&ic->notifier);
- poller_async_register(&ic->poller);
+ poller_async_register(&ic->poller, "input");
return console_register(&ic->console);
}
diff --git a/drivers/input/qt1070.c b/drivers/input/qt1070.c
index 59acee5c39..9e1dcc57ee 100644
--- a/drivers/input/qt1070.c
+++ b/drivers/input/qt1070.c
@@ -269,7 +269,7 @@ static int qt1070_probe(struct device_d *dev)
console_register(&data->cdev);
- ret = poller_register(&data->poller);
+ ret = poller_register(&data->poller, dev_name(dev));
if (ret)
goto err;
diff --git a/drivers/input/twl6030_pwrbtn.c b/drivers/input/twl6030_pwrbtn.c
index fc4c728778..481688b4a9 100644
--- a/drivers/input/twl6030_pwrbtn.c
+++ b/drivers/input/twl6030_pwrbtn.c
@@ -97,7 +97,7 @@ static int __init twl6030_pwrbtn_probe(struct device_d *dev)
idata->cdev.getc = twl6030_pwrbtn_getc;
console_register(&idata->cdev);
- return poller_register(&idata->poller);
+ return poller_register(&idata->poller, dev_name(dev));
}
static struct driver_d twl6030_pwrbtn_driver = {
diff --git a/drivers/led/core.c b/drivers/led/core.c
index e727148a24..4cce5dfc97 100644
--- a/drivers/led/core.c
+++ b/drivers/led/core.c
@@ -212,7 +212,7 @@ static struct poller_struct led_poller = {
static int led_blink_init(void)
{
- return poller_register(&led_poller);
+ return poller_register(&led_poller, "led");
}
late_initcall(led_blink_init);
diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
index 096f05ed48..126d76e8bb 100644
--- a/drivers/usb/gadget/udc-core.c
+++ b/drivers/usb/gadget/udc-core.c
@@ -319,7 +319,7 @@ static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *dri
if (udc->gadget->ops->udc_poll) {
udc->poller.func = udc_poll_driver;
- ret = poller_register(&udc->poller);
+ ret = poller_register(&udc->poller, dev_name(&udc->dev));
if (ret)
return ret;
}
diff --git a/drivers/watchdog/wd_core.c b/drivers/watchdog/wd_core.c
index 34040408f7..a17234f4b6 100644
--- a/drivers/watchdog/wd_core.c
+++ b/drivers/watchdog/wd_core.c
@@ -116,7 +116,7 @@ static int watchdog_register_poller(struct watchdog *wd)
struct param_d *p;
int ret;
- ret = poller_async_register(&wd->poller);
+ ret = poller_async_register(&wd->poller, dev_name(&wd->dev));
if (ret)
return ret;
diff --git a/include/poller.h b/include/poller.h
index b22b8a1b89..886557252b 100644
--- a/include/poller.h
+++ b/include/poller.h
@@ -12,9 +12,10 @@ struct poller_struct {
void (*func)(struct poller_struct *poller);
int registered;
struct list_head list;
+ char *name;
};
-int poller_register(struct poller_struct *poller);
+int poller_register(struct poller_struct *poller, const char *name);
int poller_unregister(struct poller_struct *poller);
struct poller_async;
@@ -27,7 +28,7 @@ struct poller_async {
int active;
};
-int poller_async_register(struct poller_async *pa);
+int poller_async_register(struct poller_async *pa, const char *name);
int poller_async_unregister(struct poller_async *pa);
int poller_call_async(struct poller_async *pa, uint64_t delay_ns,
--
2.26.1
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 02/11] poller: Add a poller command
2020-04-22 7:54 [PATCH v4 00/11] Protect code from pollers Sascha Hauer
2020-04-22 7:54 ` [PATCH 01/11] poller: Give pollers a name Sascha Hauer
@ 2020-04-22 7:54 ` Sascha Hauer
2020-04-22 7:54 ` [PATCH 03/11] Introduce slices Sascha Hauer
` (8 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Sascha Hauer @ 2020-04-22 7:54 UTC (permalink / raw)
To: Barebox List; +Cc: Edmund Henniges, Daniel Glöckner
The poller command allows to print which pollers are registered and also
how many times we can run the registered pollers in one second.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
commands/Kconfig | 9 ++++++
common/poller.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 80 insertions(+)
diff --git a/commands/Kconfig b/commands/Kconfig
index a0c2828983..7fb47b8fb5 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -260,6 +260,15 @@ config CMD_MMC_EXTCSD
-y don't request when writing to one time programmable fields
__CAUTION__: this could damage the device!
+config CMD_POLLER
+ tristate
+ prompt "poller"
+ depends on POLLER
+ help
+ Pollers are functions that are running in the background whenever code executes
+ is_timeout() or one of the various delay functions. The poller command prints
+ informations about registered pollers.
+
# end Information commands
endmenu
diff --git a/common/poller.c b/common/poller.c
index b1a2122f91..95f828b439 100644
--- a/common/poller.c
+++ b/common/poller.c
@@ -121,3 +121,74 @@ void poller_call(void)
poller_active = 0;
}
+
+#if defined CONFIG_CMD_POLLER
+
+#include <command.h>
+#include <getopt.h>
+
+static void poller_time(void)
+{
+ uint64_t start = get_time_ns();
+ int i = 0;
+
+ /*
+ * How many times we can run the registered pollers in one second?
+ *
+ * A low number here may point to problems with pollers taking too
+ * much time.
+ */
+ while (!is_timeout(start, SECOND))
+ i++;
+
+ printf("%d poller calls in 1s\n", i);
+}
+
+static void poller_info(void)
+{
+ struct poller_struct *poller;
+
+ printf("Registered pollers:\n");
+
+ if (list_empty(&poller_list)) {
+ printf("<none>\n");
+ return;
+ }
+
+ list_for_each_entry(poller, &poller_list, list)
+ printf("%s\n", poller->name);
+}
+
+BAREBOX_CMD_HELP_START(poller)
+BAREBOX_CMD_HELP_TEXT("print info about registered pollers")
+BAREBOX_CMD_HELP_TEXT("")
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT ("-i", "Print information about registered pollers")
+BAREBOX_CMD_HELP_OPT ("-t", "measure how many pollers we run in 1s")
+BAREBOX_CMD_HELP_END
+
+static int do_poller(int argc, char *argv[])
+{
+ int opt;
+
+ while ((opt = getopt(argc, argv, "it")) > 0) {
+ switch (opt) {
+ case 'i':
+ poller_info();
+ return 0;
+ case 't':
+ poller_time();
+ return 0;
+ }
+ }
+
+ return COMMAND_ERROR_USAGE;
+}
+
+BAREBOX_CMD_START(poller)
+ .cmd = do_poller,
+ BAREBOX_CMD_DESC("print info about registered pollers")
+ BAREBOX_CMD_GROUP(CMD_GRP_MISC)
+ BAREBOX_CMD_HELP(cmd_poller_help)
+BAREBOX_CMD_END
+#endif
--
2.26.1
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 03/11] Introduce slices
2020-04-22 7:54 [PATCH v4 00/11] Protect code from pollers Sascha Hauer
2020-04-22 7:54 ` [PATCH 01/11] poller: Give pollers a name Sascha Hauer
2020-04-22 7:54 ` [PATCH 02/11] poller: Add a poller command Sascha Hauer
@ 2020-04-22 7:54 ` Sascha Hauer
2020-05-08 18:01 ` Daniel Glöckner
2020-04-22 7:54 ` [PATCH 04/11] net: Add a slice to struct eth_device Sascha Hauer
` (7 subsequent siblings)
10 siblings, 1 reply; 14+ messages in thread
From: Sascha Hauer @ 2020-04-22 7:54 UTC (permalink / raw)
To: Barebox List; +Cc: Edmund Henniges, Daniel Glöckner
slices, the barebox idea of locking
barebox has pollers which execute code in the background whenever one of the
delay functions (udelay, mdelay, ...) or is_timeout() are called. This
introduces resource problems when some device triggers a poller by calling
a delay function and then the poller code calls into the same device again.
As an example consider a I2C GPIO expander which drives a LED which shall
be used as a heartbeat LED:
poller -> LED on/off -> GPIO high/low -> I2C transfer
The I2C controller has a timeout loop using is_timeout() and thus can trigger
a poller run. With this the following can happen during an unrelated I2C
transfer:
I2C transfer -> is_timeout() -> poller -> LED on/off -> GPIO high/low -> I2C transfer
We end up with issuing an I2C transfer during another I2C transfer and
things go downhill.
Due to the lack of interrupts we can't do real locking in barebox. We use
a mechanism called slices instead. A slice describes a resource to which
other slices can be attached. Whenever a slice is needed it must be acquired.
Acquiring a slice never fails, it just increases the acquired counter of
the slice and its dependent slices. when a slice shall be used inside a
poller it must first be tested if the slice is already in use. If it is,
we can't do the operation on the slice now and must return and hope that
we have more luck in the next poller call.
slices can be attached other slices as dependencies. In the example above
LED driver would add a dependency to the GPIO controller and the GPIO driver
would add a dependency to the I2C bus:
GPIO driver probe:
slice_add(&gpio->slice, i2c_device_slice(i2cdev));
LED driver probe:
slice_add(&led->slice, gpio_slice(gpio));
The GPIO code would call slice_acquire(&gpio->slice) before doing any
operation on the GPIO chip providing this GPIO, likewise the I2C core
would call slice_acquire(&i2cbus->slice) before doing an operation on
this I2C bus.
The heartbeat poller code would call slice_acquired(led_slice(led)) and
only continue when the slice is not acquired.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
commands/Kconfig | 9 ++
common/Kconfig | 4 +
common/Makefile | 1 +
common/slice.c | 294 +++++++++++++++++++++++++++++++++++++++++++++++
include/slice.h | 31 +++++
5 files changed, 339 insertions(+)
create mode 100644 common/slice.c
create mode 100644 include/slice.h
diff --git a/commands/Kconfig b/commands/Kconfig
index 7fb47b8fb5..5ff6454427 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -269,6 +269,15 @@ config CMD_POLLER
is_timeout() or one of the various delay functions. The poller command prints
informations about registered pollers.
+config CMD_SLICE
+ tristate
+ prompt "slice"
+ depends on SLICE
+ help
+ slices are a way to protect resources from being accessed by pollers. The slice
+ command can be used to print informations about slices and also to manipulate
+ them on the command line for debugging purposes.
+
# end Information commands
endmenu
diff --git a/common/Kconfig b/common/Kconfig
index 400c0553cf..bd2aebac75 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -913,6 +913,10 @@ config BAREBOXCRC32_TARGET
config POLLER
bool "generic polling infrastructure"
+config SLICE
+ depends on POLLER
+ default y
+
config STATE
bool "generic state infrastructure"
select CRC32
diff --git a/common/Makefile b/common/Makefile
index 84463b4d48..16f14db41c 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -11,6 +11,7 @@ obj-y += bootsource.o
obj-$(CONFIG_ELF) += elf.o
obj-y += restart.o
obj-y += poweroff.o
+obj-y += slice.o
obj-$(CONFIG_MACHINE_ID) += machine_id.o
obj-$(CONFIG_AUTO_COMPLETE) += complete.o
obj-y += version.o
diff --git a/common/slice.c b/common/slice.c
new file mode 100644
index 0000000000..b4460335d7
--- /dev/null
+++ b/common/slice.c
@@ -0,0 +1,294 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define pr_fmt(fmt) "slice: " fmt
+
+#include <common.h>
+#include <slice.h>
+
+/*
+ * slices, the barebox idea of locking
+ *
+ * barebox has pollers which execute code in the background whenever one of the
+ * delay functions (udelay, mdelay, ...) or is_timeout() are called. This
+ * introduces resource problems when some device triggers a poller by calling
+ * a delay function and then the poller code calls into the same device again.
+ *
+ * As an example consider a I2C GPIO expander which drives a LED which shall
+ * be used as a heartbeat LED:
+ *
+ * poller -> LED on/off -> GPIO high/low -> I2C transfer
+ *
+ * The I2C controller has a timeout loop using is_timeout() and thus can trigger
+ * a poller run. With this the following can happen during an unrelated I2C
+ * transfer:
+ *
+ * I2C transfer -> is_timeout() -> poller -> LED on/off -> GPIO high/low -> I2C transfer
+ *
+ * We end up with issuing an I2C transfer during another I2C transfer and
+ * things go downhill.
+ *
+ * Due to the lack of interrupts we can't do real locking in barebox. We use
+ * a mechanism called slices instead. A slice describes a resource to which
+ * other slices can be attached. Whenever a slice is needed it must be acquired.
+ * Acquiring a slice never fails, it just increases the acquired counter of
+ * the slice and its dependent slices. when a slice shall be used inside a
+ * poller it must first be tested if the slice is already in use. If it is,
+ * we can't do the operation on the slice now and must return and hope that
+ * we have more luck in the next poller call.
+ *
+ * slices can be attached other slices as dependencies. In the example above
+ * LED driver would add a dependency to the GPIO controller and the GPIO driver
+ * would add a dependency to the I2C bus:
+ *
+ * GPIO driver probe:
+ *
+ * slice_add(&gpio->slice, i2c_device_slice(i2cdev));
+ *
+ * LED driver probe:
+ *
+ * slice_add(&led->slice, gpio_slice(gpio));
+ *
+ * The GPIO code would call slice_acquire(&gpio->slice) before doing any
+ * operation on the GPIO chip providing this GPIO, likewise the I2C core
+ * would call slice_acquire(&i2cbus->slice) before doing an operation on
+ * this I2C bus.
+ *
+ * The heartbeat poller code would call slice_acquired(led_slice(led)) and
+ * only continue when the slice is not acquired.
+ */
+
+/*
+ * slices are not required to have a device and thus a name, but it really
+ * helps debugging when it has one.
+ */
+static const char *slice_name(struct slice *slice)
+{
+ return slice->name;
+}
+
+static void __slice_acquire(struct slice *slice, int add)
+{
+ slice->acquired += add;
+
+ if (slice->acquired < 0) {
+ pr_err("Slice %s acquired count drops below zero\n",
+ slice_name(slice));
+ dump_stack();
+ slice->acquired = 0;
+ return;
+ }
+}
+
+/**
+ * slice_acquire: acquire a slice
+ * @slice: The slice to acquire
+ *
+ * This acquires a slice and its dependencies
+ */
+void slice_acquire(struct slice *slice)
+{
+ __slice_acquire(slice, 1);
+}
+
+/**
+ * slice_release: release a slice
+ * @slice: The slice to release
+ *
+ * This releases a slice and its dependencies
+ */
+void slice_release(struct slice *slice)
+{
+ __slice_acquire(slice, -1);
+}
+
+/**
+ * slice_acquired: test if a slice is acquired
+ * @slice: The slice to test
+ *
+ * This tests if a slice is acquired. Returns true if it is, false otherwise
+ */
+bool slice_acquired(struct slice *slice)
+{
+ struct slice_entry *se;
+ bool ret = false;
+
+ if (slice->acquired > 0)
+ return true;
+
+ if (slice->acquired < 0) {
+ pr_err("Recursive dependency detected in slice %s\n",
+ slice_name(slice));
+ panic("Cannot continue");
+ }
+
+ slice->acquired = -1;
+
+ list_for_each_entry(se, &slice->deps, list)
+ if (slice_acquired(se->slice)) {
+ ret = true;
+ break;
+ }
+
+ slice->acquired = 0;
+
+ return ret;
+}
+
+static void __slice_debug_acquired(struct slice *slice, int level)
+{
+ struct slice_entry *se;
+
+ pr_debug("%*s%s: %d\n", level * 4, "",
+ slice_name(slice),
+ slice->acquired);
+
+ list_for_each_entry(se, &slice->deps, list)
+ __slice_debug_acquired(se->slice, level + 1);
+}
+
+/**
+ * slice_debug_acquired: print the acquired state of a slice
+ *
+ * @slice: The slice to print
+ *
+ * This prints the acquired state of a slice and its dependencies.
+ */
+void slice_debug_acquired(struct slice *slice)
+{
+ if (!slice_acquired(slice))
+ return;
+
+ __slice_debug_acquired(slice, 0);
+}
+
+/**
+ * slice_add: Add a dependency to a slice
+ *
+ * @slice: The slice to add the dependency to
+ * @dep: The slice @slice depends on
+ *
+ * This makes @slice dependent on @dep. In other words, acquiring @slice
+ * will lead to also acquiring @dep.
+ */
+void slice_add(struct slice *slice, struct slice *dep)
+{
+ struct slice_entry *se = xzalloc(sizeof(*se));
+
+ pr_debug("Adding dependency %s (%d) to slice %s (%d)\n",
+ slice_name(dep), dep->acquired,
+ slice_name(slice), slice->acquired);
+
+ se->slice = dep;
+
+ list_add_tail(&se->list, &slice->deps);
+}
+
+static LIST_HEAD(slices);
+
+/**
+ * slice_del: Remove a dependency previously added with slice_add
+ * @slice: The slice to remove the dependency from
+ * @dep: The slice @slice depended on
+ */
+void slice_del(struct slice *slice, struct slice *dep)
+{
+ struct slice_entry *se;
+
+ list_del(&slice->list);
+
+ list_for_each_entry(se, &slice->deps, list) {
+ if (se->slice == dep) {
+ list_del(&se->list);
+ free(se);
+ return;
+ }
+ }
+}
+
+/**
+ * slice_init - initialize a slice before usage
+ * @slice: The slice to initialize
+ * @name: The name of the slice
+ *
+ * This initializes a slice before usage. @name should be a meaningful name, when
+ * a device context is available to the caller it is recommended to pass its
+ * dev_name() as name argument.
+ */
+void slice_init(struct slice *slice, const char *name)
+{
+ INIT_LIST_HEAD(&slice->deps);
+ slice->name = xstrdup(name);
+ list_add_tail(&slice->list, &slices);
+}
+
+#if defined CONFIG_CMD_SLICE
+
+#include <command.h>
+#include <getopt.h>
+
+static void slice_info(void)
+{
+ struct slice *slice;
+ struct slice_entry *se;
+
+ list_for_each_entry(slice, &slices, list) {
+ printf("slice %s: acquired=%d\n",
+ slice_name(slice), slice->acquired);
+ list_for_each_entry(se, &slice->deps, list)
+ printf(" dep: %s\n", slice_name(se->slice));
+ }
+}
+
+static int __slice_acquire_name(const char *name, int add)
+{
+ struct slice *slice;
+
+ list_for_each_entry(slice, &slices, list) {
+ if (!strcmp(slice->name, name)) {
+ __slice_acquire(slice, add);
+ return 0;
+ }
+ }
+
+ printf("No such slice: %s\n", name);
+
+ return 1;
+}
+
+BAREBOX_CMD_HELP_START(slice)
+BAREBOX_CMD_HELP_TEXT("slice debugging command")
+BAREBOX_CMD_HELP_TEXT("")
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT ("-i", "Print information about slices")
+BAREBOX_CMD_HELP_OPT ("-a <devname>", "acquire a slice")
+BAREBOX_CMD_HELP_OPT ("-r <devname>", "release a slice")
+BAREBOX_CMD_HELP_END
+
+static int do_slice(int argc, char *argv[])
+{
+ int opt;
+
+ while ((opt = getopt(argc, argv, "a:r:i")) > 0) {
+ switch (opt) {
+ case 'a':
+ return __slice_acquire_name(optarg, 1);
+ case 'r':
+ return __slice_acquire_name(optarg, -1);
+ case 'i':
+ slice_info();
+ return 0;
+ }
+ }
+
+ return COMMAND_ERROR_USAGE;
+}
+
+BAREBOX_CMD_START(slice)
+ .cmd = do_slice,
+ BAREBOX_CMD_DESC("debug slices")
+ BAREBOX_CMD_OPTS("[-ari]")
+ BAREBOX_CMD_GROUP(CMD_GRP_MISC)
+ BAREBOX_CMD_HELP(cmd_slice_help)
+
+BAREBOX_CMD_END
+#endif
diff --git a/include/slice.h b/include/slice.h
new file mode 100644
index 0000000000..4874f1afc0
--- /dev/null
+++ b/include/slice.h
@@ -0,0 +1,31 @@
+#ifndef __SLICE_H
+#define __SLICE_H
+
+enum slice_action {
+ SLICE_ACQUIRE = 1,
+ SLICE_RELEASE = -1,
+ SLICE_TEST = 0,
+};
+
+struct slice {
+ int acquired;
+ struct list_head deps;
+ char *name;
+ struct list_head list;
+};
+
+struct slice_entry {
+ struct slice *slice;
+ struct list_head list;
+};
+
+void slice_acquire(struct slice *slice);
+void slice_release(struct slice *slice);
+bool slice_acquired(struct slice *slice);
+void slice_add(struct slice *slice, struct slice *dep);
+void slice_del(struct slice *slice, struct slice *dep);
+void slice_init(struct slice *slice, const char *name);
+
+void slice_debug_acquired(struct slice *slice);
+
+#endif /* __SLICE_H */
--
2.26.1
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 03/11] Introduce slices
2020-04-22 7:54 ` [PATCH 03/11] Introduce slices Sascha Hauer
@ 2020-05-08 18:01 ` Daniel Glöckner
2020-05-11 6:40 ` Sascha Hauer
0 siblings, 1 reply; 14+ messages in thread
From: Daniel Glöckner @ 2020-05-08 18:01 UTC (permalink / raw)
To: Sascha Hauer, Barebox List; +Cc: Edmund Henniges
Hello Sascha,
Am 22.04.20 um 09:54 schrieb Sascha Hauer:
> diff --git a/commands/Kconfig b/commands/Kconfig
> index 7fb47b8fb5..5ff6454427 100644
> --- a/commands/Kconfig
> +++ b/commands/Kconfig
> @@ -269,6 +269,15 @@ config CMD_POLLER
> is_timeout() or one of the various delay functions. The poller command prints
> informations about registered pollers.
>
> +config CMD_SLICE
> + tristate
> + prompt "slice"
> + depends on SLICE
> + help
> + slices are a way to protect resources from being accessed by pollers. The slice
> + command can be used to print informations about slices and also to manipulate
> + them on the command line for debugging purposes.
> +
> # end Information commands
> endmenu
>
> diff --git a/common/Kconfig b/common/Kconfig
> index 400c0553cf..bd2aebac75 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -913,6 +913,10 @@ config BAREBOXCRC32_TARGET
> config POLLER
> bool "generic polling infrastructure"
>
> +config SLICE
> + depends on POLLER
> + default y
> +
> config STATE
> bool "generic state infrastructure"
> select CRC32
> diff --git a/common/Makefile b/common/Makefile
> index 84463b4d48..16f14db41c 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -11,6 +11,7 @@ obj-y += bootsource.o
> obj-$(CONFIG_ELF) += elf.o
> obj-y += restart.o
> obj-y += poweroff.o
> +obj-y += slice.o
> obj-$(CONFIG_MACHINE_ID) += machine_id.o
> obj-$(CONFIG_AUTO_COMPLETE) += complete.o
> obj-y += version.o
the Kconfig logic doesn't make sense to me. slice.o gets built regardless of
CONFIG_SLICE and it will still compile and link when CONFIG_POLLER is disabled.
On the other hand poller.o will fail to link when slice.o is omitted.
I suggest we drop CONFIG_SLICE.
Best regards,
Daniel
--
Dipl.-Math. Daniel Glöckner, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11,
Gothaer Platz 3, 37083 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
Geschäftsführung: Heike Jordan, Dr. Uwe Kracke
Ust-IdNr.: DE 205 198 055
emlix - your embedded linux partner
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 03/11] Introduce slices
2020-05-08 18:01 ` Daniel Glöckner
@ 2020-05-11 6:40 ` Sascha Hauer
0 siblings, 0 replies; 14+ messages in thread
From: Sascha Hauer @ 2020-05-11 6:40 UTC (permalink / raw)
To: Daniel Glöckner; +Cc: Barebox List, Edmund Henniges
On Fri, May 08, 2020 at 08:01:00PM +0200, Daniel Glöckner wrote:
> Hello Sascha,
>
> Am 22.04.20 um 09:54 schrieb Sascha Hauer:
> > diff --git a/commands/Kconfig b/commands/Kconfig
> > index 7fb47b8fb5..5ff6454427 100644
> > --- a/commands/Kconfig
> > +++ b/commands/Kconfig
> > @@ -269,6 +269,15 @@ config CMD_POLLER
> > is_timeout() or one of the various delay functions. The poller command prints
> > informations about registered pollers.
> >
> > +config CMD_SLICE
> > + tristate
> > + prompt "slice"
> > + depends on SLICE
> > + help
> > + slices are a way to protect resources from being accessed by pollers. The slice
> > + command can be used to print informations about slices and also to manipulate
> > + them on the command line for debugging purposes.
> > +
> > # end Information commands
> > endmenu
> >
> > diff --git a/common/Kconfig b/common/Kconfig
> > index 400c0553cf..bd2aebac75 100644
> > --- a/common/Kconfig
> > +++ b/common/Kconfig
> > @@ -913,6 +913,10 @@ config BAREBOXCRC32_TARGET
> > config POLLER
> > bool "generic polling infrastructure"
> >
> > +config SLICE
> > + depends on POLLER
> > + default y
> > +
> > config STATE
> > bool "generic state infrastructure"
> > select CRC32
> > diff --git a/common/Makefile b/common/Makefile
> > index 84463b4d48..16f14db41c 100644
> > --- a/common/Makefile
> > +++ b/common/Makefile
> > @@ -11,6 +11,7 @@ obj-y += bootsource.o
> > obj-$(CONFIG_ELF) += elf.o
> > obj-y += restart.o
> > obj-y += poweroff.o
> > +obj-y += slice.o
> > obj-$(CONFIG_MACHINE_ID) += machine_id.o
> > obj-$(CONFIG_AUTO_COMPLETE) += complete.o
> > obj-y += version.o
>
> the Kconfig logic doesn't make sense to me. slice.o gets built regardless of
> CONFIG_SLICE and it will still compile and link when CONFIG_POLLER is disabled.
> On the other hand poller.o will fail to link when slice.o is omitted.
>
> I suggest we drop CONFIG_SLICE.
I probably just added slice.o using obj-y initially to get started and
then didn't look at it again.. Yes, dropping it seems right.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 04/11] net: Add a slice to struct eth_device
2020-04-22 7:54 [PATCH v4 00/11] Protect code from pollers Sascha Hauer
` (2 preceding siblings ...)
2020-04-22 7:54 ` [PATCH 03/11] Introduce slices Sascha Hauer
@ 2020-04-22 7:54 ` Sascha Hauer
2020-04-22 7:54 ` [PATCH 05/11] net: mdiobus: Add slice Sascha Hauer
` (6 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Sascha Hauer @ 2020-04-22 7:54 UTC (permalink / raw)
To: Barebox List; +Cc: Edmund Henniges, Daniel Glöckner
Add ethernet code safe for being called from a poller.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
include/net.h | 8 ++++++++
net/eth.c | 20 +++++++++++++++++---
2 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/include/net.h b/include/net.h
index 54db8a179a..8d2b4923de 100644
--- a/include/net.h
+++ b/include/net.h
@@ -19,6 +19,7 @@
#include <stdlib.h>
#include <clock.h>
#include <led.h>
+#include <slice.h>
#include <xfuncs.h>
#include <linux/phy.h>
#include <linux/string.h> /* memcpy */
@@ -63,6 +64,8 @@ struct eth_device {
char *bootarg;
char *linuxdevname;
+ struct slice slice;
+
bool ifup;
#define ETH_MODE_DHCP 0
#define ETH_MODE_STATIC 1
@@ -72,6 +75,11 @@ struct eth_device {
#define dev_to_edev(d) container_of(d, struct eth_device, dev)
+static inline struct slice *eth_device_slice(struct eth_device *edev)
+{
+ return &edev->slice;
+}
+
static inline const char *eth_name(struct eth_device *edev)
{
return edev->devname;
diff --git a/net/eth.c b/net/eth.c
index 4a8a7a8876..5cd1d703c4 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -222,20 +222,32 @@ int eth_send(struct eth_device *edev, void *packet, int length)
if (ret)
return ret;
+ slice_acquire(eth_device_slice(edev));
+
led_trigger_network(LED_TRIGGER_NET_TX);
- return edev->send(edev, packet, length);
+ ret = edev->send(edev, packet, length);
+
+ slice_release(eth_device_slice(edev));
+
+ return ret;
}
static int __eth_rx(struct eth_device *edev)
{
int ret;
+ slice_acquire(eth_device_slice(edev));
+
ret = eth_carrier_check(edev, 0);
if (ret)
- return ret;
+ goto out;
+
+ ret = edev->recv(edev);
+out:
+ slice_release(eth_device_slice(edev));
- return edev->recv(edev);
+ return ret;
}
int eth_rx(void)
@@ -356,6 +368,8 @@ int eth_register(struct eth_device *edev)
if (ret)
return ret;
+ slice_init(&edev->slice, dev_name(dev));
+
edev->devname = xstrdup(dev_name(&edev->dev));
dev_add_param_ip(dev, "ipaddr", NULL, NULL, &edev->ipaddr, edev);
--
2.26.1
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 05/11] net: mdiobus: Add slice
2020-04-22 7:54 [PATCH v4 00/11] Protect code from pollers Sascha Hauer
` (3 preceding siblings ...)
2020-04-22 7:54 ` [PATCH 04/11] net: Add a slice to struct eth_device Sascha Hauer
@ 2020-04-22 7:54 ` Sascha Hauer
2020-04-22 7:54 ` [PATCH 06/11] usb: Add a slice to usb host controllers Sascha Hauer
` (5 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Sascha Hauer @ 2020-04-22 7:54 UTC (permalink / raw)
To: Barebox List; +Cc: Edmund Henniges, Daniel Glöckner
By adding a slice to the mdio bus we make the mdio code safe for being
called in a poller.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/net/phy/mdio_bus.c | 41 ++++++++++++++++++++++++++++++++++++++
include/linux/phy.h | 38 ++++++++++++++---------------------
2 files changed, 56 insertions(+), 23 deletions(-)
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 3480e2ffb4..9a76db6ad4 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -239,6 +239,8 @@ int mdiobus_register(struct mii_bus *bus)
return -EINVAL;
}
+ slice_init(&bus->slice, dev_name(&bus->dev));
+
if (bus->reset)
bus->reset(bus);
@@ -357,6 +359,45 @@ static int mdio_bus_match(struct device_d *dev, struct driver_d *drv)
return 1;
}
+/**
+ * mdiobus_read - Convenience function for reading a given MII mgmt register
+ * @bus: the mii_bus struct
+ * @addr: the phy address
+ * @regnum: register number to read
+ */
+int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum)
+{
+ int ret;
+
+ slice_acquire(&bus->slice);
+
+ ret = bus->read(bus, addr, regnum);
+
+ slice_release(&bus->slice);
+
+ return ret;
+}
+
+/**
+ * mdiobus_write - Convenience function for writing a given MII mgmt register
+ * @bus: the mii_bus struct
+ * @addr: the phy address
+ * @regnum: register number to write
+ * @val: value to write to @regnum
+ */
+int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val)
+{
+ int ret;
+
+ slice_acquire(&bus->slice);
+
+ ret = bus->write(bus, addr, regnum, val);
+
+ slice_release(&bus->slice);
+
+ return ret;
+}
+
static ssize_t phydev_read(struct cdev *cdev, void *_buf, size_t count, loff_t offset, ulong flags)
{
int i = count;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index a9fdf44f1a..e93f6a01ff 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -16,6 +16,7 @@
#define __PHY_H
#include <driver.h>
+#include <slice.h>
#include <linux/list.h>
#include <linux/ethtool.h>
#include <linux/mii.h>
@@ -116,9 +117,16 @@ struct mii_bus {
struct list_head list;
bool is_multiplexed;
+
+ struct slice slice;
};
#define to_mii_bus(d) container_of(d, struct mii_bus, dev)
+static inline struct slice *mdiobus_slice(struct mii_bus *bus)
+{
+ return &bus->slice;
+}
+
int mdiobus_register(struct mii_bus *bus);
void mdiobus_unregister(struct mii_bus *bus);
struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr);
@@ -134,28 +142,8 @@ struct mii_bus *mdiobus_get_bus(int busnum);
struct mii_bus *of_mdio_find_bus(struct device_node *mdio_bus_np);
-/**
- * mdiobus_read - Convenience function for reading a given MII mgmt register
- * @bus: the mii_bus struct
- * @addr: the phy address
- * @regnum: register number to read
- */
-static inline int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum)
-{
- return bus->read(bus, addr, regnum);
-}
-
-/**
- * mdiobus_write - Convenience function for writing a given MII mgmt register
- * @bus: the mii_bus struct
- * @addr: the phy address
- * @regnum: register number to write
- * @val: value to write to @regnum
- */
-static inline int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val)
-{
- return bus->write(bus, addr, regnum, val);
-}
+int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
+int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
/* phy_device: An instance of a PHY
*
@@ -376,11 +364,15 @@ int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask,
int phy_register_fixup_for_id(const char *bus_id,
int (*run)(struct phy_device *));
int phy_scan_fixups(struct phy_device *phydev);
-
int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad);
void phy_write_mmd_indirect(struct phy_device *phydev, int prtad, int devad,
u16 data);
+static inline bool phy_acquired(struct phy_device *phydev)
+{
+ return phydev && slice_acquired(&phydev->bus->slice);
+}
+
#ifdef CONFIG_PHYLIB
int phy_register_fixup_for_uid(u32 phy_uid, u32 phy_uid_mask,
int (*run)(struct phy_device *));
--
2.26.1
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 06/11] usb: Add a slice to usb host controllers
2020-04-22 7:54 [PATCH v4 00/11] Protect code from pollers Sascha Hauer
` (4 preceding siblings ...)
2020-04-22 7:54 ` [PATCH 05/11] net: mdiobus: Add slice Sascha Hauer
@ 2020-04-22 7:54 ` Sascha Hauer
2020-04-22 7:54 ` [PATCH 07/11] usbnet: Add slice Sascha Hauer
` (4 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Sascha Hauer @ 2020-04-22 7:54 UTC (permalink / raw)
To: Barebox List; +Cc: Edmund Henniges, Daniel Glöckner
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/usb/core/usb.c | 6 ++++++
include/usb/usb.h | 7 +++++++
2 files changed, 13 insertions(+)
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index d0a91b9e9f..db86ab4d19 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -81,11 +81,16 @@ static inline int usb_host_acquire(struct usb_host *host)
if (host->sem)
return -EAGAIN;
host->sem++;
+
+ slice_acquire(&host->slice);
+
return 0;
}
static inline void usb_host_release(struct usb_host *host)
{
+ slice_release(&host->slice);
+
if (host->sem > 0)
host->sem--;
}
@@ -95,6 +100,7 @@ int usb_register_host(struct usb_host *host)
list_add_tail(&host->list, &host_list);
host->busnum = host_busnum++;
host->sem = 0;
+ slice_init(&host->slice, dev_name(host->hw_dev));
return 0;
}
diff --git a/include/usb/usb.h b/include/usb/usb.h
index 82a4736445..b3b6cb3178 100644
--- a/include/usb/usb.h
+++ b/include/usb/usb.h
@@ -23,6 +23,7 @@
#define _USB_H_
#include <driver.h>
+#include <slice.h>
#include <usb/ch9.h>
#include <usb/ch11.h>
#include <usb/usb_defs.h>
@@ -168,11 +169,17 @@ struct usb_host {
struct usb_device *root_dev;
int sem;
struct usb_phy *usbphy;
+ struct slice slice;
};
int usb_register_host(struct usb_host *);
void usb_unregister_host(struct usb_host *host);
+static inline struct slice *usb_device_slice(struct usb_device *udev)
+{
+ return &udev->host->slice;
+}
+
int usb_host_detect(struct usb_host *host);
int usb_set_protocol(struct usb_device *dev, int ifnum, int protocol);
--
2.26.1
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 07/11] usbnet: Add slice
2020-04-22 7:54 [PATCH v4 00/11] Protect code from pollers Sascha Hauer
` (5 preceding siblings ...)
2020-04-22 7:54 ` [PATCH 06/11] usb: Add a slice to usb host controllers Sascha Hauer
@ 2020-04-22 7:54 ` Sascha Hauer
2020-04-22 7:54 ` [PATCH 08/11] net: Call net_poll() in a poller Sascha Hauer
` (3 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Sascha Hauer @ 2020-04-22 7:54 UTC (permalink / raw)
To: Barebox List; +Cc: Edmund Henniges, Daniel Glöckner
Both the ethernet device and the mdio bus of a USB network controller
need the USB bus. Add dependencies to it.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/net/usb/usbnet.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 406b8c964f..d43b75591a 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -233,6 +233,9 @@ int usbnet_probe(struct usb_device *usbdev, const struct usb_device_id *prod)
eth_register(edev);
+ slice_add(eth_device_slice(edev), usb_device_slice(usbdev));
+ slice_add(mdiobus_slice(&undev->miibus), usb_device_slice(usbdev));
+
return 0;
out1:
dev_dbg(&edev->dev, "err: %d\n", status);
--
2.26.1
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 08/11] net: Call net_poll() in a poller
2020-04-22 7:54 [PATCH v4 00/11] Protect code from pollers Sascha Hauer
` (6 preceding siblings ...)
2020-04-22 7:54 ` [PATCH 07/11] usbnet: Add slice Sascha Hauer
@ 2020-04-22 7:54 ` Sascha Hauer
2020-04-22 7:54 ` [PATCH 09/11] net: reply to ping requests Sascha Hauer
` (2 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Sascha Hauer @ 2020-04-22 7:54 UTC (permalink / raw)
To: Barebox List; +Cc: Edmund Henniges, Daniel Glöckner
This patch moves the ethernet receive loop into a poller. With this
network protocols no longer have to call net_loop() explicitly but
can do it implicitly by calling is_timeout() when waiting for packets.
Having the network receive loop running in a poller has the advantage
that we have it running continuously and not only when we explicitly
expect packets to come in. With this we can reasonably well answer to
ping requests which is implemented in the next patch. This also helps
protocols that run in the background like fastboot over UDP.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
fs/nfs.c | 4 ++--
fs/tftp.c | 2 --
include/net.h | 3 ---
net/dhcp.c | 1 -
net/dns.c | 1 -
net/eth.c | 15 ++++++++++-----
net/net.c | 14 +++++++++++---
net/netconsole.c | 4 +---
net/nfs.c | 1 -
net/ping.c | 2 --
net/sntp.c | 2 --
11 files changed, 24 insertions(+), 25 deletions(-)
diff --git a/fs/nfs.c b/fs/nfs.c
index 227a4866d7..b28d94a2ba 100644
--- a/fs/nfs.c
+++ b/fs/nfs.c
@@ -449,9 +449,9 @@ again:
nfs_timer_start = get_time_ns();
- while (1) {
- net_poll();
+ nfs_state = STATE_START;
+ while (nfs_state != STATE_DONE) {
if (is_timeout(nfs_timer_start, NFS_TIMEOUT)) {
tries++;
if (tries == NFS_MAX_RESEND)
diff --git a/fs/tftp.c b/fs/tftp.c
index 1f36c56511..11f289584d 100644
--- a/fs/tftp.c
+++ b/fs/tftp.c
@@ -210,8 +210,6 @@ static int tftp_poll(struct file_priv *priv)
return -ETIMEDOUT;
}
- net_poll();
-
return 0;
}
diff --git a/include/net.h b/include/net.h
index 8d2b4923de..583dc14ed5 100644
--- a/include/net.h
+++ b/include/net.h
@@ -244,9 +244,6 @@ IPaddr_t net_get_nameserver(void);
const char *net_get_domainname(void);
struct eth_device *net_route(IPaddr_t ip);
-/* Do the work */
-void net_poll(void);
-
static inline struct iphdr *net_eth_to_iphdr(char *pkt)
{
return (struct iphdr *)(pkt + ETHER_HDR_SIZE);
diff --git a/net/dhcp.c b/net/dhcp.c
index a27fa89996..ef22d2cef0 100644
--- a/net/dhcp.c
+++ b/net/dhcp.c
@@ -513,7 +513,6 @@ int dhcp_request(struct eth_device *edev, const struct dhcp_req_param *param,
ret = -ETIMEDOUT;
goto out1;
}
- net_poll();
if (is_timeout(dhcp_start, 3 * SECOND)) {
dhcp_start = get_time_ns();
printf("T ");
diff --git a/net/dns.c b/net/dns.c
index ffe98ef9e3..851bf3722e 100644
--- a/net/dns.c
+++ b/net/dns.c
@@ -231,7 +231,6 @@ int resolv(const char *host, IPaddr_t *ip)
if (ctrlc()) {
break;
}
- net_poll();
if (is_timeout(dns_timer_start, SECOND)) {
dns_timer_start = get_time_ns();
printf("T ");
diff --git a/net/eth.c b/net/eth.c
index 5cd1d703c4..d06daa1f8e 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -237,14 +237,19 @@ static int __eth_rx(struct eth_device *edev)
{
int ret;
- slice_acquire(eth_device_slice(edev));
+ if (!phy_acquired(edev->phydev)) {
+ ret = eth_carrier_check(edev, 0);
+ if (ret)
+ return ret;
+ }
- ret = eth_carrier_check(edev, 0);
- if (ret)
- goto out;
+ if (slice_acquired(eth_device_slice(edev)))
+ return 0;
+
+ slice_acquire(eth_device_slice(edev));
ret = edev->recv(edev);
-out:
+
slice_release(eth_device_slice(edev));
return ret;
diff --git a/net/net.c b/net/net.c
index 0d889ddb52..6eb604e5dc 100644
--- a/net/net.c
+++ b/net/net.c
@@ -242,8 +242,6 @@ static int arp_request(struct eth_device *edev, IPaddr_t dest, unsigned char *et
if (retries > PKT_NUM_RETRIES)
return -ETIMEDOUT;
-
- net_poll();
}
pr_debug("Got ARP REPLY for %pI4: %02x:%02x:%02x:%02x:%02x:%02x\n",
@@ -252,11 +250,21 @@ static int arp_request(struct eth_device *edev, IPaddr_t dest, unsigned char *et
return 0;
}
-void net_poll(void)
+static void net_poll(struct poller_struct *poller)
{
eth_rx();
}
+static struct poller_struct net_poller = {
+ .func = net_poll,
+};
+
+static int init_net_poll(void)
+{
+ return poller_register(&net_poller, "net");
+}
+device_initcall(init_net_poll);
+
static uint16_t net_udp_new_localport(void)
{
static uint16_t localport;
diff --git a/net/netconsole.c b/net/netconsole.c
index ef8b1856b6..457a3c2971 100644
--- a/net/netconsole.c
+++ b/net/netconsole.c
@@ -62,7 +62,7 @@ static int nc_getc(struct console_device *cdev)
return 0;
while (!kfifo_len(priv->fifo))
- net_poll();
+ udelay(1);
kfifo_getc(priv->fifo, &c);
@@ -80,8 +80,6 @@ static int nc_tstc(struct console_device *cdev)
if (priv->busy)
return kfifo_len(priv->fifo) ? 1 : 0;
- net_poll();
-
return kfifo_len(priv->fifo) ? 1 : 0;
}
diff --git a/net/nfs.c b/net/nfs.c
index 63573098d7..e0479ef692 100644
--- a/net/nfs.c
+++ b/net/nfs.c
@@ -708,7 +708,6 @@ static int do_nfs(int argc, char *argv[])
nfs_err = -EINTR;
break;
}
- net_poll();
if (is_timeout(nfs_timer_start, NFS_TIMEOUT * SECOND)) {
show_progress(-1);
nfs_send();
diff --git a/net/ping.c b/net/ping.c
index a71f59a805..f3de0c27a4 100644
--- a/net/ping.c
+++ b/net/ping.c
@@ -89,8 +89,6 @@ static int do_ping(int argc, char *argv[])
break;
}
- net_poll();
-
if (is_timeout(ping_start, SECOND)) {
/* No answer, send another packet */
ping_start = get_time_ns();
diff --git a/net/sntp.c b/net/sntp.c
index b4e6d6439c..f693a2e8af 100644
--- a/net/sntp.c
+++ b/net/sntp.c
@@ -145,8 +145,6 @@ s64 sntp(const char *server)
break;
}
- net_poll();
-
if (is_timeout(sntp_start, 1 * SECOND)) {
sntp_start = get_time_ns();
ret = sntp_send();
--
2.26.1
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 09/11] net: reply to ping requests
2020-04-22 7:54 [PATCH v4 00/11] Protect code from pollers Sascha Hauer
` (7 preceding siblings ...)
2020-04-22 7:54 ` [PATCH 08/11] net: Call net_poll() in a poller Sascha Hauer
@ 2020-04-22 7:54 ` Sascha Hauer
2020-04-22 7:54 ` [PATCH 10/11] usbnet: Be more friendly in the receive path Sascha Hauer
2020-04-22 7:54 ` [PATCH 11/11] poller: Allow to run pollers inside of pollers Sascha Hauer
10 siblings, 0 replies; 14+ messages in thread
From: Sascha Hauer @ 2020-04-22 7:54 UTC (permalink / raw)
To: Barebox List; +Cc: Edmund Henniges, Daniel Glöckner
Now that we have the network receive function running in a poller we
can reasonably well answer to ping requests. Implement this feature.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
net/net.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 44 insertions(+), 2 deletions(-)
diff --git a/net/net.c b/net/net.c
index 6eb604e5dc..e0cded5783 100644
--- a/net/net.c
+++ b/net/net.c
@@ -573,12 +573,54 @@ static int net_handle_udp(unsigned char *pkt, int len)
return -EINVAL;
}
-static int net_handle_icmp(unsigned char *pkt, int len)
+static int ping_reply(struct eth_device *edev, unsigned char *pkt, int len)
+{
+ struct ethernet *et = (struct ethernet *)pkt;
+ struct icmphdr *icmp;
+ struct iphdr *ip = (struct iphdr *)(pkt + ETHER_HDR_SIZE);
+ unsigned char *packet;
+ int ret;
+
+ memcpy(et->et_dest, et->et_src, 6);
+ memcpy(et->et_src, edev->ethaddr, 6);
+ et->et_protlen = htons(PROT_IP);
+
+ icmp = net_eth_to_icmphdr(pkt);
+
+ icmp->type = ICMP_ECHO_REPLY;
+ icmp->checksum = 0;
+ icmp->checksum = ~net_checksum((unsigned char *)icmp,
+ len - sizeof(struct iphdr) - ETHER_HDR_SIZE);
+ ip->check = 0;
+ ip->frag_off = 0;
+ net_copy_ip((void *)&ip->daddr, &ip->saddr);
+ net_copy_ip((void *)&ip->saddr, &edev->ipaddr);
+ ip->check = ~net_checksum((unsigned char *)ip, sizeof(struct iphdr));
+
+ packet = net_alloc_packet();
+ if (!packet)
+ return 0;
+
+ memcpy(packet, pkt, ETHER_HDR_SIZE + len);
+
+ ret = eth_send(edev, packet, ETHER_HDR_SIZE + len);
+
+ free(packet);
+
+ return 0;
+}
+
+static int net_handle_icmp(struct eth_device *edev, unsigned char *pkt, int len)
{
struct net_connection *con;
+ struct icmphdr *icmp;
pr_debug("%s\n", __func__);
+ icmp = net_eth_to_icmphdr(pkt);
+ if (icmp->type == ICMP_ECHO_REQUEST)
+ ping_reply(edev, pkt, len);
+
list_for_each_entry(con, &connection_list, list) {
if (con->proto == IPPROTO_ICMP) {
con->handler(con->priv, pkt, len);
@@ -615,7 +657,7 @@ static int net_handle_ip(struct eth_device *edev, unsigned char *pkt, int len)
switch (ip->protocol) {
case IPPROTO_ICMP:
- return net_handle_icmp(pkt, len);
+ return net_handle_icmp(edev, pkt, len);
case IPPROTO_UDP:
return net_handle_udp(pkt, len);
}
--
2.26.1
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 10/11] usbnet: Be more friendly in the receive path
2020-04-22 7:54 [PATCH v4 00/11] Protect code from pollers Sascha Hauer
` (8 preceding siblings ...)
2020-04-22 7:54 ` [PATCH 09/11] net: reply to ping requests Sascha Hauer
@ 2020-04-22 7:54 ` Sascha Hauer
2020-04-22 7:54 ` [PATCH 11/11] poller: Allow to run pollers inside of pollers Sascha Hauer
10 siblings, 0 replies; 14+ messages in thread
From: Sascha Hauer @ 2020-04-22 7:54 UTC (permalink / raw)
To: Barebox List; +Cc: Edmund Henniges, Daniel Glöckner
To recognize if we have a receive packet pending we must set up a USB
bulk transfer. When there's no incoming packet we must wait until the
transfer times out. We do this with every poller call which can
considerably slow down the system. With this patch we do two things
against this:
- lower the timeout for the bulk transfer
- When we haven't received a packet for longer then lower the frequency
of calling into the USB stack to once every 100ms
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/net/usb/usbnet.c | 19 ++++++++++++++++++-
include/usb/usbnet.h | 3 +++
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index d43b75591a..c3ae94b727 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -122,12 +122,29 @@ static int usbnet_recv(struct eth_device *edev)
dev_dbg(&edev->dev, "%s\n",__func__);
+ /*
+ * we must let the usb_bulk_msg below timeout before we realize
+ * that we have no packet received. Since this function runs
+ * inside a poller we considerably slow down barebox when we
+ * wait for the timeout too often. To improve this we only poll
+ * with full speed when we actually have received a packet in the
+ * last 100ms.
+ */
+ if (is_timeout(dev->last_pkt_received, 100 * MSECOND) &&
+ !is_timeout(dev->last_recv_call, 100 * MSECOND)) {
+ return 0;
+ }
+
+ dev->last_recv_call = get_time_ns();
+
len = dev->rx_urb_size;
- ret = usb_bulk_msg(dev->udev, dev->in, dev->rx_buf, len, &alen, 100);
+ ret = usb_bulk_msg(dev->udev, dev->in, dev->rx_buf, len, &alen, 2);
if (ret)
return ret;
+ dev->last_pkt_received = get_time_ns();
+
if (alen) {
if (info->rx_fixup)
return info->rx_fixup(dev, dev->rx_buf, alen);
diff --git a/include/usb/usbnet.h b/include/usb/usbnet.h
index 450db47b40..7ff32f280a 100644
--- a/include/usb/usbnet.h
+++ b/include/usb/usbnet.h
@@ -54,6 +54,9 @@ struct usbnet {
# define EVENT_RX_MEMORY 2
# define EVENT_STS_SPLIT 3
# define EVENT_LINK_RESET 4
+
+ uint64_t last_pkt_received;
+ uint64_t last_recv_call;
};
#if 0
--
2.26.1
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 11/11] poller: Allow to run pollers inside of pollers
2020-04-22 7:54 [PATCH v4 00/11] Protect code from pollers Sascha Hauer
` (9 preceding siblings ...)
2020-04-22 7:54 ` [PATCH 10/11] usbnet: Be more friendly in the receive path Sascha Hauer
@ 2020-04-22 7:54 ` Sascha Hauer
10 siblings, 0 replies; 14+ messages in thread
From: Sascha Hauer @ 2020-04-22 7:54 UTC (permalink / raw)
To: Barebox List; +Cc: Edmund Henniges, Daniel Glöckner
This adds a slice to each poller which is acquired before the poller is
executed. This allows us to run pollers inside of other pollers.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
common/poller.c | 15 +++++++--------
include/poller.h | 2 ++
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/common/poller.c b/common/poller.c
index 95f828b439..cccffcb88d 100644
--- a/common/poller.c
+++ b/common/poller.c
@@ -14,7 +14,6 @@
#include <clock.h>
static LIST_HEAD(poller_list);
-static int poller_active;
int poller_register(struct poller_struct *poller, const char *name)
{
@@ -22,6 +21,7 @@ int poller_register(struct poller_struct *poller, const char *name)
return -EBUSY;
poller->name = xstrdup(name);
+ slice_init(&poller->slice, poller->name);
list_add_tail(&poller->list, &poller_list);
poller->registered = 1;
@@ -111,15 +111,14 @@ void poller_call(void)
{
struct poller_struct *poller, *tmp;
- if (poller_active)
- return;
-
- poller_active = 1;
+ list_for_each_entry_safe(poller, tmp, &poller_list, list) {
+ if (slice_acquired(&poller->slice))
+ continue;
- list_for_each_entry_safe(poller, tmp, &poller_list, list)
+ slice_acquire(&poller->slice);
poller->func(poller);
-
- poller_active = 0;
+ slice_release(&poller->slice);
+ }
}
#if defined CONFIG_CMD_POLLER
diff --git a/include/poller.h b/include/poller.h
index 886557252b..c8b6829685 100644
--- a/include/poller.h
+++ b/include/poller.h
@@ -7,11 +7,13 @@
#define POLLER_H
#include <linux/list.h>
+#include <slice.h>
struct poller_struct {
void (*func)(struct poller_struct *poller);
int registered;
struct list_head list;
+ struct slice slice;
char *name;
};
--
2.26.1
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 14+ messages in thread