From ef40d97d297439b7b058f9bfa5eb2b8e166d84c6 Mon Sep 17 00:00:00 2001 From: Martin Spinler Date: Thu, 16 Feb 2023 10:42:58 +0100 Subject: [PATCH] drivers - NDP: [FIX] force stop on stuck queue --- drivers/kernel/drivers/nfb/ndp/channel.c | 11 +++++-- drivers/kernel/drivers/nfb/ndp/char.c | 3 +- drivers/kernel/drivers/nfb/ndp/ctrl.c | 32 +++++++++++-------- drivers/kernel/drivers/nfb/ndp/ctrl_ndp.c | 21 ++++++------ drivers/kernel/drivers/nfb/ndp/kndp.c | 3 +- drivers/kernel/drivers/nfb/ndp/ndp.h | 6 ++-- drivers/kernel/drivers/nfb/ndp/subscription.c | 22 ++++++++++--- libnfb/src/ndp/ndp.c | 10 ++++-- 8 files changed, 73 insertions(+), 35 deletions(-) diff --git a/drivers/kernel/drivers/nfb/ndp/channel.c b/drivers/kernel/drivers/nfb/ndp/channel.c index f3e46594..125f58e6 100644 --- a/drivers/kernel/drivers/nfb/ndp/channel.c +++ b/drivers/kernel/drivers/nfb/ndp/channel.c @@ -178,7 +178,7 @@ int ndp_channel_start(struct ndp_subscription *sub) return ret; } -int ndp_channel_stop(struct ndp_subscription *sub) +int ndp_channel_stop(struct ndp_subscription *sub, int force) { int ret = 0; struct ndp_channel *channel = sub->channel; @@ -190,13 +190,20 @@ int ndp_channel_stop(struct ndp_subscription *sub) /* stop now (i.e. the last one)? */ if (--channel->start_count == 0) { - channel->ops->stop(channel); + ret = channel->ops->stop(channel, force); + if (ret == -EAGAIN) { + ++channel->start_count; + goto err_again; + } else { + ret = 0; + } } spin_lock(&channel->lock); list_del_init(&sub->list_item); spin_unlock(&channel->lock); +err_again: mutex_unlock(&channel->mutex); return ret; } diff --git a/drivers/kernel/drivers/nfb/ndp/char.c b/drivers/kernel/drivers/nfb/ndp/char.c index 8aa12beb..729cf053 100644 --- a/drivers/kernel/drivers/nfb/ndp/char.c +++ b/drivers/kernel/drivers/nfb/ndp/char.c @@ -55,6 +55,7 @@ void ndp_char_release(void *priv, void *app_priv, struct file *file) { struct ndp_subscriber *subscriber = app_priv; + /* FIXME: force stop ctrl */ ndp_subscriber_destroy(subscriber); } @@ -127,7 +128,7 @@ long ndp_char_ioctl(void *priv, void *app_priv, struct file *file, unsigned int if (sub == NULL) return -EBADF; - ndp_subscription_stop(sub); + ret = ndp_subscription_stop(sub, 0); break; } default: diff --git a/drivers/kernel/drivers/nfb/ndp/ctrl.c b/drivers/kernel/drivers/nfb/ndp/ctrl.c index 3ea21c83..4f8d2371 100644 --- a/drivers/kernel/drivers/nfb/ndp/ctrl.c +++ b/drivers/kernel/drivers/nfb/ndp/ctrl.c @@ -295,29 +295,33 @@ static uint64_t ndp_ctrl_set_flags(struct ndp_channel *channel, uint64_t flags) return ret; } -static void ndp_ctrl_stop(struct ndp_channel *channel) +static int ndp_ctrl_stop(struct ndp_channel *channel, int force) { + uint64_t s, e; int dirty = 0; - unsigned int counter = 0; + unsigned int cnt = 0; struct ndp_ctrl *ctrl = container_of(channel, struct ndp_ctrl, channel); if (channel->id.type == NDP_CHANNEL_TYPE_TX) { - while (1) { - uint64_t s, e; + dirty = 1; + while (cnt < 10 || (!ndp_kill_signal_pending(current) && !force)) { s = *((uint64_t *)ctrl->update_ptr); e = ctrl->swptr; - if (s == e) - break; - if (ndp_kill_signal_pending(current)) { - dev_warn(ctrl->comp->nfb->dev, - "NDP queue %s has not completed all data transfers. " - "Transfers aborted by users, queue is in dirty state.\n", - dev_name(&channel->dev)); - dirty = 1; + if (s == e) { + dirty = 0; break; + } else if (/*ret == -EAGAIN && */!force) { + return -EAGAIN; } msleep(10); + cnt++; + } + if (dirty) { + dev_warn(ctrl->comp->nfb->dev, + "NDP queue %s has not completed all data transfers. " + "Transfers aborted by users, queue is in dirty state.\n", + dev_name(&channel->dev)); } } @@ -328,12 +332,13 @@ static void ndp_ctrl_stop(struct ndp_channel *channel) ndp_ctrl_set_swptr(channel, ndp_ctrl_get_hwptr(channel)); } + cnt = 0; while (1) { uint32_t status; status = nfb_comp_read32(ctrl->comp, NDP_CTRL_REG_STATUS); if (dirty || !(status & NDP_CTRL_REG_STATUS_RUNNING)) break; - if (counter++ > 100) { + if (cnt++ > 100) { dev_warn(ctrl->comp->nfb->dev, "NDP queue %s did not stop in 1 sec. " "This may be due to hardware/firmware error.\n", @@ -342,6 +347,7 @@ static void ndp_ctrl_stop(struct ndp_channel *channel) } msleep(10); } + return 0; } static int ndp_ctrl_attach_ring(struct ndp_channel *channel) diff --git a/drivers/kernel/drivers/nfb/ndp/ctrl_ndp.c b/drivers/kernel/drivers/nfb/ndp/ctrl_ndp.c index 968db1b3..0e25a0dd 100644 --- a/drivers/kernel/drivers/nfb/ndp/ctrl_ndp.c +++ b/drivers/kernel/drivers/nfb/ndp/ctrl_ndp.c @@ -501,30 +501,33 @@ static int ndp_ctrl_start(struct ndp_channel *channel, uint64_t *hwptr) return 0; } -static void ndp_ctrl_stop(struct ndp_channel *channel) +static int ndp_ctrl_stop(struct ndp_channel *channel, int force) { - const int ignore_timeout = 1; int ret; int cnt = 0; struct ndp_ctrl *ctrl = container_of(channel, struct ndp_ctrl, channel); - do { + while (cnt < 10 || (!ndp_kill_signal_pending(current) && !force)) { ret = nc_ndp_ctrl_stop(&ctrl->c); - if (ret == -EINPROGRESS) - cnt = 0; - else if (ret != -EAGAIN) + if (ret == 0) { break; + } else if (ret == -EINPROGRESS) { + cnt = 0; + } else if (ret == -EAGAIN && !force) { + return -EAGAIN; + } msleep(10); cnt++; - } while ((cnt < 100 || ignore_timeout) && !ndp_kill_signal_pending(current)); + } if (ret) { nc_ndp_ctrl_stop_force(&ctrl->c); dev_err(ctrl->nfb->dev, - "NDP queue %s did't stop in 1 sec. " + "NDP queue %s did't stop in %d msecs. " "This may be due to firmware error.\n", - dev_name(&channel->dev)); + dev_name(&channel->dev), cnt * 10); } + return 0; } static int ndp_ctrl_hdr_mmap(struct vm_area_struct *vma, unsigned long offset, unsigned long size, void *priv) diff --git a/drivers/kernel/drivers/nfb/ndp/kndp.c b/drivers/kernel/drivers/nfb/ndp/kndp.c index f513a7fe..1c4a6990 100644 --- a/drivers/kernel/drivers/nfb/ndp/kndp.c +++ b/drivers/kernel/drivers/nfb/ndp/kndp.c @@ -47,7 +47,7 @@ inline int _ndp_queue_start(struct ndp_queue *q) inline int _ndp_queue_stop(struct ndp_queue *q) { #ifdef __KERNEL__ - ndp_subscription_stop(ndp_subscription_by_id(q->subscriber, q->sync.id)); + ndp_subscription_stop(ndp_subscription_by_id(q->subscriber, q->sync.id), 1); #else if (ioctl(q->fd, NDP_IOC_STOP, &q->sync)) return errno; @@ -119,6 +119,7 @@ void ndp_close_queue(struct ndp_queue *q) ndp_queue_stop(q); #ifdef __KERNEL__ if (q->sub) { + /* FIXME: force stop ctrl */ ndp_subscription_destroy(q->sub); q->sub = NULL; } diff --git a/drivers/kernel/drivers/nfb/ndp/ndp.h b/drivers/kernel/drivers/nfb/ndp/ndp.h index d42f5719..5c00178e 100644 --- a/drivers/kernel/drivers/nfb/ndp/ndp.h +++ b/drivers/kernel/drivers/nfb/ndp/ndp.h @@ -98,7 +98,7 @@ struct ndp_subscriber { struct ndp_channel_ops { int (*start)(struct ndp_channel *channel, uint64_t *hwptr); - void (*stop)(struct ndp_channel *channel); + int (*stop)(struct ndp_channel *channel, int force); int (*attach_ring)(struct ndp_channel *channel); void (*detach_ring)(struct ndp_channel *channel); uint64_t (*get_hwptr)(struct ndp_channel *channel); @@ -213,7 +213,7 @@ ssize_t ndp_channel_set_discard(struct device *dev, struct device_attribute *att int ndp_subscription_start(struct ndp_subscription *sub, struct ndp_subscription_sync *sync); -void ndp_subscription_stop(struct ndp_subscription *sub); +int ndp_subscription_stop(struct ndp_subscription *sub, int force); int ndp_subscription_sync(struct ndp_subscription *sub, struct ndp_subscription_sync *sync); @@ -231,7 +231,7 @@ void ndp_subscriber_destroy(struct ndp_subscriber *subscriber); int ndp_subscriber_poll(struct ndp_subscriber *subscriber, struct file *filp, struct poll_table_struct *wait); int ndp_channel_start(struct ndp_subscription *sub); -int ndp_channel_stop(struct ndp_subscription *sub); +int ndp_channel_stop(struct ndp_subscription *sub, int force); void ndp_channel_sync(struct ndp_subscription *sub, struct ndp_subscription_sync *sync); extern struct ndp_channel *ndp_channel_create(struct ndp *ndp, struct ndp_channel_ops *ctrl_ops, diff --git a/drivers/kernel/drivers/nfb/ndp/subscription.c b/drivers/kernel/drivers/nfb/ndp/subscription.c index 07b8ac74..be319d3e 100644 --- a/drivers/kernel/drivers/nfb/ndp/subscription.c +++ b/drivers/kernel/drivers/nfb/ndp/subscription.c @@ -7,6 +7,8 @@ * Martin Spinler */ +#include + #include "ndp.h" size_t ndp_subscription_rx_data_available(struct ndp_subscription *sub) @@ -64,14 +66,26 @@ int ndp_subscription_start(struct ndp_subscription *sub, return 0; } -void ndp_subscription_stop(struct ndp_subscription *sub) +int ndp_subscription_stop(struct ndp_subscription *sub, int force) { + int ret; if (sub->status != NDP_SUB_STATUS_RUNNING) { //dev_warn("Trying to stop not started subscription."); - return; + //return -EBADF; + return 0; } - ndp_channel_stop(sub); + ret = ndp_channel_stop(sub, 0); + if (ret == -EAGAIN) { + if (force) { + msleep(10); + ndp_channel_stop(sub, 1); + } else { + return ret; + } + } + sub->status = NDP_SUB_STATUS_SUBSCRIBED; + return 0; } struct ndp_subscription *ndp_subscription_create( @@ -146,7 +160,7 @@ void ndp_subscription_destroy(struct ndp_subscription *sub) struct ndp *ndp = subscriber->ndp; if (sub->status == NDP_SUB_STATUS_RUNNING) { - ndp_subscription_stop(sub); + ndp_subscription_stop(sub, 1); } ndp_channel_unsubscribe(sub); diff --git a/libnfb/src/ndp/ndp.c b/libnfb/src/ndp/ndp.c index 54d15d6f..b13b01ce 100644 --- a/libnfb/src/ndp/ndp.c +++ b/libnfb/src/ndp/ndp.c @@ -92,9 +92,15 @@ inline int _ndp_queue_start(struct ndp_queue *q) inline int _ndp_queue_stop(struct ndp_queue *q) { #ifdef __KERNEL__ - ndp_subscription_stop(ndp_subscription_by_id(q->subscriber, q->sync.id)); + ndp_subscription_stop(ndp_subscription_by_id(q->subscriber, q->sync.id), 1); #else - if (ioctl(q->fd, NDP_IOC_STOP, &q->sync)) + int ret; + do { + errno = 0; + ret = ioctl(q->fd, NDP_IOC_STOP, &q->sync); + } while (ret == -1 && errno == EAGAIN); + + if (ret) return errno; #endif return 0;