Re: [PATCH] serial: Remove NR_IRQS usage

From: Eric W. Biederman
Date: Wed Aug 06 2008 - 12:13:30 EST


Alan Cox <alan@xxxxxxxxxxxxxxxxxxx> writes:

> Works on my test box with a quick test.
>
> 8250: Remove NR_IRQ usage
>
> From: Alan Cox <alan@xxxxxxxxxx>
>
> Signed-off-by: Alan Cox <alan@xxxxxxxxxx>

This looks good.

Two final nits.

> -static struct irq_info irq_lists[NR_IRQS];
> +#define NR_IRQ_HASH 32 /* Can be adjusted later */
> +static struct hlist_head irq_lists[NR_IRQ_HASH];
> +static DEFINE_MUTEX(hash_mutex); /* Used to walk the hash */

> static int serial_link_irq_chain(struct uart_8250_port *up)
> {
> - struct irq_info *i = irq_lists + up->port.irq;
> + struct hlist_head *h;
> + struct hlist_node *n;
> + struct irq_info *i;
> int ret, irq_flags = up->port.flags & UPF_SHARE_IRQ ? IRQF_SHARED : 0;
>
> + mutex_lock(&hash_mutex);
> +
....
> + mutex_unlock(&hash_mutex);

Could you move the unlock down to the end of serial_link_irq_chain?
So that it covers serial_do_unlink on the error cleanup path?
> +
> spin_lock_irq(&i->lock);
>
> if (i->head) {
> @@ -1567,14 +1599,28 @@ static int serial_link_irq_chain(struct uart_8250_port
> *up)
>
> static void serial_unlink_irq_chain(struct uart_8250_port *up)
...
> - if (list_empty(i->head))
...
> +
> + if (list_empty(i->head)) {
> free_irq(up->port.irq, i);
> -
> + }

Adding the braces here just appears to be extra noise.

> serial_do_unlink(i, up);
> + mutex_unlock(&hash_mutex);
> }

Eric


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/