[PATCH] kernel/freezer.c : Removing extra checking.

[PATCH] kernel/freezer.c : Removing extra checking.

[Rakib Mullick]

Impact: Reduces an extra checking.

Following patch removes an extra checking. We can remove it since, the
current task is frozen. If the current task is not frozen, then it
will return from __else__ condition of freezing(current) check. So,
the check becames unnecessary. It also reduces 32 bytes of text space
on my x86  (32 bit) system.

Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>

--- linux-2.6-orig/kernel/freezer.c	2008-12-05 19:53:45.000000000 +0600
+++ linux-2.6/kernel/freezer.c	2008-12-05 19:55:40.000000000 +0600
@@ -46,8 +46,6 @@ void refrigerator(void)

 	for (;;) {
 		set_current_state(TASK_UNINTERRUPTIBLE);
-		if (!frozen(current))
-			break;
 		schedule();
 	}
 	pr_debug("%s left refrigerator\n", current->comm);

Re: [PATCH] kernel/freezer.c : Removing extra checking.

[Rafael J. Wysocki]

On Saturday, 6 of December 2008, Rakib Mullick wrote:
> Impact: Reduces an extra checking.
> 
> Following patch removes an extra checking. We can remove it since, the
> current task is frozen. If the current task is not frozen, then it
> will return from __else__ condition of freezing(current) check. So,
> the check becames unnecessary. It also reduces 32 bytes of text space
> on my x86  (32 bit) system.

What will happen with this patch applied when a task is woken up for some
reason other than thawing?

> Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>
> 
> --- linux-2.6-orig/kernel/freezer.c	2008-12-05 19:53:45.000000000 +0600
> +++ linux-2.6/kernel/freezer.c	2008-12-05 19:55:40.000000000 +0600
> @@ -46,8 +46,6 @@ void refrigerator(void)
> 
>  	for (;;) {
>  		set_current_state(TASK_UNINTERRUPTIBLE);
> -		if (!frozen(current))
> -			break;
>  		schedule();
>  	}
>  	pr_debug("%s left refrigerator\n", current->comm);
> --

Re: [PATCH] kernel/freezer.c : Removing extra checking.

[Rakib Mullick]

On 12/6/08, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Saturday, 6 of December 2008, Rakib Mullick wrote:
>  > Impact: Reduces an extra checking.
>  >
>  > Following patch removes an extra checking. We can remove it since, the
>  > current task is frozen. If the current task is not frozen, then it
>  > will return from __else__ condition of freezing(current) check. So,
>  > the check becames unnecessary. It also reduces 32 bytes of text space
>  > on my x86  (32 bit) system.
>
>
> What will happen with this patch applied when a task is woken up for some
>  reason other than thawing?
thawing is usually used for woken up a frozzen process. Would you
please tell me what are the other ways to woken up a frozzen process?
Am I missing anything ?
Thanks,

>
>  > Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>
>  >
>  > --- linux-2.6-orig/kernel/freezer.c   2008-12-05 19:53:45.000000000 +0600
>  > +++ linux-2.6/kernel/freezer.c        2008-12-05 19:55:40.000000000 +0600
>  > @@ -46,8 +46,6 @@ void refrigerator(void)
>  >
>  >       for (;;) {
>  >               set_current_state(TASK_UNINTERRUPTIBLE);
>  > -             if (!frozen(current))
>  > -                     break;
>  >               schedule();
>  >       }
>  >       pr_debug("%s left refrigerator\n", current->comm);
>
> > --
>

Re: [PATCH] kernel/freezer.c : Removing extra checking.

[Rafael J. Wysocki]

On Saturday, 6 of December 2008, Rakib Mullick wrote:
> On 12/6/08, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Saturday, 6 of December 2008, Rakib Mullick wrote:
> >  > Impact: Reduces an extra checking.
> >  >
> >  > Following patch removes an extra checking. We can remove it since, the
> >  > current task is frozen. If the current task is not frozen, then it
> >  > will return from __else__ condition of freezing(current) check. So,
> >  > the check becames unnecessary. It also reduces 32 bytes of text space
> >  > on my x86  (32 bit) system.
> >
> >
> > What will happen with this patch applied when a task is woken up for some
> >  reason other than thawing?
> thawing is usually used for woken up a frozzen process. Would you
> please tell me what are the other ways to woken up a frozzen process?
> Am I missing anything ?

Well, in theory the process can be woken up for another reason.

I don't have any particular examples in mind, but in this case the burden is
yours to show that it won't happen and to say why exactly you think so in the
changelog, because this is the very reason why the code has been written this
way in the first place.

Thanks,
Rafael


> >  > Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>
> >  >
> >  > --- linux-2.6-orig/kernel/freezer.c   2008-12-05 19:53:45.000000000 +0600
> >  > +++ linux-2.6/kernel/freezer.c        2008-12-05 19:55:40.000000000 +0600
> >  > @@ -46,8 +46,6 @@ void refrigerator(void)
> >  >
> >  >       for (;;) {
> >  >               set_current_state(TASK_UNINTERRUPTIBLE);
> >  > -             if (!frozen(current))
> >  > -                     break;
> >  >               schedule();
> >  >       }
> >  >       pr_debug("%s left refrigerator\n", current->comm);
> >
> > > --

Re: [PATCH] kernel/freezer.c : Removing extra checking.

[Pavel Machek]

On Sat 2008-12-06 17:52:57, Rafael J. Wysocki wrote:
> On Saturday, 6 of December 2008, Rakib Mullick wrote:
> > On 12/6/08, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > > On Saturday, 6 of December 2008, Rakib Mullick wrote:
> > >  > Impact: Reduces an extra checking.
> > >  >
> > >  > Following patch removes an extra checking. We can remove it since, the
> > >  > current task is frozen. If the current task is not frozen, then it
> > >  > will return from __else__ condition of freezing(current) check. So,
> > >  > the check becames unnecessary. It also reduces 32 bytes of text space
> > >  > on my x86  (32 bit) system.
> > >
> > >
> > > What will happen with this patch applied when a task is woken up for some
> > >  reason other than thawing?
> > thawing is usually used for woken up a frozzen process. Would you
> > please tell me what are the other ways to woken up a frozzen process?
> > Am I missing anything ?
> 
> Well, in theory the process can be woken up for another reason.

...like pending signal...

> I don't have any particular examples in mind, but in this case the burden is
> yours to show that it won't happen and to say why exactly you think so in the
> changelog, because this is the very reason why the code has been written this
> way in the first place.
> 
> Thanks,
> Rafael
> 
> 
> > >  > Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>
> > >  >
> > >  > --- linux-2.6-orig/kernel/freezer.c   2008-12-05 19:53:45.000000000 +0600
> > >  > +++ linux-2.6/kernel/freezer.c        2008-12-05 19:55:40.000000000 +0600
> > >  > @@ -46,8 +46,6 @@ void refrigerator(void)
> > >  >
> > >  >       for (;;) {
> > >  >               set_current_state(TASK_UNINTERRUPTIBLE);
> > >  > -             if (!frozen(current))
> > >  > -                     break;
> > >  >               schedule();
> > >  >       }
> > >  >       pr_debug("%s left refrigerator\n", current->comm);
> > >

Plus this looks like infinite loop after your change. Have you tested
it?


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] kernel/freezer.c : Removing extra checking.

[Rakib Mullick]

On 12/11/08, Pavel Machek <pavel@suse.cz> wrote:
>
> Plus this looks like infinite loop after your change. Have you tested
>  it?
No. I haven't tested it. But very soon I'll try to test it.
Well, you said pending signal. What will happen when a pending signal
will send. In current code check is made to make sure that the task is
frozen or not. I mean , when a signal is send ( from theoritical POV,
which could be any ) this code doesn't make sure what will happen to
that perticular process.
It is already a infinite loop, the check is made to make sure that
whether the task will be scheduled or not.

Thanks,
Rakib

>
>
>  --
>  (english) http://www.livejournal.com/~pavelmachek
>  (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
>