Micro-optimize arm64's uma_small_alloc()
ClosedPublic
Actions

Authored by alc on Oct 19 2020, 11:38 PM.

Details

Reviewers

andrew
markj
manu

Commits

rS366944: Micro-optimize uma_small_alloc(). Replace bzero(..., PAGE_SIZE) by

Summary

Replace bzero(..., PAGE_SIZE) by pagezero(). Ultimately, they use the same method for bulk zeroing, but the generality of bzero() requires size and alignment checks that pagezero() does not.

Eliminate an unnecessary #include.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

alc created this revision.Oct 19 2020, 11:38 PM

Herald added a reviewer: manu. · View Herald TranscriptOct 19 2020, 11:38 PM

Herald added a subscriber: emaste. · View Herald Transcript

alc requested review of this revision.Oct 19 2020, 11:38 PM

LGTM

Most other implementations also use bzero(), though on them pagezero() is nothing more than bzero(va, PAGE_SIZE) anyway.

This revision is now accepted and ready to land.Oct 20 2020, 2:36 AM

In D26876#599253, @markj wrote:

Most other implementations also use bzero(), though on them pagezero() is nothing more than bzero(va, PAGE_SIZE) anyway.

amd64 also uses pagezero().

Closed by commit rS366944: Micro-optimize uma_small_alloc(). Replace bzero(..., PAGE_SIZE) by (authored by alc). · Explain WhyOct 22 2020, 5:48 PM

This revision was automatically updated to reflect the committed changes.

alc added a commit: rS366944: Micro-optimize uma_small_alloc(). Replace bzero(..., PAGE_SIZE) by.

Herald added a subscriber: imp. · View Herald TranscriptOct 22 2020, 5:48 PM

Revision Contents
Changeset List

Path

Size

head/

sys/

arm64/

uma_machdep.c

5 lines

Diff 78607

View Options

head/sys/arm64/arm64/uma_machdep.c

	Show First 20 Lines • Show All 450 Lines • ▼ Show 20 Lines
	* while the latter sets things up from a root context. Such as might			* while the latter sets things up from a root context. Such as might
	* be called from login(1).			* be called from login(1).
	*			*
	*/			*/

	int			int
	setusercontext(login_cap_t lc, const struct passwd pwd, uid_t uid, unsigned int flags)			setusercontext(login_cap_t lc, const struct passwd pwd, uid_t uid, unsigned int flags)
	{			{
	rlim_t p;
	login_cap_t *llc = NULL;			login_cap_t *llc = NULL;
	struct rtprio rtp;
	int error;			int error;

	if (lc == NULL) {			if (lc == NULL) {
	if (pwd != NULL && (lc = login_getpwclass(pwd)) != NULL)			if (pwd != NULL && (lc = login_getpwclass(pwd)) != NULL)
	llc = lc; /* free this when we're done */			llc = lc; /* free this when we're done */
	}			}

	if (flags & LOGIN_SETPATH)			if (flags & LOGIN_SETPATH)
	pathvars[0].def = uid ? _PATH_DEFPATH : _PATH_STDPATH;			pathvars[0].def = uid ? _PATH_DEFPATH : _PATH_STDPATH;

	/* we need a passwd entry to set these */			/* we need a passwd entry to set these */
	if (pwd == NULL)			if (pwd == NULL)
	flags &= ~(LOGIN_SETGROUP \| LOGIN_SETLOGIN \| LOGIN_SETMAC);			flags &= ~(LOGIN_SETGROUP \| LOGIN_SETLOGIN \| LOGIN_SETMAC);

	/* Set the process priority */			/* Set the process priority */
	if (flags & LOGIN_SETPRIORITY) {			if (flags & LOGIN_SETPRIORITY) {
	p = login_getcapnum(lc, "priority", LOGIN_DEFPRI, LOGIN_DEFPRI);			/*
				* Make it unlikely that someone would input our default sentinel
				* (which indicates no specification).
				*/
				rlim_t const def_val = INT64_MIN + 1, err_val = INT64_MIN;
				rlim_t p = login_getcapnum(lc, "priority", def_val, err_val);
				int rc;

				if (p != def_val) {
				/* Invariant: 'lc' != NULL from here. */
				if (p == err_val) {
				syslog(LOG_WARNING,
				"%s%s%sLogin class '%s': "
				kibUnsubmitted Done Inline Actions Indents are wrong. kib: Indents are wrong.
				"Invalid priority specification: '%s'",
				pwd ? "Login '" : "",
				pwd ? pwd->pw_name : "",
				pwd ? "': " : "",
				lc->lc_class,
				login_getcapstr(lc, "priority", "", ""));
				} else {
	if (p > PRIO_MAX) {			if (p > PRIO_MAX) {
				struct rtprio rtp;
	rtp.type = RTP_PRIO_IDLE;			rtp.type = RTP_PRIO_IDLE;
	p += RTP_PRIO_MIN - (PRIO_MAX + 1);			p += RTP_PRIO_MIN - (PRIO_MAX + 1);
	rtp.prio = p > RTP_PRIO_MAX ? RTP_PRIO_MAX : p;			rtp.prio = p > RTP_PRIO_MAX ? RTP_PRIO_MAX : p;
	if (rtprio(RTP_SET, 0, &rtp))			rc = rtprio(RTP_SET, 0, &rtp);
	syslog(LOG_WARNING, "rtprio '%s' (%s): %m",
	pwd ? pwd->pw_name : "-",
	lc ? lc->lc_class : LOGIN_DEFCLASS);
	} else if (p < PRIO_MIN) {			} else if (p < PRIO_MIN) {
				struct rtprio rtp;
	rtp.type = RTP_PRIO_REALTIME;			rtp.type = RTP_PRIO_REALTIME;
	p += RTP_PRIO_MAX - (PRIO_MIN - 1);			p += RTP_PRIO_MAX - (PRIO_MIN - 1);
	rtp.prio = p < RTP_PRIO_MIN ? RTP_PRIO_MIN : p;			rtp.prio = p < RTP_PRIO_MIN ? RTP_PRIO_MIN : p;
	if (rtprio(RTP_SET, 0, &rtp))			rc = rtprio(RTP_SET, 0, &rtp);
	syslog(LOG_WARNING, "rtprio '%s' (%s): %m",			} else
	pwd ? pwd->pw_name : "-",			rc = setpriority(PRIO_PROCESS, 0, (int)p);
	lc ? lc->lc_class : LOGIN_DEFCLASS);
	} else {			if (rc != 0)
	if (setpriority(PRIO_PROCESS, 0, (int)p) != 0)			syslog(LOG_WARNING,
	syslog(LOG_WARNING, "setpriority '%s' (%s): %m",			"%s%s%sLogin class '%s': "
	pwd ? pwd->pw_name : "-",			"Setting priority failed: %m",
	lc ? lc->lc_class : LOGIN_DEFCLASS);			pwd ? "Login '" : "",
				pwd ? pwd->pw_name : "",
				pwd ? "': " : "",
				lc->lc_class);
				}
	}			}
	}			}
				emasteUnsubmitted Not Done Inline Actions as with comment in some previous review, I prefer braces around all if-else blocks if any have them emaste: as with comment in some previous review, I prefer braces around all if-else blocks if any have…
				impUnsubmitted Not Done Inline Actions style(9) tries to make this point, but is a hopeless. on clearity in its striving for brevity: Space after keywords (if, while, for, return, switch). Two styles of braces (‘{’ and ‘}’) are allowed for single line statements. Either they are used for all single statements, or they are used only where needed for clarity. Usage within a function should be consistent. Forever which can be read many ways. Certainly within a single if statement, I prefer what you're advocating too and that seemed to be the consensus of the discussions when I added the above text, but it's not 100% clear. imp: style(9) tries to make this point, but is a hopeless. on clearity in its striving for brevity…

	/* Setup the user's group permissions */			/* Setup the user's group permissions */
	if (flags & LOGIN_SETGROUP) {			if (flags & LOGIN_SETGROUP) {
	if (setgid(pwd->pw_gid) != 0) {			if (setgid(pwd->pw_gid) != 0) {
	syslog(LOG_ERR, "setgid(%lu): %m", (u_long)pwd->pw_gid);			syslog(LOG_ERR, "setgid(%lu): %m", (u_long)pwd->pw_gid);
	login_close(llc);			login_close(llc);
	return (-1);			return (-1);
	}			}
	▲ Show 20 Lines • Show All 71 Lines • Show Last 20 Lines

Micro-optimize arm64's uma_small_alloc()ClosedPublicActions