[PATCH 0/2] generalised cow per-thread structs

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/2] generalised cow per-thread structs

Mateusz Guzik
From: Mateusz Guzik <[hidden email]>

struct ucred is managed per thread as follows:
setuid and the like updated the pointer in struct proc
on kernel<->userspace boundary it is checked whether the thread needs
updating

This scheme is useful for other structures as well, so this patch
generalises it by introducing a counter which is compared instead.
This prevents introduction of further comparisons as such structures
are added.

The first patch just adds convenience funcs and adjusts cred handling
to use it.

The second patch implements lockless resource limits.

The bigger goal concerns struct filedesc - the plan is to split it to
fd part and vnode part. The latter is seldomly modified, se could be
accessed locklessly and with further effort can save some refs/unrefs
on vnodes since we will be sure they cannot go away.

Mateusz Guzik (2):
  Generalised support for copy-on-write structures shared by threads.
  Implement lockless resource limits.

 contrib/binutils/ld/emultempl/spu_ovl.o | Bin 1432 -> 0 bytes
 sys/amd64/amd64/trap.c                  |   4 +-
 sys/arm/arm/trap-v6.c                   |   4 +-
 sys/arm/arm/trap.c                      |  11 +++--
 sys/i386/i386/trap.c                    |   4 +-
 sys/kern/imgact_elf.c                   |  13 +++---
 sys/kern/init_main.c                    |   8 ++--
 sys/kern/kern_descrip.c                 |  24 +++++-----
 sys/kern/kern_event.c                   |   6 +--
 sys/kern/kern_exec.c                    |   4 +-
 sys/kern/kern_fork.c                    |   7 ++-
 sys/kern/kern_kthread.c                 |   2 +-
 sys/kern/kern_proc.c                    |   7 +--
 sys/kern/kern_prot.c                    |   5 ++-
 sys/kern/kern_resource.c                |  77 +++++++++++++++++++-------------
 sys/kern/kern_sig.c                     |   2 +-
 sys/kern/kern_syscalls.c                |   3 ++
 sys/kern/kern_thr.c                     |   6 +--
 sys/kern/kern_thread.c                  |  49 ++++++++++++++++++--
 sys/kern/subr_syscall.c                 |   4 +-
 sys/kern/subr_trap.c                    |   4 +-
 sys/kern/subr_uio.c                     |   4 +-
 sys/kern/sysv_shm.c                     |   4 +-
 sys/kern/tty_pts.c                      |   4 +-
 sys/kern/uipc_sockbuf.c                 |   4 +-
 sys/kern/vfs_vnops.c                    |   7 ++-
 sys/powerpc/powerpc/trap.c              |   4 +-
 sys/sparc64/sparc64/trap.c              |   4 +-
 sys/sys/proc.h                          |  14 +++++-
 sys/sys/resourcevar.h                   |   9 ++--
 sys/sys/vnode.h                         |   2 +-
 sys/vm/swap_pager.c                     |   4 +-
 sys/vm/vm_map.c                         |  14 +++---
 sys/vm/vm_mmap.c                        |  34 +++++++-------
 sys/vm/vm_pageout.c                     |   2 +-
 sys/vm/vm_unix.c                        |   8 ++--
 36 files changed, 208 insertions(+), 154 deletions(-)
 delete mode 100644 contrib/binutils/ld/emultempl/spu_ovl.o

--
2.3.6

_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] Generalised support for copy-on-write structures shared by threads.

Mateusz Guzik
From: Mateusz Guzik <[hidden email]>

Previously td_ucred was managed by comparing it to struct proc's version
on kernel<->userspace boundary.

Now a dedicated counter is introduced instead which makes it possible to
treat more structures this way without adding more tests for the common
case (no change).
---
 sys/amd64/amd64/trap.c                  |   4 +--
 sys/arm/arm/trap-v6.c                   |   4 +--
 sys/arm/arm/trap.c                      |  11 ++++----
 sys/i386/i386/trap.c                    |   4 +--
 sys/kern/init_main.c                    |   8 +++---
 sys/kern/kern_fork.c                    |   3 ++-
 sys/kern/kern_kthread.c                 |   2 +-
 sys/kern/kern_prot.c                    |   5 ++--
 sys/kern/kern_syscalls.c                |   2 ++
 sys/kern/kern_thr.c                     |   6 ++---
 sys/kern/kern_thread.c                  |  43 +++++++++++++++++++++++++++++---
 sys/kern/subr_syscall.c                 |   4 +--
 sys/kern/subr_trap.c                    |   4 +--
 sys/powerpc/powerpc/trap.c              |   4 +--
 sys/sparc64/sparc64/trap.c              |   4 +--
 sys/sys/proc.h                          |  11 ++++++++
 17 files changed, 86 insertions(+), 33 deletions(-)

diff --git a/sys/amd64/amd64/trap.c b/sys/amd64/amd64/trap.c
index 193d207..1883727 100644
--- a/sys/amd64/amd64/trap.c
+++ b/sys/amd64/amd64/trap.c
@@ -257,8 +257,8 @@ trap(struct trapframe *frame)
  td->td_pticks = 0;
  td->td_frame = frame;
  addr = frame->tf_rip;
- if (td->td_ucred != p->p_ucred)
- cred_update_thread(td);
+ if (td->td_cowgeneration != p->p_cowgeneration)
+ thread_update_cow(td);
 
  switch (type) {
  case T_PRIVINFLT: /* privileged instruction fault */
diff --git a/sys/arm/arm/trap-v6.c b/sys/arm/arm/trap-v6.c
index abafa86..f521785 100644
--- a/sys/arm/arm/trap-v6.c
+++ b/sys/arm/arm/trap-v6.c
@@ -394,8 +394,8 @@ abort_handler(struct trapframe *tf, int prefetch)
  p = td->td_proc;
  if (usermode) {
  td->td_pticks = 0;
- if (td->td_ucred != p->p_ucred)
- cred_update_thread(td);
+ if (td->td_cowgeneration != p->p_cowgeneration)
+ thread_update_cow(td);
  }
 
  /* Invoke the appropriate handler, if necessary. */
diff --git a/sys/arm/arm/trap.c b/sys/arm/arm/trap.c
index 0f142ce..36faac2 100644
--- a/sys/arm/arm/trap.c
+++ b/sys/arm/arm/trap.c
@@ -214,9 +214,8 @@ abort_handler(struct trapframe *tf, int type)
  if (user) {
  td->td_pticks = 0;
  td->td_frame = tf;
- if (td->td_ucred != td->td_proc->p_ucred)
- cred_update_thread(td);
-
+ if (td->td_cowgeneration != p->p_cowgeneration)
+ thread_update_cow(td);
  }
  /* Grab the current pcb */
  pcb = td->td_pcb;
@@ -644,8 +643,8 @@ prefetch_abort_handler(struct trapframe *tf)
 
  if (TRAP_USERMODE(tf)) {
  td->td_frame = tf;
- if (td->td_ucred != td->td_proc->p_ucred)
- cred_update_thread(td);
+                if (td->td_cowgeneration != p->p_cowgeneration)
+                        thread_update_cow(td);
  }
  fault_pc = tf->tf_pc;
  if (td->td_md.md_spinlock_count == 0) {
diff --git a/sys/i386/i386/trap.c b/sys/i386/i386/trap.c
index d783a2b..41e62db 100644
--- a/sys/i386/i386/trap.c
+++ b/sys/i386/i386/trap.c
@@ -306,8 +306,8 @@ trap(struct trapframe *frame)
  td->td_pticks = 0;
  td->td_frame = frame;
  addr = frame->tf_eip;
- if (td->td_ucred != p->p_ucred)
- cred_update_thread(td);
+ if (td->td_cowgeneration != p->p_cowgeneration)
+ thread_update_cow(td);
 
  switch (type) {
  case T_PRIVINFLT: /* privileged instruction fault */
diff --git a/sys/kern/init_main.c b/sys/kern/init_main.c
index b77b788..97e5878 100644
--- a/sys/kern/init_main.c
+++ b/sys/kern/init_main.c
@@ -522,8 +522,6 @@ proc0_init(void *dummy __unused)
 #ifdef MAC
  mac_cred_create_swapper(newcred);
 #endif
- td->td_ucred = crhold(newcred);
-
  /* Create sigacts. */
  p->p_sigacts = sigacts_alloc();
 
@@ -555,6 +553,10 @@ proc0_init(void *dummy __unused)
  p->p_limit->pl_rlimit[RLIMIT_MEMLOCK].rlim_max = pageablemem;
  p->p_cpulimit = RLIM_INFINITY;
 
+ PROC_LOCK(p);
+ thread_get_cow_proc(td, p);
+ PROC_UNLOCK(p);
+
  /* Initialize resource accounting structures. */
  racct_create(&p->p_racct);
 
@@ -842,10 +844,10 @@ create_init(const void *udata __unused)
  audit_cred_proc1(newcred);
 #endif
  proc_set_cred(initproc, newcred);
+ cred_update_thread(FIRST_THREAD_IN_PROC(initproc));
  PROC_UNLOCK(initproc);
  sx_xunlock(&proctree_lock);
  crfree(oldcred);
- cred_update_thread(FIRST_THREAD_IN_PROC(initproc));
  cpu_set_fork_handler(FIRST_THREAD_IN_PROC(initproc), start_init, NULL);
 }
 SYSINIT(init, SI_SUB_CREATE_INIT, SI_ORDER_FIRST, create_init, NULL);
diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c
index c3dd792..d04c3e3 100644
--- a/sys/kern/kern_fork.c
+++ b/sys/kern/kern_fork.c
@@ -496,7 +496,6 @@ do_fork(struct thread *td, int flags, struct proc *p2, struct thread *td2,
  p2->p_swtick = ticks;
  if (p1->p_flag & P_PROFIL)
  startprofclock(p2);
- td2->td_ucred = crhold(p2->p_ucred);
 
  if (flags & RFSIGSHARE) {
  p2->p_sigacts = sigacts_hold(p1->p_sigacts);
@@ -526,6 +525,8 @@ do_fork(struct thread *td, int flags, struct proc *p2, struct thread *td2,
  */
  lim_fork(p1, p2);
 
+ thread_get_cow_proc(td2, p2);
+
  pstats_fork(p1->p_stats, p2->p_stats);
 
  PROC_UNLOCK(p1);
diff --git a/sys/kern/kern_kthread.c b/sys/kern/kern_kthread.c
index ee94de0..0614d89 100644
--- a/sys/kern/kern_kthread.c
+++ b/sys/kern/kern_kthread.c
@@ -289,7 +289,7 @@ kthread_add(void (*func)(void *), void *arg, struct proc *p,
  cpu_set_fork_handler(newtd, func, arg);
 
  newtd->td_pflags |= TDP_KTHREAD;
- newtd->td_ucred = crhold(p->p_ucred);
+ thread_get_cow_proc(newtd, p);
 
  /* this code almost the same as create_thread() in kern_thr.c */
  p->p_flag |= P_HADTHREADS;
diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c
index 9c49f71..b531763 100644
--- a/sys/kern/kern_prot.c
+++ b/sys/kern/kern_prot.c
@@ -1946,9 +1946,8 @@ cred_update_thread(struct thread *td)
 
  p = td->td_proc;
  cred = td->td_ucred;
- PROC_LOCK(p);
+ PROC_LOCK_ASSERT(p, MA_OWNED);
  td->td_ucred = crhold(p->p_ucred);
- PROC_UNLOCK(p);
  if (cred != NULL)
  crfree(cred);
 }
@@ -1987,6 +1986,8 @@ proc_set_cred(struct proc *p, struct ucred *newcred)
 
  oldcred = p->p_ucred;
  p->p_ucred = newcred;
+ if (newcred != NULL)
+ PROC_UPDATE_COW(p);
  return (oldcred);
 }
 
diff --git a/sys/kern/kern_syscalls.c b/sys/kern/kern_syscalls.c
index dada746..3d3df01 100644
--- a/sys/kern/kern_syscalls.c
+++ b/sys/kern/kern_syscalls.c
@@ -31,6 +31,8 @@ __FBSDID("$FreeBSD$");
 #include <sys/kernel.h>
 #include <sys/lock.h>
 #include <sys/module.h>
+#include <sys/mutex.h>
+#include <sys/proc.h>
 #include <sys/sx.h>
 #include <sys/syscall.h>
 #include <sys/sysent.h>
diff --git a/sys/kern/kern_thr.c b/sys/kern/kern_thr.c
index d5f1ce6..242e4dd 100644
--- a/sys/kern/kern_thr.c
+++ b/sys/kern/kern_thr.c
@@ -226,13 +226,13 @@ create_thread(struct thread *td, mcontext_t *ctx,
  bcopy(&td->td_startcopy, &newtd->td_startcopy,
     __rangeof(struct thread, td_startcopy, td_endcopy));
  newtd->td_proc = td->td_proc;
- newtd->td_ucred = crhold(td->td_ucred);
+ thread_get_cow(newtd, td);
 
  if (ctx != NULL) { /* old way to set user context */
  error = set_mcontext(newtd, ctx);
  if (error != 0) {
+ thread_free_cow(newtd);
  thread_free(newtd);
- crfree(td->td_ucred);
  goto fail;
  }
  } else {
@@ -244,8 +244,8 @@ create_thread(struct thread *td, mcontext_t *ctx,
  /* Setup user TLS address and TLS pointer register. */
  error = cpu_set_user_tls(newtd, tls_base);
  if (error != 0) {
+ thread_free_cow(newtd);
  thread_free(newtd);
- crfree(td->td_ucred);
  goto fail;
  }
  }
diff --git a/sys/kern/kern_thread.c b/sys/kern/kern_thread.c
index 0a93dbd..df8511b 100644
--- a/sys/kern/kern_thread.c
+++ b/sys/kern/kern_thread.c
@@ -324,8 +324,7 @@ thread_reap(void)
  mtx_unlock_spin(&zombie_lock);
  while (td_first) {
  td_next = TAILQ_NEXT(td_first, td_slpq);
- if (td_first->td_ucred)
- crfree(td_first->td_ucred);
+ thread_free_cow(td_first);
  thread_free(td_first);
  td_first = td_next;
  }
@@ -381,6 +380,44 @@ thread_free(struct thread *td)
  uma_zfree(thread_zone, td);
 }
 
+void
+thread_get_cow_proc(struct thread *newtd, struct proc *p)
+{
+
+ PROC_LOCK_ASSERT(p, MA_OWNED);
+ newtd->td_ucred = crhold(p->p_ucred);
+ newtd->td_cowgeneration = p->p_cowgeneration;
+}
+
+void
+thread_get_cow(struct thread *newtd, struct thread *td)
+{
+
+ newtd->td_ucred = crhold(td->td_ucred);
+ newtd->td_cowgeneration = td->td_cowgeneration;
+}
+
+void
+thread_free_cow(struct thread *td)
+{
+
+ if (td->td_ucred)
+ crfree(td->td_ucred);
+}
+
+void
+thread_update_cow(struct thread *td)
+{
+ struct proc *p;
+
+ p = td->td_proc;
+ PROC_LOCK(p);
+ if (td->td_ucred != p->p_ucred)
+ cred_update_thread(td);
+ td->td_cowgeneration = p->p_cowgeneration;
+ PROC_UNLOCK(p);
+}
+
 /*
  * Discard the current thread and exit from its context.
  * Always called with scheduler locked.
@@ -518,7 +555,7 @@ thread_wait(struct proc *p)
  cpuset_rel(td->td_cpuset);
  td->td_cpuset = NULL;
  cpu_thread_clean(td);
- crfree(td->td_ucred);
+ thread_free_cow(td);
  thread_reap(); /* check for zombie threads etc. */
 }
 
diff --git a/sys/kern/subr_syscall.c b/sys/kern/subr_syscall.c
index 1bf78b8..8fdb828 100644
--- a/sys/kern/subr_syscall.c
+++ b/sys/kern/subr_syscall.c
@@ -61,8 +61,8 @@ syscallenter(struct thread *td, struct syscall_args *sa)
  p = td->td_proc;
 
  td->td_pticks = 0;
- if (td->td_ucred != p->p_ucred)
- cred_update_thread(td);
+ if (td->td_cowgeneration != p->p_cowgeneration)
+ thread_update_cow(td);
  if (p->p_flag & P_TRACED) {
  traced = 1;
  PROC_LOCK(p);
diff --git a/sys/kern/subr_trap.c b/sys/kern/subr_trap.c
index cfc3ed7..e055e54 100644
--- a/sys/kern/subr_trap.c
+++ b/sys/kern/subr_trap.c
@@ -219,8 +219,8 @@ ast(struct trapframe *framep)
  thread_unlock(td);
  PCPU_INC(cnt.v_trap);
 
- if (td->td_ucred != p->p_ucred)
- cred_update_thread(td);
+ if (td->td_cowgeneration != p->p_cowgeneration)
+ thread_update_cow(td);
  if (td->td_pflags & TDP_OWEUPC && p->p_flag & P_PROFIL) {
  addupc_task(td, td->td_profil_addr, td->td_profil_ticks);
  td->td_profil_ticks = 0;
diff --git a/sys/powerpc/powerpc/trap.c b/sys/powerpc/powerpc/trap.c
index 0ceb170..007752c 100644
--- a/sys/powerpc/powerpc/trap.c
+++ b/sys/powerpc/powerpc/trap.c
@@ -196,8 +196,8 @@ trap(struct trapframe *frame)
  if (user) {
  td->td_pticks = 0;
  td->td_frame = frame;
- if (td->td_ucred != p->p_ucred)
- cred_update_thread(td);
+ if (td->td_cowgeneration != p->p_cowgeneration)
+ thread_update_cow(td);
 
  /* User Mode Traps */
  switch (type) {
diff --git a/sys/sparc64/sparc64/trap.c b/sys/sparc64/sparc64/trap.c
index b4f0e27..54c1ebe 100644
--- a/sys/sparc64/sparc64/trap.c
+++ b/sys/sparc64/sparc64/trap.c
@@ -277,8 +277,8 @@ trap(struct trapframe *tf)
  td->td_pticks = 0;
  td->td_frame = tf;
  addr = tf->tf_tpc;
- if (td->td_ucred != p->p_ucred)
- cred_update_thread(td);
+ if (td->td_cowgeneration != p->p_cowgeneration)
+ thread_update_cow(td);
 
  switch (tf->tf_type) {
  case T_DATA_MISS:
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index 64b99fc..f29d796 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -225,6 +225,7 @@ struct thread {
 /* Cleared during fork1() */
 #define td_startzero td_flags
  int td_flags; /* (t) TDF_* flags. */
+ u_int td_cowgeneration;/* (k) Generation of COW pointers. */
  int td_inhibitors; /* (t) Why can not run. */
  int td_pflags; /* (k) Private thread (TDP_*) flags. */
  int td_dupfd; /* (k) Ret value from fdopen. XXX */
@@ -531,6 +532,7 @@ struct proc {
  pid_t p_oppid; /* (c + e) Save ppid in ptrace. XXX */
  struct vmspace *p_vmspace; /* (b) Address space. */
  u_int p_swtick; /* (c) Tick when swapped in or out. */
+ u_int p_cowgeneration;/* (c) Generation of COW pointers. */
  struct itimerval p_realtimer; /* (c) Alarm timer. */
  struct rusage p_ru; /* (a) Exit information. */
  struct rusage_ext p_rux; /* (cu) Internal resource usage. */
@@ -830,6 +832,11 @@ extern pid_t pid_max;
  KASSERT((p)->p_lock == 0, ("process held")); \
 } while (0)
 
+#define PROC_UPDATE_COW(p) do { \
+ PROC_LOCK_ASSERT((p), MA_OWNED); \
+ p->p_cowgeneration++; \
+} while (0)
+
 /* Check whether a thread is safe to be swapped out. */
 #define thread_safetoswapout(td) ((td)->td_flags & TDF_CANSWAP)
 
@@ -976,6 +983,10 @@ struct thread *thread_alloc(int pages);
 int thread_alloc_stack(struct thread *, int pages);
 void thread_exit(void) __dead2;
 void thread_free(struct thread *td);
+void thread_get_cow_proc(struct thread *newtd, struct proc *p);
+void thread_get_cow(struct thread *newtd, struct thread *td);
+void thread_free_cow(struct thread *td);
+void thread_update_cow(struct thread *td);
 void thread_link(struct thread *td, struct proc *p);
 void thread_reap(void);
 int thread_single(struct proc *p, int how);
--
2.3.6

_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2] Implement lockless resource limits.

Mateusz Guzik
In reply to this post by Mateusz Guzik
From: Mateusz Guzik <[hidden email]>

Employ the same mechanism which is used to manage per-thread
credentials.
---
 sys/kern/imgact_elf.c    | 13 ++++----
 sys/kern/kern_descrip.c  | 24 +++++++--------
 sys/kern/kern_event.c    |  6 +---
 sys/kern/kern_exec.c     |  4 +--
 sys/kern/kern_fork.c     |  4 +--
 sys/kern/kern_proc.c     |  7 ++---
 sys/kern/kern_resource.c | 77 ++++++++++++++++++++++++++++--------------------
 sys/kern/kern_sig.c      |  2 +-
 sys/kern/kern_syscalls.c |  1 +
 sys/kern/kern_thread.c   |  6 ++++
 sys/kern/subr_uio.c      |  4 +--
 sys/kern/sysv_shm.c      |  4 +--
 sys/kern/tty_pts.c       |  4 +--
 sys/kern/uipc_sockbuf.c  |  4 +--
 sys/kern/vfs_vnops.c     |  7 ++---
 sys/sys/proc.h           |  3 +-
 sys/sys/resourcevar.h    |  9 ++++--
 sys/sys/vnode.h          |  2 +-
 sys/vm/swap_pager.c      |  4 +--
 sys/vm/vm_map.c          | 14 ++++-----
 sys/vm/vm_mmap.c         | 34 +++++++++++----------
 sys/vm/vm_pageout.c      |  2 +-
 sys/vm/vm_unix.c         |  8 ++---
 23 files changed, 122 insertions(+), 121 deletions(-)

diff --git a/sys/kern/imgact_elf.c b/sys/kern/imgact_elf.c
index 39e4df3..ff3a371 100644
--- a/sys/kern/imgact_elf.c
+++ b/sys/kern/imgact_elf.c
@@ -900,13 +900,17 @@ __CONCAT(exec_, __elfN(imgact))(struct image_params *imgp)
  * limits after loading the segments since we do
  * not actually fault in all the segments pages.
  */
+#ifdef RACCT
  PROC_LOCK(imgp->proc);
- if (data_size > lim_cur(imgp->proc, RLIMIT_DATA) ||
+#endif
+ if (data_size > lim_cur(curthread, RLIMIT_DATA) ||
     text_size > maxtsiz ||
-    total_size > lim_cur(imgp->proc, RLIMIT_VMEM) ||
+    total_size > lim_cur(curthread, RLIMIT_VMEM) ||
     racct_set(imgp->proc, RACCT_DATA, data_size) != 0 ||
     racct_set(imgp->proc, RACCT_VMEM, total_size) != 0) {
+#ifdef RACCT
  PROC_UNLOCK(imgp->proc);
+#endif
  return (ENOMEM);
  }
 
@@ -922,9 +926,8 @@ __CONCAT(exec_, __elfN(imgact))(struct image_params *imgp)
  * calculation is that it leaves room for the heap to grow to
  * its maximum allowed size.
  */
- addr = round_page((vm_offset_t)vmspace->vm_daddr + lim_max(imgp->proc,
+ addr = round_page((vm_offset_t)vmspace->vm_daddr + lim_max(curthread,
     RLIMIT_DATA));
- PROC_UNLOCK(imgp->proc);
 
  imgp->entry_addr = entry;
 
@@ -1963,7 +1966,7 @@ note_procstat_rlimit(void *arg, struct sbuf *sb, size_t *sizep)
  sbuf_bcat(sb, &structsize, sizeof(structsize));
  PROC_LOCK(p);
  for (i = 0; i < RLIM_NLIMITS; i++)
- lim_rlimit(p, i, &rlim[i]);
+ lim_rlimit_proc(p, i, &rlim[i]);
  PROC_UNLOCK(p);
  sbuf_bcat(sb, rlim, sizeof(rlim));
  }
diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index f3f27bf..cc7b276 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -109,7 +109,7 @@ static void fdgrowtable(struct filedesc *fdp, int nfd);
 static void fdgrowtable_exp(struct filedesc *fdp, int nfd);
 static void fdunused(struct filedesc *fdp, int fd);
 static void fdused(struct filedesc *fdp, int fd);
-static int getmaxfd(struct proc *p);
+static int getmaxfd(struct thread *td);
 
 /* Flags for do_dup() */
 #define DUP_FIXED 0x1 /* Force fixed allocation. */
@@ -331,16 +331,19 @@ struct getdtablesize_args {
 int
 sys_getdtablesize(struct thread *td, struct getdtablesize_args *uap)
 {
- struct proc *p = td->td_proc;
+#ifdef RACCT
  uint64_t lim;
+#endif
 
- PROC_LOCK(p);
  td->td_retval[0] =
-    min((int)lim_cur(p, RLIMIT_NOFILE), maxfilesperproc);
+    min((int)lim_cur(td, RLIMIT_NOFILE), maxfilesperproc);
+#ifdef RACCT
+ PROC_LOCK(p);
  lim = racct_get_limit(td->td_proc, RACCT_NOFILE);
  PROC_UNLOCK(p);
  if (lim < td->td_retval[0])
  td->td_retval[0] = lim;
+#endif
  return (0);
 }
 
@@ -785,15 +788,10 @@ kern_fcntl(struct thread *td, int fd, int cmd, intptr_t arg)
 }
 
 static int
-getmaxfd(struct proc *p)
+getmaxfd(struct thread *td)
 {
- int maxfd;
-
- PROC_LOCK(p);
- maxfd = min((int)lim_cur(p, RLIMIT_NOFILE), maxfilesperproc);
- PROC_UNLOCK(p);
 
- return (maxfd);
+ return (min((int)lim_cur(td, RLIMIT_NOFILE), maxfilesperproc));
 }
 
 /*
@@ -821,7 +819,7 @@ do_dup(struct thread *td, int flags, int old, int new)
  return (EBADF);
  if (new < 0)
  return (flags & DUP_FCNTL ? EINVAL : EBADF);
- maxfd = getmaxfd(p);
+ maxfd = getmaxfd(td);
  if (new >= maxfd)
  return (flags & DUP_FCNTL ? EINVAL : EBADF);
 
@@ -1619,7 +1617,7 @@ fdalloc(struct thread *td, int minfd, int *result)
  if (fdp->fd_freefile > minfd)
  minfd = fdp->fd_freefile;
 
- maxfd = getmaxfd(p);
+ maxfd = getmaxfd(td);
 
  /*
  * Search the bitmap for a free descriptor starting at minfd.
diff --git a/sys/kern/kern_event.c b/sys/kern/kern_event.c
index e01f12c..618a68e 100644
--- a/sys/kern/kern_event.c
+++ b/sys/kern/kern_event.c
@@ -747,14 +747,10 @@ sys_kqueue(struct thread *td, struct kqueue_args *uap)
  p = td->td_proc;
  cred = td->td_ucred;
  crhold(cred);
- PROC_LOCK(p);
- if (!chgkqcnt(cred->cr_ruidinfo, 1, lim_cur(td->td_proc,
-    RLIMIT_KQUEUES))) {
- PROC_UNLOCK(p);
+ if (!chgkqcnt(cred->cr_ruidinfo, 1, lim_cur(td, RLIMIT_KQUEUES))) {
  crfree(cred);
  return (ENOMEM);
  }
- PROC_UNLOCK(p);
 
  fdp = p->p_fd;
  error = falloc(td, &fp, &fd, 0);
diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c
index 9d893f8..751f153 100644
--- a/sys/kern/kern_exec.c
+++ b/sys/kern/kern_exec.c
@@ -1061,9 +1061,7 @@ exec_new_vmspace(imgp, sv)
  /* Allocate a new stack */
  if (imgp->stack_sz != 0) {
  ssiz = trunc_page(imgp->stack_sz);
- PROC_LOCK(p);
- lim_rlimit(p, RLIMIT_STACK, &rlim_stack);
- PROC_UNLOCK(p);
+ lim_rlimit(curthread, RLIMIT_STACK, &rlim_stack);
  if (ssiz > rlim_stack.rlim_max)
  ssiz = rlim_stack.rlim_max;
  if (ssiz > rlim_stack.rlim_cur) {
diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c
index d04c3e3..6cde199 100644
--- a/sys/kern/kern_fork.c
+++ b/sys/kern/kern_fork.c
@@ -912,10 +912,8 @@ fork1(struct thread *td, int flags, int pages, struct proc **procp,
  if (error == 0)
  ok = chgproccnt(td->td_ucred->cr_ruidinfo, 1, 0);
  else {
- PROC_LOCK(p1);
  ok = chgproccnt(td->td_ucred->cr_ruidinfo, 1,
-    lim_cur(p1, RLIMIT_NPROC));
- PROC_UNLOCK(p1);
+    lim_cur(td, RLIMIT_NPROC));
  }
  if (ok) {
  do_fork(td, flags, newproc, td2, vm2, pdflags);
diff --git a/sys/kern/kern_proc.c b/sys/kern/kern_proc.c
index 505521d..0708d71 100644
--- a/sys/kern/kern_proc.c
+++ b/sys/kern/kern_proc.c
@@ -2597,11 +2597,8 @@ sysctl_kern_proc_rlimit(SYSCTL_HANDLER_ARGS)
  /*
  * Retrieve limit.
  */
- if (req->oldptr != NULL) {
- PROC_LOCK(p);
- lim_rlimit(p, which, &rlim);
- PROC_UNLOCK(p);
- }
+ if (req->oldptr != NULL)
+ lim_rlimit(curthread, which, &rlim);
  error = SYSCTL_OUT(req, &rlim, sizeof(rlim));
  if (error != 0)
  goto errout;
diff --git a/sys/kern/kern_resource.c b/sys/kern/kern_resource.c
index dac49cd..bc677dc 100644
--- a/sys/kern/kern_resource.c
+++ b/sys/kern/kern_resource.c
@@ -560,15 +560,11 @@ ogetrlimit(struct thread *td, register struct ogetrlimit_args *uap)
 {
  struct orlimit olim;
  struct rlimit rl;
- struct proc *p;
  int error;
 
  if (uap->which >= RLIM_NLIMITS)
  return (EINVAL);
- p = td->td_proc;
- PROC_LOCK(p);
- lim_rlimit(p, uap->which, &rl);
- PROC_UNLOCK(p);
+ lim_rlimit(td, uap->which, &rl);
 
  /*
  * XXX would be more correct to convert only RLIM_INFINITY to the
@@ -625,7 +621,7 @@ lim_cb(void *arg)
  }
  PROC_STATUNLOCK(p);
  if (p->p_rux.rux_runtime > p->p_cpulimit * cpu_tickrate()) {
- lim_rlimit(p, RLIMIT_CPU, &rlim);
+ lim_rlimit_proc(p, RLIMIT_CPU, &rlim);
  if (p->p_rux.rux_runtime >= rlim.rlim_max * cpu_tickrate()) {
  killproc(p, "exceeded maximum CPU limit");
  } else {
@@ -667,29 +663,21 @@ kern_proc_setrlimit(struct thread *td, struct proc *p, u_int which,
  limp->rlim_max = RLIM_INFINITY;
 
  oldssiz.rlim_cur = 0;
- newlim = NULL;
+ newlim = lim_alloc();
  PROC_LOCK(p);
- if (lim_shared(p->p_limit)) {
- PROC_UNLOCK(p);
- newlim = lim_alloc();
- PROC_LOCK(p);
- }
  oldlim = p->p_limit;
  alimp = &oldlim->pl_rlimit[which];
  if (limp->rlim_cur > alimp->rlim_max ||
     limp->rlim_max > alimp->rlim_max)
  if ((error = priv_check(td, PRIV_PROC_SETRLIMIT))) {
  PROC_UNLOCK(p);
- if (newlim != NULL)
- lim_free(newlim);
+ lim_free(newlim);
  return (error);
  }
  if (limp->rlim_cur > limp->rlim_max)
  limp->rlim_cur = limp->rlim_max;
- if (newlim != NULL) {
- lim_copy(newlim, oldlim);
- alimp = &newlim->pl_rlimit[which];
- }
+ lim_copy(newlim, oldlim);
+ alimp = &newlim->pl_rlimit[which];
 
  switch (which) {
 
@@ -739,11 +727,10 @@ kern_proc_setrlimit(struct thread *td, struct proc *p, u_int which,
  if (p->p_sysent->sv_fixlimit != NULL)
  p->p_sysent->sv_fixlimit(limp, which);
  *alimp = *limp;
- if (newlim != NULL)
- p->p_limit = newlim;
+ p->p_limit = newlim;
+ PROC_UPDATE_COW(p);
  PROC_UNLOCK(p);
- if (newlim != NULL)
- lim_free(oldlim);
+ lim_free(oldlim);
 
  if (which == RLIMIT_STACK &&
     /*
@@ -793,15 +780,11 @@ int
 sys_getrlimit(struct thread *td, register struct __getrlimit_args *uap)
 {
  struct rlimit rlim;
- struct proc *p;
  int error;
 
  if (uap->which >= RLIM_NLIMITS)
  return (EINVAL);
- p = td->td_proc;
- PROC_LOCK(p);
- lim_rlimit(p, uap->which, &rlim);
- PROC_UNLOCK(p);
+ lim_rlimit(td, uap->which, &rlim);
  error = copyout(&rlim, uap->rlp, sizeof(struct rlimit));
  return (error);
 }
@@ -1172,11 +1155,11 @@ lim_copy(struct plimit *dst, struct plimit *src)
  * which parameter specifies the index into the rlimit array.
  */
 rlim_t
-lim_max(struct proc *p, int which)
+lim_max(struct thread *td, int which)
 {
  struct rlimit rl;
 
- lim_rlimit(p, which, &rl);
+ lim_rlimit(td, which, &rl);
  return (rl.rlim_max);
 }
 
@@ -1185,11 +1168,11 @@ lim_max(struct proc *p, int which)
  * The which parameter which specifies the index into the rlimit array
  */
 rlim_t
-lim_cur(struct proc *p, int which)
+lim_cur(struct thread *td, int which)
 {
  struct rlimit rl;
 
- lim_rlimit(p, which, &rl);
+ lim_rlimit(td, which, &rl);
  return (rl.rlim_cur);
 }
 
@@ -1198,7 +1181,23 @@ lim_cur(struct proc *p, int which)
  * specified by 'which' in the rlimit structure pointed to by 'rlp'.
  */
 void
-lim_rlimit(struct proc *p, int which, struct rlimit *rlp)
+lim_rlimit(struct thread *td, int which, struct rlimit *rlp)
+{
+ struct proc *p = td->td_proc;
+
+ MPASS(td == curthread);
+ KASSERT(which >= 0 && which < RLIM_NLIMITS,
+    ("request for invalid resource limit"));
+ *rlp = td->td_limit->pl_rlimit[which];
+ if (p->p_sysent->sv_fixlimit != NULL)
+ p->p_sysent->sv_fixlimit(rlp, which);
+}
+
+/*
+ * Same as lim_rlimit but can be used with non-curthread.
+ */
+void
+lim_rlimit_proc(struct proc *p, int which, struct rlimit *rlp)
 {
 
  PROC_LOCK_ASSERT(p, MA_OWNED);
@@ -1441,3 +1440,17 @@ chgkqcnt(struct uidinfo *uip, int diff, rlim_t max)
  }
  return (1);
 }
+
+void
+lim_update_thread(struct thread *td)
+{
+ struct proc *p;
+ struct plimit *lim;
+
+ p = td->td_proc;
+ lim = td->td_limit;
+ PROC_LOCK_ASSERT(p, MA_OWNED);
+ td->td_limit = lim_hold(p->p_limit);
+ if (lim != NULL)
+ lim_free(lim);
+}
diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c
index 154c250..07a586f 100644
--- a/sys/kern/kern_sig.c
+++ b/sys/kern/kern_sig.c
@@ -3304,7 +3304,7 @@ coredump(struct thread *td)
  * a corefile is truncated instead of not being created,
  * if it is larger than the limit.
  */
- limit = (off_t)lim_cur(p, RLIMIT_CORE);
+ limit = (off_t)lim_cur(td, RLIMIT_CORE);
  if (limit == 0 || racct_get_available(p, RACCT_CORE) == 0) {
  PROC_UNLOCK(p);
  return (EFBIG);
diff --git a/sys/kern/kern_syscalls.c b/sys/kern/kern_syscalls.c
index 3d3df01..15574be 100644
--- a/sys/kern/kern_syscalls.c
+++ b/sys/kern/kern_syscalls.c
@@ -33,6 +33,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/module.h>
 #include <sys/mutex.h>
 #include <sys/proc.h>
+#include <sys/resourcevar.h>
 #include <sys/sx.h>
 #include <sys/syscall.h>
 #include <sys/sysent.h>
diff --git a/sys/kern/kern_thread.c b/sys/kern/kern_thread.c
index df8511b..79e9c50 100644
--- a/sys/kern/kern_thread.c
+++ b/sys/kern/kern_thread.c
@@ -386,6 +386,7 @@ thread_get_cow_proc(struct thread *newtd, struct proc *p)
 
  PROC_LOCK_ASSERT(p, MA_OWNED);
  newtd->td_ucred = crhold(p->p_ucred);
+ newtd->td_limit = lim_hold(p->p_limit);
  newtd->td_cowgeneration = p->p_cowgeneration;
 }
 
@@ -394,6 +395,7 @@ thread_get_cow(struct thread *newtd, struct thread *td)
 {
 
  newtd->td_ucred = crhold(td->td_ucred);
+ newtd->td_limit = lim_hold(td->td_limit);
  newtd->td_cowgeneration = td->td_cowgeneration;
 }
 
@@ -403,6 +405,8 @@ thread_free_cow(struct thread *td)
 
  if (td->td_ucred)
  crfree(td->td_ucred);
+ if (td->td_limit)
+ lim_free(td->td_limit);
 }
 
 void
@@ -414,6 +418,8 @@ thread_update_cow(struct thread *td)
  PROC_LOCK(p);
  if (td->td_ucred != p->p_ucred)
  cred_update_thread(td);
+ if (td->td_limit != p->p_limit)
+ lim_update_thread(td);
  td->td_cowgeneration = p->p_cowgeneration;
  PROC_UNLOCK(p);
 }
diff --git a/sys/kern/subr_uio.c b/sys/kern/subr_uio.c
index 87892fd..570298f 100644
--- a/sys/kern/subr_uio.c
+++ b/sys/kern/subr_uio.c
@@ -409,10 +409,8 @@ copyout_map(struct thread *td, vm_offset_t *addr, size_t sz)
  /*
  * Map somewhere after heap in process memory.
  */
- PROC_LOCK(td->td_proc);
  *addr = round_page((vm_offset_t)vms->vm_daddr +
-    lim_max(td->td_proc, RLIMIT_DATA));
- PROC_UNLOCK(td->td_proc);
+    lim_max(td, RLIMIT_DATA));
 
  /* round size up to page boundry */
  size = (vm_size_t)round_page(sz);
diff --git a/sys/kern/sysv_shm.c b/sys/kern/sysv_shm.c
index 274deda..00e3c0a 100644
--- a/sys/kern/sysv_shm.c
+++ b/sys/kern/sysv_shm.c
@@ -380,10 +380,8 @@ kern_shmat_locked(struct thread *td, int shmid, const void *shmaddr,
  * This is just a hint to vm_map_find() about where to
  * put it.
  */
- PROC_LOCK(p);
  attach_va = round_page((vm_offset_t)p->p_vmspace->vm_daddr +
-    lim_max(p, RLIMIT_DATA));
- PROC_UNLOCK(p);
+    lim_max(td, RLIMIT_DATA));
  }
 
  vm_object_reference(shmseg->object);
diff --git a/sys/kern/tty_pts.c b/sys/kern/tty_pts.c
index 2d1e8fe..fcc9c47 100644
--- a/sys/kern/tty_pts.c
+++ b/sys/kern/tty_pts.c
@@ -741,7 +741,7 @@ pts_alloc(int fflags, struct thread *td, struct file *fp)
  PROC_UNLOCK(p);
  return (EAGAIN);
  }
- ok = chgptscnt(cred->cr_ruidinfo, 1, lim_cur(p, RLIMIT_NPTS));
+ ok = chgptscnt(cred->cr_ruidinfo, 1, lim_cur(td, RLIMIT_NPTS));
  if (!ok) {
  racct_sub(p, RACCT_NPTS, 1);
  PROC_UNLOCK(p);
@@ -795,7 +795,7 @@ pts_alloc_external(int fflags, struct thread *td, struct file *fp,
  PROC_UNLOCK(p);
  return (EAGAIN);
  }
- ok = chgptscnt(cred->cr_ruidinfo, 1, lim_cur(p, RLIMIT_NPTS));
+ ok = chgptscnt(cred->cr_ruidinfo, 1, lim_cur(td, RLIMIT_NPTS));
  if (!ok) {
  racct_sub(p, RACCT_NPTS, 1);
  PROC_UNLOCK(p);
diff --git a/sys/kern/uipc_sockbuf.c b/sys/kern/uipc_sockbuf.c
index 88952ed..243450d 100644
--- a/sys/kern/uipc_sockbuf.c
+++ b/sys/kern/uipc_sockbuf.c
@@ -420,9 +420,7 @@ sbreserve_locked(struct sockbuf *sb, u_long cc, struct socket *so,
  if (cc > sb_max_adj)
  return (0);
  if (td != NULL) {
- PROC_LOCK(td->td_proc);
- sbsize_limit = lim_cur(td->td_proc, RLIMIT_SBSIZE);
- PROC_UNLOCK(td->td_proc);
+ sbsize_limit = lim_cur(td, RLIMIT_SBSIZE);
  } else
  sbsize_limit = RLIM_INFINITY;
  if (!chgsbsize(so->so_cred->cr_uidinfo, &sb->sb_hiwat, cc,
diff --git a/sys/kern/vfs_vnops.c b/sys/kern/vfs_vnops.c
index 01d448e..9db72c3 100644
--- a/sys/kern/vfs_vnops.c
+++ b/sys/kern/vfs_vnops.c
@@ -2098,19 +2098,18 @@ vn_vget_ino_gen(struct vnode *vp, vn_get_ino_t alloc, void *alloc_arg,
 
 int
 vn_rlimit_fsize(const struct vnode *vp, const struct uio *uio,
-    const struct thread *td)
+    struct thread *td)
 {
 
  if (vp->v_type != VREG || td == NULL)
  return (0);
- PROC_LOCK(td->td_proc);
  if ((uoff_t)uio->uio_offset + uio->uio_resid >
-    lim_cur(td->td_proc, RLIMIT_FSIZE)) {
+    lim_cur(td, RLIMIT_FSIZE)) {
+ PROC_LOCK(td->td_proc);
  kern_psignal(td->td_proc, SIGXFSZ);
  PROC_UNLOCK(td->td_proc);
  return (EFBIG);
  }
- PROC_UNLOCK(td->td_proc);
  return (0);
 }
 
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index f29d796..9d58550 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -247,6 +247,7 @@ struct thread {
  int td_intr_nesting_level; /* (k) Interrupt recursion. */
  int td_pinned; /* (k) Temporary cpu pin count. */
  struct ucred *td_ucred; /* (k) Reference to credentials. */
+ struct plimit *td_limit; /* (k) Resource limits. */
  u_int td_estcpu; /* (t) estimated cpu utilization */
  int td_slptick; /* (t) Time at sleep. */
  int td_blktick; /* (t) Time spent blocked. */
@@ -497,7 +498,7 @@ struct proc {
  struct filedesc *p_fd; /* (b) Open files. */
  struct filedesc_to_leader *p_fdtol; /* (b) Tracking node */
  struct pstats *p_stats; /* (b) Accounting/statistics (CPU). */
- struct plimit *p_limit; /* (c) Process limits. */
+ struct plimit *p_limit; /* (c) Resource limits. */
  struct callout p_limco; /* (c) Limit callout handle */
  struct sigacts *p_sigacts; /* (x) Signal actions, state (CPU). */
 
diff --git a/sys/sys/resourcevar.h b/sys/sys/resourcevar.h
index a07fdf8..426a27a 100644
--- a/sys/sys/resourcevar.h
+++ b/sys/sys/resourcevar.h
@@ -130,13 +130,14 @@ int kern_proc_setrlimit(struct thread *td, struct proc *p, u_int which,
 struct plimit
  *lim_alloc(void);
 void lim_copy(struct plimit *dst, struct plimit *src);
-rlim_t lim_cur(struct proc *p, int which);
+rlim_t lim_cur(struct thread *td, int which);
 void lim_fork(struct proc *p1, struct proc *p2);
 void lim_free(struct plimit *limp);
 struct plimit
  *lim_hold(struct plimit *limp);
-rlim_t lim_max(struct proc *p, int which);
-void lim_rlimit(struct proc *p, int which, struct rlimit *rlp);
+rlim_t lim_max(struct thread *td, int which);
+void lim_rlimit(struct thread *td, int which, struct rlimit *rlp);
+void lim_rlimit_proc(struct proc *p, int which, struct rlimit *rlp);
 void ruadd(struct rusage *ru, struct rusage_ext *rux, struct rusage *ru2,
     struct rusage_ext *rux2);
 void rucollect(struct rusage *ru, struct rusage *ru2);
@@ -156,5 +157,7 @@ void ui_racct_foreach(void (*callback)(struct racct *racct,
     void *arg2, void *arg3), void *arg2, void *arg3);
 #endif
 
+void lim_update_thread(struct thread *td);
+
 #endif /* _KERNEL */
 #endif /* !_SYS_RESOURCEVAR_H_ */
diff --git a/sys/sys/vnode.h b/sys/sys/vnode.h
index d70aa57..4aecd93 100644
--- a/sys/sys/vnode.h
+++ b/sys/sys/vnode.h
@@ -691,7 +691,7 @@ int vn_rdwr_inchunks(enum uio_rw rw, struct vnode *vp, void *base,
     struct ucred *active_cred, struct ucred *file_cred, size_t *aresid,
     struct thread *td);
 int vn_rlimit_fsize(const struct vnode *vn, const struct uio *uio,
-    const struct thread *td);
+    struct thread *td);
 int vn_stat(struct vnode *vp, struct stat *sb, struct ucred *active_cred,
     struct ucred *file_cred, struct thread *td);
 int vn_start_write(struct vnode *vp, struct mount **mpp, int flags);
diff --git a/sys/vm/swap_pager.c b/sys/vm/swap_pager.c
index 55e02c4..bdf55c5 100644
--- a/sys/vm/swap_pager.c
+++ b/sys/vm/swap_pager.c
@@ -222,16 +222,14 @@ swap_reserve_by_cred(vm_ooffset_t incr, struct ucred *cred)
  mtx_unlock(&sw_dev_mtx);
 
  if (res) {
- PROC_LOCK(curproc);
  UIDINFO_VMSIZE_LOCK(uip);
  if ((overcommit & SWAP_RESERVE_RLIMIT_ON) != 0 &&
-    uip->ui_vmsize + incr > lim_cur(curproc, RLIMIT_SWAP) &&
+    uip->ui_vmsize + incr > lim_cur(curthread, RLIMIT_SWAP) &&
     priv_check(curthread, PRIV_VM_SWAP_NORLIMIT))
  res = 0;
  else
  uip->ui_vmsize += incr;
  UIDINFO_VMSIZE_UNLOCK(uip);
- PROC_UNLOCK(curproc);
  if (!res) {
  mtx_lock(&sw_dev_mtx);
  swap_reserved -= incr;
diff --git a/sys/vm/vm_map.c b/sys/vm/vm_map.c
index b7e668b..225837f 100644
--- a/sys/vm/vm_map.c
+++ b/sys/vm/vm_map.c
@@ -3421,10 +3421,8 @@ vm_map_stack(vm_map_t map, vm_offset_t addrbos, vm_size_t max_ssize,
  growsize = sgrowsiz;
  init_ssize = (max_ssize < growsize) ? max_ssize : growsize;
  vm_map_lock(map);
- PROC_LOCK(curproc);
- lmemlim = lim_cur(curproc, RLIMIT_MEMLOCK);
- vmemlim = lim_cur(curproc, RLIMIT_VMEM);
- PROC_UNLOCK(curproc);
+ lmemlim = lim_cur(curthread, RLIMIT_MEMLOCK);
+ vmemlim = lim_cur(curthread, RLIMIT_VMEM);
  if (!old_mlock && map->flags & MAP_WIREFUTURE) {
  if (ptoa(pmap_wired_count(map->pmap)) + init_ssize > lmemlim) {
  rv = KERN_NO_SPACE;
@@ -3553,12 +3551,10 @@ vm_map_growstack(struct proc *p, vm_offset_t addr)
  int error;
 #endif
 
+ lmemlim = lim_cur(curthread, RLIMIT_MEMLOCK);
+ stacklim = lim_cur(curthread, RLIMIT_STACK);
+ vmemlim = lim_cur(curthread, RLIMIT_VMEM);
 Retry:
- PROC_LOCK(p);
- lmemlim = lim_cur(p, RLIMIT_MEMLOCK);
- stacklim = lim_cur(p, RLIMIT_STACK);
- vmemlim = lim_cur(p, RLIMIT_VMEM);
- PROC_UNLOCK(p);
 
  vm_map_lock_read(map);
 
diff --git a/sys/vm/vm_mmap.c b/sys/vm/vm_mmap.c
index 02634d6..adc7fba 100644
--- a/sys/vm/vm_mmap.c
+++ b/sys/vm/vm_mmap.c
@@ -325,14 +325,12 @@ sys_mmap(td, uap)
  * There should really be a pmap call to determine a reasonable
  * location.
  */
- PROC_LOCK(td->td_proc);
  if (addr == 0 ||
     (addr >= round_page((vm_offset_t)vms->vm_taddr) &&
     addr < round_page((vm_offset_t)vms->vm_daddr +
-    lim_max(td->td_proc, RLIMIT_DATA))))
+    lim_max(td, RLIMIT_DATA))))
  addr = round_page((vm_offset_t)vms->vm_daddr +
-    lim_max(td->td_proc, RLIMIT_DATA));
- PROC_UNLOCK(td->td_proc);
+    lim_max(td, RLIMIT_DATA));
  }
  if (flags & MAP_ANON) {
  /*
@@ -1112,13 +1110,9 @@ vm_mlock(struct proc *proc, struct ucred *cred, const void *addr0, size_t len)
  if (npages > vm_page_max_wired)
  return (ENOMEM);
  map = &proc->p_vmspace->vm_map;
- PROC_LOCK(proc);
  nsize = ptoa(npages + pmap_wired_count(map->pmap));
- if (nsize > lim_cur(proc, RLIMIT_MEMLOCK)) {
- PROC_UNLOCK(proc);
+ if (nsize > lim_cur(curthread, RLIMIT_MEMLOCK))
  return (ENOMEM);
- }
- PROC_UNLOCK(proc);
  if (npages + vm_cnt.v_wire_count > vm_page_max_wired)
  return (EAGAIN);
 #ifdef RACCT
@@ -1171,12 +1165,8 @@ sys_mlockall(td, uap)
  * a hard resource limit, return ENOMEM.
  */
  if (!old_mlock && uap->how & MCL_CURRENT) {
- PROC_LOCK(td->td_proc);
- if (map->size > lim_cur(td->td_proc, RLIMIT_MEMLOCK)) {
- PROC_UNLOCK(td->td_proc);
+ if (map->size > lim_cur(td, RLIMIT_MEMLOCK))
  return (ENOMEM);
- }
- PROC_UNLOCK(td->td_proc);
  }
 #ifdef RACCT
  PROC_LOCK(td->td_proc);
@@ -1551,21 +1541,29 @@ vm_mmap(vm_map_t map, vm_offset_t *addr, vm_size_t size, vm_prot_t prot,
  size = round_page(size);
 
  if (map == &td->td_proc->p_vmspace->vm_map) {
+#ifdef RACCT
  PROC_LOCK(td->td_proc);
- if (map->size + size > lim_cur(td->td_proc, RLIMIT_VMEM)) {
+#endif
+ if (map->size + size > lim_cur(td, RLIMIT_VMEM)) {
+#ifdef RACCT
  PROC_UNLOCK(td->td_proc);
+#endif
  return (ENOMEM);
  }
  if (racct_set(td->td_proc, RACCT_VMEM, map->size + size)) {
+#ifdef RACCT
  PROC_UNLOCK(td->td_proc);
+#endif
  return (ENOMEM);
  }
  if (!old_mlock && map->flags & MAP_WIREFUTURE) {
  if (ptoa(pmap_wired_count(map->pmap)) + size >
-    lim_cur(td->td_proc, RLIMIT_MEMLOCK)) {
+    lim_cur(td, RLIMIT_MEMLOCK)) {
  racct_set_force(td->td_proc, RACCT_VMEM,
     map->size);
+#ifdef RACCT
  PROC_UNLOCK(td->td_proc);
+#endif
  return (ENOMEM);
  }
  error = racct_set(td->td_proc, RACCT_MEMLOCK,
@@ -1573,11 +1571,15 @@ vm_mmap(vm_map_t map, vm_offset_t *addr, vm_size_t size, vm_prot_t prot,
  if (error != 0) {
  racct_set_force(td->td_proc, RACCT_VMEM,
     map->size);
+#ifdef RACCT
  PROC_UNLOCK(td->td_proc);
+#endif
  return (error);
  }
  }
+#ifdef RACCT
  PROC_UNLOCK(td->td_proc);
+#endif
  }
 
  /*
diff --git a/sys/vm/vm_pageout.c b/sys/vm/vm_pageout.c
index 6f50053..8225522 100644
--- a/sys/vm/vm_pageout.c
+++ b/sys/vm/vm_pageout.c
@@ -1844,7 +1844,7 @@ again:
  /*
  * get a limit
  */
- lim_rlimit(p, RLIMIT_RSS, &rsslim);
+ lim_rlimit_proc(p, RLIMIT_RSS, &rsslim);
  limit = OFF_TO_IDX(
     qmin(rsslim.rlim_cur, rsslim.rlim_max));
 
diff --git a/sys/vm/vm_unix.c b/sys/vm/vm_unix.c
index de9aa78..0e55ddf 100644
--- a/sys/vm/vm_unix.c
+++ b/sys/vm/vm_unix.c
@@ -83,11 +83,9 @@ sys_obreak(td, uap)
  int error = 0;
  boolean_t do_map_wirefuture;
 
- PROC_LOCK(td->td_proc);
- datalim = lim_cur(td->td_proc, RLIMIT_DATA);
- lmemlim = lim_cur(td->td_proc, RLIMIT_MEMLOCK);
- vmemlim = lim_cur(td->td_proc, RLIMIT_VMEM);
- PROC_UNLOCK(td->td_proc);
+ datalim = lim_cur(td, RLIMIT_DATA);
+ lmemlim = lim_cur(td, RLIMIT_MEMLOCK);
+ vmemlim = lim_cur(td, RLIMIT_VMEM);
 
  do_map_wirefuture = FALSE;
  new = round_page((vm_offset_t)uap->nsize);
--
2.3.6

_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] Generalised support for copy-on-write structures shared by threads.

Bruce Evans-4
In reply to this post by Mateusz Guzik
On Tue, 28 Apr 2015, Mateusz Guzik wrote:

> diff --git a/sys/amd64/amd64/trap.c b/sys/amd64/amd64/trap.c
> index 193d207..1883727 100644
> --- a/sys/amd64/amd64/trap.c
> +++ b/sys/amd64/amd64/trap.c
> @@ -257,8 +257,8 @@ trap(struct trapframe *frame)
> td->td_pticks = 0;
> td->td_frame = frame;
> addr = frame->tf_rip;
> - if (td->td_ucred != p->p_ucred)
> - cred_update_thread(td);
> + if (td->td_cowgeneration != p->p_cowgeneration)
> + thread_update_cow(td);
>
> switch (type) {
> case T_PRIVINFLT: /* privileged instruction fault */

This seems reasonable, but I don't like verbose names like p_cowgeneration.
It is especially bad to abbreviate "copy on write" to "cow" and then spell
"generation" in full.  "gen" would be a reasonable abbreviation, but "g"
goes better with "cow".

Old bad names visible in the patch include "thread" instead of "td".  "td"
is not such a good abbreviation for "thread pointer".

> diff --git a/sys/kern/kern_thr.c b/sys/kern/kern_thr.c
> index d5f1ce6..242e4dd 100644
> --- a/sys/kern/kern_thr.c
> +++ b/sys/kern/kern_thr.c

"thread" has too many different spellings.  For just file names, there
are kern_thr.c and kern_thread.c.  For variable names, there is also
"t" in "tid".  "tid" is the best of all the names mentioned so far.

> diff --git a/sys/sys/proc.h b/sys/sys/proc.h
> index 64b99fc..f29d796 100644
> --- a/sys/sys/proc.h
> +++ b/sys/sys/proc.h
> @@ -225,6 +225,7 @@ struct thread {
> /* Cleared during fork1() */
> #define td_startzero td_flags
> int td_flags; /* (t) TDF_* flags. */
> + u_int td_cowgeneration;/* (k) Generation of COW pointers. */
> int td_inhibitors; /* (t) Why can not run. */
> int td_pflags; /* (k) Private thread (TDP_*) flags. */
> int td_dupfd; /* (k) Ret value from fdopen. XXX */

This name is so verbose that it messes up the comment indentation.

> @@ -830,6 +832,11 @@ extern pid_t pid_max;
> KASSERT((p)->p_lock == 0, ("process held")); \
> } while (0)
>
> +#define PROC_UPDATE_COW(p) do { \
> + PROC_LOCK_ASSERT((p), MA_OWNED); \
> + p->p_cowgeneration++; \

Missing parentheses.

> +} while (0)
> +
> /* Check whether a thread is safe to be swapped out. */
> #define thread_safetoswapout(td) ((td)->td_flags & TDF_CANSWAP)
>
> @@ -976,6 +983,10 @@ struct thread *thread_alloc(int pages);
> int thread_alloc_stack(struct thread *, int pages);
> void thread_exit(void) __dead2;
> void thread_free(struct thread *td);
> +void thread_get_cow_proc(struct thread *newtd, struct proc *p);
> +void thread_get_cow(struct thread *newtd, struct thread *td);
> +void thread_free_cow(struct thread *td);
> +void thread_update_cow(struct thread *td);

Insertion sort errors.

Namespace errors.  I don't like the style of naming things with objects
first and verbs last, but it is good for sorting related objects.  Here
the verbs "get" and "free" are in the middle of the objects
"thread_cow_proc" and "thread_cow".  Also, shouldn't it be "thread_proc_cow"
(but less verbose, maybe "tpcow"), not "thread_cow_proc", to indicate
that the cow is hung of the proc?  I didn't notice the details, but it
makes no sense to hang a proc of a cow :-).

Bruce
_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] Generalised support for copy-on-write structures shared by threads.

Mateusz Guzik
On Tue, Apr 28, 2015 at 06:45:01PM +1000, Bruce Evans wrote:

> On Tue, 28 Apr 2015, Mateusz Guzik wrote:
> >diff --git a/sys/sys/proc.h b/sys/sys/proc.h
> >index 64b99fc..f29d796 100644
> >--- a/sys/sys/proc.h
> >+++ b/sys/sys/proc.h
> >@@ -225,6 +225,7 @@ struct thread {
> >/* Cleared during fork1() */
> >#define td_startzero td_flags
> > int td_flags; /* (t) TDF_* flags. */
> >+ u_int td_cowgeneration;/* (k) Generation of COW pointers. */
> > int td_inhibitors; /* (t) Why can not run. */
> > int td_pflags; /* (k) Private thread (TDP_*) flags. */
> > int td_dupfd; /* (k) Ret value from fdopen. XXX */
>
> This name is so verbose that it messes up the comment indentation.
>

Yeah, that's crap, but the naming is already inconsistent and verbose.
For instance there is td_generation alrady.

Is _cowgen variant ok?

> >@@ -830,6 +832,11 @@ extern pid_t pid_max;
> > KASSERT((p)->p_lock == 0, ("process held")); \
> >} while (0)
> >
> >+#define PROC_UPDATE_COW(p) do { \
> >+ PROC_LOCK_ASSERT((p), MA_OWNED); \
> >+ p->p_cowgeneration++; \
>
> Missing parentheses.

Oops, fixed.

>
> >+} while (0)
> >+
> >/* Check whether a thread is safe to be swapped out. */
> >#define thread_safetoswapout(td) ((td)->td_flags & TDF_CANSWAP)
> >
> >@@ -976,6 +983,10 @@ struct thread *thread_alloc(int pages);
> >int thread_alloc_stack(struct thread *, int pages);
> >void thread_exit(void) __dead2;
> >void thread_free(struct thread *td);
> >+void thread_get_cow_proc(struct thread *newtd, struct proc *p);
> >+void thread_get_cow(struct thread *newtd, struct thread *td);
> >+void thread_free_cow(struct thread *td);
> >+void thread_update_cow(struct thread *td);
>
> Insertion sort errors.
>
> Namespace errors.  I don't like the style of naming things with objects
> first and verbs last, but it is good for sorting related objects.  Here
> the verbs "get" and "free" are in the middle of the objects
> "thread_cow_proc" and "thread_cow".  Also, shouldn't it be "thread_proc_cow"
> (but less verbose, maybe "tpcow"), not "thread_cow_proc", to indicate
> that the cow is hung of the proc?  I didn't notice the details, but it
> makes no sense to hang a proc of a cow :-).
>

Well all current funcs are named thread_*, so tpcow and the like would
be inconsistent.

On another look existence of thread_suspend_* suggests thread_cow_*
naming.

With this putting _proc variant anywhere but at the end also breaks
consistency. 'thread_cow_from_proc' would increase verbosity.

That said, I would say the patch below is ok enough.

diff --git a/sys/amd64/amd64/trap.c b/sys/amd64/amd64/trap.c
index 193d207..cef3221 100644
--- a/sys/amd64/amd64/trap.c
+++ b/sys/amd64/amd64/trap.c
@@ -257,8 +257,8 @@ trap(struct trapframe *frame)
  td->td_pticks = 0;
  td->td_frame = frame;
  addr = frame->tf_rip;
- if (td->td_ucred != p->p_ucred)
- cred_update_thread(td);
+ if (td->td_cowgen != p->p_cowgen)
+ thread_cow_update(td);
 
  switch (type) {
  case T_PRIVINFLT: /* privileged instruction fault */
diff --git a/sys/arm/arm/trap-v6.c b/sys/arm/arm/trap-v6.c
index abafa86..7463d3c 100644
--- a/sys/arm/arm/trap-v6.c
+++ b/sys/arm/arm/trap-v6.c
@@ -394,8 +394,8 @@ abort_handler(struct trapframe *tf, int prefetch)
  p = td->td_proc;
  if (usermode) {
  td->td_pticks = 0;
- if (td->td_ucred != p->p_ucred)
- cred_update_thread(td);
+ if (td->td_cowgen != p->p_cowgen)
+ thread_cow_update(td);
  }
 
  /* Invoke the appropriate handler, if necessary. */
diff --git a/sys/arm/arm/trap.c b/sys/arm/arm/trap.c
index 0f142ce..d7fb73a 100644
--- a/sys/arm/arm/trap.c
+++ b/sys/arm/arm/trap.c
@@ -214,8 +214,8 @@ abort_handler(struct trapframe *tf, int type)
  if (user) {
  td->td_pticks = 0;
  td->td_frame = tf;
- if (td->td_ucred != td->td_proc->p_ucred)
- cred_update_thread(td);
+ if (td->td_cowgen != td->td_proc->p_cowgen)
+ thread_cow_update(td);
 
  }
  /* Grab the current pcb */
@@ -644,8 +644,8 @@ prefetch_abort_handler(struct trapframe *tf)
 
  if (TRAP_USERMODE(tf)) {
  td->td_frame = tf;
- if (td->td_ucred != td->td_proc->p_ucred)
- cred_update_thread(td);
+ if (td->td_cowgen != td->td_proc->p_cowgen)
+ thread_cow_update(td);
  }
  fault_pc = tf->tf_pc;
  if (td->td_md.md_spinlock_count == 0) {
diff --git a/sys/i386/i386/trap.c b/sys/i386/i386/trap.c
index d783a2b..b118e73 100644
--- a/sys/i386/i386/trap.c
+++ b/sys/i386/i386/trap.c
@@ -306,8 +306,8 @@ trap(struct trapframe *frame)
  td->td_pticks = 0;
  td->td_frame = frame;
  addr = frame->tf_eip;
- if (td->td_ucred != p->p_ucred)
- cred_update_thread(td);
+ if (td->td_cowgen != p->p_cowgen)
+ thread_cow_update(td);
 
  switch (type) {
  case T_PRIVINFLT: /* privileged instruction fault */
diff --git a/sys/kern/init_main.c b/sys/kern/init_main.c
index b77b788..e0042e9 100644
--- a/sys/kern/init_main.c
+++ b/sys/kern/init_main.c
@@ -522,8 +522,6 @@ proc0_init(void *dummy __unused)
 #ifdef MAC
  mac_cred_create_swapper(newcred);
 #endif
- td->td_ucred = crhold(newcred);
-
  /* Create sigacts. */
  p->p_sigacts = sigacts_alloc();
 
@@ -555,6 +553,10 @@ proc0_init(void *dummy __unused)
  p->p_limit->pl_rlimit[RLIMIT_MEMLOCK].rlim_max = pageablemem;
  p->p_cpulimit = RLIM_INFINITY;
 
+ PROC_LOCK(p);
+ thread_cow_get_proc(td, p);
+ PROC_UNLOCK(p);
+
  /* Initialize resource accounting structures. */
  racct_create(&p->p_racct);
 
@@ -842,10 +844,10 @@ create_init(const void *udata __unused)
  audit_cred_proc1(newcred);
 #endif
  proc_set_cred(initproc, newcred);
+ cred_update_thread(FIRST_THREAD_IN_PROC(initproc));
  PROC_UNLOCK(initproc);
  sx_xunlock(&proctree_lock);
  crfree(oldcred);
- cred_update_thread(FIRST_THREAD_IN_PROC(initproc));
  cpu_set_fork_handler(FIRST_THREAD_IN_PROC(initproc), start_init, NULL);
 }
 SYSINIT(init, SI_SUB_CREATE_INIT, SI_ORDER_FIRST, create_init, NULL);
diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c
index c3dd792..0dfecff 100644
--- a/sys/kern/kern_fork.c
+++ b/sys/kern/kern_fork.c
@@ -496,7 +496,6 @@ do_fork(struct thread *td, int flags, struct proc *p2, struct thread *td2,
  p2->p_swtick = ticks;
  if (p1->p_flag & P_PROFIL)
  startprofclock(p2);
- td2->td_ucred = crhold(p2->p_ucred);
 
  if (flags & RFSIGSHARE) {
  p2->p_sigacts = sigacts_hold(p1->p_sigacts);
@@ -526,6 +525,8 @@ do_fork(struct thread *td, int flags, struct proc *p2, struct thread *td2,
  */
  lim_fork(p1, p2);
 
+ thread_cow_get_proc(td2, p2);
+
  pstats_fork(p1->p_stats, p2->p_stats);
 
  PROC_UNLOCK(p1);
diff --git a/sys/kern/kern_kthread.c b/sys/kern/kern_kthread.c
index ee94de0..863bbc6 100644
--- a/sys/kern/kern_kthread.c
+++ b/sys/kern/kern_kthread.c
@@ -289,7 +289,7 @@ kthread_add(void (*func)(void *), void *arg, struct proc *p,
  cpu_set_fork_handler(newtd, func, arg);
 
  newtd->td_pflags |= TDP_KTHREAD;
- newtd->td_ucred = crhold(p->p_ucred);
+ thread_cow_get_proc(newtd, p);
 
  /* this code almost the same as create_thread() in kern_thr.c */
  p->p_flag |= P_HADTHREADS;
diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c
index 9c49f71..b531763 100644
--- a/sys/kern/kern_prot.c
+++ b/sys/kern/kern_prot.c
@@ -1946,9 +1946,8 @@ cred_update_thread(struct thread *td)
 
  p = td->td_proc;
  cred = td->td_ucred;
- PROC_LOCK(p);
+ PROC_LOCK_ASSERT(p, MA_OWNED);
  td->td_ucred = crhold(p->p_ucred);
- PROC_UNLOCK(p);
  if (cred != NULL)
  crfree(cred);
 }
@@ -1987,6 +1986,8 @@ proc_set_cred(struct proc *p, struct ucred *newcred)
 
  oldcred = p->p_ucred;
  p->p_ucred = newcred;
+ if (newcred != NULL)
+ PROC_UPDATE_COW(p);
  return (oldcred);
 }
 
diff --git a/sys/kern/kern_syscalls.c b/sys/kern/kern_syscalls.c
index dada746..3d3df01 100644
--- a/sys/kern/kern_syscalls.c
+++ b/sys/kern/kern_syscalls.c
@@ -31,6 +31,8 @@ __FBSDID("$FreeBSD$");
 #include <sys/kernel.h>
 #include <sys/lock.h>
 #include <sys/module.h>
+#include <sys/mutex.h>
+#include <sys/proc.h>
 #include <sys/sx.h>
 #include <sys/syscall.h>
 #include <sys/sysent.h>
diff --git a/sys/kern/kern_thr.c b/sys/kern/kern_thr.c
index 6911bb97..a53bd25 100644
--- a/sys/kern/kern_thr.c
+++ b/sys/kern/kern_thr.c
@@ -228,13 +228,13 @@ create_thread(struct thread *td, mcontext_t *ctx,
  bcopy(&td->td_startcopy, &newtd->td_startcopy,
     __rangeof(struct thread, td_startcopy, td_endcopy));
  newtd->td_proc = td->td_proc;
- newtd->td_ucred = crhold(td->td_ucred);
+ thread_cow_get(newtd, td);
 
  if (ctx != NULL) { /* old way to set user context */
  error = set_mcontext(newtd, ctx);
  if (error != 0) {
+ thread_cow_free(newtd);
  thread_free(newtd);
- crfree(td->td_ucred);
  goto fail;
  }
  } else {
@@ -246,8 +246,8 @@ create_thread(struct thread *td, mcontext_t *ctx,
  /* Setup user TLS address and TLS pointer register. */
  error = cpu_set_user_tls(newtd, tls_base);
  if (error != 0) {
+ thread_cow_free(newtd);
  thread_free(newtd);
- crfree(td->td_ucred);
  goto fail;
  }
  }
diff --git a/sys/kern/kern_thread.c b/sys/kern/kern_thread.c
index 0a93dbd..063dfe9 100644
--- a/sys/kern/kern_thread.c
+++ b/sys/kern/kern_thread.c
@@ -324,8 +324,7 @@ thread_reap(void)
  mtx_unlock_spin(&zombie_lock);
  while (td_first) {
  td_next = TAILQ_NEXT(td_first, td_slpq);
- if (td_first->td_ucred)
- crfree(td_first->td_ucred);
+ thread_cow_free(td_first);
  thread_free(td_first);
  td_first = td_next;
  }
@@ -381,6 +380,44 @@ thread_free(struct thread *td)
  uma_zfree(thread_zone, td);
 }
 
+void
+thread_cow_get_proc(struct thread *newtd, struct proc *p)
+{
+
+ PROC_LOCK_ASSERT(p, MA_OWNED);
+ newtd->td_ucred = crhold(p->p_ucred);
+ newtd->td_cowgen = p->p_cowgen;
+}
+
+void
+thread_cow_get(struct thread *newtd, struct thread *td)
+{
+
+ newtd->td_ucred = crhold(td->td_ucred);
+ newtd->td_cowgen = td->td_cowgen;
+}
+
+void
+thread_cow_free(struct thread *td)
+{
+
+ if (td->td_ucred)
+ crfree(td->td_ucred);
+}
+
+void
+thread_cow_update(struct thread *td)
+{
+ struct proc *p;
+
+ p = td->td_proc;
+ PROC_LOCK(p);
+ if (td->td_ucred != p->p_ucred)
+ cred_update_thread(td);
+ td->td_cowgen = p->p_cowgen;
+ PROC_UNLOCK(p);
+}
+
 /*
  * Discard the current thread and exit from its context.
  * Always called with scheduler locked.
@@ -518,7 +555,7 @@ thread_wait(struct proc *p)
  cpuset_rel(td->td_cpuset);
  td->td_cpuset = NULL;
  cpu_thread_clean(td);
- crfree(td->td_ucred);
+ thread_cow_free(td);
  thread_reap(); /* check for zombie threads etc. */
 }
 
diff --git a/sys/kern/subr_syscall.c b/sys/kern/subr_syscall.c
index 1bf78b8..070ba28 100644
--- a/sys/kern/subr_syscall.c
+++ b/sys/kern/subr_syscall.c
@@ -61,8 +61,8 @@ syscallenter(struct thread *td, struct syscall_args *sa)
  p = td->td_proc;
 
  td->td_pticks = 0;
- if (td->td_ucred != p->p_ucred)
- cred_update_thread(td);
+ if (td->td_cowgen != p->p_cowgen)
+ thread_cow_update(td);
  if (p->p_flag & P_TRACED) {
  traced = 1;
  PROC_LOCK(p);
diff --git a/sys/kern/subr_trap.c b/sys/kern/subr_trap.c
index 93f7557..e5e55dd 100644
--- a/sys/kern/subr_trap.c
+++ b/sys/kern/subr_trap.c
@@ -213,8 +213,8 @@ ast(struct trapframe *framep)
  thread_unlock(td);
  PCPU_INC(cnt.v_trap);
 
- if (td->td_ucred != p->p_ucred)
- cred_update_thread(td);
+ if (td->td_cowgen != p->p_cowgen)
+ thread_cow_update(td);
  if (td->td_pflags & TDP_OWEUPC && p->p_flag & P_PROFIL) {
  addupc_task(td, td->td_profil_addr, td->td_profil_ticks);
  td->td_profil_ticks = 0;
diff --git a/sys/powerpc/powerpc/trap.c b/sys/powerpc/powerpc/trap.c
index 0ceb170..bfbd94d 100644
--- a/sys/powerpc/powerpc/trap.c
+++ b/sys/powerpc/powerpc/trap.c
@@ -196,8 +196,8 @@ trap(struct trapframe *frame)
  if (user) {
  td->td_pticks = 0;
  td->td_frame = frame;
- if (td->td_ucred != p->p_ucred)
- cred_update_thread(td);
+ if (td->td_cowgen != p->p_cowgen)
+ thread_cow_update(td);
 
  /* User Mode Traps */
  switch (type) {
diff --git a/sys/sparc64/sparc64/trap.c b/sys/sparc64/sparc64/trap.c
index b4f0e27..e9917e5 100644
--- a/sys/sparc64/sparc64/trap.c
+++ b/sys/sparc64/sparc64/trap.c
@@ -277,8 +277,8 @@ trap(struct trapframe *tf)
  td->td_pticks = 0;
  td->td_frame = tf;
  addr = tf->tf_tpc;
- if (td->td_ucred != p->p_ucred)
- cred_update_thread(td);
+ if (td->td_cowgen != p->p_cowgen)
+ thread_cow_update(td);
 
  switch (tf->tf_type) {
  case T_DATA_MISS:
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index 64b99fc..5033957 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -225,6 +225,7 @@ struct thread {
 /* Cleared during fork1() */
 #define td_startzero td_flags
  int td_flags; /* (t) TDF_* flags. */
+ u_int td_cowgen; /* (k) Generation of COW pointers. */
  int td_inhibitors; /* (t) Why can not run. */
  int td_pflags; /* (k) Private thread (TDP_*) flags. */
  int td_dupfd; /* (k) Ret value from fdopen. XXX */
@@ -531,6 +532,7 @@ struct proc {
  pid_t p_oppid; /* (c + e) Save ppid in ptrace. XXX */
  struct vmspace *p_vmspace; /* (b) Address space. */
  u_int p_swtick; /* (c) Tick when swapped in or out. */
+ u_int p_cowgen; /* (c) Generation of COW pointers. */
  struct itimerval p_realtimer; /* (c) Alarm timer. */
  struct rusage p_ru; /* (a) Exit information. */
  struct rusage_ext p_rux; /* (cu) Internal resource usage. */
@@ -830,6 +832,11 @@ extern pid_t pid_max;
  KASSERT((p)->p_lock == 0, ("process held")); \
 } while (0)
 
+#define PROC_UPDATE_COW(p) do { \
+ PROC_LOCK_ASSERT((p), MA_OWNED); \
+ (p)->p_cowgen++; \
+} while (0)
+
 /* Check whether a thread is safe to be swapped out. */
 #define thread_safetoswapout(td) ((td)->td_flags & TDF_CANSWAP)
 
@@ -974,6 +981,10 @@ void cpu_thread_swapin(struct thread *);
 void cpu_thread_swapout(struct thread *);
 struct thread *thread_alloc(int pages);
 int thread_alloc_stack(struct thread *, int pages);
+void thread_cow_get_proc(struct thread *newtd, struct proc *p);
+void thread_cow_get(struct thread *newtd, struct thread *td);
+void thread_cow_free(struct thread *td);
+void thread_cow_update(struct thread *td);
 void thread_exit(void) __dead2;
 void thread_free(struct thread *td);
 void thread_link(struct thread *td, struct proc *p);
--
Mateusz Guzik <mjguzik gmail.com>
_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] Generalised support for copy-on-write structures shared by threads.

Julian Elischer-5
On 5/2/15 12:56 AM, Mateusz Guzik wrote:

> On Tue, Apr 28, 2015 at 06:45:01PM +1000, Bruce Evans wrote:
>> On Tue, 28 Apr 2015, Mateusz Guzik wrote:
>>> diff --git a/sys/sys/proc.h b/sys/sys/proc.h
>>> index 64b99fc..f29d796 100644
>>> --- a/sys/sys/proc.h
>>> +++ b/sys/sys/proc.h
>>> @@ -225,6 +225,7 @@ struct thread {
>>> /* Cleared during fork1() */
>>> #define td_startzero td_flags
>>> int td_flags; /* (t) TDF_* flags. */
>>> + u_int td_cowgeneration;/* (k) Generation of COW pointers. */
>>> int td_inhibitors; /* (t) Why can not run. */
>>> int td_pflags; /* (k) Private thread (TDP_*) flags. */
>>> int td_dupfd; /* (k) Ret value from fdopen. XXX */
>> This name is so verbose that it messes up the comment indentation.
>>
> Yeah, that's crap, but the naming is already inconsistent and verbose.
> For instance there is td_generation alrady.
>
> Is _cowgen variant ok?
td_cowgen is much preferable  with comment  "(k) generation of c.o.w.
pointers."

>
>>> @@ -830,6 +832,11 @@ extern pid_t pid_max;
>>> KASSERT((p)->p_lock == 0, ("process held")); \
>>> } while (0)
>>>
>>> +#define PROC_UPDATE_COW(p) do { \
>>> + PROC_LOCK_ASSERT((p), MA_OWNED); \
>>> + p->p_cowgeneration++; \
>> Missing parentheses.
> Oops, fixed.
>
>>> +} while (0)
>>> +
>>> /* Check whether a thread is safe to be swapped out. */
>>> #define thread_safetoswapout(td) ((td)->td_flags & TDF_CANSWAP)
>>>
>>> @@ -976,6 +983,10 @@ struct thread *thread_alloc(int pages);
>>> int thread_alloc_stack(struct thread *, int pages);
>>> void thread_exit(void) __dead2;
>>> void thread_free(struct thread *td);
>>> +void thread_get_cow_proc(struct thread *newtd, struct proc *p);
>>> +void thread_get_cow(struct thread *newtd, struct thread *td);
>>> +void thread_free_cow(struct thread *td);
>>> +void thread_update_cow(struct thread *td);
>> Insertion sort errors.
>>
>> Namespace errors.  I don't like the style of naming things with objects
>> first and verbs last, but it is good for sorting related objects.  Here
>> the verbs "get" and "free" are in the middle of the objects
>> "thread_cow_proc" and "thread_cow".  Also, shouldn't it be "thread_proc_cow"
>> (but less verbose, maybe "tpcow"), not "thread_cow_proc", to indicate
>> that the cow is hung of the proc?  I didn't notice the details, but it
>> makes no sense to hang a proc of a cow :-).
>>
> Well all current funcs are named thread_*, so tpcow and the like would
> be inconsistent.
>
> On another look existence of thread_suspend_* suggests thread_cow_*
> naming.
>
> With this putting _proc variant anywhere but at the end also breaks
> consistency. 'thread_cow_from_proc' would increase verbosity.
>
> That said, I would say the patch below is ok enough.
>
> diff --git a/sys/amd64/amd64/trap.c b/sys/amd64/amd64/trap.c
> index 193d207..cef3221 100644
> --- a/sys/amd64/amd64/trap.c
> +++ b/sys/amd64/amd64/trap.c
> @@ -257,8 +257,8 @@ trap(struct trapframe *frame)
>   td->td_pticks = 0;
>   td->td_frame = frame;
>   addr = frame->tf_rip;
> - if (td->td_ucred != p->p_ucred)
> - cred_update_thread(td);
> + if (td->td_cowgen != p->p_cowgen)
> + thread_cow_update(td);
>  
>   switch (type) {
>   case T_PRIVINFLT: /* privileged instruction fault */
> diff --git a/sys/arm/arm/trap-v6.c b/sys/arm/arm/trap-v6.c
> index abafa86..7463d3c 100644
> --- a/sys/arm/arm/trap-v6.c
> +++ b/sys/arm/arm/trap-v6.c
> @@ -394,8 +394,8 @@ abort_handler(struct trapframe *tf, int prefetch)
>   p = td->td_proc;
>   if (usermode) {
>   td->td_pticks = 0;
> - if (td->td_ucred != p->p_ucred)
> - cred_update_thread(td);
> + if (td->td_cowgen != p->p_cowgen)
> + thread_cow_update(td);
>   }
>  
>   /* Invoke the appropriate handler, if necessary. */
> diff --git a/sys/arm/arm/trap.c b/sys/arm/arm/trap.c
> index 0f142ce..d7fb73a 100644
> --- a/sys/arm/arm/trap.c
> +++ b/sys/arm/arm/trap.c
> @@ -214,8 +214,8 @@ abort_handler(struct trapframe *tf, int type)
>   if (user) {
>   td->td_pticks = 0;
>   td->td_frame = tf;
> - if (td->td_ucred != td->td_proc->p_ucred)
> - cred_update_thread(td);
> + if (td->td_cowgen != td->td_proc->p_cowgen)
> + thread_cow_update(td);
>  
>   }
>   /* Grab the current pcb */
> @@ -644,8 +644,8 @@ prefetch_abort_handler(struct trapframe *tf)
>  
>   if (TRAP_USERMODE(tf)) {
>   td->td_frame = tf;
> - if (td->td_ucred != td->td_proc->p_ucred)
> - cred_update_thread(td);
> + if (td->td_cowgen != td->td_proc->p_cowgen)
> + thread_cow_update(td);
>   }
>   fault_pc = tf->tf_pc;
>   if (td->td_md.md_spinlock_count == 0) {
> diff --git a/sys/i386/i386/trap.c b/sys/i386/i386/trap.c
> index d783a2b..b118e73 100644
> --- a/sys/i386/i386/trap.c
> +++ b/sys/i386/i386/trap.c
> @@ -306,8 +306,8 @@ trap(struct trapframe *frame)
>   td->td_pticks = 0;
>   td->td_frame = frame;
>   addr = frame->tf_eip;
> - if (td->td_ucred != p->p_ucred)
> - cred_update_thread(td);
> + if (td->td_cowgen != p->p_cowgen)
> + thread_cow_update(td);
>  
>   switch (type) {
>   case T_PRIVINFLT: /* privileged instruction fault */
> diff --git a/sys/kern/init_main.c b/sys/kern/init_main.c
> index b77b788..e0042e9 100644
> --- a/sys/kern/init_main.c
> +++ b/sys/kern/init_main.c
> @@ -522,8 +522,6 @@ proc0_init(void *dummy __unused)
>   #ifdef MAC
>   mac_cred_create_swapper(newcred);
>   #endif
> - td->td_ucred = crhold(newcred);
> -
>   /* Create sigacts. */
>   p->p_sigacts = sigacts_alloc();
>  
> @@ -555,6 +553,10 @@ proc0_init(void *dummy __unused)
>   p->p_limit->pl_rlimit[RLIMIT_MEMLOCK].rlim_max = pageablemem;
>   p->p_cpulimit = RLIM_INFINITY;
>  
> + PROC_LOCK(p);
> + thread_cow_get_proc(td, p);
> + PROC_UNLOCK(p);
> +
>   /* Initialize resource accounting structures. */
>   racct_create(&p->p_racct);
>  
> @@ -842,10 +844,10 @@ create_init(const void *udata __unused)
>   audit_cred_proc1(newcred);
>   #endif
>   proc_set_cred(initproc, newcred);
> + cred_update_thread(FIRST_THREAD_IN_PROC(initproc));
>   PROC_UNLOCK(initproc);
>   sx_xunlock(&proctree_lock);
>   crfree(oldcred);
> - cred_update_thread(FIRST_THREAD_IN_PROC(initproc));
>   cpu_set_fork_handler(FIRST_THREAD_IN_PROC(initproc), start_init, NULL);
>   }
>   SYSINIT(init, SI_SUB_CREATE_INIT, SI_ORDER_FIRST, create_init, NULL);
> diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c
> index c3dd792..0dfecff 100644
> --- a/sys/kern/kern_fork.c
> +++ b/sys/kern/kern_fork.c
> @@ -496,7 +496,6 @@ do_fork(struct thread *td, int flags, struct proc *p2, struct thread *td2,
>   p2->p_swtick = ticks;
>   if (p1->p_flag & P_PROFIL)
>   startprofclock(p2);
> - td2->td_ucred = crhold(p2->p_ucred);
>  
>   if (flags & RFSIGSHARE) {
>   p2->p_sigacts = sigacts_hold(p1->p_sigacts);
> @@ -526,6 +525,8 @@ do_fork(struct thread *td, int flags, struct proc *p2, struct thread *td2,
>   */
>   lim_fork(p1, p2);
>  
> + thread_cow_get_proc(td2, p2);
> +
>   pstats_fork(p1->p_stats, p2->p_stats);
>  
>   PROC_UNLOCK(p1);
> diff --git a/sys/kern/kern_kthread.c b/sys/kern/kern_kthread.c
> index ee94de0..863bbc6 100644
> --- a/sys/kern/kern_kthread.c
> +++ b/sys/kern/kern_kthread.c
> @@ -289,7 +289,7 @@ kthread_add(void (*func)(void *), void *arg, struct proc *p,
>   cpu_set_fork_handler(newtd, func, arg);
>  
>   newtd->td_pflags |= TDP_KTHREAD;
> - newtd->td_ucred = crhold(p->p_ucred);
> + thread_cow_get_proc(newtd, p);
>  
>   /* this code almost the same as create_thread() in kern_thr.c */
>   p->p_flag |= P_HADTHREADS;
> diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c
> index 9c49f71..b531763 100644
> --- a/sys/kern/kern_prot.c
> +++ b/sys/kern/kern_prot.c
> @@ -1946,9 +1946,8 @@ cred_update_thread(struct thread *td)
>  
>   p = td->td_proc;
>   cred = td->td_ucred;
> - PROC_LOCK(p);
> + PROC_LOCK_ASSERT(p, MA_OWNED);
>   td->td_ucred = crhold(p->p_ucred);
> - PROC_UNLOCK(p);
>   if (cred != NULL)
>   crfree(cred);
>   }
> @@ -1987,6 +1986,8 @@ proc_set_cred(struct proc *p, struct ucred *newcred)
>  
>   oldcred = p->p_ucred;
>   p->p_ucred = newcred;
> + if (newcred != NULL)
> + PROC_UPDATE_COW(p);
>   return (oldcred);
>   }
>  
> diff --git a/sys/kern/kern_syscalls.c b/sys/kern/kern_syscalls.c
> index dada746..3d3df01 100644
> --- a/sys/kern/kern_syscalls.c
> +++ b/sys/kern/kern_syscalls.c
> @@ -31,6 +31,8 @@ __FBSDID("$FreeBSD$");
>   #include <sys/kernel.h>
>   #include <sys/lock.h>
>   #include <sys/module.h>
> +#include <sys/mutex.h>
> +#include <sys/proc.h>
>   #include <sys/sx.h>
>   #include <sys/syscall.h>
>   #include <sys/sysent.h>
> diff --git a/sys/kern/kern_thr.c b/sys/kern/kern_thr.c
> index 6911bb97..a53bd25 100644
> --- a/sys/kern/kern_thr.c
> +++ b/sys/kern/kern_thr.c
> @@ -228,13 +228,13 @@ create_thread(struct thread *td, mcontext_t *ctx,
>   bcopy(&td->td_startcopy, &newtd->td_startcopy,
>      __rangeof(struct thread, td_startcopy, td_endcopy));
>   newtd->td_proc = td->td_proc;
> - newtd->td_ucred = crhold(td->td_ucred);
> + thread_cow_get(newtd, td);
>  
>   if (ctx != NULL) { /* old way to set user context */
>   error = set_mcontext(newtd, ctx);
>   if (error != 0) {
> + thread_cow_free(newtd);
>   thread_free(newtd);
> - crfree(td->td_ucred);
>   goto fail;
>   }
>   } else {
> @@ -246,8 +246,8 @@ create_thread(struct thread *td, mcontext_t *ctx,
>   /* Setup user TLS address and TLS pointer register. */
>   error = cpu_set_user_tls(newtd, tls_base);
>   if (error != 0) {
> + thread_cow_free(newtd);
>   thread_free(newtd);
> - crfree(td->td_ucred);
>   goto fail;
>   }
>   }
> diff --git a/sys/kern/kern_thread.c b/sys/kern/kern_thread.c
> index 0a93dbd..063dfe9 100644
> --- a/sys/kern/kern_thread.c
> +++ b/sys/kern/kern_thread.c
> @@ -324,8 +324,7 @@ thread_reap(void)
>   mtx_unlock_spin(&zombie_lock);
>   while (td_first) {
>   td_next = TAILQ_NEXT(td_first, td_slpq);
> - if (td_first->td_ucred)
> - crfree(td_first->td_ucred);
> + thread_cow_free(td_first);
>   thread_free(td_first);
>   td_first = td_next;
>   }
> @@ -381,6 +380,44 @@ thread_free(struct thread *td)
>   uma_zfree(thread_zone, td);
>   }
>  
> +void
> +thread_cow_get_proc(struct thread *newtd, struct proc *p)
> +{
> +
> + PROC_LOCK_ASSERT(p, MA_OWNED);
> + newtd->td_ucred = crhold(p->p_ucred);
> + newtd->td_cowgen = p->p_cowgen;
> +}
> +
> +void
> +thread_cow_get(struct thread *newtd, struct thread *td)
> +{
> +
> + newtd->td_ucred = crhold(td->td_ucred);
> + newtd->td_cowgen = td->td_cowgen;
> +}
> +
> +void
> +thread_cow_free(struct thread *td)
> +{
> +
> + if (td->td_ucred)
> + crfree(td->td_ucred);
> +}
> +
> +void
> +thread_cow_update(struct thread *td)
> +{
> + struct proc *p;
> +
> + p = td->td_proc;
> + PROC_LOCK(p);
> + if (td->td_ucred != p->p_ucred)
> + cred_update_thread(td);
> + td->td_cowgen = p->p_cowgen;
> + PROC_UNLOCK(p);
> +}
> +
>   /*
>    * Discard the current thread and exit from its context.
>    * Always called with scheduler locked.
> @@ -518,7 +555,7 @@ thread_wait(struct proc *p)
>   cpuset_rel(td->td_cpuset);
>   td->td_cpuset = NULL;
>   cpu_thread_clean(td);
> - crfree(td->td_ucred);
> + thread_cow_free(td);
>   thread_reap(); /* check for zombie threads etc. */
>   }
>  
> diff --git a/sys/kern/subr_syscall.c b/sys/kern/subr_syscall.c
> index 1bf78b8..070ba28 100644
> --- a/sys/kern/subr_syscall.c
> +++ b/sys/kern/subr_syscall.c
> @@ -61,8 +61,8 @@ syscallenter(struct thread *td, struct syscall_args *sa)
>   p = td->td_proc;
>  
>   td->td_pticks = 0;
> - if (td->td_ucred != p->p_ucred)
> - cred_update_thread(td);
> + if (td->td_cowgen != p->p_cowgen)
> + thread_cow_update(td);
>   if (p->p_flag & P_TRACED) {
>   traced = 1;
>   PROC_LOCK(p);
> diff --git a/sys/kern/subr_trap.c b/sys/kern/subr_trap.c
> index 93f7557..e5e55dd 100644
> --- a/sys/kern/subr_trap.c
> +++ b/sys/kern/subr_trap.c
> @@ -213,8 +213,8 @@ ast(struct trapframe *framep)
>   thread_unlock(td);
>   PCPU_INC(cnt.v_trap);
>  
> - if (td->td_ucred != p->p_ucred)
> - cred_update_thread(td);
> + if (td->td_cowgen != p->p_cowgen)
> + thread_cow_update(td);
>   if (td->td_pflags & TDP_OWEUPC && p->p_flag & P_PROFIL) {
>   addupc_task(td, td->td_profil_addr, td->td_profil_ticks);
>   td->td_profil_ticks = 0;
> diff --git a/sys/powerpc/powerpc/trap.c b/sys/powerpc/powerpc/trap.c
> index 0ceb170..bfbd94d 100644
> --- a/sys/powerpc/powerpc/trap.c
> +++ b/sys/powerpc/powerpc/trap.c
> @@ -196,8 +196,8 @@ trap(struct trapframe *frame)
>   if (user) {
>   td->td_pticks = 0;
>   td->td_frame = frame;
> - if (td->td_ucred != p->p_ucred)
> - cred_update_thread(td);
> + if (td->td_cowgen != p->p_cowgen)
> + thread_cow_update(td);
>  
>   /* User Mode Traps */
>   switch (type) {
> diff --git a/sys/sparc64/sparc64/trap.c b/sys/sparc64/sparc64/trap.c
> index b4f0e27..e9917e5 100644
> --- a/sys/sparc64/sparc64/trap.c
> +++ b/sys/sparc64/sparc64/trap.c
> @@ -277,8 +277,8 @@ trap(struct trapframe *tf)
>   td->td_pticks = 0;
>   td->td_frame = tf;
>   addr = tf->tf_tpc;
> - if (td->td_ucred != p->p_ucred)
> - cred_update_thread(td);
> + if (td->td_cowgen != p->p_cowgen)
> + thread_cow_update(td);
>  
>   switch (tf->tf_type) {
>   case T_DATA_MISS:
> diff --git a/sys/sys/proc.h b/sys/sys/proc.h
> index 64b99fc..5033957 100644
> --- a/sys/sys/proc.h
> +++ b/sys/sys/proc.h
> @@ -225,6 +225,7 @@ struct thread {
>   /* Cleared during fork1() */
>   #define td_startzero td_flags
>   int td_flags; /* (t) TDF_* flags. */
> + u_int td_cowgen; /* (k) Generation of COW pointers. */
>   int td_inhibitors; /* (t) Why can not run. */
>   int td_pflags; /* (k) Private thread (TDP_*) flags. */
>   int td_dupfd; /* (k) Ret value from fdopen. XXX */
> @@ -531,6 +532,7 @@ struct proc {
>   pid_t p_oppid; /* (c + e) Save ppid in ptrace. XXX */
>   struct vmspace *p_vmspace; /* (b) Address space. */
>   u_int p_swtick; /* (c) Tick when swapped in or out. */
> + u_int p_cowgen; /* (c) Generation of COW pointers. */
>   struct itimerval p_realtimer; /* (c) Alarm timer. */
>   struct rusage p_ru; /* (a) Exit information. */
>   struct rusage_ext p_rux; /* (cu) Internal resource usage. */
> @@ -830,6 +832,11 @@ extern pid_t pid_max;
>   KASSERT((p)->p_lock == 0, ("process held")); \
>   } while (0)
>  
> +#define PROC_UPDATE_COW(p) do { \
> + PROC_LOCK_ASSERT((p), MA_OWNED); \
> + (p)->p_cowgen++; \
> +} while (0)
> +
>   /* Check whether a thread is safe to be swapped out. */
>   #define thread_safetoswapout(td) ((td)->td_flags & TDF_CANSWAP)
>  
> @@ -974,6 +981,10 @@ void cpu_thread_swapin(struct thread *);
>   void cpu_thread_swapout(struct thread *);
>   struct thread *thread_alloc(int pages);
>   int thread_alloc_stack(struct thread *, int pages);
> +void thread_cow_get_proc(struct thread *newtd, struct proc *p);
> +void thread_cow_get(struct thread *newtd, struct thread *td);
> +void thread_cow_free(struct thread *td);
> +void thread_cow_update(struct thread *td);
>   void thread_exit(void) __dead2;
>   void thread_free(struct thread *td);
>   void thread_link(struct thread *td, struct proc *p);

_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] Generalised support for copy-on-write structures shared by threads.

Bruce Evans-4
In reply to this post by Mateusz Guzik
On Fri, 1 May 2015, Mateusz Guzik wrote:

> On Tue, Apr 28, 2015 at 06:45:01PM +1000, Bruce Evans wrote:
>> On Tue, 28 Apr 2015, Mateusz Guzik wrote:
>>> diff --git a/sys/sys/proc.h b/sys/sys/proc.h
>>> index 64b99fc..f29d796 100644
>>> --- a/sys/sys/proc.h
>>> +++ b/sys/sys/proc.h
>>> @@ -225,6 +225,7 @@ struct thread {
>>> /* Cleared during fork1() */
>>> #define td_startzero td_flags
>>> int td_flags; /* (t) TDF_* flags. */
>>> + u_int td_cowgeneration;/* (k) Generation of COW pointers. */
>>> int td_inhibitors; /* (t) Why can not run. */
>>> int td_pflags; /* (k) Private thread (TDP_*) flags. */
>>> int td_dupfd; /* (k) Ret value from fdopen. XXX */
>>
>> This name is so verbose that it messes up the comment indentation.
>
> Yeah, that's crap, but the naming is already inconsistent and verbose.
> For instance there is td_generation alrady.

td_generation is a much worse.  It looks like it might be a generation
counter for reused thread instances, but is actually just a thread
preemption counter.

> Is _cowgen variant ok?

Yes.  It is more verbose than _cowg, but easier to guess what it means.
Everyone knows what a cow is, but not what a g is.

However, since td_cowgen is not for threads generally, but just for a
couple of things hung off threads, perhaps its name should give a hint
about those things and not say anything about "cow".  I didn't notice
how you named these things.

>>> @@ -830,6 +832,11 @@ extern pid_t pid_max;
>>> KASSERT((p)->p_lock == 0, ("process held")); \
>>> } while (0)
>>>
>>> +#define PROC_UPDATE_COW(p) do { \
>>> + PROC_LOCK_ASSERT((p), MA_OWNED); \
>>> + p->p_cowgeneration++; \
>>
>> Missing parentheses.
>
> Oops, fixed.

Further naming problems.  This macro doesn't update cow things, but
just increases the generation count.

> That said, I would say the patch below is ok enough.

OK.

Bruce
_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] Generalised support for copy-on-write structures shared by threads.

Bruce Evans-4
In reply to this post by Julian Elischer-5
On Sun, 3 May 2015, Julian Elischer wrote:

> On 5/2/15 12:56 AM, Mateusz Guzik wrote:
>> On Tue, Apr 28, 2015 at 06:45:01PM +1000, Bruce Evans wrote:
>>> On Tue, 28 Apr 2015, Mateusz Guzik wrote:
>>>> diff --git a/sys/sys/proc.h b/sys/sys/proc.h
>>>> index 64b99fc..f29d796 100644
>>>> --- a/sys/sys/proc.h
>>>> +++ b/sys/sys/proc.h
>>>> @@ -225,6 +225,7 @@ struct thread {
>>>> /* Cleared during fork1() */
>>>> #define td_startzero td_flags
>>>> int td_flags; /* (t) TDF_* flags. */
>>>> + u_int td_cowgeneration;/* (k) Generation of COW pointers.
>>>> */
>>>> int td_inhibitors; /* (t) Why can not run. */
>>>> int td_pflags; /* (k) Private thread (TDP_*) flags.
>>>> */
>>>> int td_dupfd; /* (k) Ret value from fdopen. XXX */
>>> This name is so verbose that it messes up the comment indentation.
>>>
>> Yeah, that's crap, but the naming is already inconsistent and verbose.
>> For instance there is td_generation alrady.
>>
>> Is _cowgen variant ok?
> td_cowgen is much preferable  with comment  "(k) generation of c.o.w.
> pointers."

> + u_int td_cowgen; /* (k) generation of c.o.w. pointers. */

It is less preferable with such a comment.  The change in the comment just
adds 3 style bugs:
- inconsistent capitalization.  In this file, comments on struct members
   start with a capital letter
- punctuation for COW.  Acronyms are not punctuated.  Perhaps this one
   should be in lower case (like vm is usually in lower case).
- verboseness from the previous bug.  It expands the line line to 80.

But there is a problem with the comment.  It doesn't do much more than
echo the variable name with minor expansions and rearrangements.  The
only useful parts of it are "(k)" and "pointers".

Bruce
_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] Generalised support for copy-on-write structures shared by threads.

Julian Elischer-5
On 5/4/15 7:58 AM, Bruce Evans wrote:
> On Sun, 3 May 2015, Julian Elischer wrote: td_cowgen is much
> preferable  with comment  "(k) generation of c.o.w. pointers."
>
>> +    u_int        td_cowgen;    /* (k) generation of c.o.w.
>> pointers. */
>
> But there is a problem with the comment.  It doesn't do much more than
> echo the variable name with minor expansions and rearrangements. The
> only useful parts of it are "(k)" and "pointers".
cow is not immediately obvious to everyone.

>
> Bruce
>
>

_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "[hidden email]"