All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Winkler, Tomas" <tomas.winkler@intel.com>
To: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	"tpmdd-devel@lists.sourceforge.net" 
	<tpmdd-devel@lists.sourceforge.net>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] tpm: don't destroy chip device prematurely
Date: Fri, 7 Oct 2016 20:10:39 +0000	[thread overview]
Message-ID: <5B8DA87D05A7694D9FA63FD143655C1B542F6305@hasmsx108.ger.corp.intel.com> (raw)
In-Reply-To: <20161007191724.GA28795@obsidianresearch.com>

> Subject: Re: [PATCH] tpm: don't destroy chip device prematurely
> 
> On Fri, Oct 07, 2016 at 02:24:59PM +0000, Winkler, Tomas wrote:
> 
> > So here I'm to say I'm sorry for misleading this, after all the doubts
> > I got back to debugging and traces.  One thing for a reason moving the
> > device_del, had really made the problem go away, but the real problem
> > was unbalance runtime_pm PUT/GET from the tpm_crb probe function.
> 
> Oh this is very good news, I'm glad this was resolved in crb!
> 
> Presumably the unbalanced put made the ref count go negative and the
> balanced get caused it to go to zero, so pm locking was basically totally
> broken? That would explain how an idle callback could run concurrently with
> transmit_cmd.

This is not due to locking and refcount, but similar. The usage_count went negative  and the idle callback kicked in from the pm work queue, and suspended the device.

> 
> Though a bit of a mystery why device_del had any impact? I'm still very
> unclear exactly how the child device effects the parent - and that seems like
> pretty important information going forward..

Yes, there is some dependency as if device_del is not called the idle callback doesn't kick in between send and receive and that was misleading.  I'm not sure but this could be due to scheduling of the pm worker, but I'm not sure.   In any case we hit the issue even w/o device_del if the device is exercise enough.  I will dig into that later.

Thanks
Tomas

WARNING: multiple messages have this Message-ID (diff)
From: "Winkler, Tomas" <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
To: Jason Gunthorpe
	<jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Cc: "tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org"
	<tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [PATCH] tpm: don't destroy chip device prematurely
Date: Fri, 7 Oct 2016 20:10:39 +0000	[thread overview]
Message-ID: <5B8DA87D05A7694D9FA63FD143655C1B542F6305@hasmsx108.ger.corp.intel.com> (raw)
In-Reply-To: <20161007191724.GA28795-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

> Subject: Re: [PATCH] tpm: don't destroy chip device prematurely
> 
> On Fri, Oct 07, 2016 at 02:24:59PM +0000, Winkler, Tomas wrote:
> 
> > So here I'm to say I'm sorry for misleading this, after all the doubts
> > I got back to debugging and traces.  One thing for a reason moving the
> > device_del, had really made the problem go away, but the real problem
> > was unbalance runtime_pm PUT/GET from the tpm_crb probe function.
> 
> Oh this is very good news, I'm glad this was resolved in crb!
> 
> Presumably the unbalanced put made the ref count go negative and the
> balanced get caused it to go to zero, so pm locking was basically totally
> broken? That would explain how an idle callback could run concurrently with
> transmit_cmd.

This is not due to locking and refcount, but similar. The usage_count went negative  and the idle callback kicked in from the pm work queue, and suspended the device.

> 
> Though a bit of a mystery why device_del had any impact? I'm still very
> unclear exactly how the child device effects the parent - and that seems like
> pretty important information going forward..

Yes, there is some dependency as if device_del is not called the idle callback doesn't kick in between send and receive and that was misleading.  I'm not sure but this could be due to scheduling of the pm worker, but I'm not sure.   In any case we hit the issue even w/o device_del if the device is exercise enough.  I will dig into that later.

Thanks
Tomas




------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

  reply	other threads:[~2016-10-07 20:10 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-02  7:39 [PATCH] tpm: don't destroy chip device prematurely Tomas Winkler
2016-10-02 10:17 ` Jarkko Sakkinen
2016-10-02 10:17   ` Jarkko Sakkinen
2016-10-02 10:24   ` Jarkko Sakkinen
2016-10-02 10:24     ` Jarkko Sakkinen
2016-10-02 21:21     ` Jason Gunthorpe
2016-10-02 21:21       ` Jason Gunthorpe
2016-10-03  7:05       ` Winkler, Tomas
2016-10-03  7:05         ` Winkler, Tomas
2016-10-03  7:38         ` Winkler, Tomas
2016-10-03  7:38           ` Winkler, Tomas
2016-10-03 12:42           ` Jarkko Sakkinen
2016-10-03 12:42             ` Jarkko Sakkinen
2016-10-03 16:03             ` Jason Gunthorpe
2016-10-03 16:03               ` Jason Gunthorpe
2016-10-03 17:30               ` Winkler, Tomas
2016-10-03 17:30                 ` Winkler, Tomas
2016-10-03 12:48         ` Jarkko Sakkinen
2016-10-03 12:48           ` Jarkko Sakkinen
2016-10-04  5:19           ` Jarkko Sakkinen
2016-10-04  5:19             ` Jarkko Sakkinen
2016-10-04 16:47             ` Jason Gunthorpe
2016-10-04 16:47               ` Jason Gunthorpe
2016-10-04 21:55               ` Winkler, Tomas
2016-10-04 21:55                 ` Winkler, Tomas
2016-10-04 23:10                 ` Jason Gunthorpe
2016-10-04 23:10                   ` Jason Gunthorpe
2016-10-05  7:48                   ` Winkler, Tomas
2016-10-05  7:48                     ` Winkler, Tomas
2016-10-05 15:15                     ` Jarkko Sakkinen
2016-10-05 15:15                       ` Jarkko Sakkinen
2016-10-05 16:37                       ` Jason Gunthorpe
2016-10-05 16:37                         ` Jason Gunthorpe
2016-10-05 17:11                     ` Jason Gunthorpe
2016-10-05 17:11                       ` Jason Gunthorpe
2016-10-05 20:09                       ` Winkler, Tomas
2016-10-05 20:09                         ` Winkler, Tomas
2016-10-05 21:16                         ` Jason Gunthorpe
2016-10-05 21:16                           ` Jason Gunthorpe
2016-10-06  0:43                           ` Winkler, Tomas
2016-10-06  0:43                             ` Winkler, Tomas
2016-10-06  2:07                             ` Jason Gunthorpe
2016-10-06  2:07                               ` Jason Gunthorpe
2016-10-07 14:24                               ` Winkler, Tomas
2016-10-07 14:24                                 ` Winkler, Tomas
2016-10-07 19:17                                 ` Jason Gunthorpe
2016-10-07 19:17                                   ` Jason Gunthorpe
2016-10-07 20:10                                   ` Winkler, Tomas [this message]
2016-10-07 20:10                                     ` Winkler, Tomas
2016-10-08 10:47                                 ` Jarkko Sakkinen
2016-10-08 10:47                                   ` Jarkko Sakkinen
2016-10-05 10:02               ` Jarkko Sakkinen
2016-10-05 10:02                 ` Jarkko Sakkinen
2016-10-05 16:27                 ` Jason Gunthorpe
2016-10-05 16:27                   ` Jason Gunthorpe
2016-10-06 11:23                   ` Jarkko Sakkinen
2016-10-06 11:23                     ` Jarkko Sakkinen
2016-10-06 16:22                     ` Jason Gunthorpe
2016-10-06 16:22                       ` Jason Gunthorpe
2016-10-06 16:46                       ` Jarkko Sakkinen
2016-10-06 16:46                         ` Jarkko Sakkinen
2016-10-05 10:09               ` Jarkko Sakkinen
2016-10-05 10:09                 ` Jarkko Sakkinen
2016-10-03 16:00         ` Jason Gunthorpe
2016-10-03 16:00           ` Jason Gunthorpe
2016-10-03 17:16           ` Winkler, Tomas
2016-10-03 17:16             ` Winkler, Tomas
2016-10-03 17:30             ` Jason Gunthorpe
2016-10-03 17:30               ` Jason Gunthorpe

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=5B8DA87D05A7694D9FA63FD143655C1B542F6305@hasmsx108.ger.corp.intel.com \
    --to=tomas.winkler@intel.com \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=jgunthorpe@obsidianresearch.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tpmdd-devel@lists.sourceforge.net \
    /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.