Re: [PATCH 1/1] suspend: delete sys_sync()

[NeilBrown]

On Fri, 15 May 2015 11:35:57 +0100 One Thousand Gnomes
<gnomes@xxxxxxxxxxxxxxxxxxx> wrote:

> > > Data loss may be caused for hotplug storage(like USB), or all storage
> > > when power is exhausted during suspend.
> > 
> > Which also may very well happen at run time, right?
> 
> Intuitively users treat "suspended" as a bit like off and do remove
> devices they've "finished with".
> 
> Technically yes the instant off/on on a phone is the same as the suspend
> to RAM on a laptop but it doesn't mean the model in people's heads is the
> same... not yet anyway.
> 
> > > Is there obvious advantage to remove sys_sync() in the case?
> > 
> > Yes, there is.  It is not necessary to sync() every time you suspend
> > if you do that very often.
> 
> But if you do it very often you won't have any dirty pages to flush so it
> will be very fast.

Several people have said things like this, and yet Len clearly saw a problem
so sometimes it isn't as fast as you think it should be.  I guess we need
some data.

In fact there seem to be a number of open questions:

1/ Len has seen enough delays to bother sending a patch.  Twice.  Lots of
   other people claim there shouldn't be much delay.

   Len: can you provide numbers?  How long does the sys_sync take
   (min/max/mean....).  I think an interesting number would be in a quick
   "resume, do something, suspend" cycle, what percent of the time is spent
   in sys_sync.
   Maybe also report what filesystems are in use, and whether you expect
   there to be any dirty data.

   If I run "time sync; time sync; time sync" it reports about 50msec for
   each sync.  If I run "time sleep 0; time sleep 0; time sleep 0" it reports
   about 2 msec.  So sys_sync is taking at least 50msec.
   Almost all of that is in sync_inodes_sb() and much of that is in
   _raw_spin_lock.... though I might be misinterpreting perf.  It seems to
   wait for a BDI flusher thread to go off and do nothing.

   Len: is 50msec enough to bother you?


2/ Is a 'sys_sync' really necessary.  People claim that "users might lose
   data".  I claim that some user data could be still in the application
   buffers so they would lose that anyway, and applications call fsync after
   writing everything out, so sys_sync isn't necessary.
   Does anyone have an example of an application which writes important
   user data, but doesn't call "fsync" shortly after all data has been written
   out of the application's internal buffers?

3/ Is a 'sys_sync' sufficient.  Len claims that in some cases, when running
     sync; sync
   the second sync takes longer, suggesting that the first sync didn't do
   everything that was expected.   Prior to Linux 1.3.20, sys_sync didn't
   wait for everything.  Since then it seems to be designed to, but maybe
   something isn't right there.
   In that case, having sys_sync may not be helping anyway.

   Len: can you reliably reproduce this?  Can you provide a recipe?

4/ Which part of 'sync' is really important?
   sys_sync does two things.  It initiates writeout on dirty data and it
   wait for writeout to complete.  Even if the former isn't necessary, the
   latter probably is.  Do we need to reliably wait for all output queues
   to flush?  Where is the best place to do that?

Thanks,
NeilBrown



> 
> > And it is done in such a place that everything needs to wait for it to complete.
> 
> Only because the code deciding to trigger any automated suspend doesn't
> do a sync a few seconds before. In the case the user goes to the menus
> and does power->suspend then yes it's a delay. In the case where the OS
> at some level has decided that it's 10 seconds from automatically
> suspending to something the user space can issue a pre-emptive sync to
> get the queue size down.
> 
> Alan

Attachment: pgpHRtOzvtmvq.pgp
Description: OpenPGP digital signature