LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Cornelia Huck <cohuck@redhat.com>
To: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
Cc: linux-kernel@vger.kernel.org, linux-s390@vger.kernel.org,
	kvm@vger.kernel.org, borntraeger@de.ibm.com,
	bjsdjshi@linux.ibm.com, pasic@linux.ibm.com,
	pmorel@linux.ibm.com, Halil Pasic <pasic@linux.vnet.ibm.com>
Subject: Re: [PATCH v2 5/5] vfio: ccw: add traceponits for interesting error paths
Date: Fri, 27 Apr 2018 12:13:53 +0200	[thread overview]
Message-ID: <20180427121353.4453bdc2.cohuck@redhat.com> (raw)
In-Reply-To: <20180423110113.59385-6-bjsdjshi@linux.vnet.ibm.com>

On Mon, 23 Apr 2018 13:01:13 +0200
Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> wrote:

typo in subject: s/traceponits/tracepoints/

> From: Halil Pasic <pasic@linux.vnet.ibm.com>
> 
> Add some tracepoints so we can inspect what is not working as is should.
> 
> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> Signed-off-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
> ---
>  drivers/s390/cio/Makefile         |  1 +
>  drivers/s390/cio/vfio_ccw_fsm.c   | 16 +++++++-
>  drivers/s390/cio/vfio_ccw_trace.h | 77 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 93 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/s390/cio/vfio_ccw_trace.h


> @@ -135,6 +142,8 @@ static void fsm_io_request(struct vfio_ccw_private *private,
>  			goto err_out;
>  
>  		io_region->ret_code = cp_prefetch(&private->cp);
> +		trace_vfio_ccw_cp_prefetch(get_schid(private),
> +					   io_region->ret_code);
>  		if (io_region->ret_code) {
>  			cp_free(&private->cp);
>  			goto err_out;
> @@ -142,11 +151,13 @@ static void fsm_io_request(struct vfio_ccw_private *private,
>  
>  		/* Start channel program and wait for I/O interrupt. */
>  		io_region->ret_code = fsm_io_helper(private);
> +		trace_vfio_ccw_fsm_io_helper(get_schid(private),
> +					     io_region->ret_code);
>  		if (io_region->ret_code) {
>  			cp_free(&private->cp);
>  			goto err_out;
>  		}
> -		return;
> +		goto out;
>  	} else if (scsw->cmd.fctl & SCSW_FCTL_HALT_FUNC) {
>  		/* XXX: Handle halt. */
>  		io_region->ret_code = -EOPNOTSUPP;
> @@ -159,6 +170,9 @@ static void fsm_io_request(struct vfio_ccw_private *private,
>  
>  err_out:
>  	private->state = VFIO_CCW_STATE_IDLE;
> +out:
> +	trace_vfio_ccw_io_fctl(scsw->cmd.fctl, get_schid(private),
> +			       io_region->ret_code);
>  }
>  
>  /*

I really don't want to bikeshed, especially as some tracepoints are
better than no tracepoints, but...

We now trace fctl/schid/ret_code unconditionally (good).

We trace the outcome of cp_prefetch() and fsm_io_helper()
unconditionally. We don't, however, trace all things that may go wrong.
We have the tracepoint at the end, but it cannot tell us where the
error came from. Should we have tracepoints in every place (in this
function) that may generate an error? Only if there is an actual error?
Are the two enough for common debug scenarios?

Opinions? We can just go ahead with this and improve things later on, I
guess.

  reply	other threads:[~2018-04-27 10:14 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-23 11:01 [PATCH v2 0/5] vfio: ccw: error handling fixes and improvements Dong Jia Shi
2018-04-23 11:01 ` [PATCH v2 1/5] vfio: ccw: fix cleanup if cp_prefetch fails Dong Jia Shi
2018-04-23 11:38   ` Halil Pasic
2018-04-23 11:40   ` Cornelia Huck
2018-04-23 12:00     ` Halil Pasic
2018-04-24  9:31   ` Cornelia Huck
     [not found]     ` <20180425024341.GY12194@bjsdjshi@linux.vnet.ibm.com>
2018-04-25 11:19       ` Halil Pasic
2018-04-23 11:01 ` [PATCH v2 2/5] vfio: ccw: shorten kernel doc description for pfn_array_pin() Dong Jia Shi
2018-04-23 11:44   ` Cornelia Huck
2018-04-23 11:01 ` [PATCH v2 3/5] vfio: ccw: refactor and improve pfn_array_alloc_pin() Dong Jia Shi
2018-04-23 11:01 ` [PATCH v2 4/5] vfio: ccw: set ccw->cda to NULL defensively Dong Jia Shi
2018-04-23 11:01 ` [PATCH v2 5/5] vfio: ccw: add traceponits for interesting error paths Dong Jia Shi
2018-04-27 10:13   ` Cornelia Huck [this message]
     [not found]     ` <20180428055023.GS5428@bjsdjshi@linux.vnet.ibm.com>
2018-04-30 11:51       ` Cornelia Huck
2018-04-30 14:14         ` Halil Pasic
2018-04-30 15:03           ` Cornelia Huck
2018-04-30 16:51             ` Halil Pasic
     [not found]             ` <20180502022330.GT5428@bjsdjshi@linux.vnet.ibm.com>
     [not found]               ` <20180516065355.GB6363@bjsdjshi@linux.ibm.com>
2018-05-22 12:55                 ` Cornelia Huck
2018-04-23 11:32 ` [PATCH v2 0/5] vfio: ccw: error handling fixes and improvements Cornelia Huck

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=20180427121353.4453bdc2.cohuck@redhat.com \
    --to=cohuck@redhat.com \
    --cc=bjsdjshi@linux.ibm.com \
    --cc=bjsdjshi@linux.vnet.ibm.com \
    --cc=borntraeger@de.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=pasic@linux.ibm.com \
    --cc=pasic@linux.vnet.ibm.com \
    --cc=pmorel@linux.ibm.com \
    --subject='Re: [PATCH v2 5/5] vfio: ccw: add traceponits for interesting error paths' \
    /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

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).