[RESEND 3/4] vme: change bus error handling scheme
Martyn Welch
martyn at welchs.me.uk
Fri Sep 18 09:46:21 UTC 2015
On 18/09/15 00:01, dmitry.kalinkin at gmail.com wrote:
> From: Dmitry Kalinkin <dmitry.kalinkin at gmail.com>
>
> The current VME bus error handler adds errors to the bridge error list.
> vme_master_{read,write} then traverses that list to look for relevant
> errors.
>
> Such scheme didn't work well for accesses going through vme_master_mmap
> because they would also allocate a vme_bus_error, but have no way to do
> vme_clear_errors call to free that memory.
>
> This changes the error handling process to be other way around: now
> vme_master_{read,write} defines a window in VME address space that will
> catch possible errors. VME bus error interrupt only traverses these
> windows and marks those that had errors in them.
>
> Signed-off-by: Dmitry Kalinkin <dmitry.kalinkin at gmail.com>
> Cc: Igor Alekseev <igor.alekseev at itep.ru>
This one is currently causing the following warnings at build time:
CC drivers/vme/bridges/vme_tsi148.o
drivers/vme/bridges/vme_tsi148.c: In function ‘tsi148_master_read’:
drivers/vme/bridges/vme_tsi148.c:1254:14: warning: ‘handler’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
if (handler->num_errors) {
^
drivers/vme/bridges/vme_tsi148.c: In function ‘tsi148_master_write’:
drivers/vme/bridges/vme_tsi148.c:1352:14: warning: ‘handler’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
if (handler->num_errors) {
^
This appears to be because the pointer "handler" is set and checked in
one if statement controlled by "err_chk" and is used in a second. The
variable "err_chk" is a kernel param and from memory this can be set at
runtime, so there's a chance that the value of err_chk could be changed
between the two if statements (unlikely I know, but...) resulting in
handler being a null pointer.
Martyn
> ---
> drivers/vme/bridges/vme_ca91cx42.c | 3 +-
> drivers/vme/bridges/vme_tsi148.c | 83 ++++++++++++++++----------------
> drivers/vme/vme.c | 92 +++++++++++++-----------------------
> drivers/vme/vme_bridge.h | 23 +++++----
> 4 files changed, 91 insertions(+), 110 deletions(-)
>
> diff --git a/drivers/vme/bridges/vme_ca91cx42.c b/drivers/vme/bridges/vme_ca91cx42.c
> index f692efc..834883d 100644
> --- a/drivers/vme/bridges/vme_ca91cx42.c
> +++ b/drivers/vme/bridges/vme_ca91cx42.c
> @@ -204,8 +204,7 @@ static int ca91cx42_irq_init(struct vme_bridge *ca91cx42_bridge)
> /* Need pdev */
> pdev = container_of(ca91cx42_bridge->parent, struct pci_dev, dev);
>
> - /* Initialise list for VME bus errors */
> - INIT_LIST_HEAD(&ca91cx42_bridge->vme_errors);
> + INIT_LIST_HEAD(&ca91cx42_bridge->vme_error_handlers);
>
> mutex_init(&ca91cx42_bridge->irq_mtx);
>
> diff --git a/drivers/vme/bridges/vme_tsi148.c b/drivers/vme/bridges/vme_tsi148.c
> index 76ccfae..b0132e0 100644
> --- a/drivers/vme/bridges/vme_tsi148.c
> +++ b/drivers/vme/bridges/vme_tsi148.c
> @@ -314,8 +314,7 @@ static int tsi148_irq_init(struct vme_bridge *tsi148_bridge)
>
> bridge = tsi148_bridge->driver_priv;
>
> - /* Initialise list for VME bus errors */
> - INIT_LIST_HEAD(&tsi148_bridge->vme_errors);
> + INIT_LIST_HEAD(&tsi148_bridge->vme_error_handlers);
>
> mutex_init(&tsi148_bridge->irq_mtx);
>
> @@ -1187,7 +1186,7 @@ static ssize_t tsi148_master_read(struct vme_master_resource *image, void *buf,
> int retval, enabled;
> unsigned long long vme_base, size;
> u32 aspace, cycle, dwidth;
> - struct vme_bus_error *vme_err = NULL;
> + struct vme_error_handler *handler;
> struct vme_bridge *tsi148_bridge;
> void __iomem *addr = image->kern_base + offset;
> unsigned int done = 0;
> @@ -1197,6 +1196,17 @@ static ssize_t tsi148_master_read(struct vme_master_resource *image, void *buf,
>
> spin_lock(&image->lock);
>
> + if (err_chk) {
> + __tsi148_master_get(image, &enabled, &vme_base, &size, &aspace,
> + &cycle, &dwidth);
> + handler = vme_register_error_handler(tsi148_bridge, aspace,
> + vme_base + offset, count);
> + if (!handler) {
> + spin_unlock(&image->lock);
> + return -ENOMEM;
> + }
> + }
> +
> /* The following code handles VME address alignment. We cannot use
> * memcpy_xxx here because it may cut data transfers in to 8-bit
> * cycles when D16 or D32 cycles are required on the VME bus.
> @@ -1240,24 +1250,16 @@ static ssize_t tsi148_master_read(struct vme_master_resource *image, void *buf,
> out:
> retval = count;
>
> - if (!err_chk)
> - goto skip_chk;
> -
> - __tsi148_master_get(image, &enabled, &vme_base, &size, &aspace, &cycle,
> - &dwidth);
> -
> - vme_err = vme_find_error(tsi148_bridge, aspace, vme_base + offset,
> - count);
> - if (vme_err != NULL) {
> - dev_err(image->parent->parent, "First VME read error detected "
> - "an at address 0x%llx\n", vme_err->address);
> - retval = vme_err->address - (vme_base + offset);
> - /* Clear down save errors in this address range */
> - vme_clear_errors(tsi148_bridge, aspace, vme_base + offset,
> - count);
> + if (err_chk) {
> + if (handler->num_errors) {
> + dev_err(image->parent->parent,
> + "First VME read error detected an at address 0x%llx\n",
> + handler->first_error);
> + retval = handler->first_error - (vme_base + offset);
> + }
> + vme_unregister_error_handler(handler);
> }
>
> -skip_chk:
> spin_unlock(&image->lock);
>
> return retval;
> @@ -1274,7 +1276,7 @@ static ssize_t tsi148_master_write(struct vme_master_resource *image, void *buf,
> unsigned int done = 0;
> unsigned int count32;
>
> - struct vme_bus_error *vme_err = NULL;
> + struct vme_error_handler *handler;
> struct vme_bridge *tsi148_bridge;
> struct tsi148_driver *bridge;
>
> @@ -1284,6 +1286,17 @@ static ssize_t tsi148_master_write(struct vme_master_resource *image, void *buf,
>
> spin_lock(&image->lock);
>
> + if (err_chk) {
> + __tsi148_master_get(image, &enabled, &vme_base, &size, &aspace,
> + &cycle, &dwidth);
> + handler = vme_register_error_handler(tsi148_bridge, aspace,
> + vme_base + offset, count);
> + if (!handler) {
> + spin_unlock(&image->lock);
> + return -ENOMEM;
> + }
> + }
> +
> /* Here we apply for the same strategy we do in master_read
> * function in order to assure the correct cycles.
> */
> @@ -1333,30 +1346,18 @@ out:
> * We check for saved errors in the written address range/space.
> */
>
> - if (!err_chk)
> - goto skip_chk;
> -
> - /*
> - * Get window info first, to maximise the time that the buffers may
> - * fluch on their own
> - */
> - __tsi148_master_get(image, &enabled, &vme_base, &size, &aspace, &cycle,
> - &dwidth);
> -
> - ioread16(bridge->flush_image->kern_base + 0x7F000);
> + if (err_chk) {
> + ioread16(bridge->flush_image->kern_base + 0x7F000);
>
> - vme_err = vme_find_error(tsi148_bridge, aspace, vme_base + offset,
> - count);
> - if (vme_err != NULL) {
> - dev_warn(tsi148_bridge->parent, "First VME write error detected"
> - " an at address 0x%llx\n", vme_err->address);
> - retval = vme_err->address - (vme_base + offset);
> - /* Clear down save errors in this address range */
> - vme_clear_errors(tsi148_bridge, aspace, vme_base + offset,
> - count);
> + if (handler->num_errors) {
> + dev_warn(tsi148_bridge->parent,
> + "First VME write error detected an at address 0x%llx\n",
> + handler->first_error);
> + retval = handler->first_error - (vme_base + offset);
> + }
> + vme_unregister_error_handler(handler);
> }
>
> -skip_chk:
> spin_unlock(&image->lock);
>
> return retval;
> diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c
> index 2b79cd2..7a10d92 100644
> --- a/drivers/vme/vme.c
> +++ b/drivers/vme/vme.c
> @@ -1026,76 +1026,52 @@ EXPORT_SYMBOL(vme_dma_free);
> void vme_bus_error_handler(struct vme_bridge *bridge,
> unsigned long long address, int am)
> {
> - struct vme_bus_error *error;
> -
> - error = kmalloc(sizeof(struct vme_bus_error), GFP_ATOMIC);
> - if (error) {
> - error->aspace = vme_get_aspace(am);
> - error->address = address;
> - list_add_tail(&error->list, &bridge->vme_errors);
> - } else {
> - dev_err(bridge->parent,
> - "Unable to alloc memory for VMEbus Error reporting\n");
> + struct list_head *handler_pos = NULL;
> + struct vme_error_handler *handler;
> + u32 aspace = vme_get_aspace(am);
> +
> + list_for_each(handler_pos, &bridge->vme_error_handlers) {
> + handler = list_entry(handler_pos, struct vme_error_handler,
> + list);
> + if ((aspace == handler->aspace) &&
> + (address >= handler->start) &&
> + (address < handler->end)) {
> + if (!handler->num_errors)
> + handler->first_error = address;
> + if (handler->num_errors != UINT_MAX)
> + handler->num_errors++;
> + }
> }
> }
> EXPORT_SYMBOL(vme_bus_error_handler);
>
> -/*
> - * Find the first error in this address range
> - */
> -struct vme_bus_error *vme_find_error(struct vme_bridge *bridge, u32 aspace,
> - unsigned long long address, size_t count)
> +struct vme_error_handler *vme_register_error_handler(
> + struct vme_bridge *bridge, u32 aspace,
> + unsigned long long address, size_t len)
> {
> - struct list_head *err_pos;
> - struct vme_bus_error *vme_err, *valid = NULL;
> - unsigned long long bound;
> -
> - bound = address + count;
> + struct vme_error_handler *handler;
>
> - err_pos = NULL;
> - /* Iterate through errors */
> - list_for_each(err_pos, &bridge->vme_errors) {
> - vme_err = list_entry(err_pos, struct vme_bus_error, list);
> - if ((vme_err->aspace == aspace) &&
> - (vme_err->address >= address) &&
> - (vme_err->address < bound)) {
> + handler = kmalloc(sizeof(*handler), GFP_KERNEL);
> + if (!handler)
> + return NULL;
>
> - valid = vme_err;
> - break;
> - }
> - }
> + handler->aspace = aspace;
> + handler->start = address;
> + handler->end = address + len;
> + handler->num_errors = 0;
> + handler->first_error = 0;
> + list_add_tail(&handler->list, &bridge->vme_error_handlers);
>
> - return valid;
> + return handler;
> }
> -EXPORT_SYMBOL(vme_find_error);
> +EXPORT_SYMBOL(vme_register_error_handler);
>
> -/*
> - * Clear errors in the provided address range.
> - */
> -void vme_clear_errors(struct vme_bridge *bridge, u32 aspace,
> - unsigned long long address, size_t count)
> +void vme_unregister_error_handler(struct vme_error_handler *handler)
> {
> - struct list_head *err_pos, *temp;
> - struct vme_bus_error *vme_err;
> - unsigned long long bound;
> -
> - bound = address + count;
> -
> - err_pos = NULL;
> - /* Iterate through errors */
> - list_for_each_safe(err_pos, temp, &bridge->vme_errors) {
> - vme_err = list_entry(err_pos, struct vme_bus_error, list);
> -
> - if ((vme_err->aspace == aspace) &&
> - (vme_err->address >= address) &&
> - (vme_err->address < bound)) {
> -
> - list_del(err_pos);
> - kfree(vme_err);
> - }
> - }
> + list_del(&handler->list);
> + kfree(handler);
> }
> -EXPORT_SYMBOL(vme_clear_errors);
> +EXPORT_SYMBOL(vme_unregister_error_handler);
>
> void vme_irq_handler(struct vme_bridge *bridge, int level, int statid)
> {
> diff --git a/drivers/vme/vme_bridge.h b/drivers/vme/vme_bridge.h
> index 92fbe18..397578a 100644
> --- a/drivers/vme/vme_bridge.h
> +++ b/drivers/vme/vme_bridge.h
> @@ -75,10 +75,13 @@ struct vme_lm_resource {
> int monitors;
> };
>
> -struct vme_bus_error {
> +struct vme_error_handler {
> struct list_head list;
> - u32 aspace;
> - unsigned long long address;
> + unsigned long long start; /* Beginning of error window */
> + unsigned long long end; /* End of error window */
> + unsigned long long first_error; /* Address of the first error */
> + u32 aspace; /* Address space of error window*/
> + unsigned num_errors; /* Number of errors */
> };
>
> struct vme_callback {
> @@ -106,8 +109,10 @@ struct vme_bridge {
> struct list_head dma_resources;
> struct list_head lm_resources;
>
> - struct list_head vme_errors; /* List for errors generated on VME */
> - struct list_head devices; /* List of devices on this bridge */
> + /* List for registered errors handlers */
> + struct list_head vme_error_handlers;
> + /* List of devices on this bridge */
> + struct list_head devices;
>
> /* Bridge Info - XXX Move to private structure? */
> struct device *parent; /* Parent device (eg. pdev->dev for PCI) */
> @@ -168,13 +173,13 @@ struct vme_bridge {
>
> void vme_bus_error_handler(struct vme_bridge *bridge,
> unsigned long long address, int am);
> -struct vme_bus_error *vme_find_error(struct vme_bridge *bridge, u32 aspace,
> - unsigned long long address, size_t count);
> -void vme_clear_errors(struct vme_bridge *bridge, u32 aspace,
> - unsigned long long address, size_t count);
> void vme_irq_handler(struct vme_bridge *, int, int);
>
> int vme_register_bridge(struct vme_bridge *);
> void vme_unregister_bridge(struct vme_bridge *);
> +struct vme_error_handler *vme_register_error_handler(
> + struct vme_bridge *bridge, u32 aspace,
> + unsigned long long address, size_t len);
> +void vme_unregister_error_handler(struct vme_error_handler *handler);
>
> #endif /* _VME_BRIDGE_H_ */
More information about the devel
mailing list