linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH] powerpc: fix oops in pcibios_release_device() after pcibios_free_controller()
Date: Tue, 5 Jul 2016 10:34:47 -0300	[thread overview]
Message-ID: <577BB777.2080308@linux.vnet.ibm.com> (raw)
In-Reply-To: <1467687331.13965.27.camel@kernel.crashing.org>

On 07/04/2016 11:55 PM, Benjamin Herrenschmidt wrote:
> Have you considered instead adding a kref to the PHB and only freeing
> it when all devices have been freed ? Or it's too hard to tract device
> creation ?

Yes, considered it, but felt leery of possibly leaving the PHB unfreed
(or block until it is freed) in the call that is supposed to remove it:

for example, if it's not freed yet (because of an userspace program
that holds references, maybe misbehaving or in error)...

- Should the call block? -- in the sense of what would it mean to the
   DLPAR tools (say, drmgr and hmc) that might see a timeout as failure,
   and perhaps won't retry it?
   Or if the administrator requested the adapter/PHB to be removed,
   maybe he's aware of the situation and actually wants the removal
   to happen anyway.

- If it should not block, the phb struct would be left there at mercy
   of the reference holders, and perhaps complicate things if the PHB
   is re-added later on? (say, bring the adapter back to the LPAR)
   Could the tools think it's already added/still present, and then
   stop in error?

Now, putting the complicated scenarios aside, implementing krefs would
touch several other parts of code that I'm less comfortable to change
at this moment, which would probably take a lot more time to do.

So, for simplicity and schedule/backport/time constraints, I've shot
for a less intrusive change that still resolves the problem.

Do you think it's an acceptable solution as is, or needs change or to
be implemented in a different way?

Thanks,

-- 
Mauricio Faria de Oliveira
IBM Linux Technology Center

  reply	other threads:[~2016-07-05 13:34 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-05  1:44 [PATCH] powerpc: fix oops in pcibios_release_device() after pcibios_free_controller() Mauricio Faria de Oliveira
2016-07-05  2:55 ` Benjamin Herrenschmidt
2016-07-05 13:34   ` Mauricio Faria de Oliveira [this message]
2016-07-12 23:07   ` Mauricio Faria de Oliveira
2016-07-13 13:52     ` Mauricio Faria de Oliveira

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=577BB777.2080308@linux.vnet.ibm.com \
    --to=mauricfo@linux.vnet.ibm.com \
    --cc=benh@kernel.crashing.org \
    --cc=linuxppc-dev@lists.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).