[PATCH] lib/aux: Print progress output with igt_info

[PATCH] lib/aux: Print progress output with igt_info

[Daniel Vetter]

With the structured logging it makes more sense to tune this down a
bit. Also, this way it is consistent with Thomas Wood's new activity
indicator helper.

Spotted while discussing Thomas' patch with him.

Cc: Thomas Wood <thomas.wood@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 lib/igt_aux.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 180c2742ab10..f6fef5875c69 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -229,11 +229,11 @@ void igt_progress(const char *header, uint64_t i, uint64_t total)
 {
 	int divider = 200;
 
-	if (!isatty(fileno(stderr)))
+	if (!isatty(STDOUT_FILENO))
 		return;
 
 	if (i+1 >= total) {
-		igt_warn("\r%s100%%\n", header);
+		igt_info("\r%s100%%\n", header);
 		return;
 	}
 
@@ -242,7 +242,7 @@ void igt_progress(const char *header, uint64_t i, uint64_t total)
 
 	/* only bother updating about every 0.5% */
 	if (i % (total / divider) == 0)
-		igt_warn("\r%s%3llu%%", header,
+		igt_info("\r%s%3llu%%", header,
 			 (long long unsigned)i * 100 / total);
 }
 
-- 
2.1.1

Re: [PATCH] lib/aux: Print progress output with igt_info

[Chris Wilson]

On Tue, Sep 30, 2014 at 06:45:52PM +0200, Daniel Vetter wrote:
> With the structured logging it makes more sense to tune this down a
> bit. Also, this way it is consistent with Thomas Wood's new activity
> indicator helper.

Tune it down? It was on stderr to segregate it from stdout. So either
the activity indicator is sufficient to take over, or it should be made
so.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] lib/aux: Print progress output with igt_info

[Daniel Vetter]

On Tue, Sep 30, 2014 at 08:58:14PM +0100, Chris Wilson wrote:
> On Tue, Sep 30, 2014 at 06:45:52PM +0200, Daniel Vetter wrote:
> > With the structured logging it makes more sense to tune this down a
> > bit. Also, this way it is consistent with Thomas Wood's new activity
> > indicator helper.
> 
> Tune it down? It was on stderr to segregate it from stdout. So either
> the activity indicator is sufficient to take over, or it should be made
> so.

The revised patch after a bit of discussion with Thomas on irc does it
better. Let me resend.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] lib/aux: Print progress output at INFO level

[Daniel Vetter]

With the structured logging it makes more sense to tune this down a
bit. Also, this way it is consistent with Thomas Wood's new activity
indicator helper.

Spotted while discussing Thomas' patch with him.

v2: Thomas noticed that I've forgotten the fflush. Extract
igt_interactive_info for both igt_progress and igt_print_activity.

v3: Interactive output should go to stderr. Also extract the "is this
a terminal" check.

Cc: Thomas Wood <thomas.wood@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 lib/igt_aux.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 7c07b7d0722c..b32297ee508c 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -214,6 +214,23 @@ void igt_permute_array(void *array, unsigned size,
 	}
 }
 
+__attribute__((format(printf, 1, 2)))
+static void igt_interactive_info(const char *format, ...)
+{
+	va_list args;
+
+	if (!isatty(STDERR_FILENO))
+		return;
+
+	if (igt_log_level > IGT_LOG_INFO)
+		return;
+
+	va_start(args, format);
+	vfprintf(stderr, format, args);
+	va_end(args);
+}
+
+
 /**
  * igt_progress:
  * @header: header string to prepend to the progress indicator
@@ -229,11 +246,8 @@ void igt_progress(const char *header, uint64_t i, uint64_t total)
 {
 	int divider = 200;
 
-	if (!isatty(fileno(stderr)))
-		return;
-
 	if (i+1 >= total) {
-		igt_warn("\r%s100%%\n", header);
+		igt_interactive_info("\r%s100%%\n", header);
 		return;
 	}
 
@@ -242,8 +256,8 @@ void igt_progress(const char *header, uint64_t i, uint64_t total)
 
 	/* only bother updating about every 0.5% */
 	if (i % (total / divider) == 0)
-		igt_warn("\r%s%3llu%%", header,
-			 (long long unsigned)i * 100 / total);
+		igt_interactive_info("\r%s%3llu%%", header,
+				     (long long unsigned)i * 100 / total);
 }
 
 /**
@@ -254,11 +268,7 @@ void igt_progress(const char *header, uint64_t i, uint64_t total)
  */
 void igt_print_activity(void)
 {
-	if (!isatty(STDOUT_FILENO))
-		return;
-
-	igt_info(".");
-	fflush(stdout);
+	igt_interactive_info(".");
 }
 
 /* mappable aperture trasher helper */
-- 
2.1.1

Re: [PATCH] lib/aux: Print progress output at INFO level

[Chris Wilson]

On Tue, Sep 30, 2014 at 10:55:35PM +0200, Daniel Vetter wrote:
> With the structured logging it makes more sense to tune this down a
> bit. Also, this way it is consistent with Thomas Wood's new activity
> indicator helper.
> 
> Spotted while discussing Thomas' patch with him.
> 
> v2: Thomas noticed that I've forgotten the fflush. Extract
> igt_interactive_info for both igt_progress and igt_print_activity.
> 
> v3: Interactive output should go to stderr. Also extract the "is this
> a terminal" check.
> 
> Cc: Thomas Wood <thomas.wood@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Not reusing the igt_warn() is great as well (we had earlier problems
when igt_warn* became smarter...)

Nothing to complain about here, lgtm.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre