All of lore.kernel.org
 help / color / mirror / Atom feed
From: Govindarajulu Varadarajan <gvaradar@cisco.com>
To: Sinan Kaya <okaya@codeaurora.org>
Cc: <benve@cisco.com>, <bhelgaas@google.com>,
	<linux-pci@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<jlbec@evilplan.org>, <hch@lst.de>, <mingo@redhat.com>,
	<peterz@infradead.org>
Subject: Re: [PATCH V2] PCI: AER: fix deadlock in do_recovery
Date: Mon, 2 Oct 2017 17:19:31 -0700	[thread overview]
Message-ID: <alpine.LNX.2.20.1710021714260.30867@cae-iprp-alln-lb.cisco.com> (raw)
In-Reply-To: <a9b14502-4ac6-58dc-89ee-a91354f4d2da@codeaurora.org>

On Sat, 30 Sep 2017, Sinan Kaya wrote:

> On 9/30/2017 1:49 AM, Govindarajulu Varadarajan wrote:
>> This patch does a pci_bus_walk and adds all the devices to a list. After
>> unlocking (up_read) &pci_bus_sem, we go through the list and call
>> err_handler of the devices with devic_lock() held. This way, we dont try
>> to hold both locks at same time.
>
> I do like this approach with some more feedback.
>
> I need a little bit of help here from someone that knows get/put device calls.
>
> I understand get_device() and put_device() are there to increment/decrement
> reference counters. This patch seems to use them as an alternative for device_lock()
> and device_unlock() API.
>

get/put_deivce is not used as an alternative to device_lock() here. We are
incrementing the counter to make sure no one else free the device while they
are in our list. report_error_detected() and other cb will still acquire
device_lock().


> If this is a good assumption, then you can get away with just replacing device_lock()
> with get_device() and device_unlock() with put_device() in the existing code as
> well. Then, you don't need to build a linklist.

  reply	other threads:[~2017-10-03  0:19 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-30  5:49 [PATCH V2] PCI: AER: fix deadlock in do_recovery Govindarajulu Varadarajan
2017-09-30 13:31 ` Sinan Kaya
2017-10-03  0:19   ` Govindarajulu Varadarajan [this message]
2017-10-01  7:55 ` Christoph Hellwig
2017-10-03  0:14   ` Govindarajulu Varadarajan
2017-10-03  8:09     ` Christoph Hellwig
2017-10-03 21:15 ` Bjorn Helgaas
2017-10-05 15:05   ` Wei Yang
2017-10-05 18:42     ` Bjorn Helgaas
2017-10-06  1:11       ` Wei Yang

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=alpine.LNX.2.20.1710021714260.30867@cae-iprp-alln-lb.cisco.com \
    --to=gvaradar@cisco.com \
    --cc=benve@cisco.com \
    --cc=bhelgaas@google.com \
    --cc=hch@lst.de \
    --cc=jlbec@evilplan.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=okaya@codeaurora.org \
    --cc=peterz@infradead.org \
    /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.