Re: [PATCH v7 2/3] trivial: PM / Hibernate: clean up checkpatch in hibernate.c

[Sebastian Capella <sebastian.capella@linaro.org>]

Quoting Joe Perches (2014-02-04 13:21:02)
> On Tue, 2014-02-04 at 12:43 -0800, Sebastian Capella wrote:
> > Checkpatch reports several warnings in hibernate.c
> > printk use removed, long lines wrapped, whitespace cleanup,
> > extend short msleeps, while loops on two lines.
> []
> > diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> []
> > @@ -765,7 +762,7 @@ static int software_resume(void)
> >       if (isdigit(resume_file[0]) && resume_wait) {
> >               int partno;
> >               while (!get_gendisk(swsusp_resume_device, &partno))
> > -                     msleep(10);
> > +                     msleep(20);
> 
> What good is changing this from 10 to 20?
> 
> > @@ -776,8 +773,9 @@ static int software_resume(void)
> >               wait_for_device_probe();
> >  
> >               if (resume_wait) {
> > -                     while ((swsusp_resume_device = name_to_dev_t(resume_file)) == 0)
> > -                             msleep(10);
> > +                     while ((swsusp_resume_device =
> > +                                     name_to_dev_t(resume_file)) == 0)
> > +                             msleep(20);
> 
> here too.

Thanks Joe!

I'm happy to make whatever change is best.  I just ran into one
checkpatch warning around a printk I indented and figured I'd try to get
them all if I could.

The delays in question didn't appear timing critical as both are looping
waiting for device discovery to complete.  They're only enabled when using
the resumewait command line parameter.

Is this an incorrect checkpatch warning?  The message from checkpatch
implies using msleep for smaller values can be misleading.

WARNING: msleep < 20ms can sleep for up to 20ms; see
Documentation/timers/timers-howto.txt
+  msleep(10);

From Documentation/timers/timers-howto.txt

SLEEPING FOR ~USECS OR SMALL MSECS ( 10us - 20ms):                       
  * Use usleep_range                                               

  - Why not msleep for (1ms - 20ms)?                               
    Explained originally here:                               
      http://lkml.org/lkml/2007/8/3/250                
    msleep(1~20) may not do what the caller intends, and     
    will often sleep longer (~20 ms actual sleep for any     
    value given in the 1~20ms range). In many cases this     
    is not the desired behavior. 

When I look at kernel/timers.c in my current kernel, I see msleep is
using msecs_to_jiffies + 1, and on my current platform this appears to
be ~20msec as the jiffies are 10ms.

Thanks,

Sebastian

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>