All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tilman Schmidt <tilman@imap.cc>
To: Xiaotian Feng <xtfeng@gmail.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	isdn4linux <isdn4linux@listserv.isdn4linux.de>,
	Netdev <netdev@vger.kernel.org>,
	Karsten Keil <isdn@linux-pingi.de>
Subject: Re: possible circular locking dependency in ISDN PPP
Date: Wed, 21 Oct 2009 18:24:42 +0200	[thread overview]
Message-ID: <4ADF35CA.5020207@imap.cc> (raw)
In-Reply-To: <7b6bb4a50910182227y1281b40bj3fcc082d32cf4496@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2152 bytes --]

Thanks for your analysis. The patch you propose does indeed prevent the
"circular locking dependency" message, with no noticeable ill effect.

I cannot say why xmit_lock was taken around isdn_net_lp_busy() in the
first place. The ISDN4Linux people would be the ones to comment on that.
If none of them objects, I propose you add a Signed-off-by line to your
patch and submit it to Karsten Keil, the ISDN maintainer, for inclusion.
You can also add a "Tested-by: Tilman Schmidt <tilman@imap.cc>" line.

Thanks,
Tilman

Am 19.10.2009 07:27 schrieb Xiaotian Feng:
> So there's a circular locking dependency.. Looking into isdn_net_get_locked_lp()
[...]
> Why do we need to hold xmit_lock while using
> isdn_net_lp_busy(nd->queue) ? Can following patch fix this bug?
> 
> ---
> diff --git a/drivers/isdn/i4l/isdn_net.h b/drivers/isdn/i4l/isdn_net.h
> index 74032d0..7511f08 100644
> --- a/drivers/isdn/i4l/isdn_net.h
> +++ b/drivers/isdn/i4l/isdn_net.h
> @@ -83,19 +83,19 @@ static __inline__ isdn_net_local *
> isdn_net_get_locked_lp(isdn_net_dev *nd)
> 
>         spin_lock_irqsave(&nd->queue_lock, flags);
>         lp = nd->queue;         /* get lp on top of queue */
> -       spin_lock(&nd->queue->xmit_lock);
>         while (isdn_net_lp_busy(nd->queue)) {
> -               spin_unlock(&nd->queue->xmit_lock);
>                 nd->queue = nd->queue->next;
>                 if (nd->queue == lp) { /* not found -- should never happen */
>                         lp = NULL;
>                         goto errout;
>                 }
> -               spin_lock(&nd->queue->xmit_lock);
>         }
>         lp = nd->queue;
>         nd->queue = nd->queue->next;
> +       spin_unlock_irqrestore(&nd->queue_lock, flags);
> +       spin_lock(&lp->xmit_lock);
>         local_bh_disable();
> +       return lp;
>  errout:
>         spin_unlock_irqrestore(&nd->queue_lock, flags);
>         return lp;
> 

-- 
Tilman Schmidt                    E-Mail: tilman@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 254 bytes --]

  reply	other threads:[~2009-10-21 16:24 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-18 22:16 possible circular locking dependency in ISDN PPP Tilman Schmidt
2009-10-19  5:27 ` Xiaotian Feng
2009-10-21 16:24   ` Tilman Schmidt [this message]
2009-10-22  2:27     ` Xiaotian Feng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4ADF35CA.5020207@imap.cc \
    --to=tilman@imap.cc \
    --cc=isdn4linux@listserv.isdn4linux.de \
    --cc=isdn@linux-pingi.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=xtfeng@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.