* [PATCH 1/1] rapidio/rio_cm: avoid GFP_KERNEL in atomic context
@ 2016-09-15 17:54 Alexandre Bounine
2016-09-16 22:08 ` Andrew Morton
0 siblings, 1 reply; 3+ messages in thread
From: Alexandre Bounine @ 2016-09-15 17:54 UTC (permalink / raw)
To: Andrew Morton; +Cc: Alexandre Bounine, Alexey Khoroshilov, linux-kernel
As reported by Alexey Khoroshilov <khoroshilov@ispras.ru>
(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.
Reported-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Cc: Alexey Khoroshilov <khoroshilov@ispras.ru>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Alexandre Bounine <alexandre.bounine@idt.com>
---
drivers/rapidio/rio_cm.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/rapidio/rio_cm.c b/drivers/rapidio/rio_cm.c
index 3226983..0e91335 100644
--- 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;
}
--
2.9.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] rapidio/rio_cm: avoid GFP_KERNEL in atomic context
2016-09-15 17:54 [PATCH 1/1] rapidio/rio_cm: avoid GFP_KERNEL in atomic context Alexandre Bounine
@ 2016-09-16 22:08 ` Andrew Morton
2016-09-19 14:05 ` Bounine, Alexandre
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2016-09-16 22:08 UTC (permalink / raw)
To: Alexandre Bounine; +Cc: Alexey Khoroshilov, linux-kernel
On Thu, 15 Sep 2016 13:54:02 -0400 Alexandre Bounine <alexandre.bounine@idt.com> wrote:
> As reported by Alexey Khoroshilov <khoroshilov@ispras.ru>
> (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;
}
_
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH 1/1] rapidio/rio_cm: avoid GFP_KERNEL in atomic context
2016-09-16 22:08 ` Andrew Morton
@ 2016-09-19 14:05 ` Bounine, Alexandre
0 siblings, 0 replies; 3+ messages in thread
From: Bounine, Alexandre @ 2016-09-19 14:05 UTC (permalink / raw)
To: Andrew Morton; +Cc: Alexey Khoroshilov, linux-kernel
On Fri, Sep 16, 2016 6:08 PM Andrew Morton <akpm@linux-foundation.org> wrote:
> >
> > + 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?
Sure. This check is not needed.
>
> --- 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;
> }
> _
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-09-19 14:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-15 17:54 [PATCH 1/1] rapidio/rio_cm: avoid GFP_KERNEL in atomic context Alexandre Bounine
2016-09-16 22:08 ` Andrew Morton
2016-09-19 14:05 ` Bounine, Alexandre
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).