Re: [patch 1/8] Add rt_nr_running accounting

2007-10-20 Thread Steven Rostedt 

Hi Dmitry,

--
On Sat, 20 Oct 2007, Dmitry Adamushko wrote:

> On 19/10/2007, Steven Rostedt <[EMAIL PROTECTED]> wrote:
> > [ ... ]
> > Index: linux-test.git/kernel/sched.c
> > ===
> > --- linux-test.git.orig/kernel/sched.c  2007-10-19 12:32:39.0 -0400
> > +++ linux-test.git/kernel/sched.c   2007-10-19 12:33:09.0 -0400
> > @@ -300,6 +300,8 @@ struct rq {
> >  */
> > unsigned long nr_uninterruptible;
> >
> > +   unsigned long rt_nr_running;
>
> could it be a part of the 'struct rt_rq' instead?

Maybe. I didn't really look too hard to where to put it. Currently, in the
-rt patch, it is located in struct rq, so I just did the same. I may be
able to move it.

>
> >
> > +static inline void inc_rt_tasks(struct task_struct *p, struct rq *rq)
> > +{
> > +   if (rt_task(p))
> > +   rq->rt_nr_running++;
> > +}
> > +
> > +static inline void dec_rt_tasks(struct task_struct *p, struct rq *rq)
> > +{
> > +   if (rt_task(p)) {
> > +   WARN_ON(!rq->rt_nr_running);
> > +   rq->rt_nr_running--;
> > +   }
> > +}
> > +
> >  static void enqueue_task_rt(struct rq *rq, struct task_struct *p, int 
> > wakeup)
> >  {
> > struct rt_prio_array *array = >rt.active;
> >
> > list_add_tail(>run_list, array->queue + p->prio);
> > __set_bit(p->prio, array->bitmap);
> > +
> > +   inc_rt_tasks(p, rq);
>
> why do you need the rt_task(p) check in {inc,dec}_rt_tasks() ?

Me being paranoid ;-)

>
> {enqueue,dequeue}_task_rt() seem to be the only callers and they will
> crash (or corrupt memory) anyway in the case of ! rt_task(p) (sure,
> this case would mean something is broken somewhere wrt sched_class
> handling).

Exactly, I was just being safe. We could add a WARN_ON(!rt_task) there.

-- Steve

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch 1/8] Add rt_nr_running accounting

2007-10-20 Thread Dmitry Adamushko 
On 19/10/2007, Steven Rostedt <[EMAIL PROTECTED]> wrote:
> [ ... ]
> Index: linux-test.git/kernel/sched.c
> ===
> --- linux-test.git.orig/kernel/sched.c  2007-10-19 12:32:39.0 -0400
> +++ linux-test.git/kernel/sched.c   2007-10-19 12:33:09.0 -0400
> @@ -300,6 +300,8 @@ struct rq {
>  */
> unsigned long nr_uninterruptible;
>
> +   unsigned long rt_nr_running;

could it be a part of the 'struct rt_rq' instead?

>
> +static inline void inc_rt_tasks(struct task_struct *p, struct rq *rq)
> +{
> +   if (rt_task(p))
> +   rq->rt_nr_running++;
> +}
> +
> +static inline void dec_rt_tasks(struct task_struct *p, struct rq *rq)
> +{
> +   if (rt_task(p)) {
> +   WARN_ON(!rq->rt_nr_running);
> +   rq->rt_nr_running--;
> +   }
> +}
> +
>  static void enqueue_task_rt(struct rq *rq, struct task_struct *p, int wakeup)
>  {
> struct rt_prio_array *array = >rt.active;
>
> list_add_tail(>run_list, array->queue + p->prio);
> __set_bit(p->prio, array->bitmap);
> +
> +   inc_rt_tasks(p, rq);

why do you need the rt_task(p) check in {inc,dec}_rt_tasks() ?

{enqueue,dequeue}_task_rt() seem to be the only callers and they will
crash (or corrupt memory) anyway in the case of ! rt_task(p) (sure,
this case would mean something is broken somewhere wrt sched_class
handling).


-- 
Best regards,
Dmitry Adamushko
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch 1/8] Add rt_nr_running accounting

2007-10-20 Thread Dmitry Adamushko 
On 19/10/2007, Steven Rostedt [EMAIL PROTECTED] wrote:
 [ ... ]
 Index: linux-test.git/kernel/sched.c
 ===
 --- linux-test.git.orig/kernel/sched.c  2007-10-19 12:32:39.0 -0400
 +++ linux-test.git/kernel/sched.c   2007-10-19 12:33:09.0 -0400
 @@ -300,6 +300,8 @@ struct rq {
  */
 unsigned long nr_uninterruptible;

 +   unsigned long rt_nr_running;

could it be a part of the 'struct rt_rq' instead?


 +static inline void inc_rt_tasks(struct task_struct *p, struct rq *rq)
 +{
 +   if (rt_task(p))
 +   rq-rt_nr_running++;
 +}
 +
 +static inline void dec_rt_tasks(struct task_struct *p, struct rq *rq)
 +{
 +   if (rt_task(p)) {
 +   WARN_ON(!rq-rt_nr_running);
 +   rq-rt_nr_running--;
 +   }
 +}
 +
  static void enqueue_task_rt(struct rq *rq, struct task_struct *p, int wakeup)
  {
 struct rt_prio_array *array = rq-rt.active;

 list_add_tail(p-run_list, array-queue + p-prio);
 __set_bit(p-prio, array-bitmap);
 +
 +   inc_rt_tasks(p, rq);

why do you need the rt_task(p) check in {inc,dec}_rt_tasks() ?

{enqueue,dequeue}_task_rt() seem to be the only callers and they will
crash (or corrupt memory) anyway in the case of ! rt_task(p) (sure,
this case would mean something is broken somewhere wrt sched_class
handling).


-- 
Best regards,
Dmitry Adamushko
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch 1/8] Add rt_nr_running accounting

2007-10-20 Thread Steven Rostedt 

Hi Dmitry,

--
On Sat, 20 Oct 2007, Dmitry Adamushko wrote:

 On 19/10/2007, Steven Rostedt [EMAIL PROTECTED] wrote:
  [ ... ]
  Index: linux-test.git/kernel/sched.c
  ===
  --- linux-test.git.orig/kernel/sched.c  2007-10-19 12:32:39.0 -0400
  +++ linux-test.git/kernel/sched.c   2007-10-19 12:33:09.0 -0400
  @@ -300,6 +300,8 @@ struct rq {
   */
  unsigned long nr_uninterruptible;
 
  +   unsigned long rt_nr_running;

 could it be a part of the 'struct rt_rq' instead?

Maybe. I didn't really look too hard to where to put it. Currently, in the
-rt patch, it is located in struct rq, so I just did the same. I may be
able to move it.


 
  +static inline void inc_rt_tasks(struct task_struct *p, struct rq *rq)
  +{
  +   if (rt_task(p))
  +   rq-rt_nr_running++;
  +}
  +
  +static inline void dec_rt_tasks(struct task_struct *p, struct rq *rq)
  +{
  +   if (rt_task(p)) {
  +   WARN_ON(!rq-rt_nr_running);
  +   rq-rt_nr_running--;
  +   }
  +}
  +
   static void enqueue_task_rt(struct rq *rq, struct task_struct *p, int 
  wakeup)
   {
  struct rt_prio_array *array = rq-rt.active;
 
  list_add_tail(p-run_list, array-queue + p-prio);
  __set_bit(p-prio, array-bitmap);
  +
  +   inc_rt_tasks(p, rq);

 why do you need the rt_task(p) check in {inc,dec}_rt_tasks() ?

Me being paranoid ;-)


 {enqueue,dequeue}_task_rt() seem to be the only callers and they will
 crash (or corrupt memory) anyway in the case of ! rt_task(p) (sure,
 this case would mean something is broken somewhere wrt sched_class
 handling).

Exactly, I was just being safe. We could add a WARN_ON(!rt_task) there.

-- Steve

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/