From: "Dixit, Ashutosh" <ashutosh.dixit@intel.com>
To: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 7/7] drm/i915/perf: add flushing ioctl
Date: Wed, 04 Mar 2020 21:56:28 -0800 [thread overview]
Message-ID: <87o8tb2vlf.wl-ashutosh.dixit@intel.com> (raw)
In-Reply-To: <bc369a53-ec87-f459-e798-2212f9a73d90@intel.com>
On Wed, 04 Mar 2020 00:52:34 -0800, Lionel Landwerlin wrote:
>
> On 04/03/2020 07:48, Dixit, Ashutosh wrote:
> > On Tue, 03 Mar 2020 14:19:05 -0800, Umesh Nerlige Ramappa wrote:
> >> From: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> >>
> >> With the currently available parameters for the i915-perf stream,
> >> there are still situations that are not well covered :
> >>
> >> If an application opens the stream with polling disable or at very low
> >> frequency and OA interrupt enabled, no data will be available even
> >> though somewhere between nothing and half of the OA buffer worth of
> >> data might have landed in memory.
> >>
> >> To solve this issue we have a new flush ioctl on the perf stream that
> >> forces the i915-perf driver to look at the state of the buffer when
> >> called and makes any data available through both poll() & read() type
> >> syscalls.
> >>
> >> v2: Version the ioctl (Joonas)
> >> v3: Rebase (Umesh)
> >>
> >> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> >> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> > [snip]
> >
> >> +/**
> >> + * i915_perf_flush_data - handle `I915_PERF_IOCTL_FLUSH_DATA` ioctl
> >> + * @stream: An enabled i915 perf stream
> >> + *
> >> + * The intention is to flush all the data available for reading from the OA
> >> + * buffer
> >> + */
> >> +static void i915_perf_flush_data(struct i915_perf_stream *stream)
> >> +{
> >> + stream->pollin = oa_buffer_check(stream, true);
> >> +}
> > Since this function doesn't actually wake up any thread (which anyway can
> > be done by sending a signal to the blocked thread), is the only purpose of
> > this function to update OA buffer head/tail? But in that it is not clear
> > why a separate ioctl should be created for this, can't the read() call
> > itself call oa_buffer_check() to update the OA buffer head/tail?
> >
> > Again just trying to minimize uapi changes if possible.
>
> Most applications will call read() after being notified by poll()/select()
> that some data is available.
Correct this is the standard non blocking read behavior.
> Changing that behavior will break some of the existing perf tests .
I am not suggesting changing that (that standard non blocking read
behavior).
> If any data is available, this new ioctl will wake up existing waiters on
> poll()/select().
The issue is we are not calling wake_up() in the above function to wake up
any blocked waiters. The ioctl will just update the OA buffer head/tail so
that (a) a subsequent blocking read will not block, or (b) a subsequent non
blocking read will return valid data (not -EAGAIN), or (c) a poll/select
will not block but return immediately saying data is available.
That is why it seems to me the ioctl is not required, updating the OA
buffer head/tail can be done as part of the read() (and the poll/select)
calls themselves.
We will investigate if this can be done and update the patches in the next
revision accordingly. Thanks!
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2020-03-05 5:56 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-03 22:18 [Intel-gfx] [PATCH 0/7] drm/i915/perf: add OA interrupt support Umesh Nerlige Ramappa
2020-03-03 22:18 ` [Intel-gfx] [PATCH 1/7] drm/i915/perf: rework aging tail workaround Umesh Nerlige Ramappa
2020-03-03 22:19 ` [Intel-gfx] [PATCH 2/7] drm/i915/perf: move pollin setup to non hw specific code Umesh Nerlige Ramappa
2020-03-03 22:19 ` [Intel-gfx] [PATCH 3/7] drm/i915/perf: only append status when data is available Umesh Nerlige Ramappa
2020-03-03 22:19 ` [Intel-gfx] [PATCH 4/7] drm/i915/perf: add new open param to configure polling of OA buffer Umesh Nerlige Ramappa
2020-03-12 19:27 ` Dixit, Ashutosh
2020-03-12 20:37 ` Lionel Landwerlin
2020-03-12 22:20 ` Dixit, Ashutosh
2020-03-12 23:10 ` Umesh Nerlige Ramappa
2020-03-03 22:19 ` [Intel-gfx] [PATCH 5/7] drm/i915: handle interrupts from the OA unit Umesh Nerlige Ramappa
2020-03-03 22:19 ` [Intel-gfx] [PATCH 6/7] drm/i915/perf: add interrupt enabling parameter Umesh Nerlige Ramappa
2020-03-04 5:47 ` Dixit, Ashutosh
2020-03-04 8:55 ` Lionel Landwerlin
2020-03-10 20:08 ` Umesh Nerlige Ramappa
2020-03-10 20:57 ` Lionel Landwerlin
2020-03-03 22:19 ` [Intel-gfx] [PATCH 7/7] drm/i915/perf: add flushing ioctl Umesh Nerlige Ramappa
2020-03-04 5:48 ` Dixit, Ashutosh
2020-03-04 8:52 ` Lionel Landwerlin
2020-03-05 5:56 ` Dixit, Ashutosh [this message]
2020-03-09 19:51 ` Umesh Nerlige Ramappa
2020-03-10 20:44 ` Lionel Landwerlin
2020-03-11 3:05 ` Dixit, Ashutosh
2020-03-09 21:15 ` Umesh Nerlige Ramappa
2020-03-04 2:53 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/perf: add OA interrupt support (rev5) Patchwork
2020-03-04 2:58 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-03-04 3:13 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
2020-03-04 3:18 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-03-04 19:31 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
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=87o8tb2vlf.wl-ashutosh.dixit@intel.com \
--to=ashutosh.dixit@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=lionel.g.landwerlin@intel.com \
/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).