Re: kern_sched.c: unused functions

[Michael McConville]

Michal Mazurek wrote:
> On 17:19:39,  2.03.16, Martin Natano wrote:
> > On Wed, Mar 02, 2016 at 05:07:21PM +0100, Michal Mazurek wrote:
> > > kern_sched.c:
> > > - remove unused functions
> > > - mark static functions as static
> > 
> > Functions shouldn't be static in the kernel. "In userland,
> > functions local to one source module should be declared ???static???.  This
> > should not be done in kernel land since it makes it impossible to use the
> > kernel debugger." -- style(8)
> 
> Oh, right.
> 
> Remove cpuset_clear(), cpuset_add_all() and cpuset_union(), and move
> some declarations from proc.h to kern_sched.c.

ok mmcc@, pending an ok from someone who works with this code.

> Index: sys/kern/kern_sched.c
> ===
> RCS file: /cvs/src/sys/kern/kern_sched.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 kern_sched.c
> --- sys/kern/kern_sched.c 23 Dec 2015 14:51:17 -  1.41
> +++ sys/kern/kern_sched.c 2 Mar 2016 16:37:18 -
> @@ -28,11 +28,18 @@
>  
>  #include 
>  
> -void sched_kthreads_create(void *);
> -
> -int sched_proc_to_cpu_cost(struct cpu_info *ci, struct proc *p);
> +void  sched_kthreads_create(void *);
> +int   sched_proc_to_cpu_cost(struct cpu_info *ci, struct proc *p);
>  struct proc *sched_steal_proc(struct cpu_info *);
>  
> +void  cpuset_complement(struct cpuset *, struct cpuset *, struct cpuset *);
> +void  cpuset_copy(struct cpuset *, struct cpuset *);
> +struct cpu_info *cpuset_first(struct cpuset *);
> +void  cpuset_del(struct cpuset *, struct cpu_info *);
> +void  cpuset_init_cpu(struct cpu_info *);
> +void  cpuset_intersection(struct cpuset *t, struct cpuset *,
> + struct cpuset *);
> +
>  /*
>   * To help choosing which cpu should run which process we keep track
>   * of cpus which are currently idle and which cpus have processes
> @@ -717,12 +724,6 @@ cpuset_init_cpu(struct cpu_info *ci)
>  }
>  
>  void
> -cpuset_clear(struct cpuset *cs)
> -{
> - memset(cs, 0, sizeof(*cs));
> -}
> -
> -void
>  cpuset_add(struct cpuset *cs, struct cpu_info *ci)
>  {
>   unsigned int num = CPU_INFO_UNIT(ci);
> @@ -744,12 +745,6 @@ cpuset_isset(struct cpuset *cs, struct c
>  }
>  
>  void
> -cpuset_add_all(struct cpuset *cs)
> -{
> - cpuset_copy(cs, _all);
> -}
> -
> -void
>  cpuset_copy(struct cpuset *to, struct cpuset *from)
>  {
>   memcpy(to, from, sizeof(*to));
> @@ -765,15 +760,6 @@ cpuset_first(struct cpuset *cs)
>   return (cpuset_infos[i * 32 + ffs(cs->cs_set[i]) - 1]);
>  
>   return (NULL);
> -}
> -
> -void
> -cpuset_union(struct cpuset *to, struct cpuset *a, struct cpuset *b)
> -{
> - int i;
> -
> - for (i = 0; i < CPUSET_ASIZE(ncpus); i++)
> - to->cs_set[i] = a->cs_set[i] | b->cs_set[i];
>  }
>  
>  void
> Index: sys/sys/proc.h
> ===
> RCS file: /cvs/src/sys/sys/proc.h,v
> retrieving revision 1.213
> diff -u -p -r1.213 proc.h
> --- sys/sys/proc.h6 Dec 2015 17:50:21 -   1.213
> +++ sys/sys/proc.h2 Mar 2016 15:56:33 -
> @@ -205,9 +205,9 @@ struct process {
>  
>   struct uprof {  /* profile arguments */
>   caddr_t pr_base;/* buffer base */
> - size_t  pr_size;/* buffer size */
> + size_t  pr_size;/* buffer size */
>   u_long  pr_off; /* pc offset */
> - u_int   pr_scale;   /* pc scaling */
> + u_int   pr_scale;   /* pc scaling */
>   } ps_prof;
>  
>   u_short ps_acflag;  /* Accounting flags. */
> @@ -215,8 +215,8 @@ struct process {
>   uint64_t ps_pledge;
>   struct whitepaths *ps_pledgepaths;
>  
> - int64_t ps_kbind_cookie;
> - u_long  ps_kbind_addr;
> + int64_t ps_kbind_cookie;
> + u_long  ps_kbind_addr;
>  
>  /* End area that is copied on creation. */
>  #define ps_endcopy   ps_refcnt
> @@ -255,7 +255,7 @@ struct process {
>  #define  PS_EMBRYO   0x0002  /* New process, not yet fledged 
> */
>  #define  PS_ZOMBIE   0x0004  /* Dead and ready to be waited 
> for */
>  #define  PS_NOBROADCASTKILL 0x0008   /* Process excluded from kill 
> -1. */
> -#define PS_PLEDGE0x0010  /* Has called pledge(2) */
> +#define  PS_PLEDGE   0x0010  /* Has called pledge(2) */
>  
>  #define  PS_BITS \
>  ("\20" "\01CONTROLT" "\02EXEC" "\03INEXEC" "\04EXITING" "\05SUGID" \
> @@ -380,12 +380,12 @@ struct proc {
>  #define  P_WEXIT 0x2000  /* Working on exiting. */
>  #define  P_OWEUPC0x8000  /* Owe proc an addupc() at next 
> ast. */
>  #define  P_SUSPSINGLE0x0008  /* Need to stop for single 
> threading. */
> -#define P_SYSTRACE   0x0040  /* Process system call tracing active*/
> -#define P_CONTINUED  0x0080  /* Proc has continued from a stopped 
> 

Re: kern_sched.c: unused functions

[Mike Belopuhov]

On Wed, Mar 16, 2016 at 22:18 -0400, Michael McConville wrote:
> Michal Mazurek wrote:
> > On 17:19:39,  2.03.16, Martin Natano wrote:
> > > On Wed, Mar 02, 2016 at 05:07:21PM +0100, Michal Mazurek wrote:
> > > > kern_sched.c:
> > > > - remove unused functions
> > > > - mark static functions as static
> > > 
> > > Functions shouldn't be static in the kernel. "In userland,
> > > functions local to one source module should be declared ???static???.  
> > > This
> > > should not be done in kernel land since it makes it impossible to use the
> > > kernel debugger." -- style(8)
> > 
> > Oh, right.
> > 
> > Remove cpuset_clear(), cpuset_add_all() and cpuset_union(), and move
> > some declarations from proc.h to kern_sched.c.
> 
> ok mmcc@, pending an ok from someone who works with this code.
>

Art has implemented a complete API for sets (in a mathematical sense)
and I don't see how removing the set union operation helps anything.
The fact is that it's unused right now doesn't mean it's useless,
it just means that the cpuset API is complete in a way of simple
set operations.

I don't have any strong opinions on the rest of removed functions,
but cpuset_clear might be useful /in theory/.

Please don't remove cpuset_union.

> > Index: sys/kern/kern_sched.c
> > ===
> > RCS file: /cvs/src/sys/kern/kern_sched.c,v
> > retrieving revision 1.41
> > diff -u -p -r1.41 kern_sched.c
> > --- sys/kern/kern_sched.c   23 Dec 2015 14:51:17 -  1.41
> > +++ sys/kern/kern_sched.c   2 Mar 2016 16:37:18 -
> > @@ -28,11 +28,18 @@
> >  
> >  #include 
> >  
> > -void sched_kthreads_create(void *);
> > -
> > -int sched_proc_to_cpu_cost(struct cpu_info *ci, struct proc *p);
> > +voidsched_kthreads_create(void *);
> > +int sched_proc_to_cpu_cost(struct cpu_info *ci, struct proc *p);
> >  struct proc *sched_steal_proc(struct cpu_info *);
> >  
> > +voidcpuset_complement(struct cpuset *, struct cpuset *, struct 
> > cpuset *);
> > +voidcpuset_copy(struct cpuset *, struct cpuset *);
> > +struct cpu_info *cpuset_first(struct cpuset *);
> > +voidcpuset_del(struct cpuset *, struct cpu_info *);
> > +voidcpuset_init_cpu(struct cpu_info *);
> > +voidcpuset_intersection(struct cpuset *t, struct cpuset *,
> > +   struct cpuset *);
> > +
> >  /*
> >   * To help choosing which cpu should run which process we keep track
> >   * of cpus which are currently idle and which cpus have processes
> > @@ -717,12 +724,6 @@ cpuset_init_cpu(struct cpu_info *ci)
> >  }
> >  
> >  void
> > -cpuset_clear(struct cpuset *cs)
> > -{
> > -   memset(cs, 0, sizeof(*cs));
> > -}
> > -
> > -void
> >  cpuset_add(struct cpuset *cs, struct cpu_info *ci)
> >  {
> > unsigned int num = CPU_INFO_UNIT(ci);
> > @@ -744,12 +745,6 @@ cpuset_isset(struct cpuset *cs, struct c
> >  }
> >  
> >  void
> > -cpuset_add_all(struct cpuset *cs)
> > -{
> > -   cpuset_copy(cs, _all);
> > -}
> > -
> > -void
> >  cpuset_copy(struct cpuset *to, struct cpuset *from)
> >  {
> > memcpy(to, from, sizeof(*to));
> > @@ -765,15 +760,6 @@ cpuset_first(struct cpuset *cs)
> > return (cpuset_infos[i * 32 + ffs(cs->cs_set[i]) - 1]);
> >  
> > return (NULL);
> > -}
> > -
> > -void
> > -cpuset_union(struct cpuset *to, struct cpuset *a, struct cpuset *b)
> > -{
> > -   int i;
> > -
> > -   for (i = 0; i < CPUSET_ASIZE(ncpus); i++)
> > -   to->cs_set[i] = a->cs_set[i] | b->cs_set[i];
> >  }
> >  
> >  void
> > Index: sys/sys/proc.h
> > ===
> > RCS file: /cvs/src/sys/sys/proc.h,v
> > retrieving revision 1.213
> > diff -u -p -r1.213 proc.h
> > --- sys/sys/proc.h  6 Dec 2015 17:50:21 -   1.213
> > +++ sys/sys/proc.h  2 Mar 2016 15:56:33 -
> > @@ -205,9 +205,9 @@ struct process {
> >  
> > struct uprof {  /* profile arguments */
> > caddr_t pr_base;/* buffer base */
> > -   size_t  pr_size;/* buffer size */
> > +   size_t  pr_size;/* buffer size */
> > u_long  pr_off; /* pc offset */
> > -   u_int   pr_scale;   /* pc scaling */
> > +   u_int   pr_scale;   /* pc scaling */
> > } ps_prof;
> >  
> > u_short ps_acflag;  /* Accounting flags. */
> > @@ -215,8 +215,8 @@ struct process {
> > uint64_t ps_pledge;
> > struct whitepaths *ps_pledgepaths;
> >  
> > -   int64_t ps_kbind_cookie;
> > -   u_long  ps_kbind_addr;
> > +   int64_t ps_kbind_cookie;
> > +   u_long  ps_kbind_addr;
> >  
> >  /* End area that is copied on creation. */
> >  #define ps_endcopy ps_refcnt
> > @@ -255,7 +255,7 @@ struct process {
> >  #definePS_EMBRYO   0x0002  /* New process, not yet fledged 
> > */
> >  #definePS_ZOMBIE   0x0004  /* Dead and ready to be waited 
> > for */
> >  #definePS_NOBROADCASTKILL 0x0008   /* 

Re: kern_sched.c: unused functions

[Mark Kettenis]

> Date: Wed, 16 Mar 2016 22:18:41 -0400
> From: Michael McConville 
> 
> Michal Mazurek wrote:
> > On 17:19:39,  2.03.16, Martin Natano wrote:
> > > On Wed, Mar 02, 2016 at 05:07:21PM +0100, Michal Mazurek wrote:
> > > > kern_sched.c:
> > > > - remove unused functions
> > > > - mark static functions as static
> > > 
> > > Functions shouldn't be static in the kernel. "In userland,
> > > functions local to one source module should be declared ???static???.  
> > > This
> > > should not be done in kernel land since it makes it impossible to use the
> > > kernel debugger." -- style(8)
> > 
> > Oh, right.
> > 
> > Remove cpuset_clear(), cpuset_add_all() and cpuset_union(), and move
> > some declarations from proc.h to kern_sched.c.
> 
> ok mmcc@, pending an ok from someone who works with this code.

Leave them alone.  It makes sense to have the complete set of
operations implemented, even if they aren't currently used.

> > Index: sys/kern/kern_sched.c
> > ===
> > RCS file: /cvs/src/sys/kern/kern_sched.c,v
> > retrieving revision 1.41
> > diff -u -p -r1.41 kern_sched.c
> > --- sys/kern/kern_sched.c   23 Dec 2015 14:51:17 -  1.41
> > +++ sys/kern/kern_sched.c   2 Mar 2016 16:37:18 -
> > @@ -28,11 +28,18 @@
> >  
> >  #include 
> >  
> > -void sched_kthreads_create(void *);
> > -
> > -int sched_proc_to_cpu_cost(struct cpu_info *ci, struct proc *p);
> > +voidsched_kthreads_create(void *);
> > +int sched_proc_to_cpu_cost(struct cpu_info *ci, struct proc *p);
> >  struct proc *sched_steal_proc(struct cpu_info *);
> >  
> > +voidcpuset_complement(struct cpuset *, struct cpuset *, struct 
> > cpuset *);
> > +voidcpuset_copy(struct cpuset *, struct cpuset *);
> > +struct cpu_info *cpuset_first(struct cpuset *);
> > +voidcpuset_del(struct cpuset *, struct cpu_info *);
> > +voidcpuset_init_cpu(struct cpu_info *);
> > +voidcpuset_intersection(struct cpuset *t, struct cpuset *,
> > +   struct cpuset *);
> > +
> >  /*
> >   * To help choosing which cpu should run which process we keep track
> >   * of cpus which are currently idle and which cpus have processes
> > @@ -717,12 +724,6 @@ cpuset_init_cpu(struct cpu_info *ci)
> >  }
> >  
> >  void
> > -cpuset_clear(struct cpuset *cs)
> > -{
> > -   memset(cs, 0, sizeof(*cs));
> > -}
> > -
> > -void
> >  cpuset_add(struct cpuset *cs, struct cpu_info *ci)
> >  {
> > unsigned int num = CPU_INFO_UNIT(ci);
> > @@ -744,12 +745,6 @@ cpuset_isset(struct cpuset *cs, struct c
> >  }
> >  
> >  void
> > -cpuset_add_all(struct cpuset *cs)
> > -{
> > -   cpuset_copy(cs, _all);
> > -}
> > -
> > -void
> >  cpuset_copy(struct cpuset *to, struct cpuset *from)
> >  {
> > memcpy(to, from, sizeof(*to));
> > @@ -765,15 +760,6 @@ cpuset_first(struct cpuset *cs)
> > return (cpuset_infos[i * 32 + ffs(cs->cs_set[i]) - 1]);
> >  
> > return (NULL);
> > -}
> > -
> > -void
> > -cpuset_union(struct cpuset *to, struct cpuset *a, struct cpuset *b)
> > -{
> > -   int i;
> > -
> > -   for (i = 0; i < CPUSET_ASIZE(ncpus); i++)
> > -   to->cs_set[i] = a->cs_set[i] | b->cs_set[i];
> >  }
> >  
> >  void
> > Index: sys/sys/proc.h
> > ===
> > RCS file: /cvs/src/sys/sys/proc.h,v
> > retrieving revision 1.213
> > diff -u -p -r1.213 proc.h
> > --- sys/sys/proc.h  6 Dec 2015 17:50:21 -   1.213
> > +++ sys/sys/proc.h  2 Mar 2016 15:56:33 -
> > @@ -205,9 +205,9 @@ struct process {
> >  
> > struct uprof {  /* profile arguments */
> > caddr_t pr_base;/* buffer base */
> > -   size_t  pr_size;/* buffer size */
> > +   size_t  pr_size;/* buffer size */
> > u_long  pr_off; /* pc offset */
> > -   u_int   pr_scale;   /* pc scaling */
> > +   u_int   pr_scale;   /* pc scaling */
> > } ps_prof;
> >  
> > u_short ps_acflag;  /* Accounting flags. */
> > @@ -215,8 +215,8 @@ struct process {
> > uint64_t ps_pledge;
> > struct whitepaths *ps_pledgepaths;
> >  
> > -   int64_t ps_kbind_cookie;
> > -   u_long  ps_kbind_addr;
> > +   int64_t ps_kbind_cookie;
> > +   u_long  ps_kbind_addr;
> >  
> >  /* End area that is copied on creation. */
> >  #define ps_endcopy ps_refcnt
> > @@ -255,7 +255,7 @@ struct process {
> >  #definePS_EMBRYO   0x0002  /* New process, not yet fledged 
> > */
> >  #definePS_ZOMBIE   0x0004  /* Dead and ready to be waited 
> > for */
> >  #definePS_NOBROADCASTKILL 0x0008   /* Process excluded from kill 
> > -1. */
> > -#define PS_PLEDGE  0x0010  /* Has called pledge(2) */
> > +#definePS_PLEDGE   0x0010  /* Has called pledge(2) */
> >  
> >  #definePS_BITS \
> >  ("\20" "\01CONTROLT" "\02EXEC" "\03INEXEC" "\04EXITING" "\05SUGID" \
> 

Re: kern_sched.c: unused functions

[Martin Natano]

On Wed, Mar 02, 2016 at 05:07:21PM +0100, Michal Mazurek wrote:
> kern_sched.c:
> - remove unused functions
> - mark static functions as static

Functions shouldn't be static in the kernel. "In userland,
functions local to one source module should be declared ‘static’.  This
should not be done in kernel land since it makes it impossible to use the
kernel debugger." -- style(8)

kern_sched.c: unused functions

[Michal Mazurek]

kern_sched.c:
- remove unused functions
- mark static functions as static

proc.h:
- remove declarations
- convert spaces to tabs
- remove trailing empty line

Index: sys/kern/kern_sched.c
===
RCS file: /cvs/src/sys/kern/kern_sched.c,v
retrieving revision 1.41
diff -u -p -r1.41 kern_sched.c
--- sys/kern/kern_sched.c   23 Dec 2015 14:51:17 -  1.41
+++ sys/kern/kern_sched.c   2 Mar 2016 15:56:33 -
@@ -28,10 +28,18 @@
 
 #include 
 
-void sched_kthreads_create(void *);
-
-int sched_proc_to_cpu_cost(struct cpu_info *ci, struct proc *p);
-struct proc *sched_steal_proc(struct cpu_info *);
+static void sched_kthreads_create(void *);
+static int  sched_proc_to_cpu_cost(struct cpu_info *ci, struct proc *p);
+static struct proc *sched_steal_proc(struct cpu_info *);
+
+static void cpuset_complement(struct cpuset *, struct cpuset *,
+   struct cpuset *);
+static void cpuset_copy(struct cpuset *, struct cpuset *);
+static struct cpu_info *cpuset_first(struct cpuset *);
+static void cpuset_del(struct cpuset *, struct cpu_info *);
+static void cpuset_init_cpu(struct cpu_info *);
+static void cpuset_intersection(struct cpuset *t, struct cpuset *,
+   struct cpuset *);
 
 /*
  * To help choosing which cpu should run which process we keep track
@@ -100,7 +108,7 @@ sched_init_cpu(struct cpu_info *ci)
cpuset_add(_all_cpus, ci);
 }
 
-void
+static void
 sched_kthreads_create(void *v)
 {
struct cpu_info *ci = v;
@@ -454,7 +462,7 @@ sched_choosecpu(struct proc *p)
 /*
  * Attempt to steal a proc from some cpu.
  */
-struct proc *
+static struct proc *
 sched_steal_proc(struct cpu_info *self)
 {
struct proc *best = NULL;
@@ -534,7 +542,7 @@ int sched_cost_runnable = 3;
 int sched_cost_resident = 1;
 #endif
 
-int
+static int
 sched_proc_to_cpu_cost(struct cpu_info *ci, struct proc *p)
 {
int cost = 0;
@@ -709,7 +717,7 @@ sched_barrier(struct cpu_info *ci)
 struct cpu_info *cpuset_infos[MAXCPUS];
 static struct cpuset cpuset_all;
 
-void
+static void
 cpuset_init_cpu(struct cpu_info *ci)
 {
cpuset_add(_all, ci);
@@ -717,19 +725,13 @@ cpuset_init_cpu(struct cpu_info *ci)
 }
 
 void
-cpuset_clear(struct cpuset *cs)
-{
-   memset(cs, 0, sizeof(*cs));
-}
-
-void
 cpuset_add(struct cpuset *cs, struct cpu_info *ci)
 {
unsigned int num = CPU_INFO_UNIT(ci);
atomic_setbits_int(>cs_set[num/32], (1 << (num % 32)));
 }
 
-void
+static void
 cpuset_del(struct cpuset *cs, struct cpu_info *ci)
 {
unsigned int num = CPU_INFO_UNIT(ci);
@@ -743,19 +745,13 @@ cpuset_isset(struct cpuset *cs, struct c
return (cs->cs_set[num/32] & (1 << (num % 32)));
 }
 
-void
-cpuset_add_all(struct cpuset *cs)
-{
-   cpuset_copy(cs, _all);
-}
-
-void
+static void
 cpuset_copy(struct cpuset *to, struct cpuset *from)
 {
memcpy(to, from, sizeof(*to));
 }
 
-struct cpu_info *
+static struct cpu_info *
 cpuset_first(struct cpuset *cs)
 {
int i;
@@ -767,16 +763,7 @@ cpuset_first(struct cpuset *cs)
return (NULL);
 }
 
-void
-cpuset_union(struct cpuset *to, struct cpuset *a, struct cpuset *b)
-{
-   int i;
-
-   for (i = 0; i < CPUSET_ASIZE(ncpus); i++)
-   to->cs_set[i] = a->cs_set[i] | b->cs_set[i];
-}
-
-void
+static void
 cpuset_intersection(struct cpuset *to, struct cpuset *a, struct cpuset *b)
 {
int i;
@@ -785,7 +772,7 @@ cpuset_intersection(struct cpuset *to, s
to->cs_set[i] = a->cs_set[i] & b->cs_set[i];
 }
 
-void
+static void
 cpuset_complement(struct cpuset *to, struct cpuset *a, struct cpuset *b)
 {
int i;
Index: sys/sys/proc.h
===
RCS file: /cvs/src/sys/sys/proc.h,v
retrieving revision 1.213
diff -u -p -r1.213 proc.h
--- sys/sys/proc.h  6 Dec 2015 17:50:21 -   1.213
+++ sys/sys/proc.h  2 Mar 2016 15:56:33 -
@@ -205,9 +205,9 @@ struct process {
 
struct uprof {  /* profile arguments */
caddr_t pr_base;/* buffer base */
-   size_t  pr_size;/* buffer size */
+   size_t  pr_size;/* buffer size */
u_long  pr_off; /* pc offset */
-   u_int   pr_scale;   /* pc scaling */
+   u_int   pr_scale;   /* pc scaling */
} ps_prof;
 
u_short ps_acflag;  /* Accounting flags. */
@@ -215,8 +215,8 @@ struct process {
uint64_t ps_pledge;
struct whitepaths *ps_pledgepaths;
 
-   int64_t ps_kbind_cookie;
-   u_long  ps_kbind_addr;
+   int64_t ps_kbind_cookie;
+   u_long  ps_kbind_addr;
 
 /* End area that is copied on creation. */
 #define ps_endcopy ps_refcnt
@@ -255,7 +255,7 @@ struct process {
 #definePS_EMBRYO   0x0002  /* New process, not yet fledged 
*/
 #define