Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assorted cleanup for ZED, but mostly just the thousand-fold speed improvement #11834

Closed
wants to merge 10 commits into from
Closed

Assorted cleanup for ZED, but mostly just the thousand-fold speed improvement #11834

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
45b629d
zed: print combined system/user time after ZEDLET death
nabijaczleweli Apr 2, 2021
da60ced
zed: implement close_from() in terms of /proc/self/fd, if available
nabijaczleweli Apr 2, 2021
5fcb36a
zed: purge all mentions of a configuration file
nabijaczleweli Apr 2, 2021
3f01898
zed.8: don't pretend an unprivileged user could change the script owner
nabijaczleweli Apr 2, 2021
ae4a69a
zed: bump zfs_zevent_len_max if we miss any events
nabijaczleweli Apr 2, 2021
a8c8e95
zed: remove unused zed_file_read_n()
nabijaczleweli Apr 2, 2021
eb276c4
zed: merge all _NOT_IMPLEMENTED_ events
nabijaczleweli Apr 2, 2021
947e638
zed: replace zed_file_write_n() with write(2), purge it
nabijaczleweli Apr 2, 2021
8a0f7dc
zed.8: the Diagnosis Engine is implemented
nabijaczleweli Apr 2, 2021
62d353c
zed: only go up to current limit in close_from() fallback
nabijaczleweli Apr 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions cmd/zed/zed.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,6 @@ main(int argc, char *argv[])
if (geteuid() != 0)
zed_log_die("Must be run as root");

zed_conf_parse_file(zcp);

zed_file_close_from(STDERR_FILENO + 1);

(void) umask(0);
Expand Down
5 changes: 0 additions & 5 deletions cmd/zed/zed.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,6 @@
#ifndef ZED_H
#define ZED_H

/*
* Absolute path for the default zed configuration file.
*/
#define ZED_CONF_FILE SYSCONFDIR "/zfs/zed.conf"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is functionality we'd always intended to add and thus has stubbed out. Though clearly we haven't gotten to it. In particular, in zfs_diagnosis.c there are two functions fmd_prop_get_int32 and fmd_prop_get_int64 which return hard coded values we thought could be set in the config file. That said, at this point since we're only talking about a couple of value it might make more sense to add a few command line options to override the defaults. So I'm OK with shedding this long dormant and partial config file support.


/*
* Absolute path for the default zed pid file.
*/
Expand Down
32 changes: 2 additions & 30 deletions cmd/zed/zed_conf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,6 @@ zed_conf_create(void)
zcp->zevent_fd = -1; /* opened via zed_event_init() */
zcp->max_jobs = 16;

if (!(zcp->conf_file = strdup(ZED_CONF_FILE)))
goto nomem;

if (!(zcp->pid_file = strdup(ZED_PID_FILE)))
goto nomem;

Expand Down Expand Up @@ -103,10 +100,6 @@ zed_conf_destroy(struct zed_conf *zcp)
zcp->pid_file, strerror(errno));
zcp->pid_fd = -1;
}
if (zcp->conf_file) {
free(zcp->conf_file);
zcp->conf_file = NULL;
}
if (zcp->pid_file) {
free(zcp->pid_file);
zcp->pid_file = NULL;
Expand Down Expand Up @@ -163,10 +156,6 @@ _zed_conf_display_help(const char *prog, int got_err)
fprintf(fp, "%*c%*s %s\n", w1, 0x20, -w2, "-Z",
"Zero state file.");
fprintf(fp, "\n");
#if 0
fprintf(fp, "%*c%*s %s [%s]\n", w1, 0x20, -w2, "-c FILE",
"Read configuration from FILE.", ZED_CONF_FILE);
#endif
fprintf(fp, "%*c%*s %s [%s]\n", w1, 0x20, -w2, "-d DIR",
"Read enabled ZEDLETs from DIR.", ZED_ZEDLET_DIR);
fprintf(fp, "%*c%*s %s [%s]\n", w1, 0x20, -w2, "-p FILE",
Expand Down Expand Up @@ -254,7 +243,7 @@ _zed_conf_parse_path(char **resultp, const char *path)
void
zed_conf_parse_opts(struct zed_conf *zcp, int argc, char **argv)
{
const char * const opts = ":hLVc:d:p:P:s:vfFMZIj:";
const char * const opts = ":hLVd:p:P:s:vfFMZIj:";
int opt;
unsigned long raw;

Expand All @@ -274,9 +263,6 @@ zed_conf_parse_opts(struct zed_conf *zcp, int argc, char **argv)
case 'V':
_zed_conf_display_version();
break;
case 'c':
_zed_conf_parse_path(&zcp->conf_file, optarg);
break;
case 'd':
_zed_conf_parse_path(&zcp->zedlet_dir, optarg);
break;
Expand Down Expand Up @@ -331,27 +317,13 @@ zed_conf_parse_opts(struct zed_conf *zcp, int argc, char **argv)
}
}

/*
* Parse the configuration file into the configuration [zcp].
*
* FIXME: Not yet implemented.
*/
void
zed_conf_parse_file(struct zed_conf *zcp)
{
if (!zcp)
zed_log_die("Failed to parse config: %s", strerror(EINVAL));
}

/*
* Scan the [zcp] zedlet_dir for files to exec based on the event class.
* Files must be executable by user, but not writable by group or other.
* Dotfiles are ignored.
*
* Return 0 on success with an updated set of zedlets,
* or -1 on error with errno set.
*
* FIXME: Check if zedlet_dir and all parent dirs are secure.
*/
int
zed_conf_scan_dir(struct zed_conf *zcp)
Expand Down Expand Up @@ -543,7 +515,7 @@ zed_conf_write_pid(struct zed_conf *zcp)
errno = ERANGE;
zed_log_msg(LOG_ERR, "Failed to write PID file \"%s\": %s",
zcp->pid_file, strerror(errno));
} else if (zed_file_write_n(zcp->pid_fd, buf, n) != n) {
} else if (write(zcp->pid_fd, buf, n) != n) {
zed_log_msg(LOG_ERR, "Failed to write PID file \"%s\": %s",
zcp->pid_file, strerror(errno));
} else if (fdatasync(zcp->pid_fd) < 0) {
Expand Down
3 changes: 0 additions & 3 deletions cmd/zed/zed_conf.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ struct zed_conf {
int syslog_facility; /* syslog facility value */
int min_events; /* RESERVED FOR FUTURE USE */
int max_events; /* RESERVED FOR FUTURE USE */
char *conf_file; /* abs path to config file */
char *pid_file; /* abs path to pid file */
int pid_fd; /* fd to pid file for lock */
char *zedlet_dir; /* abs path to zedlet dir */
Expand All @@ -48,8 +47,6 @@ void zed_conf_destroy(struct zed_conf *zcp);

void zed_conf_parse_opts(struct zed_conf *zcp, int argc, char **argv);

void zed_conf_parse_file(struct zed_conf *zcp);

int zed_conf_scan_dir(struct zed_conf *zcp);

int zed_conf_write_pid(struct zed_conf *zcp);
Expand Down
71 changes: 46 additions & 25 deletions cmd/zed/zed_event.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,45 @@ zed_event_fini(struct zed_conf *zcp)
zed_exec_fini();
}

static void
_bump_event_queue_length(void)
{
int zzlm = -1, wr;
char qlen_buf[12] = {0}; /* parameter is int => max "-2147483647\n" */
long int qlen;

zzlm = open("/sys/module/zfs/parameters/zfs_zevent_len_max", O_RDWR);
if (zzlm < 0)
goto done;

if (read(zzlm, qlen_buf, sizeof (qlen_buf)) < 0)
goto done;
qlen_buf[sizeof (qlen_buf) - 1] = '\0';

errno = 0;
qlen = strtol(qlen_buf, NULL, 10);
if (errno == ERANGE)
goto done;

if (qlen <= 0)
qlen = 512; /* default zfs_zevent_len_max value */
else
qlen *= 2;

if (qlen > INT_MAX)
qlen = INT_MAX;
wr = snprintf(qlen_buf, sizeof (qlen_buf), "%ld", qlen);

if (pwrite(zzlm, qlen_buf, wr, 0) < 0)
goto done;

zed_log_msg(LOG_WARNING, "Bumping queue length to %ld", qlen);

done:
if (zzlm > -1)
(void) close(zzlm);
}

/*
* Seek to the event specified by [saved_eid] and [saved_etime].
* This protects against processing a given event more than once.
Expand Down Expand Up @@ -138,10 +177,7 @@ zed_event_seek(struct zed_conf *zcp, uint64_t saved_eid, int64_t saved_etime[])

if (n_dropped > 0) {
zed_log_msg(LOG_WARNING, "Missed %d events", n_dropped);
/*
* FIXME: Increase max size of event nvlist in
* /sys/module/zfs/parameters/zfs_zevent_len_max ?
*/
_bump_event_queue_length();
}
if (nvlist_lookup_uint64(nvl, "eid", &eid) != 0) {
zed_log_msg(LOG_WARNING, "Failed to lookup zevent eid");
Expand Down Expand Up @@ -213,7 +249,7 @@ _zed_event_value_is_hex(const char *name)
*
* All environment variables in [zsp] should be added through this function.
*/
static int
static __attribute__((format(printf, 5, 6))) int
_zed_event_add_var(uint64_t eid, zed_strings_t *zsp,
const char *prefix, const char *name, const char *fmt, ...)
{
Expand Down Expand Up @@ -588,8 +624,6 @@ _zed_event_add_string_array(uint64_t eid, zed_strings_t *zsp,
* Convert the nvpair [nvp] to a string which is added to the environment
* of the child process.
* Return 0 on success, -1 on error.
*
* FIXME: Refactor with cmd/zpool/zpool_main.c:zpool_do_events_nvprint()?
*/
static void
_zed_event_add_nvpair(uint64_t eid, zed_strings_t *zsp, nvpair_t *nvp)
Expand Down Expand Up @@ -688,23 +722,11 @@ _zed_event_add_nvpair(uint64_t eid, zed_strings_t *zsp, nvpair_t *nvp)
_zed_event_add_var(eid, zsp, prefix, name,
"%llu", (u_longlong_t)i64);
break;
case DATA_TYPE_NVLIST:
_zed_event_add_var(eid, zsp, prefix, name,
"%s", "_NOT_IMPLEMENTED_"); /* FIXME */
break;
case DATA_TYPE_STRING:
(void) nvpair_value_string(nvp, &str);
_zed_event_add_var(eid, zsp, prefix, name,
"%s", (str ? str : "<NULL>"));
break;
case DATA_TYPE_BOOLEAN_ARRAY:
_zed_event_add_var(eid, zsp, prefix, name,
"%s", "_NOT_IMPLEMENTED_"); /* FIXME */
break;
case DATA_TYPE_BYTE_ARRAY:
_zed_event_add_var(eid, zsp, prefix, name,
"%s", "_NOT_IMPLEMENTED_"); /* FIXME */
break;
case DATA_TYPE_INT8_ARRAY:
_zed_event_add_int8_array(eid, zsp, prefix, nvp);
break;
Expand Down Expand Up @@ -732,9 +754,11 @@ _zed_event_add_nvpair(uint64_t eid, zed_strings_t *zsp, nvpair_t *nvp)
case DATA_TYPE_STRING_ARRAY:
_zed_event_add_string_array(eid, zsp, prefix, nvp);
break;
case DATA_TYPE_NVLIST:
case DATA_TYPE_BOOLEAN_ARRAY:
case DATA_TYPE_BYTE_ARRAY:
case DATA_TYPE_NVLIST_ARRAY:
_zed_event_add_var(eid, zsp, prefix, name,
"%s", "_NOT_IMPLEMENTED_"); /* FIXME */
_zed_event_add_var(eid, zsp, prefix, name, "_NOT_IMPLEMENTED_");
break;
default:
errno = EINVAL;
Expand Down Expand Up @@ -914,10 +938,7 @@ zed_event_service(struct zed_conf *zcp)

if (n_dropped > 0) {
zed_log_msg(LOG_WARNING, "Missed %d events", n_dropped);
/*
* FIXME: Increase max size of event nvlist in
* /sys/module/zfs/parameters/zfs_zevent_len_max ?
*/
_bump_event_queue_length();
}
if (nvlist_lookup_uint64(nvl, "eid", &eid) != 0) {
zed_log_msg(LOG_WARNING, "Failed to lookup zevent eid");
Expand Down
31 changes: 25 additions & 6 deletions cmd/zed/zed_exec.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <string.h>
#include <stddef.h>
#include <sys/avl.h>
#include <sys/resource.h>
#include <sys/stat.h>
#include <sys/wait.h>
#include <time.h>
Expand Down Expand Up @@ -175,6 +176,7 @@ _zed_exec_fork_child(uint64_t eid, const char *dir, const char *prog,
node->pid = pid;
node->eid = eid;
node->name = strdup(prog);

(void) pthread_mutex_lock(&_launched_processes_lock);
avl_add(&_launched_processes, node);
(void) pthread_mutex_unlock(&_launched_processes_lock);
Expand All @@ -191,6 +193,7 @@ _reap_children(void *arg)
struct launched_process_node node, *pnode;
pid_t pid;
int status;
struct rusage usage;
struct sigaction sa = {};

(void) sigfillset(&sa.sa_mask);
Expand All @@ -203,7 +206,7 @@ _reap_children(void *arg)
(void) sigaction(SIGCHLD, &sa, NULL);

for (_reap_children_stop = B_FALSE; !_reap_children_stop; ) {
pid = waitpid(0, &status, 0);
pid = wait4(0, &status, 0, &usage);

if (pid == (pid_t)-1) {
if (errno == ECHILD)
Expand All @@ -227,21 +230,37 @@ _reap_children(void *arg)
__atomic_add_fetch(&_launched_processes_limit, 1,
__ATOMIC_SEQ_CST);

usage.ru_utime.tv_sec += usage.ru_stime.tv_sec;
usage.ru_utime.tv_usec += usage.ru_stime.tv_usec;
usage.ru_utime.tv_sec +=
usage.ru_utime.tv_usec / (1000 * 1000);
usage.ru_utime.tv_usec %= 1000 * 1000;

if (WIFEXITED(status)) {
zed_log_msg(LOG_INFO,
"Finished \"%s\" eid=%llu pid=%d exit=%d",
"Finished \"%s\" eid=%llu pid=%d "
"time=%llu.%06us exit=%d",
node.name, node.eid, pid,
(unsigned long long) usage.ru_utime.tv_sec,
(unsigned int) usage.ru_utime.tv_usec,
WEXITSTATUS(status));
} else if (WIFSIGNALED(status)) {
zed_log_msg(LOG_INFO,
"Finished \"%s\" eid=%llu pid=%d sig=%d/%s",
node.name, node.eid, pid, WTERMSIG(status),
"Finished \"%s\" eid=%llu pid=%d "
"time=%llu.%06us sig=%d/%s",
node.name, node.eid, pid,
(unsigned long long) usage.ru_utime.tv_sec,
(unsigned int) usage.ru_utime.tv_usec,
WTERMSIG(status),
strsignal(WTERMSIG(status)));
} else {
zed_log_msg(LOG_INFO,
"Finished \"%s\" eid=%llu pid=%d "
"status=0x%X",
node.name, node.eid, (unsigned int) status);
"time=%llu.%06us status=0x%X",
node.name, node.eid,
(unsigned long long) usage.ru_utime.tv_sec,
(unsigned int) usage.ru_utime.tv_usec,
(unsigned int) status);
}

free(node.name);
Expand Down
Loading