All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Ellerman <mpe@ellerman.id.au>
To: Ian Munsie <imunsie@au1.ibm.com>
Cc: Ryan Grimm <grimm@linux.vnet.ibm.com>,
	linuxppc-dev <linuxppc-dev@ozlabs.org>
Subject: Re: [PATCH] CXL: Fix device_node reference counting
Date: Wed, 28 Jan 2015 17:07:45 +1100	[thread overview]
Message-ID: <1422425265.11009.1.camel@ellerman.id.au> (raw)
In-Reply-To: <1422423577-sup-6588@delenn.ozlabs.ibm.com>

On Wed, 2015-01-28 at 16:53 +1100, Ian Munsie wrote:
> Excerpts from Michael Ellerman's message of 2015-01-28 16:04:40 +1100:
> > > I just wanted to check the status of this one? I can't see it in your
> > > tree and wanted to make sure you didn't simply miss it.
> > 
> > It looked fishy, but I never got around to replying.
> > 
> > The second sentence in the explanation should never be true:
> 
> Right, that was the point of the fix ;)

Sure, but bodging of_node_get()s all over the place is not a path to success.

> > You shouldn't have np unless you did an of_node_get() to get it, otherwise it's
> > pointing at something you don't have a reference for and it might go away at
> > any time.
> > 
> > So the patch may fix the bug but I don't think it's correct.
> > 
> > I think pnv_pci_to_phb_node() should be doing a get for you, before returning
> > the pointer.
> 
> Agreed - we should probably also rename it to have 'get' in the name,
> like pnv_pci_get_phb_node().

Yep.

> > See as a comparison pcibios_get_phb_of_node().
> 
> We could almost use that instead, except it's not exported for modules
> and I'm not sure if that even works with __weak functions?

It should. It's only weak until the final link and then you get a non-weak
version AIUI.

Try it.

And a follow up patch to have it use pci_bus_to_host() would be nice too.

cheers

  reply	other threads:[~2015-01-28  6:07 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-07  5:41 [PATCH] CXL: Fix device_node reference counting Ian Munsie
2015-01-07  5:41 ` Ian Munsie
2015-01-28  4:02 ` Ian Munsie
2015-01-28  5:04   ` Michael Ellerman
2015-01-28  5:53     ` Ian Munsie
2015-01-28  6:07       ` Michael Ellerman [this message]
2015-01-29  2:15       ` Ryan Grimm
  -- strict thread matches above, loose matches on Subject: below --
2015-01-29  2:16 Ryan Grimm
2015-01-29  2:50 ` Ian Munsie

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=1422425265.11009.1.camel@ellerman.id.au \
    --to=mpe@ellerman.id.au \
    --cc=grimm@linux.vnet.ibm.com \
    --cc=imunsie@au1.ibm.com \
    --cc=linuxppc-dev@ozlabs.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.