All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Michal Wajdeczko <michal.wajdeczko@intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH i-g-t] tests/kms_frontbuffer_tracking: Correctly handle debugfs errors
Date: Mon, 04 Dec 2017 12:45:13 +0200	[thread overview]
Message-ID: <1512384313.4394.20.camel@linux.intel.com> (raw)
In-Reply-To: <151189019088.22640.16355348567156553997@mail.alporthouse.com>

On Tue, 2017-11-28 at 17:29 +0000, Chris Wilson wrote:
> Quoting Michal Wajdeczko (2017-11-28 17:01:15)
> > In commit 3f6ae7b19 ("igt/kms_frontbuffer_tracking: Keep the debugfs
> > dir around") we introduced custom variant of __igt_debugfs_read function
> > that fires assert when debugfs returns an error. Replace that assert
> > with proper error handling to allow use of errors like -ENODEV.
> 
> Like ENODEV or just ENODEV?
> 
> Oh, it appears I goofed in c5da0662d1c0 and didn't change the open error
> from a bool to an int error code. You could also take the opportunity to
> then return -errno.
> 
> And here we can debate whether you want to use
> 
> if (len < 0) {
> 	igt_assert_eq(len, -ENODEV);
> 	len = 0;
> }

Let's be explicit at this time, can you resend (with the R-b and this
change), Michal. I'll then merge this and the kernel patch.

Regards, Joonas

> 
> Other than the topic of whether we want to flag any other error here
> (e.g. one could imagine a surprising EPERM, EACCES or ENFILE), looks ok.
> 
> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2017-12-04 10:45 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-28 17:01 [PATCH i-g-t] tests/kms_frontbuffer_tracking: Correctly handle debugfs errors Michal Wajdeczko
2017-11-28 17:23 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-11-28 17:29 ` [PATCH i-g-t] " Chris Wilson
2017-12-04 10:45   ` Joonas Lahtinen [this message]
2017-12-15 12:59     ` Chris Wilson
2017-11-29  7:58 ` ✓ Fi.CI.IGT: success for " Patchwork
2017-12-13 16:59 ` ✓ Fi.CI.BAT: " Patchwork
2017-12-13 18:34 ` ✗ Fi.CI.IGT: warning " 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=1512384313.4394.20.camel@linux.intel.com \
    --to=joonas.lahtinen@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=michal.wajdeczko@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 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.