This message generated a parse failure. Raw output follows here. Please use 'back' to navigate. From devnull@lkml.org Thu Mar 28 12:34:12 2024 Delivery-date: Tue, 06 Jan 2009 22:08:33 +0100 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751403AbZAFVH7 (ORCPT ); Tue, 6 Jan 2009 16:07:59 -0500 Received: from flock1.newmail.ru ([82.204.219.207]:56549 "HELO flock1.newmail.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751365AbZAFVH5 (ORCPT ); Tue, 6 Jan 2009 16:07:57 -0500 Received: (qmail 23530 invoked from network); 6 Jan 2009 21:07:55 -0000 Received: from unknown (HELO cooker.localnet) (arvidjaar@newmail.ru@91.77.130.216) by smtpd.newmail.ru with SMTP; 6 Jan 2009 21:07:55 -0000 From: Andrey Borzenkov To: Dave Subject: Re: 2.6.28: warn_slowpath in orinoco receive path Date: Wed, 7 Jan 2009 00:07:50 +0300 User-Agent: KMail/1.10.92 (Linux/2.6.28-1avb; KDE/4.1.85; i686; ; ) Cc: orinoco-devel@lists.sourceforge.net, linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org References: <200901052005.12428.arvidjaar@mail.ru> <200901061251.41628.arvidjaar@mail.ru> <4963B8BE.9000204@gmail.com> In-Reply-To: <4963B8BE.9000204@gmail.com> Mime-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart2062355.kNb4u0dCAV"; protocol="application/pgp-signature"; micalg=pgp-sha1 Content-Transfer-Encoding: 7bit Message-Id: <200901070007.52508.arvidjaar@mail.ru> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-Id: X-Mailing-List: linux-kernel@vger.kernel.org --nextPart2062355.kNb4u0dCAV Content-Type: multipart/mixed; boundary="Boundary-01=_og8YJDUBxLslQKI" Content-Transfer-Encoding: 7bit Content-Disposition: inline --Boundary-01=_og8YJDUBxLslQKI Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline On =D0=92=D1=82=D0=BE=D1=80=D0=BD=D0=B8=D0=BA 06 =D1=8F=D0=BD=D0=B2=D0=B0= =D1=80=D1=8F 2009 23:02:06 Dave wrote: > Since we have to use spinlock we should introduce an rx_lock > specifically to protect the list so the tasklet is not held up by > anything else the driver is doing. I'll put something together if you > don't get round to it first. > You mean something like attached patch? Not sure if it worth it frankly. --Boundary-01=_og8YJDUBxLslQKI Content-Type: message/rfc822; name="orinoco-rx_list-protect" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="orinoco-rx_list-protect" Subject: [PATCH] orinoco: protect rx_list manipulation in orinoco_rx_isr_tasklet From: Andrey Borzenkov The list is changed both in normal and interrupt context. Protect list manipulation by additional spinlock; this hopefully fixes this warning: [89479.105038] WARNING: at /home/bor/src/linux-git/lib/list_debug.c:30 __list_add+0x8f/0xa0() [89479.105058] list_add corruption. prev->next should be next (dddb3568), but was cbc28978. (prev=dddb3568). [89479.106002] Pid: 15746, comm: X Not tainted 2.6.28-1avb #26 [89479.106020] Call Trace: [89479.106062] [] warn_slowpath+0x60/0x80 [89479.106104] [] ? native_sched_clock+0x20/0x70 [89479.106194] [] ? lock_release_holdtime+0x35/0x200 [89479.106218] [] ? __slab_alloc+0x550/0x560 [89479.106254] [] ? _spin_unlock+0x1d/0x20 [89479.106270] [] ? __slab_alloc+0x550/0x560 [89479.106302] [] ? delay_tsc+0x17/0x24 [89479.106319] [] ? __const_udelay+0x21/0x30 [89479.106376] [] ? hermes_bap_seek+0x112/0x1e0 [hermes] [89479.106396] [] ? trace_hardirqs_off+0xb/0x10 [89479.106418] [] ? __kmalloc_track_caller+0xb7/0x110 [89479.106448] [] ? dev_alloc_skb+0x1c/0x30 [89479.106465] [] ? dev_alloc_skb+0x1c/0x30 [89479.106482] [] __list_add+0x8f/0xa0 [89479.106551] [] orinoco_interrupt+0xcae/0x16c0 [orinoco] [89479.106574] [] ? tick_dev_program_event+0x33/0xb0 [89479.106594] [] ? native_sched_clock+0x20/0x70 [89479.106613] [] ? lock_release_holdtime+0x35/0x200 [89479.106662] [] ? trace_hardirqs_off+0xb/0x10 [89479.106892] [] ? usb_hcd_irq+0x97/0xa0 [usbcore] [89479.106926] [] handle_IRQ_event+0x29/0x60 [89479.106947] [] handle_level_irq+0x69/0xe0 [89479.106963] [] ? handle_level_irq+0x0/0xe0 [89479.106977] [] ? tcp_v4_rcv+0x633/0x6e0 [89479.107025] [] ? common_interrupt+0x28/0x30 [89479.107057] [] ? sk_run_filter+0x320/0x7a0 [89479.107078] [] ? list_del+0x21/0x90 [89479.107106] [] ? orinoco_rx_isr_tasklet+0x2ce/0x480 [orinoco] [89479.107131] [] ? __lock_acquire+0x160/0x1650 [89479.107151] [] ? native_sched_clock+0x20/0x70 [89479.107169] [] ? lock_release_holdtime+0x35/0x200 [89479.107200] [] ? irq_enter+0xa/0x60 [89479.107217] [] ? do_IRQ+0xd2/0x130 [89479.107518] [] ? restore_nocheck_notrace+0x0/0xe [89479.107542] [] ? __do_softirq+0x0/0x110 [89479.107561] [] ? trace_hardirqs_on_caller+0x74/0x140 [89479.107583] [] ? trace_hardirqs_on_thunk+0xc/0x10 [89479.107602] [] ? tasklet_action+0x27/0x90 [89479.107620] [] ? trace_hardirqs_on_caller+0x74/0x140 [89479.107638] [] ? tasklet_action+0x43/0x90 [89479.107655] [] ? __do_softirq+0x6f/0x110 [89479.107674] [] ? __do_softirq+0x0/0x110 [89479.107685] [] ? handle_level_irq+0x0/0xe0 [89479.107715] [] ? irq_exit+0x5d/0x80 [89479.107732] [] ? do_IRQ+0xd2/0x130 [89479.107747] [] ? sysenter_exit+0xf/0x16 [89479.107765] [] ? trace_hardirqs_on_caller+0xfd/0x140 [89479.107782] [] ? common_interrupt+0x28/0x30 [89479.107797] ---[ end trace a1fc0a52df4a729d ]--- Patch based on suggestion of Dave. Signed-off-by: Andrey Borzenkov --- drivers/net/wireless/orinoco.c | 13 ++++++++++++- drivers/net/wireless/orinoco.h | 1 + 2 files changed, 13 insertions(+), 1 deletions(-) diff --git a/drivers/net/wireless/orinoco.c b/drivers/net/wireless/orinoco.c index 1d7e14b..27e29bb 100644 --- a/drivers/net/wireless/orinoco.c +++ b/drivers/net/wireless/orinoco.c @@ -72,6 +72,8 @@ * they are doing and drop the lock again. The orinoco_lock() * function handles this (it unlocks and returns -EBUSY if * hw_unavailable is non-zero). + * + * Additional lock priv->rx_lock protects received SKB list */ #define DRIVER_NAME "orinoco" @@ -1430,7 +1432,9 @@ static void __orinoco_ev_rx(struct net_device *dev, hermes_t *hw) } rx_data->desc = desc; rx_data->skb = skb; + spin_lock(&priv->rx_lock); list_add_tail(&rx_data->list, &priv->rx_list); + spin_unlock(&priv->rx_lock); tasklet_schedule(&priv->rx_tasklet); return; @@ -1568,9 +1572,15 @@ static void orinoco_rx_isr_tasklet(unsigned long data) struct orinoco_rx_data *rx_data, *temp; struct hermes_rx_descriptor *desc; struct sk_buff *skb; + struct list_head temp_rx_list; + + /* Move list to temporary head to avoid races with interrupt handler */ + spin_lock_irq(&priv->rx_lock); + list_replace_init(&priv->rx_list, &temp_rx_list); + spin_unlock_irq(&priv->rx_lock); /* extract desc and skb from queue */ - list_for_each_entry_safe(rx_data, temp, &priv->rx_list, list) { + list_for_each_entry_safe(rx_data, temp, &temp_rx_list, list) { desc = rx_data->desc; skb = rx_data->skb; list_del(&rx_data->list); @@ -3536,6 +3546,7 @@ struct net_device INIT_WORK(&priv->join_work, orinoco_join_ap); INIT_WORK(&priv->wevent_work, orinoco_send_wevents); + spin_lock_init(&priv->rx_lock); INIT_LIST_HEAD(&priv->rx_list); tasklet_init(&priv->rx_tasklet, orinoco_rx_isr_tasklet, (unsigned long) dev); diff --git a/drivers/net/wireless/orinoco.h b/drivers/net/wireless/orinoco.h index 8c29538..680c66e 100644 --- a/drivers/net/wireless/orinoco.h +++ b/drivers/net/wireless/orinoco.h @@ -83,6 +83,7 @@ struct orinoco_private { struct tasklet_struct rx_tasklet; struct list_head rx_list; struct orinoco_rx_data *rx_data; + spinlock_t rx_lock; /* driver state */ int open; --Boundary-01=_og8YJDUBxLslQKI-- --nextPart2062355.kNb4u0dCAV Content-Type: application/pgp-signature; name=signature.asc Content-Description: This is a digitally signed message part. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) iEYEABECAAYFAkljyCgACgkQR6LMutpd94wplQCgxOHP51Mi7s3oI0wzpXJzFCA7 drIAoILVNx72NNH7jZZdSDa2wdpPNr75 =v1wd -----END PGP SIGNATURE----- --nextPart2062355.kNb4u0dCAV-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/