From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757270AbcIPWIL (ORCPT ); Fri, 16 Sep 2016 18:08:11 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:58590 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750945AbcIPWID (ORCPT ); Fri, 16 Sep 2016 18:08:03 -0400 Date: Fri, 16 Sep 2016 15:08:01 -0700 From: Andrew Morton To: Alexandre Bounine Cc: Alexey Khoroshilov , linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/1] rapidio/rio_cm: avoid GFP_KERNEL in atomic context Message-Id: <20160916150801.24b13aed1984dd37de21592c@linux-foundation.org> In-Reply-To: <20160915175402.10122-1-alexandre.bounine@idt.com> References: <20160915175402.10122-1-alexandre.bounine@idt.com> X-Mailer: Sylpheed 3.4.1 (GTK+ 2.24.23; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 15 Sep 2016 13:54:02 -0400 Alexandre Bounine wrote: > As reported by Alexey Khoroshilov > (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; } _