Paths

Table of Contentst

-
head/
-
lib/libutil/
-
libutil/
1/2
gr_util.c
1/2
pw_util.c
-
usr.sbin/pw/
-
pw/
1/2
grupd.c
1/2
pw_nis.c
-
pwupd.c

passwd
ClosedPublic
Actions

Authored by asomers on Oct 21 2016, 8:36 PM.

Details

Reviewers

dwmalone
allanjude
bapt
garga
vangyzen

Commits

rS308806: Speed up pw operations that edit /etc/group or /etc/passwd

Summary

r285050 fixed a bug in pw that could lead to /etc/passwd or /etc/group
corruption on power loss. However, it fixed it by opening those files with
O_SYNC, which is very slow, especially on ZFS. This change replaces O_SYNC
with appropriately place fsync()s instead, which is much faster. Using a
ZFS tmpdir, the time to run pw's kyua tests drops from 245s to 35s.

Test Plan

pw's kyua tests

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

asomers updated this revision to Diff 21605.Oct 21 2016, 8:36 PM

asomers retitled this revision from to Speed up pw operations that edit /etc/group or /etc/passwd.

asomers updated this object.

asomers edited the test plan for this revision. (Show Details)

asomers added reviewers: garga, dwmalone.

asomers added subscribers: truckman, bapt, vangyzen, se.

Herald added a subscriber: imp. · View Herald TranscriptOct 21 2016, 8:36 PM

allanjude accepted this revision.Oct 21 2016, 8:39 PM

allanjude added a reviewer: allanjude.

This revision is now accepted and ready to land.Oct 21 2016, 8:39 PM

Is anybody able to test the pfsense issue with my patch?
https://redmine.pfsense.org/issues/4523

bapt accepted this revision.Oct 22 2016, 1:17 AM

bapt added a reviewer: bapt.

vangyzen accepted this revision.Oct 28 2016, 2:44 PM

vangyzen added a reviewer: vangyzen.

I tried to break it on pfSense and couldn't reproduce original issue. Looks OK to me

Closed by commit rS308806: Speed up pw operations that edit /etc/group or /etc/passwd (authored by asomers). · Explain WhyNov 18 2016, 4:07 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

head/

lib/

libutil/

gr_util.c

2 lines

pw_util.c

2 lines

usr.sbin/

pw/

grupd.c

1 line

pw_nis.c

1 line

pwupd.c

1 line

Diff 22331

View Options

head/lib/libutil/gr_util.c

Context not available.
	regs[0] = leaf + 1;	regs[0] = leaf + 1;

	if (regs[0] >= leaf) {	if (regs[0] >= leaf) {
		enum VM_GUEST prev_vm_guest = vm_guest;

	for (i = 0; i < nitems(vm_cpuids); i++)	for (i = 0; i < nitems(vm_cpuids); i++)
	if (strncmp((const char *)&regs[1],	if (strncmp((const char *)&regs[1],
	vm_cpuids[i].vm_cpuid, 12) == 0) {	vm_cpuids[i].vm_cpuid, 12) == 0) {
Context not available.
	* specific hypervisor, record the base, high value,	* specific hypervisor, record the base, high value,
	* and vendor identifier.	* and vendor identifier.
	*/	*/
	if (vm_guest != VM_GUEST_VM \|\| leaf == 0x40000000) {	if (vm_guest != prev_vm_guest \|\| leaf == 0x40000000) {
	hv_base = leaf;	hv_base = leaf;
	hv_high = regs[0];	hv_high = regs[0];
	((u_int *)&hv_vendor)[0] = regs[1];	((u_int *)&hv_vendor)[0] = regs[1];
Context not available.
	* If we found a specific hypervisor, then	* If we found a specific hypervisor, then
	* we are finished.	* we are finished.
	*/	*/
	if (vm_guest != VM_GUEST_VM)	if (vm_guest != VM_GUEST_VM &&
		/*
		* Xen and other hypervisors can expose the
		* HyperV signature in addition to the
		* native one in order to support Viridian
		* extensions for Windows guests.
		*
		* Do the full cpuid scan if HyperV is
		* detected, as the native hypervisor is
		* preferred.
		markjUnsubmitted Not Done Inline Actions Do you need to modify the enclosing if statement as well? I think it assumes that `vm_guest` will only be assigned once. markj: Do you need to modify the enclosing if statement as well? I think it assumes that `vm_guest`…
		roygerAuthorUnsubmitted Done Inline Actions bah, indeed, or else it will update the cached leaf even if no hypervisor was found. Will update the diff. royger: bah, indeed, or else it will update the cached leaf even if no hypervisor was found. Will…
		*/
		vm_guest != VM_GUEST_HV)
	return;	return;
	}	}
	}	}
Context not available.

View Options

head/lib/libutil/pw_util.c

Context not available.
	regs[0] = leaf + 1;	regs[0] = leaf + 1;

	if (regs[0] >= leaf) {	if (regs[0] >= leaf) {
		enum VM_GUEST prev_vm_guest = vm_guest;

	for (i = 0; i < nitems(vm_cpuids); i++)	for (i = 0; i < nitems(vm_cpuids); i++)
	if (strncmp((const char *)&regs[1],	if (strncmp((const char *)&regs[1],
	vm_cpuids[i].vm_cpuid, 12) == 0) {	vm_cpuids[i].vm_cpuid, 12) == 0) {
Context not available.
	* specific hypervisor, record the base, high value,	* specific hypervisor, record the base, high value,
	* and vendor identifier.	* and vendor identifier.
	*/	*/
	if (vm_guest != VM_GUEST_VM \|\| leaf == 0x40000000) {	if (vm_guest != prev_vm_guest \|\| leaf == 0x40000000) {
	hv_base = leaf;	hv_base = leaf;
	hv_high = regs[0];	hv_high = regs[0];
	((u_int *)&hv_vendor)[0] = regs[1];	((u_int *)&hv_vendor)[0] = regs[1];
Context not available.
	* If we found a specific hypervisor, then	* If we found a specific hypervisor, then
	* we are finished.	* we are finished.
	*/	*/
	if (vm_guest != VM_GUEST_VM)	if (vm_guest != VM_GUEST_VM &&
		/*
		* Xen and other hypervisors can expose the
		* HyperV signature in addition to the
		* native one in order to support Viridian
		* extensions for Windows guests.
		*
		* Do the full cpuid scan if HyperV is
		* detected, as the native hypervisor is
		* preferred.
		markjUnsubmitted Not Done Inline Actions Do you need to modify the enclosing if statement as well? I think it assumes that `vm_guest` will only be assigned once. markj: Do you need to modify the enclosing if statement as well? I think it assumes that `vm_guest`…
		roygerAuthorUnsubmitted Done Inline Actions bah, indeed, or else it will update the cached leaf even if no hypervisor was found. Will update the diff. royger: bah, indeed, or else it will update the cached leaf even if no hypervisor was found. Will…
		*/
		vm_guest != VM_GUEST_HV)
	return;	return;
	}	}
	}	}
Context not available.

View Options

head/usr.sbin/pw/grupd.c

Context not available.
	regs[0] = leaf + 1;	regs[0] = leaf + 1;

	if (regs[0] >= leaf) {	if (regs[0] >= leaf) {
		enum VM_GUEST prev_vm_guest = vm_guest;

	for (i = 0; i < nitems(vm_cpuids); i++)	for (i = 0; i < nitems(vm_cpuids); i++)
	if (strncmp((const char *)&regs[1],	if (strncmp((const char *)&regs[1],
	vm_cpuids[i].vm_cpuid, 12) == 0) {	vm_cpuids[i].vm_cpuid, 12) == 0) {
Context not available.
	* specific hypervisor, record the base, high value,	* specific hypervisor, record the base, high value,
	* and vendor identifier.	* and vendor identifier.
	*/	*/
	if (vm_guest != VM_GUEST_VM \|\| leaf == 0x40000000) {	if (vm_guest != prev_vm_guest \|\| leaf == 0x40000000) {
	hv_base = leaf;	hv_base = leaf;
	hv_high = regs[0];	hv_high = regs[0];
	((u_int *)&hv_vendor)[0] = regs[1];	((u_int *)&hv_vendor)[0] = regs[1];
Context not available.
	* If we found a specific hypervisor, then	* If we found a specific hypervisor, then
	* we are finished.	* we are finished.
	*/	*/
	if (vm_guest != VM_GUEST_VM)	if (vm_guest != VM_GUEST_VM &&
		/*
		* Xen and other hypervisors can expose the
		* HyperV signature in addition to the
		* native one in order to support Viridian
		* extensions for Windows guests.
		*
		* Do the full cpuid scan if HyperV is
		* detected, as the native hypervisor is
		* preferred.
		markjUnsubmitted Not Done Inline Actions Do you need to modify the enclosing if statement as well? I think it assumes that `vm_guest` will only be assigned once. markj: Do you need to modify the enclosing if statement as well? I think it assumes that `vm_guest`…
		roygerAuthorUnsubmitted Done Inline Actions bah, indeed, or else it will update the cached leaf even if no hypervisor was found. Will update the diff. royger: bah, indeed, or else it will update the cached leaf even if no hypervisor was found. Will…
		*/
		vm_guest != VM_GUEST_HV)
	return;	return;
	}	}
	}	}
Context not available.

View Options

head/usr.sbin/pw/pw_nis.c

Context not available.
	regs[0] = leaf + 1;	regs[0] = leaf + 1;

	if (regs[0] >= leaf) {	if (regs[0] >= leaf) {
		enum VM_GUEST prev_vm_guest = vm_guest;

	for (i = 0; i < nitems(vm_cpuids); i++)	for (i = 0; i < nitems(vm_cpuids); i++)
	if (strncmp((const char *)&regs[1],	if (strncmp((const char *)&regs[1],
	vm_cpuids[i].vm_cpuid, 12) == 0) {	vm_cpuids[i].vm_cpuid, 12) == 0) {
Context not available.
	* specific hypervisor, record the base, high value,	* specific hypervisor, record the base, high value,
	* and vendor identifier.	* and vendor identifier.
	*/	*/
	if (vm_guest != VM_GUEST_VM \|\| leaf == 0x40000000) {	if (vm_guest != prev_vm_guest \|\| leaf == 0x40000000) {
	hv_base = leaf;	hv_base = leaf;
	hv_high = regs[0];	hv_high = regs[0];
	((u_int *)&hv_vendor)[0] = regs[1];	((u_int *)&hv_vendor)[0] = regs[1];
Context not available.
	* If we found a specific hypervisor, then	* If we found a specific hypervisor, then
	* we are finished.	* we are finished.
	*/	*/
	if (vm_guest != VM_GUEST_VM)	if (vm_guest != VM_GUEST_VM &&
		/*
		* Xen and other hypervisors can expose the
		* HyperV signature in addition to the
		* native one in order to support Viridian
		* extensions for Windows guests.
		*
		* Do the full cpuid scan if HyperV is
		* detected, as the native hypervisor is
		* preferred.
		markjUnsubmitted Not Done Inline Actions Do you need to modify the enclosing if statement as well? I think it assumes that `vm_guest` will only be assigned once. markj: Do you need to modify the enclosing if statement as well? I think it assumes that `vm_guest`…
		roygerAuthorUnsubmitted Done Inline Actions bah, indeed, or else it will update the cached leaf even if no hypervisor was found. Will update the diff. royger: bah, indeed, or else it will update the cached leaf even if no hypervisor was found. Will…
		*/
		vm_guest != VM_GUEST_HV)
	return;	return;
	}	}
	}	}
Context not available.

View Options

Speed up pw operations that edit /etc/group or /etc/passwdClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 22331

head/lib/libutil/gr_util.c

head/lib/libutil/pw_util.c

head/usr.sbin/pw/grupd.c

head/usr.sbin/pw/pw_nis.c

head/usr.sbin/pw/pwupd.c

Speed up pw operations that edit /etc/group or /etc/passwd
ClosedPublic
Actions

Revision Contents
Changeset List