From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.7 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6626DC43331 for ; Fri, 27 Mar 2020 23:11:36 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 410D020717 for ; Fri, 27 Mar 2020 23:11:36 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 410D020717 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id CB05189C63; Fri, 27 Mar 2020 23:11:35 +0000 (UTC) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3979C89C63 for ; Fri, 27 Mar 2020 23:11:34 +0000 (UTC) IronPort-SDR: imS9FKncYq5+WCflaf9P2WPClnAhDT4XZLrV8GP+OIJLG2hTG3Ng8q69Gtk98k32bVCd2f92dC wh2YZyHDOcHQ== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Mar 2020 16:11:33 -0700 IronPort-SDR: iFlQAc+b8bcS0zaSq3Ztw6Cqc6O65rkSJh+iK7v4kJ0VHRrScACnnFIDy3YX2az+fGhltCEV0J KBkldsPIUWpw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,314,1580803200"; d="scan'208";a="358630999" Received: from adixit-mobl.amr.corp.intel.com (HELO localhost.localdomain) ([10.251.29.122]) by fmsmga001.fm.intel.com with ESMTP; 27 Mar 2020 16:11:33 -0700 From: Ashutosh Dixit To: intel-gfx@lists.freedesktop.org Date: Fri, 27 Mar 2020 16:11:31 -0700 Message-Id: <7f5ea67b41ef47b99599fec5473ce216f7e39e91.1585350600.git.ashutosh.dixit@intel.com> X-Mailer: git-send-email 2.25.2 MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH] drm/i915/perf: Do not clear pollin for small user read buffers X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" It is wrong to block the user thread in the next poll when OA data is already available which could not fit in the user buffer provided in the previous read. In several cases the exact user buffer size is not known. Blocking user space in poll can lead to data loss when the buffer size used is smaller than the available data. This change fixes this issue and allows user space to read all OA data even when using a buffer size smaller than the available data using multiple non-blocking reads rather than staying blocked in poll till the next timer interrupt. v2: Fix ret value for blocking reads (Umesh) v2: Remove -EAGAIN from comment (Umesh) Cc: Umesh Nerlige Ramappa Cc: Lionel Landwerlin Signed-off-by: Ashutosh Dixit --- drivers/gpu/drm/i915/i915_perf.c | 59 +++++++------------------------- 1 file changed, 12 insertions(+), 47 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index c74ebac50015..5f6d9bff99c8 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -2914,49 +2914,6 @@ void i915_oa_init_reg_state(const struct intel_context *ce, gen8_update_reg_state_unlocked(ce, stream); } -/** - * i915_perf_read_locked - &i915_perf_stream_ops->read with error normalisation - * @stream: An i915 perf stream - * @file: An i915 perf stream file - * @buf: destination buffer given by userspace - * @count: the number of bytes userspace wants to read - * @ppos: (inout) file seek position (unused) - * - * Besides wrapping &i915_perf_stream_ops->read this provides a common place to - * ensure that if we've successfully copied any data then reporting that takes - * precedence over any internal error status, so the data isn't lost. - * - * For example ret will be -ENOSPC whenever there is more buffered data than - * can be copied to userspace, but that's only interesting if we weren't able - * to copy some data because it implies the userspace buffer is too small to - * receive a single record (and we never split records). - * - * Another case with ret == -EFAULT is more of a grey area since it would seem - * like bad form for userspace to ask us to overrun its buffer, but the user - * knows best: - * - * http://yarchive.net/comp/linux/partial_reads_writes.html - * - * Returns: The number of bytes copied or a negative error code on failure. - */ -static ssize_t i915_perf_read_locked(struct i915_perf_stream *stream, - struct file *file, - char __user *buf, - size_t count, - loff_t *ppos) -{ - /* Note we keep the offset (aka bytes read) separate from any - * error status so that the final check for whether we return - * the bytes read with a higher precedence than any error (see - * comment below) doesn't need to be handled/duplicated in - * stream->ops->read() implementations. - */ - size_t offset = 0; - int ret = stream->ops->read(stream, buf, count, &offset); - - return offset ?: (ret ?: -EAGAIN); -} - /** * i915_perf_read - handles read() FOP for i915 perf stream FDs * @file: An i915 perf stream file @@ -2982,6 +2939,8 @@ static ssize_t i915_perf_read(struct file *file, { struct i915_perf_stream *stream = file->private_data; struct i915_perf *perf = stream->perf; + size_t offset = 0; + int __ret; ssize_t ret; /* To ensure it's handled consistently we simply treat all reads of a @@ -3005,16 +2964,19 @@ static ssize_t i915_perf_read(struct file *file, return ret; mutex_lock(&perf->lock); - ret = i915_perf_read_locked(stream, file, - buf, count, ppos); + __ret = stream->ops->read(stream, buf, count, &offset); + ret = offset ?: (__ret ?: -EAGAIN); mutex_unlock(&perf->lock); } while (ret == -EAGAIN); } else { mutex_lock(&perf->lock); - ret = i915_perf_read_locked(stream, file, buf, count, ppos); + __ret = stream->ops->read(stream, buf, count, &offset); + ret = offset ?: (__ret ?: -EAGAIN); mutex_unlock(&perf->lock); } + /* Possible values for __ret are 0, -EFAULT, -ENOSPC, -EIO, ... */ + /* We allow the poll checking to sometimes report false positive EPOLLIN * events where we might actually report EAGAIN on read() if there's * not really any data available. In this situation though we don't @@ -3022,8 +2984,11 @@ static ssize_t i915_perf_read(struct file *file, * and read() returning -EAGAIN. Clearing the oa.pollin state here * effectively ensures we back off until the next hrtimer callback * before reporting another EPOLLIN event. + * The exception to this is if ops->read() returned -ENOSPC which means + * that more OA data is available than could fit in the user provided + * buffer. In this case we want the next poll() call to not block. */ - if (ret >= 0 || ret == -EAGAIN) + if ((ret > 0 || ret == -EAGAIN) && __ret != -ENOSPC) stream->pollin = false; return ret; -- 2.25.2 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx