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.

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

Fixed memory leaks in zevent handling #2291

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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: 1 addition & 1 deletion include/sys/fm/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ typedef struct zfs_zevent {
extern void fm_init(void);
extern void fm_fini(void);
extern void fm_nvprint(nvlist_t *);
extern void zfs_zevent_post(nvlist_t *, nvlist_t *, zevent_cb_t *);
extern int zfs_zevent_post(nvlist_t *, nvlist_t *, zevent_cb_t *);
extern void zfs_zevent_drain_all(int *);
extern int zfs_zevent_fd_hold(int, minor_t *, zfs_zevent_t **);
extern void zfs_zevent_fd_rele(int);
Expand Down
41 changes: 32 additions & 9 deletions module/zfs/fm.c
Original file line number Diff line number Diff line change
Expand Up @@ -499,35 +499,51 @@ zfs_zevent_insert(zevent_t *ev)
}

/*
* Post a zevent
* Post a zevent. The cb will be called when nvl and detector are no longer
* needed, i.e.:
* - An error happened and a zevent can't be posted. In this case, cb is called
* before zfs_zevent_post() returns.
* - The event is being drained and freed.
*/
void
int
zfs_zevent_post(nvlist_t *nvl, nvlist_t *detector, zevent_cb_t *cb)
{
int64_t tv_array[2];
timestruc_t tv;
uint64_t eid;
size_t nvl_size = 0;
zevent_t *ev;
int error;

ASSERT(cb != NULL);

gethrestime(&tv);
tv_array[0] = tv.tv_sec;
tv_array[1] = tv.tv_nsec;
if (nvlist_add_int64_array(nvl, FM_EREPORT_TIME, tv_array, 2)) {

error = nvlist_add_int64_array(nvl, FM_EREPORT_TIME, tv_array, 2);
if (error) {
atomic_add_64(&erpt_kstat_data.erpt_set_failed.value.ui64, 1);
return;
goto out;
}

eid = atomic_inc_64_nv(&zevent_eid);
if (nvlist_add_uint64(nvl, FM_EREPORT_EID, eid)) {
error = nvlist_add_uint64(nvl, FM_EREPORT_EID, eid);
if (error) {
atomic_add_64(&erpt_kstat_data.erpt_set_failed.value.ui64, 1);
return;
goto out;
}

error = nvlist_size(nvl, &nvl_size, NV_ENCODE_NATIVE);
if (error) {
atomic_add_64(&erpt_kstat_data.erpt_dropped.value.ui64, 1);
goto out;
}

(void) nvlist_size(nvl, &nvl_size, NV_ENCODE_NATIVE);
if (nvl_size > ERPT_DATA_SZ || nvl_size == 0) {
atomic_add_64(&erpt_kstat_data.erpt_dropped.value.ui64, 1);
return;
error = EOVERFLOW;
goto out;
}

if (zfs_zevent_console)
Expand All @@ -536,7 +552,8 @@ zfs_zevent_post(nvlist_t *nvl, nvlist_t *detector, zevent_cb_t *cb)
ev = zfs_zevent_alloc();
if (ev == NULL) {
atomic_add_64(&erpt_kstat_data.erpt_dropped.value.ui64, 1);
return;
error = ENOMEM;
goto out;
}

ev->ev_nvl = nvl;
Expand All @@ -548,6 +565,12 @@ zfs_zevent_post(nvlist_t *nvl, nvlist_t *detector, zevent_cb_t *cb)
zfs_zevent_insert(ev);
cv_broadcast(&zevent_cv);
mutex_exit(&zevent_lock);

out:
if (error)
cb(nvl, detector);

return (error);
}

static int
Expand Down
21 changes: 12 additions & 9 deletions module/zfs/zfs_fm.c
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ zfs_zevent_post_cb(nvlist_t *nvl, nvlist_t *detector)
fm_nvlist_destroy(detector, FM_NVA_FREE);
}

static void
zfs_zevent_post_cb_noop(nvlist_t *nvl, nvlist_t *detector)
{
}

static void
zfs_ereport_start(nvlist_t **ereport_out, nvlist_t **detector_out,
const char *subclass, spa_t *spa, vdev_t *vd, zio_t *zio,
Expand Down Expand Up @@ -768,12 +773,7 @@ zfs_ereport_start_checksum(spa_t *spa, vdev_t *vd,
FM_EREPORT_ZFS_CHECKSUM, spa, vd, zio, offset, length);

if (report->zcr_ereport == NULL) {
report->zcr_free(report->zcr_cbdata, report->zcr_cbinfo);
if (report->zcr_ckinfo != NULL) {
kmem_free(report->zcr_ckinfo,
sizeof (*report->zcr_ckinfo));
}
kmem_free(report, sizeof (*report));
zfs_ereport_free_checksum(report);
return;
}
#endif
Expand All @@ -789,13 +789,15 @@ zfs_ereport_finish_checksum(zio_cksum_report_t *report,
const void *good_data, const void *bad_data, boolean_t drop_if_identical)
{
#ifdef _KERNEL
zfs_ecksum_info_t *info = NULL;
zfs_ecksum_info_t *info;

info = annotate_ecksum(report->zcr_ereport, report->zcr_ckinfo,
good_data, bad_data, report->zcr_length, drop_if_identical);

if (info != NULL)
zfs_zevent_post(report->zcr_ereport,
report->zcr_detector, zfs_zevent_post_cb);
else
zfs_zevent_post_cb(report->zcr_ereport, report->zcr_detector);

report->zcr_ereport = report->zcr_detector = NULL;
if (info != NULL)
Expand Down Expand Up @@ -826,7 +828,8 @@ void
zfs_ereport_send_interim_checksum(zio_cksum_report_t *report)
{
#ifdef _KERNEL
zfs_zevent_post(report->zcr_ereport, report->zcr_detector, NULL);
zfs_zevent_post(report->zcr_ereport, report->zcr_detector,
zfs_zevent_post_cb_noop);
#endif
}

Expand Down