Re: [PATCH 1/1] rapidio/rio_cm: avoid GFP_KERNEL in atomic context

From: Andrew Morton
Date: Fri Sep 16 2016 - 18:08:14 EST


On Thu, 15 Sep 2016 13:54:02 -0400 Alexandre Bounine <alexandre.bounine@xxxxxxx> wrote:

> As reported by Alexey Khoroshilov <khoroshilov@xxxxxxxxx>
> (see https://lkml.org/lkml/2016/9/9/737):
> riocm_send_close() is called from rio_cm_shutdown() under
> spin_lock_bh(idr_lock), but riocm_send_close() uses a GFP_KERNEL
> allocation.
>
> Fix by taking riocm_send_close() outside of spinlock protected code.
>
> --- a/drivers/rapidio/rio_cm.c
> +++ b/drivers/rapidio/rio_cm.c
> @@ -2242,17 +2242,31 @@ static int rio_cm_shutdown(struct notifier_block *nb, unsigned long code,
> {
> struct rio_channel *ch;
> unsigned int i;
> + LIST_HEAD(list);
>
> riocm_debug(EXIT, ".");
>
> + /*
> + * If there are any channels left in connected state send
> + * close notification to the connection partner.
> + * First build a list of channels that require a closing
> + * notification because function riocm_send_close() should
> + * be called outside of spinlock protected code.
> + */
> spin_lock_bh(&idr_lock);
> idr_for_each_entry(&ch_idr, ch, i) {
> - riocm_debug(EXIT, "close ch %d", ch->id);
> - if (ch->state == RIO_CM_CONNECTED)
> - riocm_send_close(ch);
> + if (ch->state == RIO_CM_CONNECTED) {
> + riocm_debug(EXIT, "close ch %d", ch->id);
> + idr_remove(&ch_idr, ch->id);
> + list_add(&ch->ch_node, &list);
> + }
> }
> spin_unlock_bh(&idr_lock);
>
> + if (!list_empty(&list))
> + list_for_each_entry(ch, &list, ch_node)
> + riocm_send_close(ch);
> +
> return NOTIFY_DONE;
> }

Fair enough.

Can we remove the !list_empty() test?

--- a/drivers/rapidio/rio_cm.c~rapidio-rio_cm-avoid-gfp_kernel-in-atomic-context-fix
+++ a/drivers/rapidio/rio_cm.c
@@ -2268,9 +2268,8 @@ static int rio_cm_shutdown(struct notifi
}
spin_unlock_bh(&idr_lock);

- if (!list_empty(&list))
- list_for_each_entry(ch, &list, ch_node)
- riocm_send_close(ch);
+ list_for_each_entry(ch, &list, ch_node)
+ riocm_send_close(ch);

return NOTIFY_DONE;
}
_