On Wed, 11 Jun 2025, Jiri Slaby (SUSE) wrote:
> This find-or-create-irq part of the serial_link_irq_chain()'s code is
> logically bounded and self-standing. For easier-to-follow code flow,
> extract the code to a separate function:
> serial_get_or_create_irq_info().
>
> This allows for an easier found-an-irq handling -- simple jump to the
> 'unlock' label and return. That results in one less 'if' levels.
>
> Note when using guard()s in the upcoming patchset, the label can dropped
> altogether.
>
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
> drivers/tty/serial/8250/8250_core.c | 37 ++++++++++++++++++++---------
> 1 file changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index 7a6050f1c094..d42ceb6ffdc2 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -129,11 +129,15 @@ static void serial_do_unlink(struct irq_info *i, struct uart_8250_port *up)
> }
> }
>
> -static int serial_link_irq_chain(struct uart_8250_port *up)
> +/*
> + * Either:
> + * - find the corresponding info in the hashtable and return it, or
> + * - allocate a new one, add it to the hashtable and return it.
> + */
> +static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_port *up)
> {
> struct hlist_head *h;
> struct irq_info *i;
> - int ret;
>
> mutex_lock(&hash_mutex);
>
> @@ -141,20 +145,31 @@ static int serial_link_irq_chain(struct uart_8250_port *up)
>
> hlist_for_each_entry(i, h, node)
> if (i->irq == up->port.irq)
> - break;
> + goto unlock;
>
> + i = kzalloc(sizeof(*i), GFP_KERNEL);
> if (i == NULL) {
> - i = kzalloc(sizeof(struct irq_info), GFP_KERNEL);
> - if (i == NULL) {
> - mutex_unlock(&hash_mutex);
> - return -ENOMEM;
> - }
> - spin_lock_init(&i->lock);
> - i->irq = up->port.irq;
> - hlist_add_head(&i->node, h);
> + i = ERR_PTR(-ENOMEM);
> + goto unlock;
> }
> + spin_lock_init(&i->lock);
> + i->irq = up->port.irq;
> + hlist_add_head(&i->node, h);
> +unlock:
> mutex_unlock(&hash_mutex);
>
> + return i;
> +}
> +
> +static int serial_link_irq_chain(struct uart_8250_port *up)
> +{
> + struct irq_info *i;
> + int ret;
> +
> + i = serial_get_or_create_irq_info(up);
> + if (IS_ERR(i))
> + return PTR_ERR(i);
> +
> spin_lock_irq(&i->lock);
>
> if (i->head) {
>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Unrelated to the patch, I didn't like the use of 'i' as a variable name
to store a struct pointer.
--
i.