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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx