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

2015-08-04 Thread Pavel Machek
On Fri 2015-07-10 01:22:55, Rafael J. Wysocki wrote: > On Thursday, July 09, 2015 09:32:51 AM Oliver Neukum wrote: > > On Thu, 2015-07-09 at 00:03 +0200, Rafael J. Wysocki wrote: > > > > > Nothing and I'm not discussing that (I've said that already at least once > > > in > > > this thread). > >

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

2015-08-04 Thread Pavel Machek
On Fri 2015-07-10 01:22:55, Rafael J. Wysocki wrote: On Thursday, July 09, 2015 09:32:51 AM Oliver Neukum wrote: On Thu, 2015-07-09 at 00:03 +0200, Rafael J. Wysocki wrote: Nothing and I'm not discussing that (I've said that already at least once in this thread). What I'm

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

2015-07-09 Thread Rafael J. Wysocki
On Thursday, July 09, 2015 09:32:51 AM Oliver Neukum wrote: > On Thu, 2015-07-09 at 00:03 +0200, Rafael J. Wysocki wrote: > > > Nothing and I'm not discussing that (I've said that already at least once in > > this thread). > > > > What I'm questioning is the "why" of calling sys_sync() from the

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

2015-07-09 Thread Oliver Neukum
On Thu, 2015-07-09 at 00:03 +0200, Rafael J. Wysocki wrote: > Nothing and I'm not discussing that (I've said that already at least once in > this thread). > > What I'm questioning is the "why" of calling sys_sync() from the kernel. That's strictly speaking two questions 1. Why do it in the

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

2015-07-09 Thread Oliver Neukum
On Thu, 2015-07-09 at 00:03 +0200, Rafael J. Wysocki wrote: Nothing and I'm not discussing that (I've said that already at least once in this thread). What I'm questioning is the why of calling sys_sync() from the kernel. That's strictly speaking two questions 1. Why do it in the kernel

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

2015-07-09 Thread Rafael J. Wysocki
On Thursday, July 09, 2015 09:32:51 AM Oliver Neukum wrote: On Thu, 2015-07-09 at 00:03 +0200, Rafael J. Wysocki wrote: Nothing and I'm not discussing that (I've said that already at least once in this thread). What I'm questioning is the why of calling sys_sync() from the kernel.

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

2015-07-08 Thread Rafael J. Wysocki
On Wednesday, July 08, 2015 10:40:00 AM Alan Stern wrote: > On Wed, 8 Jul 2015, Pavel Machek wrote: > > > > well, that depends on what the purpose of the sync is supposed to be. > > > > > > If it is there to prevent users from corrupting their filesystems as a > > > result > > > of a mistake,

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

2015-07-08 Thread Rafael J. Wysocki
On Wednesday, July 08, 2015 09:51:13 AM Oliver Neukum wrote: > On Wed, 2015-07-08 at 00:11 +0200, Rafael J. Wysocki wrote: > > On Tuesday, July 07, 2015 04:38:26 PM Oliver Neukum wrote: > > > > > That is a tough nut. But that's not a reason to make it worse. > > > I'd say there's no reason not

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

2015-07-08 Thread Alan Stern
On Wed, 8 Jul 2015, Pavel Machek wrote: > > well, that depends on what the purpose of the sync is supposed to be. > > > > If it is there to prevent users from corrupting their filesystems as a > > result > > of a mistake, it is insufficient. If it's there for other reasons, I'm > > wondering

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

2015-07-08 Thread Pavel Machek
On Wed 2015-07-08 00:20:05, Rafael J. Wysocki wrote: > On Tuesday, July 07, 2015 11:03:20 AM Alan Stern wrote: > > On Tue, 7 Jul 2015, Oliver Neukum wrote: > > > > > > he (or she) pulls the storage device out of the system, moves it to > > > > another > > > > system, makes changes (say removes

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

2015-07-08 Thread Pavel Machek
On Tue 2015-07-07 16:32:08, Rafael J. Wysocki wrote: > On Tuesday, July 07, 2015 03:16:48 PM Oliver Neukum wrote: > > On Tue, 2015-07-07 at 14:14 +0200, Rafael J. Wysocki wrote: > > > For example, on desktop systems I use user space syncs filesystems > > > before > > > writing to /sys/power/state,

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

2015-07-08 Thread Oliver Neukum
On Wed, 2015-07-08 at 00:11 +0200, Rafael J. Wysocki wrote: > On Tuesday, July 07, 2015 04:38:26 PM Oliver Neukum wrote: > > > That is a tough nut. But that's not a reason to make it worse. > > I'd say there's no reason not to use a secondary interface to > > suspend without syncing or to extend

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

2015-07-08 Thread Oliver Neukum
On Wed, 2015-07-08 at 00:11 +0200, Rafael J. Wysocki wrote: On Tuesday, July 07, 2015 04:38:26 PM Oliver Neukum wrote: That is a tough nut. But that's not a reason to make it worse. I'd say there's no reason not to use a secondary interface to suspend without syncing or to extend or

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

2015-07-08 Thread Pavel Machek
On Tue 2015-07-07 16:32:08, Rafael J. Wysocki wrote: On Tuesday, July 07, 2015 03:16:48 PM Oliver Neukum wrote: On Tue, 2015-07-07 at 14:14 +0200, Rafael J. Wysocki wrote: For example, on desktop systems I use user space syncs filesystems before writing to /sys/power/state, so the

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

2015-07-08 Thread Pavel Machek
On Wed 2015-07-08 00:20:05, Rafael J. Wysocki wrote: On Tuesday, July 07, 2015 11:03:20 AM Alan Stern wrote: On Tue, 7 Jul 2015, Oliver Neukum wrote: he (or she) pulls the storage device out of the system, moves it to another system, makes changes (say removes the file written

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

2015-07-08 Thread Alan Stern
On Wed, 8 Jul 2015, Pavel Machek wrote: well, that depends on what the purpose of the sync is supposed to be. If it is there to prevent users from corrupting their filesystems as a result of a mistake, it is insufficient. If it's there for other reasons, I'm wondering what those

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

2015-07-08 Thread Rafael J. Wysocki
On Wednesday, July 08, 2015 09:51:13 AM Oliver Neukum wrote: On Wed, 2015-07-08 at 00:11 +0200, Rafael J. Wysocki wrote: On Tuesday, July 07, 2015 04:38:26 PM Oliver Neukum wrote: That is a tough nut. But that's not a reason to make it worse. I'd say there's no reason not to use a

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

2015-07-08 Thread Rafael J. Wysocki
On Wednesday, July 08, 2015 10:40:00 AM Alan Stern wrote: On Wed, 8 Jul 2015, Pavel Machek wrote: well, that depends on what the purpose of the sync is supposed to be. If it is there to prevent users from corrupting their filesystems as a result of a mistake, it is

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

2015-07-07 Thread Rafael J. Wysocki
On Tuesday, July 07, 2015 11:03:20 AM Alan Stern wrote: > On Tue, 7 Jul 2015, Oliver Neukum wrote: > > > > he (or she) pulls the storage device out of the system, moves it to > > > another > > > system, makes changes (say removes the file written to by the process > > > above, > > > so the

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

2015-07-07 Thread Rafael J. Wysocki
On Tuesday, July 07, 2015 04:38:26 PM Oliver Neukum wrote: > On Tue, 2015-07-07 at 16:32 +0200, Rafael J. Wysocki wrote: > > On Tuesday, July 07, 2015 03:16:48 PM Oliver Neukum wrote: > > > On Tue, 2015-07-07 at 14:14 +0200, Rafael J. Wysocki wrote: > > > > For example, on desktop systems I use

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

2015-07-07 Thread Alan Stern
On Tue, 7 Jul 2015, Oliver Neukum wrote: > > he (or she) pulls the storage device out of the system, moves it to another > > system, makes changes (say removes the file written to by the process above, > > so the blocks previously occupied by that file are now used for some > > metadata) > > and

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

2015-07-07 Thread Oliver Neukum
On Tue, 2015-07-07 at 16:32 +0200, Rafael J. Wysocki wrote: > On Tuesday, July 07, 2015 03:16:48 PM Oliver Neukum wrote: > > On Tue, 2015-07-07 at 14:14 +0200, Rafael J. Wysocki wrote: > > > For example, on desktop systems I use user space syncs filesystems > > > before > > > writing to

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

2015-07-07 Thread Rafael J. Wysocki
On Tuesday, July 07, 2015 03:16:48 PM Oliver Neukum wrote: > On Tue, 2015-07-07 at 14:14 +0200, Rafael J. Wysocki wrote: > > For example, on desktop systems I use user space syncs filesystems > > before > > writing to /sys/power/state, so the additional sys_sync() in the > > kernel doesn't > >

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

2015-07-07 Thread Takashi Iwai
At Tue, 7 Jul 2015 11:17:56 +1000, Dave Chinner wrote: > > > > > Moreover, question is if we really need to carry out the sync on *every* > > > > suspend even if it is not pointless overall. That shouldn't really be > > > > necessary if we suspend and resume often enough or if we resume only for

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

2015-07-07 Thread Oliver Neukum
On Tue, 2015-07-07 at 14:14 +0200, Rafael J. Wysocki wrote: > For example, on desktop systems I use user space syncs filesystems > before > writing to /sys/power/state, so the additional sys_sync() in the > kernel doesn't > seem to serve any purpose. There is a race you cannot close in user

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

2015-07-07 Thread Rafael J. Wysocki
On Tuesday, July 07, 2015 12:25:07 PM Pavel Machek wrote: > On Mon 2015-07-06 15:59:15, Rafael J. Wysocki wrote: > > On Monday, July 06, 2015 01:06:45 PM Pavel Machek wrote: > > > On Mon 2015-07-06 01:28:20, Rafael J. Wysocki wrote: > > > > On Saturday, July 04, 2015 10:19:55 AM Alan Stern wrote:

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

2015-07-07 Thread Rafael J. Wysocki
On Tuesday, July 07, 2015 11:17:56 AM Dave Chinner wrote: > On Mon, Jul 06, 2015 at 03:52:25PM +0200, Rafael J. Wysocki wrote: > > On Monday, July 06, 2015 10:06:14 AM Dave Chinner wrote: > > > On Sat, Jul 04, 2015 at 03:03:46AM +0200, Rafael J. Wysocki wrote: > > > > > No, your observation was

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

2015-07-07 Thread Pavel Machek
On Mon 2015-07-06 15:59:15, Rafael J. Wysocki wrote: > On Monday, July 06, 2015 01:06:45 PM Pavel Machek wrote: > > On Mon 2015-07-06 01:28:20, Rafael J. Wysocki wrote: > > > On Saturday, July 04, 2015 10:19:55 AM Alan Stern wrote: > > > > On Sat, 4 Jul 2015, Rafael J. Wysocki wrote: > > > > > >

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

2015-07-07 Thread Rafael J. Wysocki
On Tuesday, July 07, 2015 11:03:20 AM Alan Stern wrote: On Tue, 7 Jul 2015, Oliver Neukum wrote: he (or she) pulls the storage device out of the system, moves it to another system, makes changes (say removes the file written to by the process above, so the blocks previously

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

2015-07-07 Thread Rafael J. Wysocki
On Tuesday, July 07, 2015 04:38:26 PM Oliver Neukum wrote: On Tue, 2015-07-07 at 16:32 +0200, Rafael J. Wysocki wrote: On Tuesday, July 07, 2015 03:16:48 PM Oliver Neukum wrote: On Tue, 2015-07-07 at 14:14 +0200, Rafael J. Wysocki wrote: For example, on desktop systems I use user space

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

2015-07-07 Thread Rafael J. Wysocki
On Tuesday, July 07, 2015 11:17:56 AM Dave Chinner wrote: On Mon, Jul 06, 2015 at 03:52:25PM +0200, Rafael J. Wysocki wrote: On Monday, July 06, 2015 10:06:14 AM Dave Chinner wrote: On Sat, Jul 04, 2015 at 03:03:46AM +0200, Rafael J. Wysocki wrote: No, your observation was that sync is

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

2015-07-07 Thread Pavel Machek
On Mon 2015-07-06 15:59:15, Rafael J. Wysocki wrote: On Monday, July 06, 2015 01:06:45 PM Pavel Machek wrote: On Mon 2015-07-06 01:28:20, Rafael J. Wysocki wrote: On Saturday, July 04, 2015 10:19:55 AM Alan Stern wrote: On Sat, 4 Jul 2015, Rafael J. Wysocki wrote: The only

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

2015-07-07 Thread Rafael J. Wysocki
On Tuesday, July 07, 2015 12:25:07 PM Pavel Machek wrote: On Mon 2015-07-06 15:59:15, Rafael J. Wysocki wrote: On Monday, July 06, 2015 01:06:45 PM Pavel Machek wrote: On Mon 2015-07-06 01:28:20, Rafael J. Wysocki wrote: On Saturday, July 04, 2015 10:19:55 AM Alan Stern wrote: On

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

2015-07-07 Thread Rafael J. Wysocki
On Tuesday, July 07, 2015 03:16:48 PM Oliver Neukum wrote: On Tue, 2015-07-07 at 14:14 +0200, Rafael J. Wysocki wrote: For example, on desktop systems I use user space syncs filesystems before writing to /sys/power/state, so the additional sys_sync() in the kernel doesn't seem to serve

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

2015-07-07 Thread Takashi Iwai
At Tue, 7 Jul 2015 11:17:56 +1000, Dave Chinner wrote: Moreover, question is if we really need to carry out the sync on *every* suspend even if it is not pointless overall. That shouldn't really be necessary if we suspend and resume often enough or if we resume only for a while

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

2015-07-07 Thread Oliver Neukum
On Tue, 2015-07-07 at 14:14 +0200, Rafael J. Wysocki wrote: For example, on desktop systems I use user space syncs filesystems before writing to /sys/power/state, so the additional sys_sync() in the kernel doesn't seem to serve any purpose. There is a race you cannot close in user space.

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

2015-07-07 Thread Oliver Neukum
On Tue, 2015-07-07 at 16:32 +0200, Rafael J. Wysocki wrote: On Tuesday, July 07, 2015 03:16:48 PM Oliver Neukum wrote: On Tue, 2015-07-07 at 14:14 +0200, Rafael J. Wysocki wrote: For example, on desktop systems I use user space syncs filesystems before writing to /sys/power/state, so

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

2015-07-07 Thread Alan Stern
On Tue, 7 Jul 2015, Oliver Neukum wrote: he (or she) pulls the storage device out of the system, moves it to another system, makes changes (say removes the file written to by the process above, so the blocks previously occupied by that file are now used for some metadata) and moves the

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

2015-07-06 Thread Dave Chinner
On Mon, Jul 06, 2015 at 03:52:25PM +0200, Rafael J. Wysocki wrote: > On Monday, July 06, 2015 10:06:14 AM Dave Chinner wrote: > > On Sat, Jul 04, 2015 at 03:03:46AM +0200, Rafael J. Wysocki wrote: > > > > No, your observation was that "sync is slow". Your *solution* is "we > > > > need to remove

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

2015-07-06 Thread Rafael J. Wysocki
On Monday, July 06, 2015 01:06:45 PM Pavel Machek wrote: > On Mon 2015-07-06 01:28:20, Rafael J. Wysocki wrote: > > On Saturday, July 04, 2015 10:19:55 AM Alan Stern wrote: > > > On Sat, 4 Jul 2015, Rafael J. Wysocki wrote: > > > > > > > The only argument against dropping sys_sync() from the

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

2015-07-06 Thread Rafael J. Wysocki
On Monday, July 06, 2015 10:06:14 AM Dave Chinner wrote: > On Sat, Jul 04, 2015 at 03:03:46AM +0200, Rafael J. Wysocki wrote: > > On Friday, July 03, 2015 11:42:50 AM Dave Chinner wrote: > > > On Wed, Jul 01, 2015 at 11:07:29PM -0400, Len Brown wrote: > > > > >> The _vast_ majority of systems

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

2015-07-06 Thread Pavel Machek
Hi! > > Moreover, question is if we really need to carry out the sync on *every* > > suspend even if it is not pointless overall. That shouldn't really be > > necessary if we suspend and resume often enough or if we resume only for > > a while and then suspend again. Maybe it should be rate

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

2015-07-06 Thread Pavel Machek
On Mon 2015-07-06 01:28:20, Rafael J. Wysocki wrote: > On Saturday, July 04, 2015 10:19:55 AM Alan Stern wrote: > > On Sat, 4 Jul 2015, Rafael J. Wysocki wrote: > > > > > The only argument against dropping sys_sync() from the suspend code path > > > I've seen in this thread that I entirely agree

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

2015-07-06 Thread Ming Lei
On Sat, Jul 4, 2015 at 9:03 AM, Rafael J. Wysocki wrote: > On Friday, July 03, 2015 11:42:50 AM Dave Chinner wrote: >> On Wed, Jul 01, 2015 at 11:07:29PM -0400, Len Brown wrote: >> > >> The _vast_ majority of systems using Linux suspend today are under >> > >> an Android user-space. Android has

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

2015-07-06 Thread Pavel Machek
Hi! > conditions, or s2disk fails because a kernel upgrade was done before > the s2disk and so can't be resumed. With your change, users lose all s2disk should be able to survive kernel change on x86-64 for long time now, and I have patches for x86, too. [But yes, I think sync() should be

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

2015-07-06 Thread Pavel Machek
Hi! > > Understand, however, there are systems which suspend/resume reliably > > many times per second, making policy choice of having the kernel hard-code > > a sys_sync() into the suspend path a bad idea. > > My current view on that is that whether or not to do a sync() before > suspending >

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

2015-07-06 Thread Pavel Machek
Hi! Understand, however, there are systems which suspend/resume reliably many times per second, making policy choice of having the kernel hard-code a sys_sync() into the suspend path a bad idea. My current view on that is that whether or not to do a sync() before suspending ultimately

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

2015-07-06 Thread Pavel Machek
Hi! conditions, or s2disk fails because a kernel upgrade was done before the s2disk and so can't be resumed. With your change, users lose all s2disk should be able to survive kernel change on x86-64 for long time now, and I have patches for x86, too. [But yes, I think sync() should be kept.]

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

2015-07-06 Thread Pavel Machek
On Mon 2015-07-06 01:28:20, Rafael J. Wysocki wrote: On Saturday, July 04, 2015 10:19:55 AM Alan Stern wrote: On Sat, 4 Jul 2015, Rafael J. Wysocki wrote: The only argument against dropping sys_sync() from the suspend code path I've seen in this thread that I entirely agree with is

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

2015-07-06 Thread Ming Lei
On Sat, Jul 4, 2015 at 9:03 AM, Rafael J. Wysocki r...@rjwysocki.net wrote: On Friday, July 03, 2015 11:42:50 AM Dave Chinner wrote: On Wed, Jul 01, 2015 at 11:07:29PM -0400, Len Brown wrote: The _vast_ majority of systems using Linux suspend today are under an Android user-space. Android

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

2015-07-06 Thread Pavel Machek
Hi! Moreover, question is if we really need to carry out the sync on *every* suspend even if it is not pointless overall. That shouldn't really be necessary if we suspend and resume often enough or if we resume only for a while and then suspend again. Maybe it should be rate limited

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

2015-07-06 Thread Rafael J. Wysocki
On Monday, July 06, 2015 10:06:14 AM Dave Chinner wrote: On Sat, Jul 04, 2015 at 03:03:46AM +0200, Rafael J. Wysocki wrote: On Friday, July 03, 2015 11:42:50 AM Dave Chinner wrote: On Wed, Jul 01, 2015 at 11:07:29PM -0400, Len Brown wrote: The _vast_ majority of systems using Linux

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

2015-07-06 Thread Rafael J. Wysocki
On Monday, July 06, 2015 01:06:45 PM Pavel Machek wrote: On Mon 2015-07-06 01:28:20, Rafael J. Wysocki wrote: On Saturday, July 04, 2015 10:19:55 AM Alan Stern wrote: On Sat, 4 Jul 2015, Rafael J. Wysocki wrote: The only argument against dropping sys_sync() from the suspend code path

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

2015-07-06 Thread Dave Chinner
On Mon, Jul 06, 2015 at 03:52:25PM +0200, Rafael J. Wysocki wrote: On Monday, July 06, 2015 10:06:14 AM Dave Chinner wrote: On Sat, Jul 04, 2015 at 03:03:46AM +0200, Rafael J. Wysocki wrote: No, your observation was that sync is slow. Your *solution* is we need to remove sync.

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

2015-07-05 Thread Dave Chinner
On Sat, Jul 04, 2015 at 03:03:46AM +0200, Rafael J. Wysocki wrote: > On Friday, July 03, 2015 11:42:50 AM Dave Chinner wrote: > > On Wed, Jul 01, 2015 at 11:07:29PM -0400, Len Brown wrote: > > > >> The _vast_ majority of systems using Linux suspend today are under > > > >> an Android user-space.

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

2015-07-05 Thread Rafael J. Wysocki
On Saturday, July 04, 2015 10:19:55 AM Alan Stern wrote: > On Sat, 4 Jul 2015, Rafael J. Wysocki wrote: > > > The only argument against dropping sys_sync() from the suspend code path > > I've seen in this thread that I entirely agree with is that it may lead to > > regressions, because we've done

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

2015-07-05 Thread Rafael J. Wysocki
On Saturday, July 04, 2015 10:50:36 AM Geert Uytterhoeven wrote: > On Sat, Jul 4, 2015 at 3:03 AM, Rafael J. Wysocki wrote: > > [The argument that the user can pull removable storage devices out of the > > system while suspended doesn't hold any water to me, because the user can > > pull them out

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

2015-07-05 Thread Geert Uytterhoeven
On Sat, Jul 4, 2015 at 3:03 AM, Rafael J. Wysocki wrote: > [The argument that the user can pull removable storage devices out of the > system while suspended doesn't hold any water to me, because the user can > pull them out of the system when not suspended just as well and cause the > same kind

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

2015-07-05 Thread Alan Stern
On Sat, 4 Jul 2015, Rafael J. Wysocki wrote: > The only argument against dropping sys_sync() from the suspend code path > I've seen in this thread that I entirely agree with is that it may lead to > regressions, because we've done it practically forever and it may hide latent > bugs somewhere in

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

2015-07-05 Thread Geert Uytterhoeven
On Sat, Jul 4, 2015 at 3:03 AM, Rafael J. Wysocki r...@rjwysocki.net wrote: [The argument that the user can pull removable storage devices out of the system while suspended doesn't hold any water to me, because the user can pull them out of the system when not suspended just as well and cause

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

2015-07-05 Thread Alan Stern
On Sat, 4 Jul 2015, Rafael J. Wysocki wrote: The only argument against dropping sys_sync() from the suspend code path I've seen in this thread that I entirely agree with is that it may lead to regressions, because we've done it practically forever and it may hide latent bugs somewhere in

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

2015-07-05 Thread Rafael J. Wysocki
On Saturday, July 04, 2015 10:50:36 AM Geert Uytterhoeven wrote: On Sat, Jul 4, 2015 at 3:03 AM, Rafael J. Wysocki r...@rjwysocki.net wrote: [The argument that the user can pull removable storage devices out of the system while suspended doesn't hold any water to me, because the user can

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

2015-07-05 Thread Rafael J. Wysocki
On Saturday, July 04, 2015 10:19:55 AM Alan Stern wrote: On Sat, 4 Jul 2015, Rafael J. Wysocki wrote: The only argument against dropping sys_sync() from the suspend code path I've seen in this thread that I entirely agree with is that it may lead to regressions, because we've done it

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

2015-07-05 Thread Dave Chinner
On Sat, Jul 04, 2015 at 03:03:46AM +0200, Rafael J. Wysocki wrote: On Friday, July 03, 2015 11:42:50 AM Dave Chinner wrote: On Wed, Jul 01, 2015 at 11:07:29PM -0400, Len Brown wrote: The _vast_ majority of systems using Linux suspend today are under an Android user-space. Android has

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

2015-07-03 Thread Rafael J. Wysocki
On Friday, July 03, 2015 11:42:50 AM Dave Chinner wrote: > On Wed, Jul 01, 2015 at 11:07:29PM -0400, Len Brown wrote: > > >> The _vast_ majority of systems using Linux suspend today are under > > >> an Android user-space. Android has no assumption that that suspend to > > >> mem will necessarily

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

2015-07-03 Thread Rafael J. Wysocki
On Friday, July 03, 2015 11:42:50 AM Dave Chinner wrote: On Wed, Jul 01, 2015 at 11:07:29PM -0400, Len Brown wrote: The _vast_ majority of systems using Linux suspend today are under an Android user-space. Android has no assumption that that suspend to mem will necessarily stay

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

2015-07-02 Thread Dave Chinner
On Wed, Jul 01, 2015 at 11:07:29PM -0400, Len Brown wrote: > >> The _vast_ majority of systems using Linux suspend today are under > >> an Android user-space. Android has no assumption that that suspend to > >> mem will necessarily stay suspended for a long time. > > > > Indeed, however your

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

2015-07-02 Thread Dave Chinner
On Wed, Jul 01, 2015 at 11:07:29PM -0400, Len Brown wrote: The _vast_ majority of systems using Linux suspend today are under an Android user-space. Android has no assumption that that suspend to mem will necessarily stay suspended for a long time. Indeed, however your change was not

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

2015-07-01 Thread Len Brown
>> The _vast_ majority of systems using Linux suspend today are under >> an Android user-space. Android has no assumption that that suspend to >> mem will necessarily stay suspended for a long time. > > Indeed, however your change was not android-specific, and it is not > "comfortable" on

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

2015-07-01 Thread Henrique de Moraes Holschuh
On Tue, Jun 30, 2015, at 17:04, Len Brown wrote: > > Entering "mem" suspend mode through sysfs currently has the implied meaning > > of "prepare the *entire* system to stay on a powered down state for > > pontentially a _long_ time", where long means "certainly more than 10 > > seconds" ;-) This

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

2015-07-01 Thread Henrique de Moraes Holschuh
On Tue, Jun 30, 2015, at 17:04, Len Brown wrote: Entering mem suspend mode through sysfs currently has the implied meaning of prepare the *entire* system to stay on a powered down state for pontentially a _long_ time, where long means certainly more than 10 seconds ;-) This is unlikely to

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

2015-07-01 Thread Len Brown
The _vast_ majority of systems using Linux suspend today are under an Android user-space. Android has no assumption that that suspend to mem will necessarily stay suspended for a long time. Indeed, however your change was not android-specific, and it is not comfortable on x86-style hardware

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

2015-06-30 Thread Len Brown
On Thu, Jun 25, 2015 at 1:11 PM, Henrique de Moraes Holschuh wrote: > On Mon, 11 May 2015, Len Brown wrote: >> On Sat, May 9, 2015 at 4:25 PM, Henrique de Moraes Holschuh >> wrote: >> > On Sat, 09 May 2015, Alan Stern wrote: >> >> On Fri, 8 May 2015, Rafael J. Wysocki wrote: >> >> > My current

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

2015-06-30 Thread Len Brown
On Thu, Jun 25, 2015 at 1:11 PM, Henrique de Moraes Holschuh h...@hmh.eng.br wrote: On Mon, 11 May 2015, Len Brown wrote: On Sat, May 9, 2015 at 4:25 PM, Henrique de Moraes Holschuh h...@hmh.eng.br wrote: On Sat, 09 May 2015, Alan Stern wrote: On Fri, 8 May 2015, Rafael J. Wysocki wrote:

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

2015-06-25 Thread Henrique de Moraes Holschuh
On Mon, 11 May 2015, Len Brown wrote: > On Sat, May 9, 2015 at 4:25 PM, Henrique de Moraes Holschuh > wrote: > > On Sat, 09 May 2015, Alan Stern wrote: > >> On Fri, 8 May 2015, Rafael J. Wysocki wrote: > >> > My current view on that is that whether or not to do a sync() before > >> > suspending

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

2015-06-25 Thread Henrique de Moraes Holschuh
On Mon, 11 May 2015, Len Brown wrote: On Sat, May 9, 2015 at 4:25 PM, Henrique de Moraes Holschuh h...@hmh.eng.br wrote: On Sat, 09 May 2015, Alan Stern wrote: On Fri, 8 May 2015, Rafael J. Wysocki wrote: My current view on that is that whether or not to do a sync() before suspending

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

2015-06-19 Thread Len Brown
>> Putting a function trace on sys_sync and executing sync manually, >> I was able to see it take 100ms, >> though function trace itself could be contributing to that... > > It would seem that way - you need to get the traces to dump to > something that has no sync overhead I don't think that

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

2015-06-19 Thread Dave Chinner
On Fri, Jun 19, 2015 at 02:34:37AM -0400, Len Brown wrote: > > Can you repeat this test on your system, so that we can determine if > > the 5ms ""sync time" is actually just the overhead of inode cache > > traversal? If that is the case, the speed of sync on a clean > > filesystem is already a

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

2015-06-19 Thread Len Brown
> Can you repeat this test on your system, so that we can determine if > the 5ms ""sync time" is actually just the overhead of inode cache > traversal? If that is the case, the speed of sync on a clean > filesystem is already a solved problem - the patchset should be > merged in the 4.2 cycle

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

2015-06-19 Thread Dave Chinner
On Fri, Jun 19, 2015 at 02:34:37AM -0400, Len Brown wrote: Can you repeat this test on your system, so that we can determine if the 5ms sync time is actually just the overhead of inode cache traversal? If that is the case, the speed of sync on a clean filesystem is already a solved problem

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

2015-06-19 Thread Len Brown
Putting a function trace on sys_sync and executing sync manually, I was able to see it take 100ms, though function trace itself could be contributing to that... It would seem that way - you need to get the traces to dump to something that has no sync overhead I don't think that ftrace

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

2015-06-19 Thread Len Brown
Can you repeat this test on your system, so that we can determine if the 5ms sync time is actually just the overhead of inode cache traversal? If that is the case, the speed of sync on a clean filesystem is already a solved problem - the patchset should be merged in the 4.2 cycle Yes,

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

2015-06-18 Thread Dave Chinner
On Thu, Jun 18, 2015 at 10:35:44PM -0400, Len Brown wrote: > On Thu, Jun 18, 2015 at 9:09 PM, Dave Chinner wrote: > > On Thu, Jun 18, 2015 at 02:14:31AM -0400, Len Brown wrote: > >> On Sun, May 17, 2015 at 9:57 PM, NeilBrown wrote: > >> > 1/ Len has seen enough delays to bother sending a patch.

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

2015-06-18 Thread Len Brown
On Thu, Jun 18, 2015 at 9:09 PM, Dave Chinner wrote: > On Thu, Jun 18, 2015 at 02:14:31AM -0400, Len Brown wrote: >> On Sun, May 17, 2015 at 9:57 PM, NeilBrown wrote: >> > On Fri, 15 May 2015 11:35:57 +0100 One Thousand Gnomes >> > wrote: >> > >> >> > > Data loss may be caused for hotplug

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

2015-06-18 Thread Dave Chinner
On Thu, Jun 18, 2015 at 02:14:31AM -0400, Len Brown wrote: > On Sun, May 17, 2015 at 9:57 PM, NeilBrown wrote: > > On Fri, 15 May 2015 11:35:57 +0100 One Thousand Gnomes > > wrote: > > > >> > > Data loss may be caused for hotplug storage(like USB), or all storage > >> > > when power is exhausted

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

2015-06-18 Thread Dave Chinner
On Thu, Jun 18, 2015 at 10:35:44PM -0400, Len Brown wrote: On Thu, Jun 18, 2015 at 9:09 PM, Dave Chinner da...@fromorbit.com wrote: On Thu, Jun 18, 2015 at 02:14:31AM -0400, Len Brown wrote: On Sun, May 17, 2015 at 9:57 PM, NeilBrown ne...@suse.de wrote: 1/ Len has seen enough delays to

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

2015-06-18 Thread Dave Chinner
On Thu, Jun 18, 2015 at 02:14:31AM -0400, Len Brown wrote: On Sun, May 17, 2015 at 9:57 PM, NeilBrown ne...@suse.de wrote: On Fri, 15 May 2015 11:35:57 +0100 One Thousand Gnomes gno...@lxorguk.ukuu.org.uk wrote: Data loss may be caused for hotplug storage(like USB), or all storage

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

2015-06-18 Thread Len Brown
On Thu, Jun 18, 2015 at 9:09 PM, Dave Chinner da...@fromorbit.com wrote: On Thu, Jun 18, 2015 at 02:14:31AM -0400, Len Brown wrote: On Sun, May 17, 2015 at 9:57 PM, NeilBrown ne...@suse.de wrote: On Fri, 15 May 2015 11:35:57 +0100 One Thousand Gnomes gno...@lxorguk.ukuu.org.uk wrote:

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

2015-05-17 Thread NeilBrown
On Fri, 15 May 2015 11:35:57 +0100 One Thousand Gnomes 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

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

2015-05-17 Thread NeilBrown
On Fri, 15 May 2015 11:35:57 +0100 One Thousand Gnomes gno...@lxorguk.ukuu.org.uk 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

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

2015-05-15 Thread Alan Stern
On Fri, 15 May 2015, Alan Stern wrote: > On Fri, 15 May 2015, NeilBrown wrote: > > > Some storage devices don't handle suspend as well as they should and lose > > requests resulting in corruption. They should obviously be fixed, but it is > > you who gets the problem reports and you are not in

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

2015-05-15 Thread Alan Stern
On Fri, 15 May 2015, NeilBrown wrote: > Some storage devices don't handle suspend as well as they should and lose > requests resulting in corruption. They should obviously be fixed, but it is > you who gets the problem reports and you are not in a position to fix them. > So you want a general

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

2015-05-15 Thread Alan Stern
On Fri, 15 May 2015, Dave Chinner wrote: > It has nothing to do with journalling, and everything to do with > bring filesystems to an *idle state* before suspend runs. We have a > long history of bug reports with XFS that go: suspend, resume, XFS > almost immediately detects corruption, shuts

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

2015-05-15 Thread One Thousand Gnomes
> > 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

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

2015-05-15 Thread One Thousand Gnomes
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

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

2015-05-15 Thread Alan Stern
On Fri, 15 May 2015, NeilBrown wrote: Some storage devices don't handle suspend as well as they should and lose requests resulting in corruption. They should obviously be fixed, but it is you who gets the problem reports and you are not in a position to fix them. So you want a general

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

2015-05-15 Thread Alan Stern
On Fri, 15 May 2015, Dave Chinner wrote: It has nothing to do with journalling, and everything to do with bring filesystems to an *idle state* before suspend runs. We have a long history of bug reports with XFS that go: suspend, resume, XFS almost immediately detects corruption, shuts down.

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

2015-05-15 Thread Alan Stern
On Fri, 15 May 2015, Alan Stern wrote: On Fri, 15 May 2015, NeilBrown wrote: Some storage devices don't handle suspend as well as they should and lose requests resulting in corruption. They should obviously be fixed, but it is you who gets the problem reports and you are not in a

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

2015-05-14 Thread Ming Lei
On Fri, May 15, 2015 at 8:59 AM, Rafael J. Wysocki wrote: > On Fri, May 15, 2015 at 2:40 AM, Ming Lei wrote: >> On Fri, May 15, 2015 at 8:34 AM, Rafael J. Wysocki >> wrote: >>> On Friday, May 15, 2015 09:54:26 AM Dave Chinner wrote: ng back On Thu, May 14, 2015 at 09:22:51AM +1000,

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

2015-05-14 Thread NeilBrown
On Fri, 15 May 2015 09:54:26 +1000 Dave Chinner wrote: > ng back On Thu, May 14, 2015 at 09:22:51AM +1000, NeilBrown wrote: > > On Mon, 11 May 2015 11:44:28 +1000 Dave Chinner wrote: > > > > > On Fri, May 08, 2015 at 03:08:43AM -0400, Len Brown wrote: > > > > From: Len Brown > > > > > > > >

  1   2   >