* [PATCH i-g-t 1/3] lib/{igt_sysfs, igt_aux}: Make available to other users kick_fbcon() (unbind_fbcon()), and added helpers to igt_aux.
2016-10-20 19:36 [PATCH i-g-t 0/3] Convert sh scripts to C variants Marius Vlad
@ 2016-10-20 19:36 ` Marius Vlad
2016-10-20 20:09 ` Chris Wilson
2016-10-24 8:40 ` Daniel Vetter
2016-10-20 19:36 ` [PATCH i-g-t 2/3] tests/drv_module_reload_basic: Convert sh script to C version Marius Vlad
` (2 subsequent siblings)
3 siblings, 2 replies; 19+ messages in thread
From: Marius Vlad @ 2016-10-20 19:36 UTC (permalink / raw)
To: intel-gfx
Previously under unbind_fbcon(), disable/enable framebuffer console.
lib/igt_aux: Added helpers to help convert sh scripts to C version. libkmod and
procps interface.
Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
---
configure.ac | 2 +
lib/Makefile.am | 2 +
lib/igt_aux.c | 278 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
lib/igt_aux.h | 7 ++
lib/igt_gvt.c | 43 +--------
lib/igt_sysfs.c | 54 +++++++++++
lib/igt_sysfs.h | 2 +
7 files changed, 347 insertions(+), 41 deletions(-)
diff --git a/configure.ac b/configure.ac
index 735cfd5..2c6e49d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -121,6 +121,8 @@ AC_SUBST(ASSEMBLER_WARN_CFLAGS)
PKG_CHECK_MODULES(DRM, [libdrm])
PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10])
+PKG_CHECK_MODULES(KMOD, [libkmod])
+PKG_CHECK_MODULES(PROCPS, [libprocps])
case "$target_cpu" in
x86*|i?86)
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 4c0893d..e1737bd 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -34,6 +34,8 @@ AM_CFLAGS += $(CAIRO_CFLAGS)
libintel_tools_la_LIBADD = \
$(DRM_LIBS) \
$(PCIACCESS_LIBS) \
+ $(PROCPS_LIBS) \
+ $(KMOD_LIBS) \
$(CAIRO_LIBS) \
$(LIBUDEV_LIBS) \
$(LIBUNWIND_LIBS) \
diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 421f6d4..d150818 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -51,6 +51,9 @@
#include <termios.h>
#include <assert.h>
+#include <proc/readproc.h>
+#include <libkmod.h>
+
#include "drmtest.h"
#include "i915_drm.h"
#include "intel_chipset.h"
@@ -1193,6 +1196,281 @@ void igt_set_module_param_int(const char *name, int val)
igt_set_module_param(name, str);
}
+/**
+ * igt_pkill:
+ * @sig: Signal to send
+ * @name: Name of process in the form found in /proc/pid/comm (limited to 15
+ * chars)
+ * @returns: 0 in case the process is not found running or the signal has been
+ * sent successfully or -1 otherwise.
+ *
+ * This function sends the signal @sig for a process found in process table
+ * with name @comm.
+ */
+int
+igt_pkill(int sig, const char *comm)
+{
+ int err = 0, try = 5;
+ PROCTAB *proc;
+ proc_t *proc_info;
+
+ proc = openproc(PROC_FILLCOM | PROC_FILLSTAT | PROC_FILLARG);
+ igt_assert(proc != NULL);
+
+ while ((proc_info = readproc(proc, NULL))) {
+ if (proc_info &&
+ !strncasecmp(proc_info->cmd, comm, sizeof(proc_info->cmd))) {
+ switch (sig) {
+ case SIGTERM:
+ case SIGKILL:
+ do {
+ kill(proc_info->tid, sig);
+ } while (kill(proc_info->tid, 0) < 0 && try--);
+
+ if (kill(proc_info->tid, 0) < 0)
+ err = -1;
+ break;
+ default:
+ if (kill(proc_info->tid, sig) < 0)
+ err = -1;
+ break;
+ }
+
+ freeproc(proc_info);
+ break;
+ }
+ freeproc(proc_info);
+ }
+
+ closeproc(proc);
+ return err;
+}
+
+/**
+ * igt_kill:
+ * @sig: Signal to send.
+ * @pid: Process pid to send.
+ * @returns: 0 in case of success or -1 otherwise.
+ *
+ * This function is identical to igt_pkill() only that it searches the process
+ * table using @pid instead of comm name.
+ *
+ */
+int
+igt_kill(int sig, pid_t pid)
+{
+ int err = 0, try = 5;
+ PROCTAB *proc;
+ proc_t *proc_info;
+
+ proc = openproc(PROC_PID | PROC_FILLSTAT | PROC_FILLARG);
+ igt_assert(proc != NULL);
+
+ while ((proc_info = readproc(proc, NULL))) {
+ if (proc_info && proc_info->tid == pid) {
+ switch (sig) {
+ case SIGTERM:
+ case SIGKILL:
+ do {
+ kill(proc_info->tid, sig);
+ } while (kill(proc_info->tid, 0) < 0 && try--);
+
+ if (kill(proc_info->tid, 0) < 0)
+ err = -1;
+ break;
+ default:
+ if (kill(proc_info->tid, sig) < 0)
+ err = -1;
+ break;
+ }
+ freeproc(proc_info);
+ break;
+ }
+ freeproc(proc_info);
+ }
+
+ closeproc(proc);
+ return err;
+}
+
+static bool
+igt_module_in_use(struct kmod_module *kmod)
+{
+ int err;
+
+ err = kmod_module_get_initstate(kmod);
+
+ /* if compiled builtin or does not exist */
+ if (err == KMOD_MODULE_BUILTIN || err < 0)
+ return false;
+
+ if (kmod_module_get_refcnt(kmod) != 0 ||
+ kmod_module_get_holders(kmod) != NULL)
+ return true;
+
+
+ return false;
+}
+
+/**
+ * igt_lsmod:
+ * @mod_name: The name of the module.
+ * @returns: True in case the module has been found or false otherwise.
+ *
+ *
+ * Function to check the existance of module @mod_name in list of loaded kernel
+ * modules.
+ *
+ */
+bool
+igt_lsmod_has_module(const char *mod_name)
+{
+ struct kmod_list *mod, *list;
+ struct kmod_ctx *ctx;
+ bool ret = false;
+
+ ctx = kmod_new(NULL, NULL);
+ igt_assert(ctx != NULL);
+
+ if (kmod_module_new_from_loaded(ctx, &list) < 0) {
+ kmod_unref(ctx);
+ return ret;
+ }
+
+ kmod_list_foreach(mod, list) {
+ struct kmod_module *kmod = kmod_module_get_module(mod);
+ const char *kmod_name = kmod_module_get_name(kmod);
+
+ if (!strncasecmp(kmod_name, mod_name, strlen(kmod_name))) {
+ kmod_module_unref(kmod);
+ ret = true;
+ break;
+ }
+ kmod_module_unref(kmod);
+ }
+
+ kmod_module_unref_list(list);
+ kmod_unref(ctx);
+
+ return ret;
+}
+
+
+/**
+ * igt_insmod:
+ * @mod_name: The name of the module
+ * @opts: Parameters for the module. NULL in case no parameters
+ * are to be passed, or a '\0' terminated string otherwise.
+ * @returns: 0 in case of success or -1 in case the module could not
+ * be loaded.
+ *
+ *
+ * This function loads a kernel module using the name specified in @mod_name.
+ *
+ * @Note: This functions doesn't automatically resolve other module
+ * dependencies so make make sure you load the dependencies module(s) before
+ * this one.
+ */
+int
+igt_insmod(const char *mod_name, const char *opts)
+{
+ struct kmod_ctx *ctx;
+ struct kmod_module *kmod;
+ int err = 0;
+
+ ctx = kmod_new(NULL, NULL);
+ igt_assert(ctx != NULL);
+
+
+ err = kmod_module_new_from_name(ctx, mod_name, &kmod);
+ if (err < 0) {
+ goto out;
+ }
+
+ err = kmod_module_insert_module(kmod, 0, opts);
+ if (err < 0) {
+ switch (err) {
+ case -EEXIST:
+ igt_info("Module %s already inserted\n",
+ kmod_module_get_name(kmod));
+ err = -1;
+ break;
+ case -ENOENT:
+ igt_info("Unknown symbol in module %s or "
+ "unknown parameter\n",
+ kmod_module_get_name(kmod));
+ err = -1;
+ break;
+ default:
+ igt_info("Could not insert %s (%s)\n",
+ kmod_module_get_name(kmod), strerror(-err));
+ err = -1;
+ break;
+ }
+ }
+out:
+ kmod_module_unref(kmod);
+ kmod_unref(ctx);
+
+ return err;
+}
+
+
+/**
+ * igt_rmmod:
+ * @mod_name: Module name.
+ * @force: A bool value to force removal. Note that his flag does not remove
+ * the module(s) that the module specified by @mod_name depends on. In other
+ * words you must igt_rmmod() the module(s) dependencies before calling it with
+ * @mod_name.
+ * @returns: 0 in case of success or -1 otherwise.
+ *
+ * Removes the module @mod_name.
+ *
+ */
+int
+igt_rmmod(const char *mod_name, bool force)
+{
+ struct kmod_ctx *ctx;
+ struct kmod_module *kmod;
+ int err, flags = 0;
+
+ ctx = kmod_new(NULL, NULL);
+ igt_assert(ctx != NULL);
+
+ err = kmod_module_new_from_name(ctx, mod_name, &kmod);
+ if (err < 0) {
+ igt_info("Could not use module %s (%s)\n", mod_name,
+ strerror(-err));
+ err = -1;
+ goto out;
+ }
+
+ if (igt_module_in_use(kmod)) {
+ igt_info("Module %s is in use\n", mod_name);
+ err = -1;
+ goto out;
+ }
+
+ if (force) {
+ flags |= KMOD_REMOVE_FORCE;
+ }
+
+ err = kmod_module_remove_module(kmod, flags);
+ if (err < 0) {
+ igt_info("Could not remove module %s (%s)\n", mod_name,
+ strerror(-err));
+ err = -1;
+ }
+
+out:
+ kmod_module_unref(kmod);
+ kmod_unref(ctx);
+
+ return err;
+}
+
+
static struct igt_siglatency {
timer_t timer;
struct timespec target;
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index d30196b..7946923 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -264,4 +264,11 @@ double igt_stop_siglatency(struct igt_mean *result);
void igt_set_module_param(const char *name, const char *val);
void igt_set_module_param_int(const char *name, int val);
+int igt_pkill(int sig, const char *comm);
+int igt_kill(int sig, pid_t pid);
+
+bool igt_lsmod_has_module(const char *mod_name);
+int igt_insmod(const char *mod_name, const char *opts);
+int igt_rmmod(const char *mod_name, bool force);
+
#endif /* IGT_AUX_H */
diff --git a/lib/igt_gvt.c b/lib/igt_gvt.c
index 0f332d1..8bbf9bd 100644
--- a/lib/igt_gvt.c
+++ b/lib/igt_gvt.c
@@ -23,6 +23,7 @@
#include "igt.h"
#include "igt_gvt.h"
+#include "igt_sysfs.h"
#include <dirent.h>
#include <unistd.h>
@@ -46,49 +47,9 @@ static bool is_gvt_enabled(void)
return enabled;
}
-static void unbind_fbcon(void)
-{
- char buf[128];
- const char *path = "/sys/class/vtconsole";
- DIR *dir;
- struct dirent *vtcon;
-
- dir = opendir(path);
- if (!dir)
- return;
-
- while ((vtcon = readdir(dir))) {
- int fd, len;
-
- if (strncmp(vtcon->d_name, "vtcon", 5))
- continue;
-
- sprintf(buf, "%s/%s/name", path, vtcon->d_name);
- fd = open(buf, O_RDONLY);
- if (fd < 0)
- continue;
-
- len = read(fd, buf, sizeof(buf) - 1);
- close(fd);
- if (len >= 0)
- buf[len] = '\0';
-
- if (strstr(buf, "frame buffer device")) {
- sprintf(buf, "%s/%s/bind", path, vtcon->d_name);
- fd = open(buf, O_WRONLY);
- if (fd != -1) {
- igt_ignore_warn(write(fd, "1\n", 2));
- close(fd);
- }
- break;
- }
- }
- closedir(dir);
-}
-
static void unload_i915(void)
{
- unbind_fbcon();
+ kick_fbcon(false);
/* pkill alsact */
igt_ignore_warn(system("/sbin/modprobe -s -r i915"));
diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
index 612de75..0803659 100644
--- a/lib/igt_sysfs.c
+++ b/lib/igt_sysfs.c
@@ -34,7 +34,11 @@
#include <fcntl.h>
#include <unistd.h>
#include <i915_drm.h>
+#include <dirent.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include "igt_core.h"
#include "igt_sysfs.h"
/**
@@ -391,3 +395,53 @@ bool igt_sysfs_set_boolean(int dir, const char *attr, bool value)
{
return igt_sysfs_printf(dir, attr, "%d", value) == 1;
}
+
+/**
+ * kick_fbcon:
+ * @enable: boolean value
+ *
+ * This functions enables/disables the text console running on top of the
+ * framebuffer device.
+ */
+void kick_fbcon(bool enable)
+{
+ char buf[128];
+ const char *path = "/sys/class/vtconsole";
+ DIR *dir;
+ struct dirent *vtcon;
+
+ dir = opendir(path);
+ if (!dir)
+ return;
+
+ while ((vtcon = readdir(dir))) {
+ int fd, len;
+
+ if (strncmp(vtcon->d_name, "vtcon", 5))
+ continue;
+
+ sprintf(buf, "%s/%s/name", path, vtcon->d_name);
+ fd = open(buf, O_RDONLY);
+ if (fd < 0)
+ continue;
+
+ len = read(fd, buf, sizeof(buf) - 1);
+ close(fd);
+ if (len >= 0)
+ buf[len] = '\0';
+
+ if (strstr(buf, "frame buffer device")) {
+ sprintf(buf, "%s/%s/bind", path, vtcon->d_name);
+ fd = open(buf, O_WRONLY);
+ if (fd != -1) {
+ if (enable)
+ igt_ignore_warn(write(fd, "1\n", 2));
+ else
+ igt_ignore_warn(write(fd, "0\n", 2));
+ close(fd);
+ }
+ break;
+ }
+ }
+ closedir(dir);
+}
diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
index 4820066..a141894 100644
--- a/lib/igt_sysfs.h
+++ b/lib/igt_sysfs.h
@@ -43,4 +43,6 @@ bool igt_sysfs_set_u32(int dir, const char *attr, uint32_t value);
bool igt_sysfs_get_boolean(int dir, const char *attr);
bool igt_sysfs_set_boolean(int dir, const char *attr, bool value);
+void kick_fbcon(bool enable);
+
#endif /* __IGT_SYSFS_H__ */
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH i-g-t 1/3] lib/{igt_sysfs, igt_aux}: Make available to other users kick_fbcon() (unbind_fbcon()), and added helpers to igt_aux.
2016-10-20 19:36 ` [PATCH i-g-t 1/3] lib/{igt_sysfs, igt_aux}: Make available to other users kick_fbcon() (unbind_fbcon()), and added helpers to igt_aux Marius Vlad
@ 2016-10-20 20:09 ` Chris Wilson
2016-10-24 18:08 ` Marius Vlad
2016-10-24 8:40 ` Daniel Vetter
1 sibling, 1 reply; 19+ messages in thread
From: Chris Wilson @ 2016-10-20 20:09 UTC (permalink / raw)
To: Marius Vlad; +Cc: intel-gfx
On Thu, Oct 20, 2016 at 10:36:47PM +0300, Marius Vlad wrote:
> +int
> +igt_pkill(int sig, const char *comm)
> +{
> + int err = 0, try = 5;
> + PROCTAB *proc;
> + proc_t *proc_info;
> +
> + proc = openproc(PROC_FILLCOM | PROC_FILLSTAT | PROC_FILLARG);
> + igt_assert(proc != NULL);
> +
> + while ((proc_info = readproc(proc, NULL))) {
> + if (proc_info &&
proc_info cannot be NULL, you've already tested for that.
> + !strncasecmp(proc_info->cmd, comm, sizeof(proc_info->cmd))) {
> + switch (sig) {
> + case SIGTERM:
> + case SIGKILL:
> + do {
> + kill(proc_info->tid, sig);
> + } while (kill(proc_info->tid, 0) < 0 && try--);
> +
> + if (kill(proc_info->tid, 0) < 0)
> + err = -1;
Not convinced this is good behaviour for an API, to repeatedly call
kill(SIGTERM) until bored. If the function didn't take a int sig and was
called igt_terminate_process(const char *name), then repeating a few
SIGTERM; before sending SIGKILL makes sense. But as it it, named like
kill() I expect this to send exactly one signal.
> +/**
> + * igt_kill:
> + * @sig: Signal to send.
> + * @pid: Process pid to send.
> + * @returns: 0 in case of success or -1 otherwise.
> + *
> + * This function is identical to igt_pkill() only that it searches the process
> + * table using @pid instead of comm name.
There's a function called kill() that does exactly that, you even use it
here ;)
> +int
> +igt_rmmod(const char *mod_name, bool force)
> +{
> + struct kmod_ctx *ctx;
> + struct kmod_module *kmod;
> + int err, flags = 0;
> +
> + ctx = kmod_new(NULL, NULL);
> + igt_assert(ctx != NULL);
> +
> + err = kmod_module_new_from_name(ctx, mod_name, &kmod);
> + if (err < 0) {
> + igt_info("Could not use module %s (%s)\n", mod_name,
> + strerror(-err));
> + err = -1;
> + goto out;
> + }
> +
> + if (igt_module_in_use(kmod)) {
> + igt_info("Module %s is in use\n", mod_name);
> + err = -1;
> + goto out;
> + }
Pointless (this is redundant).
> +
> + if (force) {
> + flags |= KMOD_REMOVE_FORCE;
Will it not be wiser (future proof) just to pass flags from the caller?
> + }
> +
> + err = kmod_module_remove_module(kmod, flags);
> + if (err < 0) {
> + igt_info("Could not remove module %s (%s)\n", mod_name,
> + strerror(-err));
> + err = -1;
> + }
> +
> +out:
> + kmod_module_unref(kmod);
> + kmod_unref(ctx);
> +
> + return err;
> +}
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH i-g-t 1/3] lib/{igt_sysfs, igt_aux}: Make available to other users kick_fbcon() (unbind_fbcon()), and added helpers to igt_aux.
2016-10-20 20:09 ` Chris Wilson
@ 2016-10-24 18:08 ` Marius Vlad
0 siblings, 0 replies; 19+ messages in thread
From: Marius Vlad @ 2016-10-24 18:08 UTC (permalink / raw)
To: Chris Wilson, intel-gfx, petri.latvala, jani.nikula
[-- Attachment #1.1: Type: text/plain, Size: 2885 bytes --]
On Thu, Oct 20, 2016 at 09:09:17PM +0100, Chris Wilson wrote:
> On Thu, Oct 20, 2016 at 10:36:47PM +0300, Marius Vlad wrote:
> > +int
> > +igt_pkill(int sig, const char *comm)
> > +{
> > + int err = 0, try = 5;
> > + PROCTAB *proc;
> > + proc_t *proc_info;
> > +
> > + proc = openproc(PROC_FILLCOM | PROC_FILLSTAT | PROC_FILLARG);
> > + igt_assert(proc != NULL);
> > +
> > + while ((proc_info = readproc(proc, NULL))) {
> > + if (proc_info &&
>
> proc_info cannot be NULL, you've already tested for that.
True. Removed.
>
> > + !strncasecmp(proc_info->cmd, comm, sizeof(proc_info->cmd))) {
> > + switch (sig) {
> > + case SIGTERM:
> > + case SIGKILL:
> > + do {
> > + kill(proc_info->tid, sig);
> > + } while (kill(proc_info->tid, 0) < 0 && try--);
> > +
> > + if (kill(proc_info->tid, 0) < 0)
> > + err = -1;
>
> Not convinced this is good behaviour for an API, to repeatedly call
> kill(SIGTERM) until bored. If the function didn't take a int sig and was
> called igt_terminate_process(const char *name), then repeating a few
> SIGTERM; before sending SIGKILL makes sense. But as it it, named like
> kill() I expect this to send exactly one signal.
I got mixed feelings about this as well. I'll keep it simple.
>
> > +/**
> > + * igt_kill:
> > + * @sig: Signal to send.
> > + * @pid: Process pid to send.
> > + * @returns: 0 in case of success or -1 otherwise.
> > + *
> > + * This function is identical to igt_pkill() only that it searches the process
> > + * table using @pid instead of comm name.
>
> There's a function called kill() that does exactly that, you even use it
> here ;)
True.
>
> > +int
> > +igt_rmmod(const char *mod_name, bool force)
> > +{
> > + struct kmod_ctx *ctx;
> > + struct kmod_module *kmod;
> > + int err, flags = 0;
> > +
> > + ctx = kmod_new(NULL, NULL);
> > + igt_assert(ctx != NULL);
> > +
> > + err = kmod_module_new_from_name(ctx, mod_name, &kmod);
> > + if (err < 0) {
> > + igt_info("Could not use module %s (%s)\n", mod_name,
> > + strerror(-err));
> > + err = -1;
> > + goto out;
> > + }
> > +
> > + if (igt_module_in_use(kmod)) {
> > + igt_info("Module %s is in use\n", mod_name);
> > + err = -1;
> > + goto out;
> > + }
>
> Pointless (this is redundant).
Indeed.
>
> > +
> > + if (force) {
> > + flags |= KMOD_REMOVE_FORCE;
>
> Will it not be wiser (future proof) just to pass flags from the caller?
I'll pass it directly.
>
> > + }
> > +
> > + err = kmod_module_remove_module(kmod, flags);
> > + if (err < 0) {
> > + igt_info("Could not remove module %s (%s)\n", mod_name,
> > + strerror(-err));
> > + err = -1;
> > + }
> > +
> > +out:
> > + kmod_module_unref(kmod);
> > + kmod_unref(ctx);
> > +
> > + return err;
> > +}
>
> --
> Chris Wilson, Intel Open Source Technology Centre
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH i-g-t 1/3] lib/{igt_sysfs, igt_aux}: Make available to other users kick_fbcon() (unbind_fbcon()), and added helpers to igt_aux.
2016-10-20 19:36 ` [PATCH i-g-t 1/3] lib/{igt_sysfs, igt_aux}: Make available to other users kick_fbcon() (unbind_fbcon()), and added helpers to igt_aux Marius Vlad
2016-10-20 20:09 ` Chris Wilson
@ 2016-10-24 8:40 ` Daniel Vetter
2016-10-26 21:02 ` Marius Vlad
1 sibling, 1 reply; 19+ messages in thread
From: Daniel Vetter @ 2016-10-24 8:40 UTC (permalink / raw)
To: Marius Vlad; +Cc: intel-gfx
On Thu, Oct 20, 2016 at 10:36:47PM +0300, Marius Vlad wrote:
> Previously under unbind_fbcon(), disable/enable framebuffer console.
>
> lib/igt_aux: Added helpers to help convert sh scripts to C version. libkmod and
> procps interface.
>
> Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> ---
> configure.ac | 2 +
> lib/Makefile.am | 2 +
> lib/igt_aux.c | 278 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> lib/igt_aux.h | 7 ++
> lib/igt_gvt.c | 43 +--------
> lib/igt_sysfs.c | 54 +++++++++++
> lib/igt_sysfs.h | 2 +
> 7 files changed, 347 insertions(+), 41 deletions(-)
If you go with extracting stuff into helpers I'd go with a new helper
library like igt_kmod.c. Or just keep it in-line with tests, extracting
code is imo overrated ;-)
Also pls make sure the docs are correct and look good, there's a bunch of
issues with them (like non-matching function names between comment and
actual code).
-Daniel
>
> diff --git a/configure.ac b/configure.ac
> index 735cfd5..2c6e49d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -121,6 +121,8 @@ AC_SUBST(ASSEMBLER_WARN_CFLAGS)
>
> PKG_CHECK_MODULES(DRM, [libdrm])
> PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10])
> +PKG_CHECK_MODULES(KMOD, [libkmod])
> +PKG_CHECK_MODULES(PROCPS, [libprocps])
>
> case "$target_cpu" in
> x86*|i?86)
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index 4c0893d..e1737bd 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -34,6 +34,8 @@ AM_CFLAGS += $(CAIRO_CFLAGS)
> libintel_tools_la_LIBADD = \
> $(DRM_LIBS) \
> $(PCIACCESS_LIBS) \
> + $(PROCPS_LIBS) \
> + $(KMOD_LIBS) \
> $(CAIRO_LIBS) \
> $(LIBUDEV_LIBS) \
> $(LIBUNWIND_LIBS) \
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index 421f6d4..d150818 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -51,6 +51,9 @@
> #include <termios.h>
> #include <assert.h>
>
> +#include <proc/readproc.h>
> +#include <libkmod.h>
> +
> #include "drmtest.h"
> #include "i915_drm.h"
> #include "intel_chipset.h"
> @@ -1193,6 +1196,281 @@ void igt_set_module_param_int(const char *name, int val)
> igt_set_module_param(name, str);
> }
>
> +/**
> + * igt_pkill:
> + * @sig: Signal to send
> + * @name: Name of process in the form found in /proc/pid/comm (limited to 15
> + * chars)
> + * @returns: 0 in case the process is not found running or the signal has been
> + * sent successfully or -1 otherwise.
> + *
> + * This function sends the signal @sig for a process found in process table
> + * with name @comm.
> + */
> +int
> +igt_pkill(int sig, const char *comm)
> +{
> + int err = 0, try = 5;
> + PROCTAB *proc;
> + proc_t *proc_info;
> +
> + proc = openproc(PROC_FILLCOM | PROC_FILLSTAT | PROC_FILLARG);
> + igt_assert(proc != NULL);
> +
> + while ((proc_info = readproc(proc, NULL))) {
> + if (proc_info &&
> + !strncasecmp(proc_info->cmd, comm, sizeof(proc_info->cmd))) {
> + switch (sig) {
> + case SIGTERM:
> + case SIGKILL:
> + do {
> + kill(proc_info->tid, sig);
> + } while (kill(proc_info->tid, 0) < 0 && try--);
> +
> + if (kill(proc_info->tid, 0) < 0)
> + err = -1;
> + break;
> + default:
> + if (kill(proc_info->tid, sig) < 0)
> + err = -1;
> + break;
> + }
> +
> + freeproc(proc_info);
> + break;
> + }
> + freeproc(proc_info);
> + }
> +
> + closeproc(proc);
> + return err;
> +}
> +
> +/**
> + * igt_kill:
> + * @sig: Signal to send.
> + * @pid: Process pid to send.
> + * @returns: 0 in case of success or -1 otherwise.
> + *
> + * This function is identical to igt_pkill() only that it searches the process
> + * table using @pid instead of comm name.
> + *
> + */
> +int
> +igt_kill(int sig, pid_t pid)
> +{
> + int err = 0, try = 5;
> + PROCTAB *proc;
> + proc_t *proc_info;
> +
> + proc = openproc(PROC_PID | PROC_FILLSTAT | PROC_FILLARG);
> + igt_assert(proc != NULL);
> +
> + while ((proc_info = readproc(proc, NULL))) {
> + if (proc_info && proc_info->tid == pid) {
> + switch (sig) {
> + case SIGTERM:
> + case SIGKILL:
> + do {
> + kill(proc_info->tid, sig);
> + } while (kill(proc_info->tid, 0) < 0 && try--);
> +
> + if (kill(proc_info->tid, 0) < 0)
> + err = -1;
> + break;
> + default:
> + if (kill(proc_info->tid, sig) < 0)
> + err = -1;
> + break;
> + }
> + freeproc(proc_info);
> + break;
> + }
> + freeproc(proc_info);
> + }
> +
> + closeproc(proc);
> + return err;
> +}
> +
> +static bool
> +igt_module_in_use(struct kmod_module *kmod)
> +{
> + int err;
> +
> + err = kmod_module_get_initstate(kmod);
> +
> + /* if compiled builtin or does not exist */
> + if (err == KMOD_MODULE_BUILTIN || err < 0)
> + return false;
> +
> + if (kmod_module_get_refcnt(kmod) != 0 ||
> + kmod_module_get_holders(kmod) != NULL)
> + return true;
> +
> +
> + return false;
> +}
> +
> +/**
> + * igt_lsmod:
> + * @mod_name: The name of the module.
> + * @returns: True in case the module has been found or false otherwise.
> + *
> + *
> + * Function to check the existance of module @mod_name in list of loaded kernel
> + * modules.
> + *
> + */
> +bool
> +igt_lsmod_has_module(const char *mod_name)
> +{
> + struct kmod_list *mod, *list;
> + struct kmod_ctx *ctx;
> + bool ret = false;
> +
> + ctx = kmod_new(NULL, NULL);
> + igt_assert(ctx != NULL);
> +
> + if (kmod_module_new_from_loaded(ctx, &list) < 0) {
> + kmod_unref(ctx);
> + return ret;
> + }
> +
> + kmod_list_foreach(mod, list) {
> + struct kmod_module *kmod = kmod_module_get_module(mod);
> + const char *kmod_name = kmod_module_get_name(kmod);
> +
> + if (!strncasecmp(kmod_name, mod_name, strlen(kmod_name))) {
> + kmod_module_unref(kmod);
> + ret = true;
> + break;
> + }
> + kmod_module_unref(kmod);
> + }
> +
> + kmod_module_unref_list(list);
> + kmod_unref(ctx);
> +
> + return ret;
> +}
> +
> +
> +/**
> + * igt_insmod:
> + * @mod_name: The name of the module
> + * @opts: Parameters for the module. NULL in case no parameters
> + * are to be passed, or a '\0' terminated string otherwise.
> + * @returns: 0 in case of success or -1 in case the module could not
> + * be loaded.
> + *
> + *
> + * This function loads a kernel module using the name specified in @mod_name.
> + *
> + * @Note: This functions doesn't automatically resolve other module
> + * dependencies so make make sure you load the dependencies module(s) before
> + * this one.
> + */
> +int
> +igt_insmod(const char *mod_name, const char *opts)
> +{
> + struct kmod_ctx *ctx;
> + struct kmod_module *kmod;
> + int err = 0;
> +
> + ctx = kmod_new(NULL, NULL);
> + igt_assert(ctx != NULL);
> +
> +
> + err = kmod_module_new_from_name(ctx, mod_name, &kmod);
> + if (err < 0) {
> + goto out;
> + }
> +
> + err = kmod_module_insert_module(kmod, 0, opts);
> + if (err < 0) {
> + switch (err) {
> + case -EEXIST:
> + igt_info("Module %s already inserted\n",
> + kmod_module_get_name(kmod));
> + err = -1;
> + break;
> + case -ENOENT:
> + igt_info("Unknown symbol in module %s or "
> + "unknown parameter\n",
> + kmod_module_get_name(kmod));
> + err = -1;
> + break;
> + default:
> + igt_info("Could not insert %s (%s)\n",
> + kmod_module_get_name(kmod), strerror(-err));
> + err = -1;
> + break;
> + }
> + }
> +out:
> + kmod_module_unref(kmod);
> + kmod_unref(ctx);
> +
> + return err;
> +}
> +
> +
> +/**
> + * igt_rmmod:
> + * @mod_name: Module name.
> + * @force: A bool value to force removal. Note that his flag does not remove
> + * the module(s) that the module specified by @mod_name depends on. In other
> + * words you must igt_rmmod() the module(s) dependencies before calling it with
> + * @mod_name.
> + * @returns: 0 in case of success or -1 otherwise.
> + *
> + * Removes the module @mod_name.
> + *
> + */
> +int
> +igt_rmmod(const char *mod_name, bool force)
> +{
> + struct kmod_ctx *ctx;
> + struct kmod_module *kmod;
> + int err, flags = 0;
> +
> + ctx = kmod_new(NULL, NULL);
> + igt_assert(ctx != NULL);
> +
> + err = kmod_module_new_from_name(ctx, mod_name, &kmod);
> + if (err < 0) {
> + igt_info("Could not use module %s (%s)\n", mod_name,
> + strerror(-err));
> + err = -1;
> + goto out;
> + }
> +
> + if (igt_module_in_use(kmod)) {
> + igt_info("Module %s is in use\n", mod_name);
> + err = -1;
> + goto out;
> + }
> +
> + if (force) {
> + flags |= KMOD_REMOVE_FORCE;
> + }
> +
> + err = kmod_module_remove_module(kmod, flags);
> + if (err < 0) {
> + igt_info("Could not remove module %s (%s)\n", mod_name,
> + strerror(-err));
> + err = -1;
> + }
> +
> +out:
> + kmod_module_unref(kmod);
> + kmod_unref(ctx);
> +
> + return err;
> +}
> +
> +
> static struct igt_siglatency {
> timer_t timer;
> struct timespec target;
> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> index d30196b..7946923 100644
> --- a/lib/igt_aux.h
> +++ b/lib/igt_aux.h
> @@ -264,4 +264,11 @@ double igt_stop_siglatency(struct igt_mean *result);
> void igt_set_module_param(const char *name, const char *val);
> void igt_set_module_param_int(const char *name, int val);
>
> +int igt_pkill(int sig, const char *comm);
> +int igt_kill(int sig, pid_t pid);
> +
> +bool igt_lsmod_has_module(const char *mod_name);
> +int igt_insmod(const char *mod_name, const char *opts);
> +int igt_rmmod(const char *mod_name, bool force);
> +
> #endif /* IGT_AUX_H */
> diff --git a/lib/igt_gvt.c b/lib/igt_gvt.c
> index 0f332d1..8bbf9bd 100644
> --- a/lib/igt_gvt.c
> +++ b/lib/igt_gvt.c
> @@ -23,6 +23,7 @@
>
> #include "igt.h"
> #include "igt_gvt.h"
> +#include "igt_sysfs.h"
>
> #include <dirent.h>
> #include <unistd.h>
> @@ -46,49 +47,9 @@ static bool is_gvt_enabled(void)
> return enabled;
> }
>
> -static void unbind_fbcon(void)
> -{
> - char buf[128];
> - const char *path = "/sys/class/vtconsole";
> - DIR *dir;
> - struct dirent *vtcon;
> -
> - dir = opendir(path);
> - if (!dir)
> - return;
> -
> - while ((vtcon = readdir(dir))) {
> - int fd, len;
> -
> - if (strncmp(vtcon->d_name, "vtcon", 5))
> - continue;
> -
> - sprintf(buf, "%s/%s/name", path, vtcon->d_name);
> - fd = open(buf, O_RDONLY);
> - if (fd < 0)
> - continue;
> -
> - len = read(fd, buf, sizeof(buf) - 1);
> - close(fd);
> - if (len >= 0)
> - buf[len] = '\0';
> -
> - if (strstr(buf, "frame buffer device")) {
> - sprintf(buf, "%s/%s/bind", path, vtcon->d_name);
> - fd = open(buf, O_WRONLY);
> - if (fd != -1) {
> - igt_ignore_warn(write(fd, "1\n", 2));
> - close(fd);
> - }
> - break;
> - }
> - }
> - closedir(dir);
> -}
> -
> static void unload_i915(void)
> {
> - unbind_fbcon();
> + kick_fbcon(false);
> /* pkill alsact */
>
> igt_ignore_warn(system("/sbin/modprobe -s -r i915"));
> diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
> index 612de75..0803659 100644
> --- a/lib/igt_sysfs.c
> +++ b/lib/igt_sysfs.c
> @@ -34,7 +34,11 @@
> #include <fcntl.h>
> #include <unistd.h>
> #include <i915_drm.h>
> +#include <dirent.h>
> +#include <unistd.h>
> +#include <fcntl.h>
>
> +#include "igt_core.h"
> #include "igt_sysfs.h"
>
> /**
> @@ -391,3 +395,53 @@ bool igt_sysfs_set_boolean(int dir, const char *attr, bool value)
> {
> return igt_sysfs_printf(dir, attr, "%d", value) == 1;
> }
> +
> +/**
> + * kick_fbcon:
> + * @enable: boolean value
> + *
> + * This functions enables/disables the text console running on top of the
> + * framebuffer device.
> + */
> +void kick_fbcon(bool enable)
> +{
> + char buf[128];
> + const char *path = "/sys/class/vtconsole";
> + DIR *dir;
> + struct dirent *vtcon;
> +
> + dir = opendir(path);
> + if (!dir)
> + return;
> +
> + while ((vtcon = readdir(dir))) {
> + int fd, len;
> +
> + if (strncmp(vtcon->d_name, "vtcon", 5))
> + continue;
> +
> + sprintf(buf, "%s/%s/name", path, vtcon->d_name);
> + fd = open(buf, O_RDONLY);
> + if (fd < 0)
> + continue;
> +
> + len = read(fd, buf, sizeof(buf) - 1);
> + close(fd);
> + if (len >= 0)
> + buf[len] = '\0';
> +
> + if (strstr(buf, "frame buffer device")) {
> + sprintf(buf, "%s/%s/bind", path, vtcon->d_name);
> + fd = open(buf, O_WRONLY);
> + if (fd != -1) {
> + if (enable)
> + igt_ignore_warn(write(fd, "1\n", 2));
> + else
> + igt_ignore_warn(write(fd, "0\n", 2));
> + close(fd);
> + }
> + break;
> + }
> + }
> + closedir(dir);
> +}
> diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
> index 4820066..a141894 100644
> --- a/lib/igt_sysfs.h
> +++ b/lib/igt_sysfs.h
> @@ -43,4 +43,6 @@ bool igt_sysfs_set_u32(int dir, const char *attr, uint32_t value);
> bool igt_sysfs_get_boolean(int dir, const char *attr);
> bool igt_sysfs_set_boolean(int dir, const char *attr, bool value);
>
> +void kick_fbcon(bool enable);
> +
> #endif /* __IGT_SYSFS_H__ */
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH i-g-t 1/3] lib/{igt_sysfs, igt_aux}: Make available to other users kick_fbcon() (unbind_fbcon()), and added helpers to igt_aux.
2016-10-24 8:40 ` Daniel Vetter
@ 2016-10-26 21:02 ` Marius Vlad
2016-10-27 6:40 ` Daniel Vetter
0 siblings, 1 reply; 19+ messages in thread
From: Marius Vlad @ 2016-10-26 21:02 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 14839 bytes --]
On Mon, Oct 24, 2016 at 10:40:37AM +0200, Daniel Vetter wrote:
> On Thu, Oct 20, 2016 at 10:36:47PM +0300, Marius Vlad wrote:
> > Previously under unbind_fbcon(), disable/enable framebuffer console.
> >
> > lib/igt_aux: Added helpers to help convert sh scripts to C version. libkmod and
> > procps interface.
> >
> > Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> > ---
> > configure.ac | 2 +
> > lib/Makefile.am | 2 +
> > lib/igt_aux.c | 278 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > lib/igt_aux.h | 7 ++
> > lib/igt_gvt.c | 43 +--------
> > lib/igt_sysfs.c | 54 +++++++++++
> > lib/igt_sysfs.h | 2 +
> > 7 files changed, 347 insertions(+), 41 deletions(-)
>
> If you go with extracting stuff into helpers I'd go with a new helper
> library like igt_kmod.c. Or just keep it in-line with tests, extracting
> code is imo overrated ;-)
Have no issue putting kmod helpers into their own file. Can't really
tell which is the best approach though: the only user is drv_module_reload
yet the test already got ``fatten'' up with the gem subtests (see v3).
>
> Also pls make sure the docs are correct and look good, there's a bunch of
> issues with them (like non-matching function names between comment and
> actual code).
Sorry, I totally missed your input, will double check on this.
> -Daniel
>
> >
> > diff --git a/configure.ac b/configure.ac
> > index 735cfd5..2c6e49d 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -121,6 +121,8 @@ AC_SUBST(ASSEMBLER_WARN_CFLAGS)
> >
> > PKG_CHECK_MODULES(DRM, [libdrm])
> > PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10])
> > +PKG_CHECK_MODULES(KMOD, [libkmod])
> > +PKG_CHECK_MODULES(PROCPS, [libprocps])
> >
> > case "$target_cpu" in
> > x86*|i?86)
> > diff --git a/lib/Makefile.am b/lib/Makefile.am
> > index 4c0893d..e1737bd 100644
> > --- a/lib/Makefile.am
> > +++ b/lib/Makefile.am
> > @@ -34,6 +34,8 @@ AM_CFLAGS += $(CAIRO_CFLAGS)
> > libintel_tools_la_LIBADD = \
> > $(DRM_LIBS) \
> > $(PCIACCESS_LIBS) \
> > + $(PROCPS_LIBS) \
> > + $(KMOD_LIBS) \
> > $(CAIRO_LIBS) \
> > $(LIBUDEV_LIBS) \
> > $(LIBUNWIND_LIBS) \
> > diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> > index 421f6d4..d150818 100644
> > --- a/lib/igt_aux.c
> > +++ b/lib/igt_aux.c
> > @@ -51,6 +51,9 @@
> > #include <termios.h>
> > #include <assert.h>
> >
> > +#include <proc/readproc.h>
> > +#include <libkmod.h>
> > +
> > #include "drmtest.h"
> > #include "i915_drm.h"
> > #include "intel_chipset.h"
> > @@ -1193,6 +1196,281 @@ void igt_set_module_param_int(const char *name, int val)
> > igt_set_module_param(name, str);
> > }
> >
> > +/**
> > + * igt_pkill:
> > + * @sig: Signal to send
> > + * @name: Name of process in the form found in /proc/pid/comm (limited to 15
> > + * chars)
> > + * @returns: 0 in case the process is not found running or the signal has been
> > + * sent successfully or -1 otherwise.
> > + *
> > + * This function sends the signal @sig for a process found in process table
> > + * with name @comm.
> > + */
> > +int
> > +igt_pkill(int sig, const char *comm)
> > +{
> > + int err = 0, try = 5;
> > + PROCTAB *proc;
> > + proc_t *proc_info;
> > +
> > + proc = openproc(PROC_FILLCOM | PROC_FILLSTAT | PROC_FILLARG);
> > + igt_assert(proc != NULL);
> > +
> > + while ((proc_info = readproc(proc, NULL))) {
> > + if (proc_info &&
> > + !strncasecmp(proc_info->cmd, comm, sizeof(proc_info->cmd))) {
> > + switch (sig) {
> > + case SIGTERM:
> > + case SIGKILL:
> > + do {
> > + kill(proc_info->tid, sig);
> > + } while (kill(proc_info->tid, 0) < 0 && try--);
> > +
> > + if (kill(proc_info->tid, 0) < 0)
> > + err = -1;
> > + break;
> > + default:
> > + if (kill(proc_info->tid, sig) < 0)
> > + err = -1;
> > + break;
> > + }
> > +
> > + freeproc(proc_info);
> > + break;
> > + }
> > + freeproc(proc_info);
> > + }
> > +
> > + closeproc(proc);
> > + return err;
> > +}
> > +
> > +/**
> > + * igt_kill:
> > + * @sig: Signal to send.
> > + * @pid: Process pid to send.
> > + * @returns: 0 in case of success or -1 otherwise.
> > + *
> > + * This function is identical to igt_pkill() only that it searches the process
> > + * table using @pid instead of comm name.
> > + *
> > + */
> > +int
> > +igt_kill(int sig, pid_t pid)
> > +{
> > + int err = 0, try = 5;
> > + PROCTAB *proc;
> > + proc_t *proc_info;
> > +
> > + proc = openproc(PROC_PID | PROC_FILLSTAT | PROC_FILLARG);
> > + igt_assert(proc != NULL);
> > +
> > + while ((proc_info = readproc(proc, NULL))) {
> > + if (proc_info && proc_info->tid == pid) {
> > + switch (sig) {
> > + case SIGTERM:
> > + case SIGKILL:
> > + do {
> > + kill(proc_info->tid, sig);
> > + } while (kill(proc_info->tid, 0) < 0 && try--);
> > +
> > + if (kill(proc_info->tid, 0) < 0)
> > + err = -1;
> > + break;
> > + default:
> > + if (kill(proc_info->tid, sig) < 0)
> > + err = -1;
> > + break;
> > + }
> > + freeproc(proc_info);
> > + break;
> > + }
> > + freeproc(proc_info);
> > + }
> > +
> > + closeproc(proc);
> > + return err;
> > +}
> > +
> > +static bool
> > +igt_module_in_use(struct kmod_module *kmod)
> > +{
> > + int err;
> > +
> > + err = kmod_module_get_initstate(kmod);
> > +
> > + /* if compiled builtin or does not exist */
> > + if (err == KMOD_MODULE_BUILTIN || err < 0)
> > + return false;
> > +
> > + if (kmod_module_get_refcnt(kmod) != 0 ||
> > + kmod_module_get_holders(kmod) != NULL)
> > + return true;
> > +
> > +
> > + return false;
> > +}
> > +
> > +/**
> > + * igt_lsmod:
> > + * @mod_name: The name of the module.
> > + * @returns: True in case the module has been found or false otherwise.
> > + *
> > + *
> > + * Function to check the existance of module @mod_name in list of loaded kernel
> > + * modules.
> > + *
> > + */
> > +bool
> > +igt_lsmod_has_module(const char *mod_name)
> > +{
> > + struct kmod_list *mod, *list;
> > + struct kmod_ctx *ctx;
> > + bool ret = false;
> > +
> > + ctx = kmod_new(NULL, NULL);
> > + igt_assert(ctx != NULL);
> > +
> > + if (kmod_module_new_from_loaded(ctx, &list) < 0) {
> > + kmod_unref(ctx);
> > + return ret;
> > + }
> > +
> > + kmod_list_foreach(mod, list) {
> > + struct kmod_module *kmod = kmod_module_get_module(mod);
> > + const char *kmod_name = kmod_module_get_name(kmod);
> > +
> > + if (!strncasecmp(kmod_name, mod_name, strlen(kmod_name))) {
> > + kmod_module_unref(kmod);
> > + ret = true;
> > + break;
> > + }
> > + kmod_module_unref(kmod);
> > + }
> > +
> > + kmod_module_unref_list(list);
> > + kmod_unref(ctx);
> > +
> > + return ret;
> > +}
> > +
> > +
> > +/**
> > + * igt_insmod:
> > + * @mod_name: The name of the module
> > + * @opts: Parameters for the module. NULL in case no parameters
> > + * are to be passed, or a '\0' terminated string otherwise.
> > + * @returns: 0 in case of success or -1 in case the module could not
> > + * be loaded.
> > + *
> > + *
> > + * This function loads a kernel module using the name specified in @mod_name.
> > + *
> > + * @Note: This functions doesn't automatically resolve other module
> > + * dependencies so make make sure you load the dependencies module(s) before
> > + * this one.
> > + */
> > +int
> > +igt_insmod(const char *mod_name, const char *opts)
> > +{
> > + struct kmod_ctx *ctx;
> > + struct kmod_module *kmod;
> > + int err = 0;
> > +
> > + ctx = kmod_new(NULL, NULL);
> > + igt_assert(ctx != NULL);
> > +
> > +
> > + err = kmod_module_new_from_name(ctx, mod_name, &kmod);
> > + if (err < 0) {
> > + goto out;
> > + }
> > +
> > + err = kmod_module_insert_module(kmod, 0, opts);
> > + if (err < 0) {
> > + switch (err) {
> > + case -EEXIST:
> > + igt_info("Module %s already inserted\n",
> > + kmod_module_get_name(kmod));
> > + err = -1;
> > + break;
> > + case -ENOENT:
> > + igt_info("Unknown symbol in module %s or "
> > + "unknown parameter\n",
> > + kmod_module_get_name(kmod));
> > + err = -1;
> > + break;
> > + default:
> > + igt_info("Could not insert %s (%s)\n",
> > + kmod_module_get_name(kmod), strerror(-err));
> > + err = -1;
> > + break;
> > + }
> > + }
> > +out:
> > + kmod_module_unref(kmod);
> > + kmod_unref(ctx);
> > +
> > + return err;
> > +}
> > +
> > +
> > +/**
> > + * igt_rmmod:
> > + * @mod_name: Module name.
> > + * @force: A bool value to force removal. Note that his flag does not remove
> > + * the module(s) that the module specified by @mod_name depends on. In other
> > + * words you must igt_rmmod() the module(s) dependencies before calling it with
> > + * @mod_name.
> > + * @returns: 0 in case of success or -1 otherwise.
> > + *
> > + * Removes the module @mod_name.
> > + *
> > + */
> > +int
> > +igt_rmmod(const char *mod_name, bool force)
> > +{
> > + struct kmod_ctx *ctx;
> > + struct kmod_module *kmod;
> > + int err, flags = 0;
> > +
> > + ctx = kmod_new(NULL, NULL);
> > + igt_assert(ctx != NULL);
> > +
> > + err = kmod_module_new_from_name(ctx, mod_name, &kmod);
> > + if (err < 0) {
> > + igt_info("Could not use module %s (%s)\n", mod_name,
> > + strerror(-err));
> > + err = -1;
> > + goto out;
> > + }
> > +
> > + if (igt_module_in_use(kmod)) {
> > + igt_info("Module %s is in use\n", mod_name);
> > + err = -1;
> > + goto out;
> > + }
> > +
> > + if (force) {
> > + flags |= KMOD_REMOVE_FORCE;
> > + }
> > +
> > + err = kmod_module_remove_module(kmod, flags);
> > + if (err < 0) {
> > + igt_info("Could not remove module %s (%s)\n", mod_name,
> > + strerror(-err));
> > + err = -1;
> > + }
> > +
> > +out:
> > + kmod_module_unref(kmod);
> > + kmod_unref(ctx);
> > +
> > + return err;
> > +}
> > +
> > +
> > static struct igt_siglatency {
> > timer_t timer;
> > struct timespec target;
> > diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> > index d30196b..7946923 100644
> > --- a/lib/igt_aux.h
> > +++ b/lib/igt_aux.h
> > @@ -264,4 +264,11 @@ double igt_stop_siglatency(struct igt_mean *result);
> > void igt_set_module_param(const char *name, const char *val);
> > void igt_set_module_param_int(const char *name, int val);
> >
> > +int igt_pkill(int sig, const char *comm);
> > +int igt_kill(int sig, pid_t pid);
> > +
> > +bool igt_lsmod_has_module(const char *mod_name);
> > +int igt_insmod(const char *mod_name, const char *opts);
> > +int igt_rmmod(const char *mod_name, bool force);
> > +
> > #endif /* IGT_AUX_H */
> > diff --git a/lib/igt_gvt.c b/lib/igt_gvt.c
> > index 0f332d1..8bbf9bd 100644
> > --- a/lib/igt_gvt.c
> > +++ b/lib/igt_gvt.c
> > @@ -23,6 +23,7 @@
> >
> > #include "igt.h"
> > #include "igt_gvt.h"
> > +#include "igt_sysfs.h"
> >
> > #include <dirent.h>
> > #include <unistd.h>
> > @@ -46,49 +47,9 @@ static bool is_gvt_enabled(void)
> > return enabled;
> > }
> >
> > -static void unbind_fbcon(void)
> > -{
> > - char buf[128];
> > - const char *path = "/sys/class/vtconsole";
> > - DIR *dir;
> > - struct dirent *vtcon;
> > -
> > - dir = opendir(path);
> > - if (!dir)
> > - return;
> > -
> > - while ((vtcon = readdir(dir))) {
> > - int fd, len;
> > -
> > - if (strncmp(vtcon->d_name, "vtcon", 5))
> > - continue;
> > -
> > - sprintf(buf, "%s/%s/name", path, vtcon->d_name);
> > - fd = open(buf, O_RDONLY);
> > - if (fd < 0)
> > - continue;
> > -
> > - len = read(fd, buf, sizeof(buf) - 1);
> > - close(fd);
> > - if (len >= 0)
> > - buf[len] = '\0';
> > -
> > - if (strstr(buf, "frame buffer device")) {
> > - sprintf(buf, "%s/%s/bind", path, vtcon->d_name);
> > - fd = open(buf, O_WRONLY);
> > - if (fd != -1) {
> > - igt_ignore_warn(write(fd, "1\n", 2));
> > - close(fd);
> > - }
> > - break;
> > - }
> > - }
> > - closedir(dir);
> > -}
> > -
> > static void unload_i915(void)
> > {
> > - unbind_fbcon();
> > + kick_fbcon(false);
> > /* pkill alsact */
> >
> > igt_ignore_warn(system("/sbin/modprobe -s -r i915"));
> > diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
> > index 612de75..0803659 100644
> > --- a/lib/igt_sysfs.c
> > +++ b/lib/igt_sysfs.c
> > @@ -34,7 +34,11 @@
> > #include <fcntl.h>
> > #include <unistd.h>
> > #include <i915_drm.h>
> > +#include <dirent.h>
> > +#include <unistd.h>
> > +#include <fcntl.h>
> >
> > +#include "igt_core.h"
> > #include "igt_sysfs.h"
> >
> > /**
> > @@ -391,3 +395,53 @@ bool igt_sysfs_set_boolean(int dir, const char *attr, bool value)
> > {
> > return igt_sysfs_printf(dir, attr, "%d", value) == 1;
> > }
> > +
> > +/**
> > + * kick_fbcon:
> > + * @enable: boolean value
> > + *
> > + * This functions enables/disables the text console running on top of the
> > + * framebuffer device.
> > + */
> > +void kick_fbcon(bool enable)
> > +{
> > + char buf[128];
> > + const char *path = "/sys/class/vtconsole";
> > + DIR *dir;
> > + struct dirent *vtcon;
> > +
> > + dir = opendir(path);
> > + if (!dir)
> > + return;
> > +
> > + while ((vtcon = readdir(dir))) {
> > + int fd, len;
> > +
> > + if (strncmp(vtcon->d_name, "vtcon", 5))
> > + continue;
> > +
> > + sprintf(buf, "%s/%s/name", path, vtcon->d_name);
> > + fd = open(buf, O_RDONLY);
> > + if (fd < 0)
> > + continue;
> > +
> > + len = read(fd, buf, sizeof(buf) - 1);
> > + close(fd);
> > + if (len >= 0)
> > + buf[len] = '\0';
> > +
> > + if (strstr(buf, "frame buffer device")) {
> > + sprintf(buf, "%s/%s/bind", path, vtcon->d_name);
> > + fd = open(buf, O_WRONLY);
> > + if (fd != -1) {
> > + if (enable)
> > + igt_ignore_warn(write(fd, "1\n", 2));
> > + else
> > + igt_ignore_warn(write(fd, "0\n", 2));
> > + close(fd);
> > + }
> > + break;
> > + }
> > + }
> > + closedir(dir);
> > +}
> > diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
> > index 4820066..a141894 100644
> > --- a/lib/igt_sysfs.h
> > +++ b/lib/igt_sysfs.h
> > @@ -43,4 +43,6 @@ bool igt_sysfs_set_u32(int dir, const char *attr, uint32_t value);
> > bool igt_sysfs_get_boolean(int dir, const char *attr);
> > bool igt_sysfs_set_boolean(int dir, const char *attr, bool value);
> >
> > +void kick_fbcon(bool enable);
> > +
> > #endif /* __IGT_SYSFS_H__ */
> > --
> > 2.7.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH i-g-t 1/3] lib/{igt_sysfs, igt_aux}: Make available to other users kick_fbcon() (unbind_fbcon()), and added helpers to igt_aux.
2016-10-26 21:02 ` Marius Vlad
@ 2016-10-27 6:40 ` Daniel Vetter
0 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2016-10-27 6:40 UTC (permalink / raw)
To: Daniel Vetter, intel-gfx
On Thu, Oct 27, 2016 at 12:02:30AM +0300, Marius Vlad wrote:
> On Mon, Oct 24, 2016 at 10:40:37AM +0200, Daniel Vetter wrote:
> > On Thu, Oct 20, 2016 at 10:36:47PM +0300, Marius Vlad wrote:
> > > Previously under unbind_fbcon(), disable/enable framebuffer console.
> > >
> > > lib/igt_aux: Added helpers to help convert sh scripts to C version. libkmod and
> > > procps interface.
> > >
> > > Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> > > ---
> > > configure.ac | 2 +
> > > lib/Makefile.am | 2 +
> > > lib/igt_aux.c | 278 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > lib/igt_aux.h | 7 ++
> > > lib/igt_gvt.c | 43 +--------
> > > lib/igt_sysfs.c | 54 +++++++++++
> > > lib/igt_sysfs.h | 2 +
> > > 7 files changed, 347 insertions(+), 41 deletions(-)
> >
> > If you go with extracting stuff into helpers I'd go with a new helper
> > library like igt_kmod.c. Or just keep it in-line with tests, extracting
> > code is imo overrated ;-)
> Have no issue putting kmod helpers into their own file. Can't really
> tell which is the best approach though: the only user is drv_module_reload
> yet the test already got ``fatten'' up with the gem subtests (see v3).
tbh helper libraries only used by one testcase aren't all that useful. If
it's only used by this testcase then I'd say don't bother with the
library: It's a lot more work, and with just 1 user you're pretty much
guaranteed to come up with a sub-par library interface. It takes a few
different users of the same feature to see what's really needed in a good
library.
-Daniel
> >
> > Also pls make sure the docs are correct and look good, there's a bunch of
> > issues with them (like non-matching function names between comment and
> > actual code).
> Sorry, I totally missed your input, will double check on this.
> > -Daniel
> >
> > >
> > > diff --git a/configure.ac b/configure.ac
> > > index 735cfd5..2c6e49d 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -121,6 +121,8 @@ AC_SUBST(ASSEMBLER_WARN_CFLAGS)
> > >
> > > PKG_CHECK_MODULES(DRM, [libdrm])
> > > PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10])
> > > +PKG_CHECK_MODULES(KMOD, [libkmod])
> > > +PKG_CHECK_MODULES(PROCPS, [libprocps])
> > >
> > > case "$target_cpu" in
> > > x86*|i?86)
> > > diff --git a/lib/Makefile.am b/lib/Makefile.am
> > > index 4c0893d..e1737bd 100644
> > > --- a/lib/Makefile.am
> > > +++ b/lib/Makefile.am
> > > @@ -34,6 +34,8 @@ AM_CFLAGS += $(CAIRO_CFLAGS)
> > > libintel_tools_la_LIBADD = \
> > > $(DRM_LIBS) \
> > > $(PCIACCESS_LIBS) \
> > > + $(PROCPS_LIBS) \
> > > + $(KMOD_LIBS) \
> > > $(CAIRO_LIBS) \
> > > $(LIBUDEV_LIBS) \
> > > $(LIBUNWIND_LIBS) \
> > > diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> > > index 421f6d4..d150818 100644
> > > --- a/lib/igt_aux.c
> > > +++ b/lib/igt_aux.c
> > > @@ -51,6 +51,9 @@
> > > #include <termios.h>
> > > #include <assert.h>
> > >
> > > +#include <proc/readproc.h>
> > > +#include <libkmod.h>
> > > +
> > > #include "drmtest.h"
> > > #include "i915_drm.h"
> > > #include "intel_chipset.h"
> > > @@ -1193,6 +1196,281 @@ void igt_set_module_param_int(const char *name, int val)
> > > igt_set_module_param(name, str);
> > > }
> > >
> > > +/**
> > > + * igt_pkill:
> > > + * @sig: Signal to send
> > > + * @name: Name of process in the form found in /proc/pid/comm (limited to 15
> > > + * chars)
> > > + * @returns: 0 in case the process is not found running or the signal has been
> > > + * sent successfully or -1 otherwise.
> > > + *
> > > + * This function sends the signal @sig for a process found in process table
> > > + * with name @comm.
> > > + */
> > > +int
> > > +igt_pkill(int sig, const char *comm)
> > > +{
> > > + int err = 0, try = 5;
> > > + PROCTAB *proc;
> > > + proc_t *proc_info;
> > > +
> > > + proc = openproc(PROC_FILLCOM | PROC_FILLSTAT | PROC_FILLARG);
> > > + igt_assert(proc != NULL);
> > > +
> > > + while ((proc_info = readproc(proc, NULL))) {
> > > + if (proc_info &&
> > > + !strncasecmp(proc_info->cmd, comm, sizeof(proc_info->cmd))) {
> > > + switch (sig) {
> > > + case SIGTERM:
> > > + case SIGKILL:
> > > + do {
> > > + kill(proc_info->tid, sig);
> > > + } while (kill(proc_info->tid, 0) < 0 && try--);
> > > +
> > > + if (kill(proc_info->tid, 0) < 0)
> > > + err = -1;
> > > + break;
> > > + default:
> > > + if (kill(proc_info->tid, sig) < 0)
> > > + err = -1;
> > > + break;
> > > + }
> > > +
> > > + freeproc(proc_info);
> > > + break;
> > > + }
> > > + freeproc(proc_info);
> > > + }
> > > +
> > > + closeproc(proc);
> > > + return err;
> > > +}
> > > +
> > > +/**
> > > + * igt_kill:
> > > + * @sig: Signal to send.
> > > + * @pid: Process pid to send.
> > > + * @returns: 0 in case of success or -1 otherwise.
> > > + *
> > > + * This function is identical to igt_pkill() only that it searches the process
> > > + * table using @pid instead of comm name.
> > > + *
> > > + */
> > > +int
> > > +igt_kill(int sig, pid_t pid)
> > > +{
> > > + int err = 0, try = 5;
> > > + PROCTAB *proc;
> > > + proc_t *proc_info;
> > > +
> > > + proc = openproc(PROC_PID | PROC_FILLSTAT | PROC_FILLARG);
> > > + igt_assert(proc != NULL);
> > > +
> > > + while ((proc_info = readproc(proc, NULL))) {
> > > + if (proc_info && proc_info->tid == pid) {
> > > + switch (sig) {
> > > + case SIGTERM:
> > > + case SIGKILL:
> > > + do {
> > > + kill(proc_info->tid, sig);
> > > + } while (kill(proc_info->tid, 0) < 0 && try--);
> > > +
> > > + if (kill(proc_info->tid, 0) < 0)
> > > + err = -1;
> > > + break;
> > > + default:
> > > + if (kill(proc_info->tid, sig) < 0)
> > > + err = -1;
> > > + break;
> > > + }
> > > + freeproc(proc_info);
> > > + break;
> > > + }
> > > + freeproc(proc_info);
> > > + }
> > > +
> > > + closeproc(proc);
> > > + return err;
> > > +}
> > > +
> > > +static bool
> > > +igt_module_in_use(struct kmod_module *kmod)
> > > +{
> > > + int err;
> > > +
> > > + err = kmod_module_get_initstate(kmod);
> > > +
> > > + /* if compiled builtin or does not exist */
> > > + if (err == KMOD_MODULE_BUILTIN || err < 0)
> > > + return false;
> > > +
> > > + if (kmod_module_get_refcnt(kmod) != 0 ||
> > > + kmod_module_get_holders(kmod) != NULL)
> > > + return true;
> > > +
> > > +
> > > + return false;
> > > +}
> > > +
> > > +/**
> > > + * igt_lsmod:
> > > + * @mod_name: The name of the module.
> > > + * @returns: True in case the module has been found or false otherwise.
> > > + *
> > > + *
> > > + * Function to check the existance of module @mod_name in list of loaded kernel
> > > + * modules.
> > > + *
> > > + */
> > > +bool
> > > +igt_lsmod_has_module(const char *mod_name)
> > > +{
> > > + struct kmod_list *mod, *list;
> > > + struct kmod_ctx *ctx;
> > > + bool ret = false;
> > > +
> > > + ctx = kmod_new(NULL, NULL);
> > > + igt_assert(ctx != NULL);
> > > +
> > > + if (kmod_module_new_from_loaded(ctx, &list) < 0) {
> > > + kmod_unref(ctx);
> > > + return ret;
> > > + }
> > > +
> > > + kmod_list_foreach(mod, list) {
> > > + struct kmod_module *kmod = kmod_module_get_module(mod);
> > > + const char *kmod_name = kmod_module_get_name(kmod);
> > > +
> > > + if (!strncasecmp(kmod_name, mod_name, strlen(kmod_name))) {
> > > + kmod_module_unref(kmod);
> > > + ret = true;
> > > + break;
> > > + }
> > > + kmod_module_unref(kmod);
> > > + }
> > > +
> > > + kmod_module_unref_list(list);
> > > + kmod_unref(ctx);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +
> > > +/**
> > > + * igt_insmod:
> > > + * @mod_name: The name of the module
> > > + * @opts: Parameters for the module. NULL in case no parameters
> > > + * are to be passed, or a '\0' terminated string otherwise.
> > > + * @returns: 0 in case of success or -1 in case the module could not
> > > + * be loaded.
> > > + *
> > > + *
> > > + * This function loads a kernel module using the name specified in @mod_name.
> > > + *
> > > + * @Note: This functions doesn't automatically resolve other module
> > > + * dependencies so make make sure you load the dependencies module(s) before
> > > + * this one.
> > > + */
> > > +int
> > > +igt_insmod(const char *mod_name, const char *opts)
> > > +{
> > > + struct kmod_ctx *ctx;
> > > + struct kmod_module *kmod;
> > > + int err = 0;
> > > +
> > > + ctx = kmod_new(NULL, NULL);
> > > + igt_assert(ctx != NULL);
> > > +
> > > +
> > > + err = kmod_module_new_from_name(ctx, mod_name, &kmod);
> > > + if (err < 0) {
> > > + goto out;
> > > + }
> > > +
> > > + err = kmod_module_insert_module(kmod, 0, opts);
> > > + if (err < 0) {
> > > + switch (err) {
> > > + case -EEXIST:
> > > + igt_info("Module %s already inserted\n",
> > > + kmod_module_get_name(kmod));
> > > + err = -1;
> > > + break;
> > > + case -ENOENT:
> > > + igt_info("Unknown symbol in module %s or "
> > > + "unknown parameter\n",
> > > + kmod_module_get_name(kmod));
> > > + err = -1;
> > > + break;
> > > + default:
> > > + igt_info("Could not insert %s (%s)\n",
> > > + kmod_module_get_name(kmod), strerror(-err));
> > > + err = -1;
> > > + break;
> > > + }
> > > + }
> > > +out:
> > > + kmod_module_unref(kmod);
> > > + kmod_unref(ctx);
> > > +
> > > + return err;
> > > +}
> > > +
> > > +
> > > +/**
> > > + * igt_rmmod:
> > > + * @mod_name: Module name.
> > > + * @force: A bool value to force removal. Note that his flag does not remove
> > > + * the module(s) that the module specified by @mod_name depends on. In other
> > > + * words you must igt_rmmod() the module(s) dependencies before calling it with
> > > + * @mod_name.
> > > + * @returns: 0 in case of success or -1 otherwise.
> > > + *
> > > + * Removes the module @mod_name.
> > > + *
> > > + */
> > > +int
> > > +igt_rmmod(const char *mod_name, bool force)
> > > +{
> > > + struct kmod_ctx *ctx;
> > > + struct kmod_module *kmod;
> > > + int err, flags = 0;
> > > +
> > > + ctx = kmod_new(NULL, NULL);
> > > + igt_assert(ctx != NULL);
> > > +
> > > + err = kmod_module_new_from_name(ctx, mod_name, &kmod);
> > > + if (err < 0) {
> > > + igt_info("Could not use module %s (%s)\n", mod_name,
> > > + strerror(-err));
> > > + err = -1;
> > > + goto out;
> > > + }
> > > +
> > > + if (igt_module_in_use(kmod)) {
> > > + igt_info("Module %s is in use\n", mod_name);
> > > + err = -1;
> > > + goto out;
> > > + }
> > > +
> > > + if (force) {
> > > + flags |= KMOD_REMOVE_FORCE;
> > > + }
> > > +
> > > + err = kmod_module_remove_module(kmod, flags);
> > > + if (err < 0) {
> > > + igt_info("Could not remove module %s (%s)\n", mod_name,
> > > + strerror(-err));
> > > + err = -1;
> > > + }
> > > +
> > > +out:
> > > + kmod_module_unref(kmod);
> > > + kmod_unref(ctx);
> > > +
> > > + return err;
> > > +}
> > > +
> > > +
> > > static struct igt_siglatency {
> > > timer_t timer;
> > > struct timespec target;
> > > diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> > > index d30196b..7946923 100644
> > > --- a/lib/igt_aux.h
> > > +++ b/lib/igt_aux.h
> > > @@ -264,4 +264,11 @@ double igt_stop_siglatency(struct igt_mean *result);
> > > void igt_set_module_param(const char *name, const char *val);
> > > void igt_set_module_param_int(const char *name, int val);
> > >
> > > +int igt_pkill(int sig, const char *comm);
> > > +int igt_kill(int sig, pid_t pid);
> > > +
> > > +bool igt_lsmod_has_module(const char *mod_name);
> > > +int igt_insmod(const char *mod_name, const char *opts);
> > > +int igt_rmmod(const char *mod_name, bool force);
> > > +
> > > #endif /* IGT_AUX_H */
> > > diff --git a/lib/igt_gvt.c b/lib/igt_gvt.c
> > > index 0f332d1..8bbf9bd 100644
> > > --- a/lib/igt_gvt.c
> > > +++ b/lib/igt_gvt.c
> > > @@ -23,6 +23,7 @@
> > >
> > > #include "igt.h"
> > > #include "igt_gvt.h"
> > > +#include "igt_sysfs.h"
> > >
> > > #include <dirent.h>
> > > #include <unistd.h>
> > > @@ -46,49 +47,9 @@ static bool is_gvt_enabled(void)
> > > return enabled;
> > > }
> > >
> > > -static void unbind_fbcon(void)
> > > -{
> > > - char buf[128];
> > > - const char *path = "/sys/class/vtconsole";
> > > - DIR *dir;
> > > - struct dirent *vtcon;
> > > -
> > > - dir = opendir(path);
> > > - if (!dir)
> > > - return;
> > > -
> > > - while ((vtcon = readdir(dir))) {
> > > - int fd, len;
> > > -
> > > - if (strncmp(vtcon->d_name, "vtcon", 5))
> > > - continue;
> > > -
> > > - sprintf(buf, "%s/%s/name", path, vtcon->d_name);
> > > - fd = open(buf, O_RDONLY);
> > > - if (fd < 0)
> > > - continue;
> > > -
> > > - len = read(fd, buf, sizeof(buf) - 1);
> > > - close(fd);
> > > - if (len >= 0)
> > > - buf[len] = '\0';
> > > -
> > > - if (strstr(buf, "frame buffer device")) {
> > > - sprintf(buf, "%s/%s/bind", path, vtcon->d_name);
> > > - fd = open(buf, O_WRONLY);
> > > - if (fd != -1) {
> > > - igt_ignore_warn(write(fd, "1\n", 2));
> > > - close(fd);
> > > - }
> > > - break;
> > > - }
> > > - }
> > > - closedir(dir);
> > > -}
> > > -
> > > static void unload_i915(void)
> > > {
> > > - unbind_fbcon();
> > > + kick_fbcon(false);
> > > /* pkill alsact */
> > >
> > > igt_ignore_warn(system("/sbin/modprobe -s -r i915"));
> > > diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
> > > index 612de75..0803659 100644
> > > --- a/lib/igt_sysfs.c
> > > +++ b/lib/igt_sysfs.c
> > > @@ -34,7 +34,11 @@
> > > #include <fcntl.h>
> > > #include <unistd.h>
> > > #include <i915_drm.h>
> > > +#include <dirent.h>
> > > +#include <unistd.h>
> > > +#include <fcntl.h>
> > >
> > > +#include "igt_core.h"
> > > #include "igt_sysfs.h"
> > >
> > > /**
> > > @@ -391,3 +395,53 @@ bool igt_sysfs_set_boolean(int dir, const char *attr, bool value)
> > > {
> > > return igt_sysfs_printf(dir, attr, "%d", value) == 1;
> > > }
> > > +
> > > +/**
> > > + * kick_fbcon:
> > > + * @enable: boolean value
> > > + *
> > > + * This functions enables/disables the text console running on top of the
> > > + * framebuffer device.
> > > + */
> > > +void kick_fbcon(bool enable)
> > > +{
> > > + char buf[128];
> > > + const char *path = "/sys/class/vtconsole";
> > > + DIR *dir;
> > > + struct dirent *vtcon;
> > > +
> > > + dir = opendir(path);
> > > + if (!dir)
> > > + return;
> > > +
> > > + while ((vtcon = readdir(dir))) {
> > > + int fd, len;
> > > +
> > > + if (strncmp(vtcon->d_name, "vtcon", 5))
> > > + continue;
> > > +
> > > + sprintf(buf, "%s/%s/name", path, vtcon->d_name);
> > > + fd = open(buf, O_RDONLY);
> > > + if (fd < 0)
> > > + continue;
> > > +
> > > + len = read(fd, buf, sizeof(buf) - 1);
> > > + close(fd);
> > > + if (len >= 0)
> > > + buf[len] = '\0';
> > > +
> > > + if (strstr(buf, "frame buffer device")) {
> > > + sprintf(buf, "%s/%s/bind", path, vtcon->d_name);
> > > + fd = open(buf, O_WRONLY);
> > > + if (fd != -1) {
> > > + if (enable)
> > > + igt_ignore_warn(write(fd, "1\n", 2));
> > > + else
> > > + igt_ignore_warn(write(fd, "0\n", 2));
> > > + close(fd);
> > > + }
> > > + break;
> > > + }
> > > + }
> > > + closedir(dir);
> > > +}
> > > diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
> > > index 4820066..a141894 100644
> > > --- a/lib/igt_sysfs.h
> > > +++ b/lib/igt_sysfs.h
> > > @@ -43,4 +43,6 @@ bool igt_sysfs_set_u32(int dir, const char *attr, uint32_t value);
> > > bool igt_sysfs_get_boolean(int dir, const char *attr);
> > > bool igt_sysfs_set_boolean(int dir, const char *attr, bool value);
> > >
> > > +void kick_fbcon(bool enable);
> > > +
> > > #endif /* __IGT_SYSFS_H__ */
> > > --
> > > 2.7.4
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH i-g-t 2/3] tests/drv_module_reload_basic: Convert sh script to C version.
2016-10-20 19:36 [PATCH i-g-t 0/3] Convert sh scripts to C variants Marius Vlad
2016-10-20 19:36 ` [PATCH i-g-t 1/3] lib/{igt_sysfs, igt_aux}: Make available to other users kick_fbcon() (unbind_fbcon()), and added helpers to igt_aux Marius Vlad
@ 2016-10-20 19:36 ` Marius Vlad
2016-10-20 19:52 ` Chris Wilson
2016-10-21 9:39 ` Petri Latvala
2016-10-20 19:36 ` [PATCH i-g-t 3/3] tests/kms_sysfs_edid_timing: " Marius Vlad
2016-10-20 21:00 ` [PATCH i-g-t 0/3] Convert sh scripts to C variants Jani Nikula
3 siblings, 2 replies; 19+ messages in thread
From: Marius Vlad @ 2016-10-20 19:36 UTC (permalink / raw)
To: intel-gfx
Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
---
tests/Makefile.sources | 2 +-
tests/drv_module_reload_basic | 97 -----------------------
tests/drv_module_reload_basic.c | 166 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 167 insertions(+), 98 deletions(-)
delete mode 100755 tests/drv_module_reload_basic
create mode 100644 tests/drv_module_reload_basic.c
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 6d081c3..c35ea11 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -211,6 +211,7 @@ TESTS_progs = \
kms_pwrite_crc \
kms_sink_crc_basic \
prime_udl \
+ drv_module_reload_basic \
$(NULL)
# IMPORTANT: The ZZ_ tests need to be run last!
@@ -221,7 +222,6 @@ TESTS_scripts_M = \
TESTS_scripts = \
debugfs_emon_crash \
drv_debugfs_reader \
- drv_module_reload_basic \
kms_sysfs_edid_timing \
sysfs_l3_parity \
test_rte_check \
diff --git a/tests/drv_module_reload_basic b/tests/drv_module_reload_basic
deleted file mode 100755
index a8d628d..0000000
--- a/tests/drv_module_reload_basic
+++ /dev/null
@@ -1,97 +0,0 @@
-#!/bin/bash
-#
-# Testcase: Reload the drm module
-#
-# ... we've broken this way too often :(
-#
-
-SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
-. $SOURCE_DIR/drm_lib.sh
-
-# no other drm service should be running, so we can just unbind
-
-# return 0 if module by name $1 is loaded according to lsmod
-function mod_loaded()
-{
- lsmod | grep -w "^$1" &> /dev/null
-}
-
-function reload() {
- local snd_hda_intel_unloaded
-
- echo Reloading i915.ko with $*
-
- # we must kick away fbcon (but only fbcon)
- for vtcon in /sys/class/vtconsole/vtcon*/ ; do
- if grep "frame buffer device" $vtcon/name > /dev/null ; then
- echo unbinding $vtcon: `cat $vtcon/name`
- echo 0 > $vtcon/bind
- fi
- done
-
- # The sound driver uses our power well
- pkill alsactl
- snd_hda_intel_unloaded=0
- if mod_loaded snd_hda_intel; then
- rmmod snd_hda_intel && snd_hda_intel_unloaded=1
- fi
-
- # gen5 only
- if mod_loaded intel_ips; then
- rmmod intel_ips
- fi
- rmmod i915 || return $IGT_EXIT_SKIP
- #ignore errors in intel-gtt, often built-in
- rmmod intel-gtt &> /dev/null
- # drm may be used by other devices (nouveau, radeon, udl, etc)
- rmmod drm_kms_helper &> /dev/null
- rmmod drm &> /dev/null
-
- if mod_loaded i915; then
- echo WARNING: i915.ko still loaded!
- return $IGT_EXIT_FAILURE
- else
- echo module successfully unloaded
- fi
-
- modprobe i915 $*
-
- if [ -f /sys/class/vtconsole/vtcon1/bind ]; then
- echo 1 > /sys/class/vtconsole/vtcon1/bind
- fi
-
- modprobe -q snd_hda_intel || return $snd_hda_intel_unloaded
-}
-
-function finish_load() {
- # does the device exist?
- if $SOURCE_DIR/gem_alive > /dev/null ; then
- echo "module successfully loaded again"
- else
- echo "failed to reload module successfully"
- return $IGT_EXIT_FAILURE
- fi
-
- # then try to run something
- if ! $SOURCE_DIR/gem_exec_store > /dev/null ; then
- echo "failed to execute a simple batch after reload"
- return $IGT_EXIT_FAILURE
- fi
-
- return $IGT_EXIT_SUCCESS
-}
-
-hda_dynamic_debug_enable
-
-reload || exit $?
-finish_load || exit $?
-
-# Repeat the module reload trying to to generate faults
-for i in $(seq 1 4); do
- reload inject_load_failure=$i
-done
-
-reload || exit $?
-finish_load
-
-exit $?
diff --git a/tests/drv_module_reload_basic.c b/tests/drv_module_reload_basic.c
new file mode 100644
index 0000000..d36afde
--- /dev/null
+++ b/tests/drv_module_reload_basic.c
@@ -0,0 +1,166 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+#include "igt.h"
+#include "igt_debugfs.h"
+#include "igt_aux.h"
+#include "igt_sysfs.h"
+#include "igt_core.h"
+
+#include <dirent.h>
+#include <sys/utsname.h>
+#include <linux/limits.h>
+#include <signal.h>
+#include <libgen.h>
+
+#include <libkmod.h>
+#include <proc/readproc.h>
+
+static const char *tests[] = {
+ "gem_alive", "gem_exec_store"
+};
+
+static int
+reload(const char *opts_i915)
+{
+ kick_fbcon(0);
+
+ if (opts_i915)
+ igt_info("Reloading i915 with %s\n\n", opts_i915);
+
+ if (igt_lsmod_has_module("snd_hda_intel")) {
+ if (igt_pkill(SIGTERM, "alsactl") == -1) {
+ return IGT_EXIT_FAILURE;
+ }
+ if (igt_rmmod("snd_hda_intel", false) == -1)
+ return IGT_EXIT_FAILURE;
+ }
+
+ /* gen5 */
+ if (igt_lsmod_has_module("intel_ips")) {
+ igt_rmmod("intel_ips", false);
+ }
+
+ if (igt_rmmod("i915", false) == -1) {
+ return IGT_EXIT_SKIP;
+ }
+
+ igt_info("i915.ko has been unloaded!\n");
+
+ if (igt_lsmod_has_module("intel-gtt")) {
+ igt_rmmod("intel-gtt", false);
+ }
+
+ igt_rmmod("drm_kms_helper", false);
+ igt_rmmod("drm", false);
+
+ if (igt_lsmod_has_module("i915")) {
+ igt_info("WARNING: i915.ko still loaded!\n");
+ return IGT_EXIT_FAILURE;
+ } else {
+ igt_info("module successfully unloaded\n");
+ }
+
+ /* we do not have automatic loading of dependencies */
+ igt_insmod("drm", NULL);
+ igt_insmod("drm_kms_helper", NULL);
+
+ if (igt_insmod("i915", opts_i915) == -1) {
+ igt_info("Could not load i915\n");
+ return IGT_EXIT_FAILURE;
+ }
+
+ kick_fbcon(1);
+
+ if (igt_insmod("snd_hda_intel", NULL) == -1)
+ return IGT_EXIT_FAILURE;
+
+ return IGT_EXIT_SUCCESS;
+}
+
+static void
+igt_execv(char **argv)
+{
+ igt_fork(child, 1) {
+ if (execv(argv[0], argv) < 0) {
+ igt_info("Failed to exec %s\n",
+ argv[0]);
+ exit(IGT_EXIT_FAILURE);
+ }
+ }
+ igt_waitchildren();
+}
+
+static void
+finish_load(char *dirname)
+{
+ char buf[PATH_MAX];
+ char *__argv[2] = { buf, NULL };
+
+ memset(buf, 0, PATH_MAX);
+ snprintf(buf, sizeof(buf), "%s/%s", dirname, tests[0]);
+
+ igt_execv(__argv);
+
+ memset(buf, 0, sizeof(buf));
+ snprintf(buf, sizeof(buf), "%s/%s", dirname, tests[1]);
+
+ igt_execv(__argv);
+}
+
+int main(int argc, char *argv[])
+{
+ int i, err;
+ char buf[64];
+ char *prg, *dir;
+
+ prg = strdup(argv[0]);
+ igt_subtest_init_parse_opts(&argc, argv, "", NULL,
+ NULL, NULL, NULL);
+
+
+ igt_subtest("basic-reload") {
+ if ((err = reload(NULL)))
+ igt_fail(err);
+ }
+
+ igt_subtest("basic-exec") {
+ dir = dirname(prg);
+ finish_load(dir);
+ }
+
+ igt_subtest("basic-reload-inject") {
+ for (i = 0; i < 4; i++) {
+ memset(buf, 0, sizeof(buf));
+ snprintf(buf, sizeof(buf), "inject_load_failure=%d", i);
+ reload(buf);
+ }
+ }
+
+ igt_subtest("basic-reload-final")
+ if ((err = reload(NULL)))
+ igt_fail(err);
+
+ free(prg);
+
+ igt_exit();
+}
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH i-g-t 2/3] tests/drv_module_reload_basic: Convert sh script to C version.
2016-10-20 19:36 ` [PATCH i-g-t 2/3] tests/drv_module_reload_basic: Convert sh script to C version Marius Vlad
@ 2016-10-20 19:52 ` Chris Wilson
2016-10-24 18:05 ` Marius Vlad
2016-10-21 9:39 ` Petri Latvala
1 sibling, 1 reply; 19+ messages in thread
From: Chris Wilson @ 2016-10-20 19:52 UTC (permalink / raw)
To: Marius Vlad; +Cc: intel-gfx
On Thu, Oct 20, 2016 at 10:36:48PM +0300, Marius Vlad wrote:
> +static const char *tests[] = {
> + "gem_alive", "gem_exec_store"
> +};
gem_alive is just a single ioctl query, simpler and move obvious to do
it inline. Then remove tests/gem_alive.c, but it may live on as
tools/gem_alive.c (or better yet tools/gem_info.c).
gem_exec_store is a couple of ioctls...
A rewritten C test should not be i915 specific if we can help it. The
core of it can be driver agnostic (same steps required to unbind from
console and reload after all).
> +
> +static int
> +reload(const char *opts_i915)
> +{
> + kick_fbcon(0);
> +
> + if (opts_i915)
> + igt_info("Reloading i915 with %s\n\n", opts_i915);
> +
> + if (igt_lsmod_has_module("snd_hda_intel")) {
> + if (igt_pkill(SIGTERM, "alsactl") == -1) {
> + return IGT_EXIT_FAILURE;
> + }
> + if (igt_rmmod("snd_hda_intel", false) == -1)
> + return IGT_EXIT_FAILURE;
> + }
> +
> + /* gen5 */
> + if (igt_lsmod_has_module("intel_ips")) {
> + igt_rmmod("intel_ips", false);
> + }
> +
> + if (igt_rmmod("i915", false) == -1) {
> + return IGT_EXIT_SKIP;
> + }
Ugh. These names leave much to be desired.
igt_kmod_load()
igt_kmod_unload()
igt_kmod_is_loaded() (can return refcnt >= 0 and -1 for unloaded)
> +
> + igt_info("i915.ko has been unloaded!\n");
> +
> + if (igt_lsmod_has_module("intel-gtt")) {
> + igt_rmmod("intel-gtt", false);
> + }
> +
> + igt_rmmod("drm_kms_helper", false);
> + igt_rmmod("drm", false);
> +
> + if (igt_lsmod_has_module("i915")) {
> + igt_info("WARNING: i915.ko still loaded!\n");
> + return IGT_EXIT_FAILURE;
> + } else {
> + igt_info("module successfully unloaded\n");
> + }
> +
> + /* we do not have automatic loading of dependencies */
> + igt_insmod("drm", NULL);
> + igt_insmod("drm_kms_helper", NULL);
> +
> + if (igt_insmod("i915", opts_i915) == -1) {
> + igt_info("Could not load i915\n");
> + return IGT_EXIT_FAILURE;
> + }
> +
> + kick_fbcon(1);
> +
> + if (igt_insmod("snd_hda_intel", NULL) == -1)
> + return IGT_EXIT_FAILURE;
> +
> + return IGT_EXIT_SUCCESS;
> +}
> +
> +static void
> +igt_execv(char **argv)
> +{
> + igt_fork(child, 1) {
> + if (execv(argv[0], argv) < 0) {
> + igt_info("Failed to exec %s\n",
> + argv[0]);
> + exit(IGT_EXIT_FAILURE);
> + }
> + }
> + igt_waitchildren();
> +}
> +
> +static void
> +finish_load(char *dirname)
> +{
> + char buf[PATH_MAX];
> + char *__argv[2] = { buf, NULL };
> +
> + memset(buf, 0, PATH_MAX);
> + snprintf(buf, sizeof(buf), "%s/%s", dirname, tests[0]);
> +
> + igt_execv(__argv);
> +
> + memset(buf, 0, sizeof(buf));
> + snprintf(buf, sizeof(buf), "%s/%s", dirname, tests[1]);
> +
> + igt_execv(__argv);
> +}
> +
> +int main(int argc, char *argv[])
igt_main
{
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH i-g-t 2/3] tests/drv_module_reload_basic: Convert sh script to C version.
2016-10-20 19:52 ` Chris Wilson
@ 2016-10-24 18:05 ` Marius Vlad
2016-10-24 20:34 ` Chris Wilson
0 siblings, 1 reply; 19+ messages in thread
From: Marius Vlad @ 2016-10-24 18:05 UTC (permalink / raw)
To: Chris Wilson, intel-gfx, petri.latvala, jani.nikula
[-- Attachment #1.1: Type: text/plain, Size: 3647 bytes --]
On Thu, Oct 20, 2016 at 08:52:23PM +0100, Chris Wilson wrote:
> On Thu, Oct 20, 2016 at 10:36:48PM +0300, Marius Vlad wrote:
> > +static const char *tests[] = {
> > + "gem_alive", "gem_exec_store"
> > +};
>
> gem_alive is just a single ioctl query, simpler and move obvious to do
> it inline. Then remove tests/gem_alive.c, but it may live on as
> tools/gem_alive.c (or better yet tools/gem_info.c).
>
> gem_exec_store is a couple of ioctls...
Initially I tried embeddeding them directly. Problem is the at exit drm
fd handler:
if (is_i915_device(fd)) {
if (__sync_fetch_and_add(&open_count, 1) == 0) {
gem_quiescent_gpu(fd);
at_exit_drm_fd = __drm_open_driver(chipset); <-- leaked until igt_exit, at_exit(), exit.
igt_install_exit_handler(quiescent_gpu_at_exit);
}
}
A drm_open_driver() w/o that opened fd allows reloading the driver after
running those basic tests/subtests.
>
> A rewritten C test should not be i915 specific if we can help it. The
> core of it can be driver agnostic (same steps required to unbind from
> console and reload after all).
>
> > +
> > +static int
> > +reload(const char *opts_i915)
> > +{
> > + kick_fbcon(0);
> > +
> > + if (opts_i915)
> > + igt_info("Reloading i915 with %s\n\n", opts_i915);
> > +
> > + if (igt_lsmod_has_module("snd_hda_intel")) {
> > + if (igt_pkill(SIGTERM, "alsactl") == -1) {
> > + return IGT_EXIT_FAILURE;
> > + }
> > + if (igt_rmmod("snd_hda_intel", false) == -1)
> > + return IGT_EXIT_FAILURE;
> > + }
> > +
> > + /* gen5 */
> > + if (igt_lsmod_has_module("intel_ips")) {
> > + igt_rmmod("intel_ips", false);
> > + }
> > +
> > + if (igt_rmmod("i915", false) == -1) {
> > + return IGT_EXIT_SKIP;
> > + }
>
> Ugh. These names leave much to be desired.
>
> igt_kmod_load()
> igt_kmod_unload()
> igt_kmod_is_loaded() (can return refcnt >= 0 and -1 for unloaded)
>
> > +
> > + igt_info("i915.ko has been unloaded!\n");
> > +
> > + if (igt_lsmod_has_module("intel-gtt")) {
> > + igt_rmmod("intel-gtt", false);
> > + }
> > +
> > + igt_rmmod("drm_kms_helper", false);
> > + igt_rmmod("drm", false);
> > +
> > + if (igt_lsmod_has_module("i915")) {
> > + igt_info("WARNING: i915.ko still loaded!\n");
> > + return IGT_EXIT_FAILURE;
> > + } else {
> > + igt_info("module successfully unloaded\n");
> > + }
> > +
> > + /* we do not have automatic loading of dependencies */
> > + igt_insmod("drm", NULL);
> > + igt_insmod("drm_kms_helper", NULL);
> > +
> > + if (igt_insmod("i915", opts_i915) == -1) {
> > + igt_info("Could not load i915\n");
> > + return IGT_EXIT_FAILURE;
> > + }
> > +
> > + kick_fbcon(1);
> > +
> > + if (igt_insmod("snd_hda_intel", NULL) == -1)
> > + return IGT_EXIT_FAILURE;
> > +
> > + return IGT_EXIT_SUCCESS;
> > +}
> > +
> > +static void
> > +igt_execv(char **argv)
> > +{
> > + igt_fork(child, 1) {
> > + if (execv(argv[0], argv) < 0) {
> > + igt_info("Failed to exec %s\n",
> > + argv[0]);
> > + exit(IGT_EXIT_FAILURE);
> > + }
> > + }
> > + igt_waitchildren();
> > +}
> > +
> > +static void
> > +finish_load(char *dirname)
> > +{
> > + char buf[PATH_MAX];
> > + char *__argv[2] = { buf, NULL };
> > +
> > + memset(buf, 0, PATH_MAX);
> > + snprintf(buf, sizeof(buf), "%s/%s", dirname, tests[0]);
> > +
> > + igt_execv(__argv);
> > +
> > + memset(buf, 0, sizeof(buf));
> > + snprintf(buf, sizeof(buf), "%s/%s", dirname, tests[1]);
> > +
> > + igt_execv(__argv);
> > +}
> > +
> > +int main(int argc, char *argv[])
>
> igt_main
> {
>
> --
> Chris Wilson, Intel Open Source Technology Centre
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH i-g-t 2/3] tests/drv_module_reload_basic: Convert sh script to C version.
2016-10-24 18:05 ` Marius Vlad
@ 2016-10-24 20:34 ` Chris Wilson
0 siblings, 0 replies; 19+ messages in thread
From: Chris Wilson @ 2016-10-24 20:34 UTC (permalink / raw)
To: intel-gfx, petri.latvala, jani.nikula
On Mon, Oct 24, 2016 at 09:05:28PM +0300, Marius Vlad wrote:
> On Thu, Oct 20, 2016 at 08:52:23PM +0100, Chris Wilson wrote:
> > On Thu, Oct 20, 2016 at 10:36:48PM +0300, Marius Vlad wrote:
> > > +static const char *tests[] = {
> > > + "gem_alive", "gem_exec_store"
> > > +};
> >
> > gem_alive is just a single ioctl query, simpler and move obvious to do
> > it inline. Then remove tests/gem_alive.c, but it may live on as
> > tools/gem_alive.c (or better yet tools/gem_info.c).
> >
> > gem_exec_store is a couple of ioctls...
> Initially I tried embeddeding them directly. Problem is the at exit drm
> fd handler:
>
> if (is_i915_device(fd)) {
> if (__sync_fetch_and_add(&open_count, 1) == 0) {
> gem_quiescent_gpu(fd);
>
> at_exit_drm_fd = __drm_open_driver(chipset); <-- leaked until igt_exit, at_exit(), exit.
> igt_install_exit_handler(quiescent_gpu_at_exit);
> }
> }
>
> A drm_open_driver() w/o that opened fd allows reloading the driver after
> running those basic tests/subtests.
Ah, yes, for module reload in C, you have to use __drm_open_driver() to
avoid that stray open device filehandle.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH i-g-t 2/3] tests/drv_module_reload_basic: Convert sh script to C version.
2016-10-20 19:36 ` [PATCH i-g-t 2/3] tests/drv_module_reload_basic: Convert sh script to C version Marius Vlad
2016-10-20 19:52 ` Chris Wilson
@ 2016-10-21 9:39 ` Petri Latvala
2016-10-24 18:06 ` Marius Vlad
1 sibling, 1 reply; 19+ messages in thread
From: Petri Latvala @ 2016-10-21 9:39 UTC (permalink / raw)
To: Marius Vlad, intel-gfx
On 10/20/2016 10:36 PM, Marius Vlad wrote:
> Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> ---
> tests/Makefile.sources | 2 +-
> tests/drv_module_reload_basic | 97 -----------------------
> tests/drv_module_reload_basic.c | 166 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 167 insertions(+), 98 deletions(-)
> delete mode 100755 tests/drv_module_reload_basic
> create mode 100644 tests/drv_module_reload_basic.c
>
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 6d081c3..c35ea11 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -211,6 +211,7 @@ TESTS_progs = \
> kms_pwrite_crc \
> kms_sink_crc_basic \
> prime_udl \
> + drv_module_reload_basic \
> $(NULL)
>
> # IMPORTANT: The ZZ_ tests need to be run last!
> @@ -221,7 +222,6 @@ TESTS_scripts_M = \
> TESTS_scripts = \
> debugfs_emon_crash \
> drv_debugfs_reader \
> - drv_module_reload_basic \
> kms_sysfs_edid_timing \
> sysfs_l3_parity \
> test_rte_check \
> diff --git a/tests/drv_module_reload_basic b/tests/drv_module_reload_basic
> deleted file mode 100755
> index a8d628d..0000000
> --- a/tests/drv_module_reload_basic
> +++ /dev/null
> @@ -1,97 +0,0 @@
> -#!/bin/bash
> -#
> -# Testcase: Reload the drm module
> -#
> -# ... we've broken this way too often :(
> -#
> -
> -SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
> -. $SOURCE_DIR/drm_lib.sh
> -
> -# no other drm service should be running, so we can just unbind
> -
> -# return 0 if module by name $1 is loaded according to lsmod
> -function mod_loaded()
> -{
> - lsmod | grep -w "^$1" &> /dev/null
> -}
> -
> -function reload() {
> - local snd_hda_intel_unloaded
> -
> - echo Reloading i915.ko with $*
> -
> - # we must kick away fbcon (but only fbcon)
> - for vtcon in /sys/class/vtconsole/vtcon*/ ; do
> - if grep "frame buffer device" $vtcon/name > /dev/null ; then
> - echo unbinding $vtcon: `cat $vtcon/name`
> - echo 0 > $vtcon/bind
> - fi
> - done
> -
> - # The sound driver uses our power well
> - pkill alsactl
> - snd_hda_intel_unloaded=0
> - if mod_loaded snd_hda_intel; then
> - rmmod snd_hda_intel && snd_hda_intel_unloaded=1
> - fi
> -
> - # gen5 only
> - if mod_loaded intel_ips; then
> - rmmod intel_ips
> - fi
> - rmmod i915 || return $IGT_EXIT_SKIP
> - #ignore errors in intel-gtt, often built-in
> - rmmod intel-gtt &> /dev/null
> - # drm may be used by other devices (nouveau, radeon, udl, etc)
> - rmmod drm_kms_helper &> /dev/null
> - rmmod drm &> /dev/null
> -
> - if mod_loaded i915; then
> - echo WARNING: i915.ko still loaded!
> - return $IGT_EXIT_FAILURE
> - else
> - echo module successfully unloaded
> - fi
> -
> - modprobe i915 $*
> -
> - if [ -f /sys/class/vtconsole/vtcon1/bind ]; then
> - echo 1 > /sys/class/vtconsole/vtcon1/bind
> - fi
> -
> - modprobe -q snd_hda_intel || return $snd_hda_intel_unloaded
> -}
> -
> -function finish_load() {
> - # does the device exist?
> - if $SOURCE_DIR/gem_alive > /dev/null ; then
> - echo "module successfully loaded again"
> - else
> - echo "failed to reload module successfully"
> - return $IGT_EXIT_FAILURE
> - fi
> -
> - # then try to run something
> - if ! $SOURCE_DIR/gem_exec_store > /dev/null ; then
> - echo "failed to execute a simple batch after reload"
> - return $IGT_EXIT_FAILURE
> - fi
> -
> - return $IGT_EXIT_SUCCESS
> -}
> -
> -hda_dynamic_debug_enable
I don't see the equivalent of hda_dynamic_debug_enable in the C version.
> -
> -reload || exit $?
> -finish_load || exit $?
> -
> -# Repeat the module reload trying to to generate faults
> -for i in $(seq 1 4); do
> - reload inject_load_failure=$i
> -done
> -
> -reload || exit $?
> -finish_load
> -
> -exit $?
> diff --git a/tests/drv_module_reload_basic.c b/tests/drv_module_reload_basic.c
> new file mode 100644
> index 0000000..d36afde
> --- /dev/null
> +++ b/tests/drv_module_reload_basic.c
> @@ -0,0 +1,166 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +#include "igt.h"
> +#include "igt_debugfs.h"
> +#include "igt_aux.h"
> +#include "igt_sysfs.h"
> +#include "igt_core.h"
> +
> +#include <dirent.h>
> +#include <sys/utsname.h>
> +#include <linux/limits.h>
> +#include <signal.h>
> +#include <libgen.h>
> +
> +#include <libkmod.h>
> +#include <proc/readproc.h>
> +
> +static const char *tests[] = {
> + "gem_alive", "gem_exec_store"
> +};
> +
> +static int
> +reload(const char *opts_i915)
> +{
> + kick_fbcon(0);
> +
> + if (opts_i915)
> + igt_info("Reloading i915 with %s\n\n", opts_i915);
> +
> + if (igt_lsmod_has_module("snd_hda_intel")) {
> + if (igt_pkill(SIGTERM, "alsactl") == -1) {
> + return IGT_EXIT_FAILURE;
> + }
> + if (igt_rmmod("snd_hda_intel", false) == -1)
> + return IGT_EXIT_FAILURE;
> + }
> +
> + /* gen5 */
> + if (igt_lsmod_has_module("intel_ips")) {
> + igt_rmmod("intel_ips", false);
> + }
> +
> + if (igt_rmmod("i915", false) == -1) {
> + return IGT_EXIT_SKIP;
> + }
> +
> + igt_info("i915.ko has been unloaded!\n");
> +
> + if (igt_lsmod_has_module("intel-gtt")) {
> + igt_rmmod("intel-gtt", false);
> + }
> +
> + igt_rmmod("drm_kms_helper", false);
> + igt_rmmod("drm", false);
> +
> + if (igt_lsmod_has_module("i915")) {
> + igt_info("WARNING: i915.ko still loaded!\n");
> + return IGT_EXIT_FAILURE;
> + } else {
> + igt_info("module successfully unloaded\n");
> + }
> +
> + /* we do not have automatic loading of dependencies */
> + igt_insmod("drm", NULL);
> + igt_insmod("drm_kms_helper", NULL);
> +
> + if (igt_insmod("i915", opts_i915) == -1) {
> + igt_info("Could not load i915\n");
> + return IGT_EXIT_FAILURE;
> + }
> +
> + kick_fbcon(1);
> +
> + if (igt_insmod("snd_hda_intel", NULL) == -1)
> + return IGT_EXIT_FAILURE;
> +
> + return IGT_EXIT_SUCCESS;
> +}
> +
> +static void
> +igt_execv(char **argv)
> +{
> + igt_fork(child, 1) {
> + if (execv(argv[0], argv) < 0) {
> + igt_info("Failed to exec %s\n",
> + argv[0]);
> + exit(IGT_EXIT_FAILURE);
> + }
> + }
> + igt_waitchildren();
> +}
> +
> +static void
> +finish_load(char *dirname)
> +{
> + char buf[PATH_MAX];
> + char *__argv[2] = { buf, NULL };
> +
> + memset(buf, 0, PATH_MAX);
> + snprintf(buf, sizeof(buf), "%s/%s", dirname, tests[0]);
> +
> + igt_execv(__argv);
> +
> + memset(buf, 0, sizeof(buf));
> + snprintf(buf, sizeof(buf), "%s/%s", dirname, tests[1]);
> +
> + igt_execv(__argv);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + int i, err;
> + char buf[64];
> + char *prg, *dir;
> +
> + prg = strdup(argv[0]);
> + igt_subtest_init_parse_opts(&argc, argv, "", NULL,
> + NULL, NULL, NULL);
> +
> +
> + igt_subtest("basic-reload") {
> + if ((err = reload(NULL)))
> + igt_fail(err);
> + }
> +
> + igt_subtest("basic-exec") {
> + dir = dirname(prg);
> + finish_load(dir);
> + }
> +
> + igt_subtest("basic-reload-inject") {
> + for (i = 0; i < 4; i++) {
> + memset(buf, 0, sizeof(buf));
> + snprintf(buf, sizeof(buf), "inject_load_failure=%d", i);
> + reload(buf);
> + }
> + }
> +
> + igt_subtest("basic-reload-final")
> + if ((err = reload(NULL)))
> + igt_fail(err);
> +
An accompanying change to tests/intel-ci/fast-feedback.testlist in the
same commit required.
--
Petri Latvala
> + free(prg);
> +
> + igt_exit();
> +}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH i-g-t 2/3] tests/drv_module_reload_basic: Convert sh script to C version.
2016-10-21 9:39 ` Petri Latvala
@ 2016-10-24 18:06 ` Marius Vlad
0 siblings, 0 replies; 19+ messages in thread
From: Marius Vlad @ 2016-10-24 18:06 UTC (permalink / raw)
To: Petri Latvala; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 9470 bytes --]
On Fri, Oct 21, 2016 at 12:39:22PM +0300, Petri Latvala wrote:
>
>
> On 10/20/2016 10:36 PM, Marius Vlad wrote:
> > Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> > ---
> > tests/Makefile.sources | 2 +-
> > tests/drv_module_reload_basic | 97 -----------------------
> > tests/drv_module_reload_basic.c | 166 ++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 167 insertions(+), 98 deletions(-)
> > delete mode 100755 tests/drv_module_reload_basic
> > create mode 100644 tests/drv_module_reload_basic.c
> >
> > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > index 6d081c3..c35ea11 100644
> > --- a/tests/Makefile.sources
> > +++ b/tests/Makefile.sources
> > @@ -211,6 +211,7 @@ TESTS_progs = \
> > kms_pwrite_crc \
> > kms_sink_crc_basic \
> > prime_udl \
> > + drv_module_reload_basic \
> > $(NULL)
> > # IMPORTANT: The ZZ_ tests need to be run last!
> > @@ -221,7 +222,6 @@ TESTS_scripts_M = \
> > TESTS_scripts = \
> > debugfs_emon_crash \
> > drv_debugfs_reader \
> > - drv_module_reload_basic \
> > kms_sysfs_edid_timing \
> > sysfs_l3_parity \
> > test_rte_check \
> > diff --git a/tests/drv_module_reload_basic b/tests/drv_module_reload_basic
> > deleted file mode 100755
> > index a8d628d..0000000
> > --- a/tests/drv_module_reload_basic
> > +++ /dev/null
> > @@ -1,97 +0,0 @@
> > -#!/bin/bash
> > -#
> > -# Testcase: Reload the drm module
> > -#
> > -# ... we've broken this way too often :(
> > -#
> > -
> > -SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
> > -. $SOURCE_DIR/drm_lib.sh
> > -
> > -# no other drm service should be running, so we can just unbind
> > -
> > -# return 0 if module by name $1 is loaded according to lsmod
> > -function mod_loaded()
> > -{
> > - lsmod | grep -w "^$1" &> /dev/null
> > -}
> > -
> > -function reload() {
> > - local snd_hda_intel_unloaded
> > -
> > - echo Reloading i915.ko with $*
> > -
> > - # we must kick away fbcon (but only fbcon)
> > - for vtcon in /sys/class/vtconsole/vtcon*/ ; do
> > - if grep "frame buffer device" $vtcon/name > /dev/null ; then
> > - echo unbinding $vtcon: `cat $vtcon/name`
> > - echo 0 > $vtcon/bind
> > - fi
> > - done
> > -
> > - # The sound driver uses our power well
> > - pkill alsactl
> > - snd_hda_intel_unloaded=0
> > - if mod_loaded snd_hda_intel; then
> > - rmmod snd_hda_intel && snd_hda_intel_unloaded=1
> > - fi
> > -
> > - # gen5 only
> > - if mod_loaded intel_ips; then
> > - rmmod intel_ips
> > - fi
> > - rmmod i915 || return $IGT_EXIT_SKIP
> > - #ignore errors in intel-gtt, often built-in
> > - rmmod intel-gtt &> /dev/null
> > - # drm may be used by other devices (nouveau, radeon, udl, etc)
> > - rmmod drm_kms_helper &> /dev/null
> > - rmmod drm &> /dev/null
> > -
> > - if mod_loaded i915; then
> > - echo WARNING: i915.ko still loaded!
> > - return $IGT_EXIT_FAILURE
> > - else
> > - echo module successfully unloaded
> > - fi
> > -
> > - modprobe i915 $*
> > -
> > - if [ -f /sys/class/vtconsole/vtcon1/bind ]; then
> > - echo 1 > /sys/class/vtconsole/vtcon1/bind
> > - fi
> > -
> > - modprobe -q snd_hda_intel || return $snd_hda_intel_unloaded
> > -}
> > -
> > -function finish_load() {
> > - # does the device exist?
> > - if $SOURCE_DIR/gem_alive > /dev/null ; then
> > - echo "module successfully loaded again"
> > - else
> > - echo "failed to reload module successfully"
> > - return $IGT_EXIT_FAILURE
> > - fi
> > -
> > - # then try to run something
> > - if ! $SOURCE_DIR/gem_exec_store > /dev/null ; then
> > - echo "failed to execute a simple batch after reload"
> > - return $IGT_EXIT_FAILURE
> > - fi
> > -
> > - return $IGT_EXIT_SUCCESS
> > -}
> > -
> > -hda_dynamic_debug_enable
>
> I don't see the equivalent of hda_dynamic_debug_enable in the C version.
Indeed, this got updated prior to me looking at it.
>
>
> > -
> > -reload || exit $?
> > -finish_load || exit $?
> > -
> > -# Repeat the module reload trying to to generate faults
> > -for i in $(seq 1 4); do
> > - reload inject_load_failure=$i
> > -done
> > -
> > -reload || exit $?
> > -finish_load
> > -
> > -exit $?
> > diff --git a/tests/drv_module_reload_basic.c b/tests/drv_module_reload_basic.c
> > new file mode 100644
> > index 0000000..d36afde
> > --- /dev/null
> > +++ b/tests/drv_module_reload_basic.c
> > @@ -0,0 +1,166 @@
> > +/*
> > + * Copyright © 2016 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the "Software"),
> > + * to deal in the Software without restriction, including without limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the next
> > + * paragraph) shall be included in all copies or substantial portions of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> > + * IN THE SOFTWARE.
> > + */
> > +#include "igt.h"
> > +#include "igt_debugfs.h"
> > +#include "igt_aux.h"
> > +#include "igt_sysfs.h"
> > +#include "igt_core.h"
> > +
> > +#include <dirent.h>
> > +#include <sys/utsname.h>
> > +#include <linux/limits.h>
> > +#include <signal.h>
> > +#include <libgen.h>
> > +
> > +#include <libkmod.h>
> > +#include <proc/readproc.h>
> > +
> > +static const char *tests[] = {
> > + "gem_alive", "gem_exec_store"
> > +};
> > +
> > +static int
> > +reload(const char *opts_i915)
> > +{
> > + kick_fbcon(0);
> > +
> > + if (opts_i915)
> > + igt_info("Reloading i915 with %s\n\n", opts_i915);
> > +
> > + if (igt_lsmod_has_module("snd_hda_intel")) {
> > + if (igt_pkill(SIGTERM, "alsactl") == -1) {
> > + return IGT_EXIT_FAILURE;
> > + }
> > + if (igt_rmmod("snd_hda_intel", false) == -1)
> > + return IGT_EXIT_FAILURE;
> > + }
> > +
> > + /* gen5 */
> > + if (igt_lsmod_has_module("intel_ips")) {
> > + igt_rmmod("intel_ips", false);
> > + }
> > +
> > + if (igt_rmmod("i915", false) == -1) {
> > + return IGT_EXIT_SKIP;
> > + }
> > +
> > + igt_info("i915.ko has been unloaded!\n");
> > +
> > + if (igt_lsmod_has_module("intel-gtt")) {
> > + igt_rmmod("intel-gtt", false);
> > + }
> > +
> > + igt_rmmod("drm_kms_helper", false);
> > + igt_rmmod("drm", false);
> > +
> > + if (igt_lsmod_has_module("i915")) {
> > + igt_info("WARNING: i915.ko still loaded!\n");
> > + return IGT_EXIT_FAILURE;
> > + } else {
> > + igt_info("module successfully unloaded\n");
> > + }
> > +
> > + /* we do not have automatic loading of dependencies */
> > + igt_insmod("drm", NULL);
> > + igt_insmod("drm_kms_helper", NULL);
> > +
> > + if (igt_insmod("i915", opts_i915) == -1) {
> > + igt_info("Could not load i915\n");
> > + return IGT_EXIT_FAILURE;
> > + }
> > +
> > + kick_fbcon(1);
> > +
> > + if (igt_insmod("snd_hda_intel", NULL) == -1)
> > + return IGT_EXIT_FAILURE;
> > +
> > + return IGT_EXIT_SUCCESS;
> > +}
> > +
> > +static void
> > +igt_execv(char **argv)
> > +{
> > + igt_fork(child, 1) {
> > + if (execv(argv[0], argv) < 0) {
> > + igt_info("Failed to exec %s\n",
> > + argv[0]);
> > + exit(IGT_EXIT_FAILURE);
> > + }
> > + }
> > + igt_waitchildren();
> > +}
> > +
> > +static void
> > +finish_load(char *dirname)
> > +{
> > + char buf[PATH_MAX];
> > + char *__argv[2] = { buf, NULL };
> > +
> > + memset(buf, 0, PATH_MAX);
> > + snprintf(buf, sizeof(buf), "%s/%s", dirname, tests[0]);
> > +
> > + igt_execv(__argv);
> > +
> > + memset(buf, 0, sizeof(buf));
> > + snprintf(buf, sizeof(buf), "%s/%s", dirname, tests[1]);
> > +
> > + igt_execv(__argv);
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > + int i, err;
> > + char buf[64];
> > + char *prg, *dir;
> > +
> > + prg = strdup(argv[0]);
> > + igt_subtest_init_parse_opts(&argc, argv, "", NULL,
> > + NULL, NULL, NULL);
> > +
> > +
> > + igt_subtest("basic-reload") {
> > + if ((err = reload(NULL)))
> > + igt_fail(err);
> > + }
> > +
> > + igt_subtest("basic-exec") {
> > + dir = dirname(prg);
> > + finish_load(dir);
> > + }
> > +
> > + igt_subtest("basic-reload-inject") {
> > + for (i = 0; i < 4; i++) {
> > + memset(buf, 0, sizeof(buf));
> > + snprintf(buf, sizeof(buf), "inject_load_failure=%d", i);
> > + reload(buf);
> > + }
> > + }
> > +
> > + igt_subtest("basic-reload-final")
> > + if ((err = reload(NULL)))
> > + igt_fail(err);
> > +
>
> An accompanying change to tests/intel-ci/fast-feedback.testlist in the same
> commit required.
Will do.
>
>
> --
> Petri Latvala
>
>
> > + free(prg);
> > +
> > + igt_exit();
> > +}
>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH i-g-t 3/3] tests/kms_sysfs_edid_timing: Convert sh script to C version.
2016-10-20 19:36 [PATCH i-g-t 0/3] Convert sh scripts to C variants Marius Vlad
2016-10-20 19:36 ` [PATCH i-g-t 1/3] lib/{igt_sysfs, igt_aux}: Make available to other users kick_fbcon() (unbind_fbcon()), and added helpers to igt_aux Marius Vlad
2016-10-20 19:36 ` [PATCH i-g-t 2/3] tests/drv_module_reload_basic: Convert sh script to C version Marius Vlad
@ 2016-10-20 19:36 ` Marius Vlad
2016-10-20 19:58 ` Chris Wilson
2016-10-20 21:00 ` [PATCH i-g-t 0/3] Convert sh scripts to C variants Jani Nikula
3 siblings, 1 reply; 19+ messages in thread
From: Marius Vlad @ 2016-10-20 19:36 UTC (permalink / raw)
To: intel-gfx
While at it, make available time macros to other users.
Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
---
benchmarks/gem_syslatency.c | 4 ---
lib/igt_core.c | 3 --
lib/igt_core.h | 3 ++
tests/Makefile.sources | 2 +-
tests/drv_hangman.c | 1 -
tests/gem_wait.c | 4 ---
tests/kms_flip.c | 3 --
tests/kms_sysfs_edid_timing | 25 -------------
tests/kms_sysfs_edid_timing.c | 82 +++++++++++++++++++++++++++++++++++++++++++
9 files changed, 86 insertions(+), 41 deletions(-)
delete mode 100755 tests/kms_sysfs_edid_timing
create mode 100644 tests/kms_sysfs_edid_timing.c
diff --git a/benchmarks/gem_syslatency.c b/benchmarks/gem_syslatency.c
index 6cad3a0..83bfac7 100644
--- a/benchmarks/gem_syslatency.c
+++ b/benchmarks/gem_syslatency.c
@@ -133,10 +133,6 @@ static void *gem_busyspin(void *arg)
return NULL;
}
-#define MSEC_PER_SEC (1000)
-#define USEC_PER_SEC (1000 * MSEC_PER_SEC)
-#define NSEC_PER_SEC (1000 * USEC_PER_SEC)
-
static double elapsed(const struct timespec *a, const struct timespec *b)
{
return 1e9*(b->tv_sec - a->tv_sec) + (b->tv_nsec - a ->tv_nsec);
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 9cd5f98..f64c809 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -398,9 +398,6 @@ error:
return -errno;
}
-#define MSEC_PER_SEC (1000)
-#define USEC_PER_SEC (1000*MSEC_PER_SEC)
-#define NSEC_PER_SEC (1000*USEC_PER_SEC)
uint64_t igt_nsec_elapsed(struct timespec *start)
{
struct timespec now;
diff --git a/lib/igt_core.h b/lib/igt_core.h
index 03be757..a45e334 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -847,6 +847,9 @@ extern enum igt_log_level igt_log_level;
void igt_set_timeout(unsigned int seconds,
const char *op);
+#define MSEC_PER_SEC (1000)
+#define USEC_PER_SEC (1000*MSEC_PER_SEC)
+#define NSEC_PER_SEC (1000*USEC_PER_SEC)
/**
* igt_nsec_elapsed:
* @start: measure from this point in time
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index c35ea11..969ef0b 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -210,6 +210,7 @@ TESTS_progs = \
kms_mmap_write_crc \
kms_pwrite_crc \
kms_sink_crc_basic \
+ kms_sysfs_edid_timing \
prime_udl \
drv_module_reload_basic \
$(NULL)
@@ -222,7 +223,6 @@ TESTS_scripts_M = \
TESTS_scripts = \
debugfs_emon_crash \
drv_debugfs_reader \
- kms_sysfs_edid_timing \
sysfs_l3_parity \
test_rte_check \
tools_test \
diff --git a/tests/drv_hangman.c b/tests/drv_hangman.c
index 953a4c6..19d809c 100644
--- a/tests/drv_hangman.c
+++ b/tests/drv_hangman.c
@@ -282,7 +282,6 @@ static void test_error_state_capture(unsigned ring_id,
* case and it takes a lot more time to wrap, so the acthd can potentially keep
* increasing for a long time
*/
-#define NSEC_PER_SEC 1000000000LL
static void hangcheck_unterminated(void)
{
int fd;
diff --git a/tests/gem_wait.c b/tests/gem_wait.c
index b4127de..db04958 100644
--- a/tests/gem_wait.c
+++ b/tests/gem_wait.c
@@ -83,10 +83,6 @@ static void sigiter(int sig, siginfo_t *info, void *arg)
__sync_synchronize();
}
-#define MSEC_PER_SEC (1000)
-#define USEC_PER_SEC (1000 * MSEC_PER_SEC)
-#define NSEC_PER_SEC (1000 * USEC_PER_SEC)
-
#define BUSY 1
#define HANG 2
static void basic(int fd, unsigned engine, unsigned flags)
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 7646aaf..842bc3a 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -83,9 +83,6 @@
#define DRM_CAP_TIMESTAMP_MONOTONIC 6
#endif
-#define USEC_PER_SEC 1000000L
-#define NSEC_PER_SEC 1000000000L
-
drmModeRes *resources;
int drm_fd;
static drm_intel_bufmgr *bufmgr;
diff --git a/tests/kms_sysfs_edid_timing b/tests/kms_sysfs_edid_timing
deleted file mode 100755
index 46ea540..0000000
--- a/tests/kms_sysfs_edid_timing
+++ /dev/null
@@ -1,25 +0,0 @@
-#!/bin/bash
-#
-# This check the time we take to read the content of all the possible connectors.
-# Without the edid -ENXIO patch (http://permalink.gmane.org/gmane.comp.video.dri.devel/62083),
-# we sometimes take a *really* long time. So let's just check for some reasonable timing here
-#
-
-DRM_LIB_ALLOW_NO_MASTER=1
-
-SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
-. $SOURCE_DIR/drm_lib.sh
-
-TIME1=$(date +%s%N)
-cat $(find /sys/devices/|grep drm | grep /status) > /dev/null
-TIME2=$(date +%s%N)
-
-# time in ms
-RES=$(((TIME2 - TIME1) / 1000000))
-
-if [ $RES -gt 600 ]; then
- echo "Talking to outputs took ${RES}ms, something is wrong"
- exit $IGT_EXIT_FAILURE
-fi
-
-exit $IGT_EXIT_SUCCESS
diff --git a/tests/kms_sysfs_edid_timing.c b/tests/kms_sysfs_edid_timing.c
new file mode 100644
index 0000000..8de4b78
--- /dev/null
+++ b/tests/kms_sysfs_edid_timing.c
@@ -0,0 +1,82 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+#include "igt.h"
+#include "igt_debugfs.h"
+#include "igt_sysfs.h"
+#include "igt_core.h"
+
+#include <dirent.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <time.h>
+
+#define MSEC_PER_SEC (1000)
+#define USEC_PER_SEC (1000*MSEC_PER_SEC)
+#define NSEC_PER_SEC (1000*USEC_PER_SEC)
+
+IGT_TEST_DESCRIPTION("This check the time we take to read the content of all "
+ "the possible connectors. Without the edid -ENXIO patch "
+ "(http://permalink.gmane.org/gmane.comp.video.dri.devel/62083), "
+ "we sometimes take a *really* long time. "
+ "So let's just check for some reasonable timing here");
+
+igt_simple_main
+{
+ int dir = igt_sysfs_open(-1, NULL);
+ DIR *dirp = fdopendir(dir);
+ struct dirent *de;
+
+ int i, fd = 0;
+ int fds[32];
+ uint64_t elapsed;
+
+ struct stat st;
+ struct timespec start = {};
+
+ memset(fds, -1, sizeof(fds));
+ while ((de = readdir(dirp))) {
+
+ if (*de->d_name == '.')
+ continue;
+
+ if (fstatat(dir, de->d_name, &st, 0))
+ continue;
+
+ if (S_ISDIR(st.st_mode) &&
+ strncmp(de->d_name, "card0-", 6) == 0) {
+ int dfd = openat(dir, de->d_name, O_RDONLY);
+ igt_assert(dfd > 0);
+ fds[fd++] = dfd;
+ }
+ }
+ closedir(dirp);
+
+ elapsed = igt_nsec_elapsed(&start);
+ for (i = 0; i < fd - 1; i++) {
+ igt_assert(igt_sysfs_get(fds[i], "status") != NULL);
+ close(fds[i]);
+ }
+ elapsed = igt_nsec_elapsed(&start);
+
+ igt_fail_on(elapsed > 600 * USEC_PER_SEC);
+}
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH i-g-t 3/3] tests/kms_sysfs_edid_timing: Convert sh script to C version.
2016-10-20 19:36 ` [PATCH i-g-t 3/3] tests/kms_sysfs_edid_timing: " Marius Vlad
@ 2016-10-20 19:58 ` Chris Wilson
0 siblings, 0 replies; 19+ messages in thread
From: Chris Wilson @ 2016-10-20 19:58 UTC (permalink / raw)
To: Marius Vlad; +Cc: intel-gfx
On Thu, Oct 20, 2016 at 10:36:49PM +0300, Marius Vlad wrote:
> While at it, make available time macros to other users.
>
> Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> ---
> benchmarks/gem_syslatency.c | 4 ---
> lib/igt_core.c | 3 --
> lib/igt_core.h | 3 ++
> tests/Makefile.sources | 2 +-
> tests/drv_hangman.c | 1 -
> tests/gem_wait.c | 4 ---
> tests/kms_flip.c | 3 --
> tests/kms_sysfs_edid_timing | 25 -------------
> tests/kms_sysfs_edid_timing.c | 82 +++++++++++++++++++++++++++++++++++++++++++
> 9 files changed, 86 insertions(+), 41 deletions(-)
> delete mode 100755 tests/kms_sysfs_edid_timing
> create mode 100644 tests/kms_sysfs_edid_timing.c
>
> diff --git a/benchmarks/gem_syslatency.c b/benchmarks/gem_syslatency.c
> index 6cad3a0..83bfac7 100644
> --- a/benchmarks/gem_syslatency.c
> +++ b/benchmarks/gem_syslatency.c
> @@ -133,10 +133,6 @@ static void *gem_busyspin(void *arg)
> return NULL;
> }
>
> -#define MSEC_PER_SEC (1000)
> -#define USEC_PER_SEC (1000 * MSEC_PER_SEC)
> -#define NSEC_PER_SEC (1000 * USEC_PER_SEC)
> -
> static double elapsed(const struct timespec *a, const struct timespec *b)
> {
> return 1e9*(b->tv_sec - a->tv_sec) + (b->tv_nsec - a ->tv_nsec);
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 9cd5f98..f64c809 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -398,9 +398,6 @@ error:
> return -errno;
> }
>
> -#define MSEC_PER_SEC (1000)
> -#define USEC_PER_SEC (1000*MSEC_PER_SEC)
> -#define NSEC_PER_SEC (1000*USEC_PER_SEC)
> uint64_t igt_nsec_elapsed(struct timespec *start)
> {
> struct timespec now;
> diff --git a/lib/igt_core.h b/lib/igt_core.h
> index 03be757..a45e334 100644
> --- a/lib/igt_core.h
> +++ b/lib/igt_core.h
> @@ -847,6 +847,9 @@ extern enum igt_log_level igt_log_level;
> void igt_set_timeout(unsigned int seconds,
> const char *op);
>
> +#define MSEC_PER_SEC (1000)
> +#define USEC_PER_SEC (1000*MSEC_PER_SEC)
> +#define NSEC_PER_SEC (1000*USEC_PER_SEC)
> /**
> * igt_nsec_elapsed:
> * @start: measure from this point in time
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index c35ea11..969ef0b 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -210,6 +210,7 @@ TESTS_progs = \
> kms_mmap_write_crc \
> kms_pwrite_crc \
> kms_sink_crc_basic \
> + kms_sysfs_edid_timing \
> prime_udl \
> drv_module_reload_basic \
> $(NULL)
> @@ -222,7 +223,6 @@ TESTS_scripts_M = \
> TESTS_scripts = \
> debugfs_emon_crash \
> drv_debugfs_reader \
> - kms_sysfs_edid_timing \
> sysfs_l3_parity \
> test_rte_check \
> tools_test \
> diff --git a/tests/drv_hangman.c b/tests/drv_hangman.c
> index 953a4c6..19d809c 100644
> --- a/tests/drv_hangman.c
> +++ b/tests/drv_hangman.c
> @@ -282,7 +282,6 @@ static void test_error_state_capture(unsigned ring_id,
> * case and it takes a lot more time to wrap, so the acthd can potentially keep
> * increasing for a long time
> */
> -#define NSEC_PER_SEC 1000000000LL
> static void hangcheck_unterminated(void)
> {
> int fd;
> diff --git a/tests/gem_wait.c b/tests/gem_wait.c
> index b4127de..db04958 100644
> --- a/tests/gem_wait.c
> +++ b/tests/gem_wait.c
> @@ -83,10 +83,6 @@ static void sigiter(int sig, siginfo_t *info, void *arg)
> __sync_synchronize();
> }
>
> -#define MSEC_PER_SEC (1000)
> -#define USEC_PER_SEC (1000 * MSEC_PER_SEC)
> -#define NSEC_PER_SEC (1000 * USEC_PER_SEC)
> -
> #define BUSY 1
> #define HANG 2
> static void basic(int fd, unsigned engine, unsigned flags)
> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> index 7646aaf..842bc3a 100644
> --- a/tests/kms_flip.c
> +++ b/tests/kms_flip.c
> @@ -83,9 +83,6 @@
> #define DRM_CAP_TIMESTAMP_MONOTONIC 6
> #endif
>
> -#define USEC_PER_SEC 1000000L
> -#define NSEC_PER_SEC 1000000000L
> -
> drmModeRes *resources;
> int drm_fd;
> static drm_intel_bufmgr *bufmgr;
> diff --git a/tests/kms_sysfs_edid_timing b/tests/kms_sysfs_edid_timing
> deleted file mode 100755
> index 46ea540..0000000
> --- a/tests/kms_sysfs_edid_timing
> +++ /dev/null
> @@ -1,25 +0,0 @@
> -#!/bin/bash
> -#
> -# This check the time we take to read the content of all the possible connectors.
> -# Without the edid -ENXIO patch (http://permalink.gmane.org/gmane.comp.video.dri.devel/62083),
> -# we sometimes take a *really* long time. So let's just check for some reasonable timing here
> -#
> -
> -DRM_LIB_ALLOW_NO_MASTER=1
> -
> -SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
> -. $SOURCE_DIR/drm_lib.sh
> -
> -TIME1=$(date +%s%N)
> -cat $(find /sys/devices/|grep drm | grep /status) > /dev/null
> -TIME2=$(date +%s%N)
> -
> -# time in ms
> -RES=$(((TIME2 - TIME1) / 1000000))
> -
> -if [ $RES -gt 600 ]; then
> - echo "Talking to outputs took ${RES}ms, something is wrong"
> - exit $IGT_EXIT_FAILURE
> -fi
> -
> -exit $IGT_EXIT_SUCCESS
> diff --git a/tests/kms_sysfs_edid_timing.c b/tests/kms_sysfs_edid_timing.c
> new file mode 100644
> index 0000000..8de4b78
> --- /dev/null
> +++ b/tests/kms_sysfs_edid_timing.c
> @@ -0,0 +1,82 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +#include "igt.h"
> +#include "igt_debugfs.h"
> +#include "igt_sysfs.h"
> +#include "igt_core.h"
> +
> +#include <dirent.h>
> +#include <fcntl.h>
> +#include <sys/stat.h>
> +#include <time.h>
> +
> +#define MSEC_PER_SEC (1000)
> +#define USEC_PER_SEC (1000*MSEC_PER_SEC)
> +#define NSEC_PER_SEC (1000*USEC_PER_SEC)
> +
> +IGT_TEST_DESCRIPTION("This check the time we take to read the content of all "
> + "the possible connectors. Without the edid -ENXIO patch "
> + "(http://permalink.gmane.org/gmane.comp.video.dri.devel/62083), "
> + "we sometimes take a *really* long time. "
> + "So let's just check for some reasonable timing here");
> +
> +igt_simple_main
> +{
> + int dir = igt_sysfs_open(-1, NULL);
> + DIR *dirp = fdopendir(dir);
> + struct dirent *de;
> +
> + int i, fd = 0;
> + int fds[32];
> + uint64_t elapsed;
> +
> + struct stat st;
> + struct timespec start = {};
> +
> + memset(fds, -1, sizeof(fds));
> + while ((de = readdir(dirp))) {
> +
> + if (*de->d_name == '.')
> + continue;
> +
> + if (fstatat(dir, de->d_name, &st, 0))
> + continue;
> +
> + if (S_ISDIR(st.st_mode) &&
> + strncmp(de->d_name, "card0-", 6) == 0) {
Whilst you are here, you may as well generalise this to work on all
outputs with an edid.
> + int dfd = openat(dir, de->d_name, O_RDONLY);
> + igt_assert(dfd > 0);
> + fds[fd++] = dfd;
> + }
> + }
> + closedir(dirp);
> +
> + elapsed = igt_nsec_elapsed(&start);
> + for (i = 0; i < fd - 1; i++) {
> + igt_assert(igt_sysfs_get(fds[i], "status") != NULL);
This is reading the cached status. This no longer performs the original
intentional.
igt_sysfs_set(fd[i], "status", "detect");
> + close(fds[i]);
> + }
> + elapsed = igt_nsec_elapsed(&start);
> +
> + igt_fail_on(elapsed > 600 * USEC_PER_SEC);
You should time each individually, and repeat a few times to identify
outliers. (Warn if outlier > threshold, fail if mean > threshold).
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH i-g-t 0/3] Convert sh scripts to C variants.
2016-10-20 19:36 [PATCH i-g-t 0/3] Convert sh scripts to C variants Marius Vlad
` (2 preceding siblings ...)
2016-10-20 19:36 ` [PATCH i-g-t 3/3] tests/kms_sysfs_edid_timing: " Marius Vlad
@ 2016-10-20 21:00 ` Jani Nikula
2016-10-21 7:38 ` Joonas Lahtinen
3 siblings, 1 reply; 19+ messages in thread
From: Jani Nikula @ 2016-10-20 21:00 UTC (permalink / raw)
To: Marius Vlad, intel-gfx
On Thu, 20 Oct 2016, Marius Vlad <marius.c.vlad@intel.com> wrote:
> This series adds some library support to help converting sh
> scripts to C version. Converted drv_module_reload_basic and
> kms_sysfs_edid_timing.
> 18 files changed, 600 insertions(+), 180 deletions(-)
Someone please justify this, plus pulling in two new dependencies. I can
think of a thing or two, but it needs to be in the commit messages. And
I'm not convinced by the justification I came up with.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH i-g-t 0/3] Convert sh scripts to C variants.
2016-10-20 21:00 ` [PATCH i-g-t 0/3] Convert sh scripts to C variants Jani Nikula
@ 2016-10-21 7:38 ` Joonas Lahtinen
2016-10-24 8:46 ` Daniel Vetter
0 siblings, 1 reply; 19+ messages in thread
From: Joonas Lahtinen @ 2016-10-21 7:38 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On pe, 2016-10-21 at 00:00 +0300, Jani Nikula wrote:
> > On Thu, 20 Oct 2016, Marius Vlad <marius.c.vlad@intel.com> wrote:
> >
> > This series adds some library support to help converting sh
> > scripts to C version. Converted drv_module_reload_basic and
> > kms_sysfs_edid_timing.
>
> >
> > 18 files changed, 600 insertions(+), 180 deletions(-)
>
> Someone please justify this, plus pulling in two new dependencies. I can
> think of a thing or two, but it needs to be in the commit messages. And
> I'm not convinced by the justification I came up with.
Hmm, not sure how Daniel instructed things. Original idea was to just
execv the same commands as the scripts do. To get rid of the
interpreter differences and allow running in a minimal environment.
I'm myself fine with using the libraries too, I think the tests can
then be improved upon, just like Chris has commented on a few of them.
Regards, Joonas
--
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH i-g-t 0/3] Convert sh scripts to C variants.
2016-10-21 7:38 ` Joonas Lahtinen
@ 2016-10-24 8:46 ` Daniel Vetter
2016-10-24 8:49 ` Daniel Vetter
0 siblings, 1 reply; 19+ messages in thread
From: Daniel Vetter @ 2016-10-24 8:46 UTC (permalink / raw)
To: Joonas Lahtinen; +Cc: Daniel Vetter, intel-gfx
On Fri, Oct 21, 2016 at 10:38:17AM +0300, Joonas Lahtinen wrote:
> On pe, 2016-10-21 at 00:00 +0300, Jani Nikula wrote:
> > > On Thu, 20 Oct 2016, Marius Vlad <marius.c.vlad@intel.com> wrote:
> > >
> > > This series adds some library support to help converting sh
> > > scripts to C version. Converted drv_module_reload_basic and
> > > kms_sysfs_edid_timing.
> >
> > >
> > > 18 files changed, 600 insertions(+), 180 deletions(-)
> >
> > Someone please justify this, plus pulling in two new dependencies. I can
> > think of a thing or two, but it needs to be in the commit messages. And
> > I'm not convinced by the justification I came up with.
- Be able to reuse the subtest/logging/whatever else igt stuff for more
consistency, without having to reinvent the wheel in the bash world a
2nd time. Often this is also tricky (we have unit tests for igt
libraries for a reason). I rejected C++ as a new language for similar
reasons, I think getting rid of bash is useful.
- dmesg logging. Imo the piglit dmesg capturing serious sucks, it'd be
great to move it into igt. Reasons for that: a) in C it's much faster,
b) integrated with igt logging (consistent timestamps, ordering,
crashbox log, ...) c) we could put the filtering of dmesg next to the
tests, atm it's some rough filter in piglit's igt.py. c) has been one of
Chris' wishlist things, e.g. make underrun tests hunt for underruns
explicitly.
- It's a nice ramp-up task for igt, that's why I bumped it up a bit. Would
be fairly low-prio otherwise.
> Hmm, not sure how Daniel instructed things. Original idea was to just
> execv the same commands as the scripts do. To get rid of the
> interpreter differences and allow running in a minimal environment.
Yeah, I was thinking of a pretty minimal conversion to C with just lots of
calls to system. Maybe long-term we could extract some shared code, but
meh. Creating good libraries is a lot more work than it generally looks
like, and code reuse for code reuse's sake is in my experience often not
worth the hassle.
> I'm myself fine with using the libraries too, I think the tests can
> then be improved upon, just like Chris has commented on a few of them.
Yeah, not against it, but maybe as step 2.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH i-g-t 0/3] Convert sh scripts to C variants.
2016-10-24 8:46 ` Daniel Vetter
@ 2016-10-24 8:49 ` Daniel Vetter
0 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2016-10-24 8:49 UTC (permalink / raw)
To: Joonas Lahtinen; +Cc: Daniel Vetter, intel-gfx
On Mon, Oct 24, 2016 at 10:46:14AM +0200, Daniel Vetter wrote:
> On Fri, Oct 21, 2016 at 10:38:17AM +0300, Joonas Lahtinen wrote:
> > On pe, 2016-10-21 at 00:00 +0300, Jani Nikula wrote:
> > > > On Thu, 20 Oct 2016, Marius Vlad <marius.c.vlad@intel.com> wrote:
> > > >
> > > > This series adds some library support to help converting sh
> > > > scripts to C version. Converted drv_module_reload_basic and
> > > > kms_sysfs_edid_timing.
> > >
> > > >
> > > > 18 files changed, 600 insertions(+), 180 deletions(-)
> > >
> > > Someone please justify this, plus pulling in two new dependencies. I can
> > > think of a thing or two, but it needs to be in the commit messages. And
> > > I'm not convinced by the justification I came up with.
>
> - Be able to reuse the subtest/logging/whatever else igt stuff for more
> consistency, without having to reinvent the wheel in the bash world a
> 2nd time. Often this is also tricky (we have unit tests for igt
> libraries for a reason). I rejected C++ as a new language for similar
> reasons, I think getting rid of bash is useful.
>
> - dmesg logging. Imo the piglit dmesg capturing serious sucks, it'd be
> great to move it into igt. Reasons for that: a) in C it's much faster,
> b) integrated with igt logging (consistent timestamps, ordering,
> crashbox log, ...) c) we could put the filtering of dmesg next to the
> tests, atm it's some rough filter in piglit's igt.py. c) has been one of
> Chris' wishlist things, e.g. make underrun tests hunt for underruns
> explicitly.
>
> - It's a nice ramp-up task for igt, that's why I bumped it up a bit. Would
> be fairly low-prio otherwise.
>
> > Hmm, not sure how Daniel instructed things. Original idea was to just
> > execv the same commands as the scripts do. To get rid of the
> > interpreter differences and allow running in a minimal environment.
>
> Yeah, I was thinking of a pretty minimal conversion to C with just lots of
> calls to system. Maybe long-term we could extract some shared code, but
> meh. Creating good libraries is a lot more work than it generally looks
> like, and code reuse for code reuse's sake is in my experience often not
> worth the hassle.
On that note, we'd probably need an igt helper to run a shell command with
full igt integration. The important bits there would be to read
stderr/stdout and feed them into igt_warn and igt_debug levels. Of course
in parallel to running the command to make sure the timestamps are
correct, and will line up with the dmesg timestampes. And long-term also
interleave with dmesg output (once we've moded that into igt proper).
That function also needs some flags to optionally ignore stderr or stdout,
which essentially would be reimplenting > /dev/null and &> /dev/null.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread