Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1353466
| From | "Hon Ching(Vicky) Lo" <honclo@linux.vnet.ibm.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH] vTPM: fix missing error handling for suspend operation |
| Date | 2016-03-09 00:00 +0100 |
| Message-ID | <rayLE-7lC-9@gated-at.bofh.it> (permalink) |
| References | <r88si-3Jj-5@gated-at.bofh.it> <r91f4-1ez-3@gated-at.bofh.it> <rawJR-659-47@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
> > > + } else if (crq.msg == (u8) PARTNER_PARTITION_FAILED) {
> > > + dev_err(ibmvtpm->dev,
> > > + "vtpm has terminated fatally; reboot to reinstate a trusted state.\n");
> > > + } else if (crq.msg == (u8) PARTNER_PARTITION_DEREG_CRQ) {
> > > + /* The vtpm is in the process of being reloaded by
> > > + * firmware and has de-registered CRQ. The client
> > > + * must wait for the CRQ INITIALIZATION message and
> > > + * respond and must resubmit suspend message.
> > > + */
> > > + sig =
> > > + wait_event_interruptible(ibmvtpm->wq,
> > > + crq_initialized == 1);
> > > + if (sig)
> > > + return -EINTR;
> > > +
> > > + if (suspend_again_count < 1) {
> > > + suspend_again_count++;
> > > + goto suspendagain;
> > > + }
> > > + } else
> > > + ;
> > > + }
> > > +
> > > if (rc != H_SUCCESS)
> > > - dev_err(ibmvtpm->dev,
> > > - "tpm_ibmvtpm_suspend failed rc=%d\n", rc);
> > > + dev_err(ibmvtpm->dev, "tpm_ibmvtpm_suspend failed rc=%d\n", rc);
> > >
> > > return rc;
> > > +
> > > +suspendagain:
> > > + rc = tpm_ibmvtpm_suspend(ibmvtpm->dev);
> > > + suspend_again_count = 0;
> > > +
> > > + if (rc != H_SUCCESS)
> > > + dev_err(ibmvtpm->dev, "tpm_ibmvtpm_suspend failed rc=%d\n", rc);
> > > +
> > > + return rc;
> > > +
> >
> > Get rid of this horrible looking tail recursion thing.
> >
> > What the heck is suspend_again_count and why it can be module scope
> > variable? You could use a local variable instead if you would iterate
> > with a loop.
> >
> > /Jarkko
> >
>
> The reason for the 'goto' statement and the suspend_again_count was to
> prevent the suspend function recurse again. In the case if vtpm is in
> the process of being reloaded by firmware, we want to wait for the CRQ
> INITIALIZATION and resubmit suspend message i.e. recurse only once.
>
Never mind.. I don't really save any repetitive code by using recursion
now. I'll rework and resubmit the patch.
Thanks,
Vicky
Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread
Re: [PATCH] vTPM: fix missing error handling for suspend operation Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-03-04 18:00 +0100
Re: [PATCH] vTPM: fix missing error handling for suspend operation "Hon Ching(Vicky) Lo" <honclo@linux.vnet.ibm.com> - 2016-03-08 21:50 +0100
Re: [PATCH] vTPM: fix missing error handling for suspend operation "Hon Ching(Vicky) Lo" <honclo@linux.vnet.ibm.com> - 2016-03-09 00:00 +0100
csiph-web