Page MenuHomeFreeBSD
Differential D23712

Correct use of UMA_ZONE_VM
ClosedPublic
Actions

Authored by jeff on Feb 16 2020, 9:25 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Mar 10, 2:00 AM
Unknown Object (File)
Sat, Mar 8, 8:12 AM
Unknown Object (File)
Jan 11 2025, 2:05 PM
Unknown Object (File)
Jan 9 2025, 5:55 AM
Unknown Object (File)
Jan 7 2025, 2:30 PM
Unknown Object (File)
Jan 7 2025, 11:39 AM
Unknown Object (File)
Jan 4 2025, 2:00 AM
Unknown Object (File)
Nov 27 2024, 3:51 PM
View All 57 Files
Subscribers
imp

Details

Reviewers
kib
markj
rlibby
Commits
rS358097: Eliminate some unnecessary uses of UMA_ZONE_VM. Only zones involved in
Summary

UMA_ZONE_VM is only for consumers that are involved in the KVA or page allocation path. Setting this restricts layout and prevents the allocation of pages for buckets. It should be used sparingly. It really doesn't have anything to do with low memory handling. If we're low on pages we already will avoid allocating buckets for any zone.

I would actually like it if all zones were able to allocate buckets from pages. It is almost safe on radix for direct map architectures. I may need a per-thread recursion flag to resolve that.

Test Plan

stress2 on amd64. I have asked pho to run on 32bit.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jeff created this revision.Feb 16 2020, 9:25 AM
Harbormaster completed remote builds in B29419: Diff 68397.Feb 16 2020, 9:26 AM
jeff edited the summary of this revision. (Show Details)Feb 16 2020, 9:29 AM
jeff edited the test plan for this revision. (Show Details)
jeff added reviewers: kib, markj, rlibby.
jeff set the repository for this revision to rS FreeBSD src repository - subversion.
Herald added a subscriber: imp. · View Herald TranscriptFeb 16 2020, 9:29 AM
jeff added inline comments.Feb 16 2020, 9:36 AM
sys/kern/subr_vmem.c
565 ↗(On Diff #68397)

We don't use this for kernel_vmem

674 ↗(On Diff #68397)

I don't know why this was nofree. We have an allocator that works.

sys/kern/vfs_subr.c
674 ↗(On Diff #68397)

This is nofree for ugly reasons but it does not need to be ZONE_VM.

sys/vm/vm_page.c
1981 ↗(On Diff #68397)

I like this being more explicitly associated with the zalloc. This allows me to loosen the definition of ZONE_VM without making the page cache recurse into itself.

markj accepted this revision.Feb 17 2020, 2:22 PM
markj added inline comments.
sys/kern/vfs_subr.c
674 ↗(On Diff #68397)

Could you add a comment explaining the nofree? I presume it's to ensure that the preallocation doesn't get reclaimed, but I'm not sure.

This revision is now accepted and ready to land.Feb 17 2020, 2:22 PM
jeff added a comment.Feb 17 2020, 2:28 PM

I have tested this off and on for several days without issue.

sys/kern/vfs_subr.c
674 ↗(On Diff #68397)

There is a comment above uma_zcreate() that describes it already.

  • Preallocate enough nodes to support one-per buf so that
  • we can not fail an insert. reassignbuf() callers can not
  • tolerate the insertion failure.

We bdirty()/bundirty() in bufdone() which is an ithread. You would have to do something like pmap does to reclaim pv entries if you really wanted this to work. Since the preallocation happens at boot it's not really contributing to fragmentation. It is unfortunate but we also limit the buf zone to a fraction of memory so it's not as bad as the radix for pages.

markj added inline comments.Feb 17 2020, 2:42 PM
sys/kern/vfs_subr.c
674 ↗(On Diff #68397)

I just mean that that comment doesn't directly explain why it's nofree. It looks like it's referring to the uma_prealloc() call. Someone might presume that we also rely on type-stability of the trie nodes somewhere.

Closed by commit rS358097: Eliminate some unnecessary uses of UMA_ZONE_VM. Only zones involved in (authored by jeff). · Explain WhyFeb 19 2020, 8:17 AM
This revision was automatically updated to reflect the committed changes.
jeff added a commit: rS358097: Eliminate some unnecessary uses of UMA_ZONE_VM. Only zones involved in.

Revision Contents
Changeset List

PathSize
head/
sys/
kern/
7 lines
2 lines
vm/
4 lines
4 lines
2 lines

Diff 68521

View Options

head/sys/kern/subr_vmem.c

  • This file was added.
--- internal/ssh/ssh.go.orig 1979-11-29 23:00:00 UTC
+++ internal/ssh/ssh.go
@@ -6,7 +6,6 @@ import (
import (
"context"
- "fmt"
"io"
"net"
"os"
@@ -54,28 +53,6 @@ func handleServerConn(keyID string, chans <-chan ssh.N
for req := range in {
payload := cleanCommand(string(req.Payload))
switch req.Type {
- case "env":
- var env struct {
- Name string
- Value string
- }
- if err := ssh.Unmarshal(req.Payload, &env); err != nil {
- log.Warn("SSH: Invalid env payload %q: %v", req.Payload, err)
- continue
- }
- // Sometimes the client could send malformed command (i.e. missing "="),
- // see https://discuss.gogs.io/t/ssh/3106.
- if env.Name == "" || env.Value == "" {
- log.Warn("SSH: Invalid env arguments: %+v", env)
- continue
- }
-
- _, stderr, err := com.ExecCmd("env", fmt.Sprintf("%s=%s", env.Name, env.Value))
- if err != nil {
- log.Error("env: %v - %s", err, stderr)
- return
- }
-
case "exec":
cmdName := strings.TrimLeft(payload, "'()")
log.Trace("SSH: Payload: %v", cmdName)
View Options

head/sys/kern/vfs_subr.c

  • This file was added.
--- internal/ssh/ssh.go.orig 1979-11-29 23:00:00 UTC
+++ internal/ssh/ssh.go
@@ -6,7 +6,6 @@ import (
import (
"context"
- "fmt"
"io"
"net"
"os"
@@ -54,28 +53,6 @@ func handleServerConn(keyID string, chans <-chan ssh.N
for req := range in {
payload := cleanCommand(string(req.Payload))
switch req.Type {
- case "env":
- var env struct {
- Name string
- Value string
- }
- if err := ssh.Unmarshal(req.Payload, &env); err != nil {
- log.Warn("SSH: Invalid env payload %q: %v", req.Payload, err)
- continue
- }
- // Sometimes the client could send malformed command (i.e. missing "="),
- // see https://discuss.gogs.io/t/ssh/3106.
- if env.Name == "" || env.Value == "" {
- log.Warn("SSH: Invalid env arguments: %+v", env)
- continue
- }
-
- _, stderr, err := com.ExecCmd("env", fmt.Sprintf("%s=%s", env.Name, env.Value))
- if err != nil {
- log.Error("env: %v - %s", err, stderr)
- return
- }
-
case "exec":
cmdName := strings.TrimLeft(payload, "'()")
log.Trace("SSH: Payload: %v", cmdName)
View Options

head/sys/vm/swap_pager.c

  • This file was added.
--- internal/ssh/ssh.go.orig 1979-11-29 23:00:00 UTC
+++ internal/ssh/ssh.go
@@ -6,7 +6,6 @@ import (
import (
"context"
- "fmt"
"io"
"net"
"os"
@@ -54,28 +53,6 @@ func handleServerConn(keyID string, chans <-chan ssh.N
for req := range in {
payload := cleanCommand(string(req.Payload))
switch req.Type {
- case "env":
- var env struct {
- Name string
- Value string
- }
- if err := ssh.Unmarshal(req.Payload, &env); err != nil {
- log.Warn("SSH: Invalid env payload %q: %v", req.Payload, err)
- continue
- }
- // Sometimes the client could send malformed command (i.e. missing "="),
- // see https://discuss.gogs.io/t/ssh/3106.
- if env.Name == "" || env.Value == "" {
- log.Warn("SSH: Invalid env arguments: %+v", env)
- continue
- }
-
- _, stderr, err := com.ExecCmd("env", fmt.Sprintf("%s=%s", env.Name, env.Value))
- if err != nil {
- log.Error("env: %v - %s", err, stderr)
- return
- }
-
case "exec":
cmdName := strings.TrimLeft(payload, "'()")
log.Trace("SSH: Payload: %v", cmdName)
View Options

head/sys/vm/vm_page.c

  • This file was added.
--- internal/ssh/ssh.go.orig 1979-11-29 23:00:00 UTC
+++ internal/ssh/ssh.go
@@ -6,7 +6,6 @@ import (
import (
"context"
- "fmt"
"io"
"net"
"os"
@@ -54,28 +53,6 @@ func handleServerConn(keyID string, chans <-chan ssh.N
for req := range in {
payload := cleanCommand(string(req.Payload))
switch req.Type {
- case "env":
- var env struct {
- Name string
- Value string
- }
- if err := ssh.Unmarshal(req.Payload, &env); err != nil {
- log.Warn("SSH: Invalid env payload %q: %v", req.Payload, err)
- continue
- }
- // Sometimes the client could send malformed command (i.e. missing "="),
- // see https://discuss.gogs.io/t/ssh/3106.
- if env.Name == "" || env.Value == "" {
- log.Warn("SSH: Invalid env arguments: %+v", env)
- continue
- }
-
- _, stderr, err := com.ExecCmd("env", fmt.Sprintf("%s=%s", env.Name, env.Value))
- if err != nil {
- log.Error("env: %v - %s", err, stderr)
- return
- }
-
case "exec":
cmdName := strings.TrimLeft(payload, "'()")
log.Trace("SSH: Payload: %v", cmdName)
View Options

head/sys/vm/vm_pager.c

  • This file was added.
--- internal/ssh/ssh.go.orig 1979-11-29 23:00:00 UTC
+++ internal/ssh/ssh.go
@@ -6,7 +6,6 @@ import (
import (
"context"
- "fmt"
"io"
"net"
"os"
@@ -54,28 +53,6 @@ func handleServerConn(keyID string, chans <-chan ssh.N
for req := range in {
payload := cleanCommand(string(req.Payload))
switch req.Type {
- case "env":
- var env struct {
- Name string
- Value string
- }
- if err := ssh.Unmarshal(req.Payload, &env); err != nil {
- log.Warn("SSH: Invalid env payload %q: %v", req.Payload, err)
- continue
- }
- // Sometimes the client could send malformed command (i.e. missing "="),
- // see https://discuss.gogs.io/t/ssh/3106.
- if env.Name == "" || env.Value == "" {
- log.Warn("SSH: Invalid env arguments: %+v", env)
- continue
- }
-
- _, stderr, err := com.ExecCmd("env", fmt.Sprintf("%s=%s", env.Name, env.Value))
- if err != nil {
- log.Error("env: %v - %s", err, stderr)
- return
- }
-
case "exec":
cmdName := strings.TrimLeft(payload, "'()")
log.Trace("SSH: Payload: %v", cmdName)