Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Rafael J. Wysocki]

Hi,

On Tue, Nov 3, 2015 at 10:31 AM, Jan Kara  wrote:
> On Tue 03-11-15 11:10:53, Dave Chinner wrote:
>> On Mon, Nov 02, 2015 at 03:43:07AM +0100, Rafael J. Wysocki wrote:
>> > I guess it may also helps to address the case when a device is removed 
>> > from a
>> > suspended system, written to on another system in the meantime and inserted
>> > back into the (still suspended) original system which then is resumed.  
>> > Today
>> > this is an almost guaranteed data corruption scenario, but if the 
>> > filesystem in
>> > question is properly frozen during suspend, the driver should be able to 
>> > detect
>> > superblock changes during unfreeze.
>>
>> Never going to work. There is no guarantee that a write to a
>> filesystem by a third party device is going to change the superblock
>> (or any metadata in the rest of the filesystem) in any detectable
>> way.  Hence freezing filesystems will not prevent Bad Things
>> Happening if you do this while your system is suspended.
>
> Agreed, we should never advertise something like this works. OTOH the truth
> is that e.g. in ext4 case a simple check in ext4_unfreeze() could catch 90%
> of cases where user shot himself in the foot like this (i.e., ext4 driver
> will update write time in superblock if it gets mounted somewhere else and
> we can check whether that didn't change in ext4_unfreeze()) and refuse to
> touch the filesystem... It is not 100% reliable since user could have used
> e.g. debuge2fs to arbitrarily modify the filesystem but in such cases they
> have to know what they are doing anyway.

Well, my idea was to use this for sanity checking.  I guess that
sanity checks here don't really hurt, do they?  And if they fail,
perhaps we can just avoid touching the fs again for safety reasons?

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Jan Kara]

On Tue 03-11-15 11:10:53, Dave Chinner wrote:
> On Mon, Nov 02, 2015 at 03:43:07AM +0100, Rafael J. Wysocki wrote:
> > I guess it may also helps to address the case when a device is removed from 
> > a
> > suspended system, written to on another system in the meantime and inserted
> > back into the (still suspended) original system which then is resumed.  
> > Today
> > this is an almost guaranteed data corruption scenario, but if the 
> > filesystem in
> > question is properly frozen during suspend, the driver should be able to 
> > detect
> > superblock changes during unfreeze.
> 
> Never going to work. There is no guarantee that a write to a
> filesystem by a third party device is going to change the superblock
> (or any metadata in the rest of the filesystem) in any detectable
> way.  Hence freezing filesystems will not prevent Bad Things
> Happening if you do this while your system is suspended.

Agreed, we should never advertise something like this works. OTOH the truth
is that e.g. in ext4 case a simple check in ext4_unfreeze() could catch 90%
of cases where user shot himself in the foot like this (i.e., ext4 driver
will update write time in superblock if it gets mounted somewhere else and
we can check whether that didn't change in ext4_unfreeze()) and refuse to
touch the filesystem... It is not 100% reliable since user could have used
e.g. debuge2fs to arbitrarily modify the filesystem but in such cases they
have to know what they are doing anyway.

Honza
-- 
Jan Kara 
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Jan Kara]

On Tue 03-11-15 11:10:53, Dave Chinner wrote:
> On Mon, Nov 02, 2015 at 03:43:07AM +0100, Rafael J. Wysocki wrote:
> > I guess it may also helps to address the case when a device is removed from 
> > a
> > suspended system, written to on another system in the meantime and inserted
> > back into the (still suspended) original system which then is resumed.  
> > Today
> > this is an almost guaranteed data corruption scenario, but if the 
> > filesystem in
> > question is properly frozen during suspend, the driver should be able to 
> > detect
> > superblock changes during unfreeze.
> 
> Never going to work. There is no guarantee that a write to a
> filesystem by a third party device is going to change the superblock
> (or any metadata in the rest of the filesystem) in any detectable
> way.  Hence freezing filesystems will not prevent Bad Things
> Happening if you do this while your system is suspended.

Agreed, we should never advertise something like this works. OTOH the truth
is that e.g. in ext4 case a simple check in ext4_unfreeze() could catch 90%
of cases where user shot himself in the foot like this (i.e., ext4 driver
will update write time in superblock if it gets mounted somewhere else and
we can check whether that didn't change in ext4_unfreeze()) and refuse to
touch the filesystem... It is not 100% reliable since user could have used
e.g. debuge2fs to arbitrarily modify the filesystem but in such cases they
have to know what they are doing anyway.

Honza
-- 
Jan Kara 
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Rafael J. Wysocki]

Hi,

On Tue, Nov 3, 2015 at 10:31 AM, Jan Kara  wrote:
> On Tue 03-11-15 11:10:53, Dave Chinner wrote:
>> On Mon, Nov 02, 2015 at 03:43:07AM +0100, Rafael J. Wysocki wrote:
>> > I guess it may also helps to address the case when a device is removed 
>> > from a
>> > suspended system, written to on another system in the meantime and inserted
>> > back into the (still suspended) original system which then is resumed.  
>> > Today
>> > this is an almost guaranteed data corruption scenario, but if the 
>> > filesystem in
>> > question is properly frozen during suspend, the driver should be able to 
>> > detect
>> > superblock changes during unfreeze.
>>
>> Never going to work. There is no guarantee that a write to a
>> filesystem by a third party device is going to change the superblock
>> (or any metadata in the rest of the filesystem) in any detectable
>> way.  Hence freezing filesystems will not prevent Bad Things
>> Happening if you do this while your system is suspended.
>
> Agreed, we should never advertise something like this works. OTOH the truth
> is that e.g. in ext4 case a simple check in ext4_unfreeze() could catch 90%
> of cases where user shot himself in the foot like this (i.e., ext4 driver
> will update write time in superblock if it gets mounted somewhere else and
> we can check whether that didn't change in ext4_unfreeze()) and refuse to
> touch the filesystem... It is not 100% reliable since user could have used
> e.g. debuge2fs to arbitrarily modify the filesystem but in such cases they
> have to know what they are doing anyway.

Well, my idea was to use this for sanity checking.  I guess that
sanity checks here don't really hurt, do they?  And if they fail,
perhaps we can just avoid touching the fs again for safety reasons?

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Rafael J. Wysocki]

On Tuesday, November 03, 2015 11:10:53 AM Dave Chinner wrote:
> On Mon, Nov 02, 2015 at 03:43:07AM +0100, Rafael J. Wysocki wrote:
> > I guess it may also helps to address the case when a device is removed from 
> > a
> > suspended system, written to on another system in the meantime and inserted
> > back into the (still suspended) original system which then is resumed.  
> > Today
> > this is an almost guaranteed data corruption scenario, but if the 
> > filesystem in
> > question is properly frozen during suspend, the driver should be able to 
> > detect
> > superblock changes during unfreeze.
> 
> Never going to work. There is no guarantee that a write to a
> filesystem by a third party device is going to change the superblock
> (or any metadata in the rest of the filesystem) in any detectable
> way.  Hence freezing filesystems will not prevent Bad Things
> Happening if you do this while your system is suspended.

OK, thanks for the clarification.

Cheers,
Rafael

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

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Dave Chinner]

On Mon, Nov 02, 2015 at 03:43:07AM +0100, Rafael J. Wysocki wrote:
> I guess it may also helps to address the case when a device is removed from a
> suspended system, written to on another system in the meantime and inserted
> back into the (still suspended) original system which then is resumed.  Today
> this is an almost guaranteed data corruption scenario, but if the filesystem 
> in
> question is properly frozen during suspend, the driver should be able to 
> detect
> superblock changes during unfreeze.

Never going to work. There is no guarantee that a write to a
filesystem by a third party device is going to change the superblock
(or any metadata in the rest of the filesystem) in any detectable
way.  Hence freezing filesystems will not prevent Bad Things
Happening if you do this while your system is suspended.

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Alan Stern]

On Mon, 2 Nov 2015, Jiri Kosina wrote:

> On Mon, 2 Nov 2015, Rafael J. Wysocki wrote:

> > BTW, the freezing of filesystems during system suspend (not hibernation) 
> > makes
> > sense too, because it will help to address the long-standing issue with 
> > storage
> > devices that go away while the system is suspended.
> > 
> > I guess it may also helps to address the case when a device is removed from 
> > a
> > suspended system, written to on another system in the meantime and inserted
> > back into the (still suspended) original system which then is resumed.  
> > Today
> > this is an almost guaranteed data corruption scenario, but if the 
> > filesystem in
> > question is properly frozen during suspend, the driver should be able to 
> > detect
> > superblock changes during unfreeze.

I agree completely; adding filesystem freezing is a great idea.

> > So the approach I'd suggest would be to add the freezing of filesystems to 
> > the
> > suspend/resume code paths just for the above reasons and drop the kthreads
> > freezing from the filesystems that support the proper freezing.  The rest
> > should be easier to deal with then.
> 
> That alone makes sense. It'll however leave a load of freezer users in the 
> kernel that make no sense (one example picked completely out of the air: 
> w1_process(); what is the reason for it there?) and are likely broken 
> (completely random examples again: md, xfsaild -- they think they are 
> freezable, but they are not).

If you fix just the kthreads which do this (along with an explanation
in the changelog of why the kthread is wrong), while leaving the others
alone, that ought to be acceptable.

Alan Stern

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

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Oliver Neukum]

On Mon, 2015-11-02 at 11:45 +0100, Jiri Kosina wrote:
> > For example, if user space does a "read" or "write" on a character
> device
> > which is runtime-suspended at that point, the driver may want to
> resume the
> > device temporarily, carry out the operation and suspend it again,
> but that
> > generally won't work for the system suspend case.
> 
> But why would this even be relevant in this discussion, given that at
> the 
> point we are talking about, the whole userspace has been frozen
> already?

It really doesn't matter whether the thread in question is a kernel
thread or user space. Device detection is even worse.
Kernel threads that do either of these things must stop at
defined points. You can use the freezer or go to another mechanism.
I just doubt they'd act much different in the end.

Regards
Oliver


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

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Jiri Kosina]

On Mon, 2 Nov 2015, Rafael J. Wysocki wrote:

> > > > BTW, a quite some of this has been already "pre-discussed" in 
> > > > Documentation/power/freezing-of-tasks.txt (which has BTW been written 
> > > > before we've had the possibility to freeze filesystems, and this fact 
> > > > is 
> > > > even point there out).
> > > 
> > > That is somewhat outdated in my view.  At least my list of reasons for 
> > > using
> > > the freezer is now somewhat different from the one given in that file.
> > 
> > Well, so what is the list then, and how do you know that kthread_run() 
> > users are behaving according to that list? That's basically exactly what 
> > this effort is about -- making some sense out of current situation.
> 
> Currently, the #1 reason for using the freezer is to prevent user space
> from interacting with devices during system suspend/resume.  BTW, that also
> covers devices with runtime PM support, because the handling of user space
> access to them in the runtime PM case may be different.
> 
> For example, if user space does a "read" or "write" on a character device
> which is runtime-suspended at that point, the driver may want to resume the
> device temporarily, carry out the operation and suspend it again, but that
> generally won't work for the system suspend case.

But why would this even be relevant in this discussion, given that at the 
point we are talking about, the whole userspace has been frozen already?

> The #2 reason in my view is that essentially the freezer is what makes a
> difference between runtime idle and system suspend on platforms without
> firmware/hardware suspend support (or the lightweight variants of suspend
> in general).  Namely, it effectively prevents user space from setting up
> timers in the future and helps to reduce interrupt noise causing the CPUs
> to leave deep low-power states too often (at least in some cases).

I am of course at all not trying to remove freezer for userspace. This 
whole discussion is solely about explicit usage of freezer in kthreads.

> The #3 reason is to provide a way for things that might touch persistent
> storage after a hibernation image had been created to prevent themselves from
> running during that time.  Note that it also is a good idea to prevent such
> things from running during system suspend/resume in general so they don't
> try to access devices at wrong times.
> 
> BTW, the freezing of filesystems during system suspend (not hibernation) makes
> sense too, because it will help to address the long-standing issue with 
> storage
> devices that go away while the system is suspended.
> 
> I guess it may also helps to address the case when a device is removed from a
> suspended system, written to on another system in the meantime and inserted
> back into the (still suspended) original system which then is resumed.  Today
> this is an almost guaranteed data corruption scenario, but if the filesystem 
> in
> question is properly frozen during suspend, the driver should be able to 
> detect
> superblock changes during unfreeze.
> 
> So the approach I'd suggest would be to add the freezing of filesystems to the
> suspend/resume code paths just for the above reasons and drop the kthreads
> freezing from the filesystems that support the proper freezing.  The rest
> should be easier to deal with then.

That alone makes sense. It'll however leave a load of freezer users in the 
kernel that make no sense (one example picked completely out of the air: 
w1_process(); what is the reason for it there?) and are likely broken 
(completely random examples again: md, xfsaild -- they think they are 
freezable, but they are not).

Thanks,

-- 
Jiri Kosina
SUSE Labs

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

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Alan Stern]

On Mon, 2 Nov 2015, Jiri Kosina wrote:

> On Mon, 2 Nov 2015, Rafael J. Wysocki wrote:

> > BTW, the freezing of filesystems during system suspend (not hibernation) 
> > makes
> > sense too, because it will help to address the long-standing issue with 
> > storage
> > devices that go away while the system is suspended.
> > 
> > I guess it may also helps to address the case when a device is removed from 
> > a
> > suspended system, written to on another system in the meantime and inserted
> > back into the (still suspended) original system which then is resumed.  
> > Today
> > this is an almost guaranteed data corruption scenario, but if the 
> > filesystem in
> > question is properly frozen during suspend, the driver should be able to 
> > detect
> > superblock changes during unfreeze.

I agree completely; adding filesystem freezing is a great idea.

> > So the approach I'd suggest would be to add the freezing of filesystems to 
> > the
> > suspend/resume code paths just for the above reasons and drop the kthreads
> > freezing from the filesystems that support the proper freezing.  The rest
> > should be easier to deal with then.
> 
> That alone makes sense. It'll however leave a load of freezer users in the 
> kernel that make no sense (one example picked completely out of the air: 
> w1_process(); what is the reason for it there?) and are likely broken 
> (completely random examples again: md, xfsaild -- they think they are 
> freezable, but they are not).

If you fix just the kthreads which do this (along with an explanation
in the changelog of why the kthread is wrong), while leaving the others
alone, that ought to be acceptable.

Alan Stern

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

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Dave Chinner]

On Mon, Nov 02, 2015 at 03:43:07AM +0100, Rafael J. Wysocki wrote:
> I guess it may also helps to address the case when a device is removed from a
> suspended system, written to on another system in the meantime and inserted
> back into the (still suspended) original system which then is resumed.  Today
> this is an almost guaranteed data corruption scenario, but if the filesystem 
> in
> question is properly frozen during suspend, the driver should be able to 
> detect
> superblock changes during unfreeze.

Never going to work. There is no guarantee that a write to a
filesystem by a third party device is going to change the superblock
(or any metadata in the rest of the filesystem) in any detectable
way.  Hence freezing filesystems will not prevent Bad Things
Happening if you do this while your system is suspended.

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Jiri Kosina]

On Mon, 2 Nov 2015, Rafael J. Wysocki wrote:

> > > > BTW, a quite some of this has been already "pre-discussed" in 
> > > > Documentation/power/freezing-of-tasks.txt (which has BTW been written 
> > > > before we've had the possibility to freeze filesystems, and this fact 
> > > > is 
> > > > even point there out).
> > > 
> > > That is somewhat outdated in my view.  At least my list of reasons for 
> > > using
> > > the freezer is now somewhat different from the one given in that file.
> > 
> > Well, so what is the list then, and how do you know that kthread_run() 
> > users are behaving according to that list? That's basically exactly what 
> > this effort is about -- making some sense out of current situation.
> 
> Currently, the #1 reason for using the freezer is to prevent user space
> from interacting with devices during system suspend/resume.  BTW, that also
> covers devices with runtime PM support, because the handling of user space
> access to them in the runtime PM case may be different.
> 
> For example, if user space does a "read" or "write" on a character device
> which is runtime-suspended at that point, the driver may want to resume the
> device temporarily, carry out the operation and suspend it again, but that
> generally won't work for the system suspend case.

But why would this even be relevant in this discussion, given that at the 
point we are talking about, the whole userspace has been frozen already?

> The #2 reason in my view is that essentially the freezer is what makes a
> difference between runtime idle and system suspend on platforms without
> firmware/hardware suspend support (or the lightweight variants of suspend
> in general).  Namely, it effectively prevents user space from setting up
> timers in the future and helps to reduce interrupt noise causing the CPUs
> to leave deep low-power states too often (at least in some cases).

I am of course at all not trying to remove freezer for userspace. This 
whole discussion is solely about explicit usage of freezer in kthreads.

> The #3 reason is to provide a way for things that might touch persistent
> storage after a hibernation image had been created to prevent themselves from
> running during that time.  Note that it also is a good idea to prevent such
> things from running during system suspend/resume in general so they don't
> try to access devices at wrong times.
> 
> BTW, the freezing of filesystems during system suspend (not hibernation) makes
> sense too, because it will help to address the long-standing issue with 
> storage
> devices that go away while the system is suspended.
> 
> I guess it may also helps to address the case when a device is removed from a
> suspended system, written to on another system in the meantime and inserted
> back into the (still suspended) original system which then is resumed.  Today
> this is an almost guaranteed data corruption scenario, but if the filesystem 
> in
> question is properly frozen during suspend, the driver should be able to 
> detect
> superblock changes during unfreeze.
> 
> So the approach I'd suggest would be to add the freezing of filesystems to the
> suspend/resume code paths just for the above reasons and drop the kthreads
> freezing from the filesystems that support the proper freezing.  The rest
> should be easier to deal with then.

That alone makes sense. It'll however leave a load of freezer users in the 
kernel that make no sense (one example picked completely out of the air: 
w1_process(); what is the reason for it there?) and are likely broken 
(completely random examples again: md, xfsaild -- they think they are 
freezable, but they are not).

Thanks,

-- 
Jiri Kosina
SUSE Labs

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

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Oliver Neukum]

On Mon, 2015-11-02 at 11:45 +0100, Jiri Kosina wrote:
> > For example, if user space does a "read" or "write" on a character
> device
> > which is runtime-suspended at that point, the driver may want to
> resume the
> > device temporarily, carry out the operation and suspend it again,
> but that
> > generally won't work for the system suspend case.
> 
> But why would this even be relevant in this discussion, given that at
> the 
> point we are talking about, the whole userspace has been frozen
> already?

It really doesn't matter whether the thread in question is a kernel
thread or user space. Device detection is even worse.
Kernel threads that do either of these things must stop at
defined points. You can use the freezer or go to another mechanism.
I just doubt they'd act much different in the end.

Regards
Oliver


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

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Rafael J. Wysocki]

On Tuesday, November 03, 2015 11:10:53 AM Dave Chinner wrote:
> On Mon, Nov 02, 2015 at 03:43:07AM +0100, Rafael J. Wysocki wrote:
> > I guess it may also helps to address the case when a device is removed from 
> > a
> > suspended system, written to on another system in the meantime and inserted
> > back into the (still suspended) original system which then is resumed.  
> > Today
> > this is an almost guaranteed data corruption scenario, but if the 
> > filesystem in
> > question is properly frozen during suspend, the driver should be able to 
> > detect
> > superblock changes during unfreeze.
> 
> Never going to work. There is no guarantee that a write to a
> filesystem by a third party device is going to change the superblock
> (or any metadata in the rest of the filesystem) in any detectable
> way.  Hence freezing filesystems will not prevent Bad Things
> Happening if you do this while your system is suspended.

OK, thanks for the clarification.

Cheers,
Rafael

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

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Rafael J. Wysocki]

On Saturday, October 31, 2015 09:19:33 AM Jiri Kosina wrote:
> On Sat, 31 Oct 2015, Rafael J. Wysocki wrote:
> 
> > > > > > > I would say instead "no I/O is allowed from now on".  Maybe 
> > > > > > > that's an 
> > > > > > > overstatement, but I think it comes closer to the truth.
> > > > > 
> > > > > But that's what PM callbacks are for.
> > 
> > Not really.  In fact, PM callbacks may not be suitable for some use cases 
> > even
> > in theory (that's if you want something to stop running before any PM 
> > callbacks
> > are executed during suspend).
> 
> If that's the case, wouldn't then it be better to have .pre_suspend 
> callback as well, instead of having to do this kind of polling?

If that's an individual driver's kthread, I guess it should be sufficient to
stop it from the .prepare callback.  If it is something more generic, creating
a device for it just in order to be able to execute a PM callback from there
may be slightly overkill.

But again, I'm not aware of any real cases like that, so it may not be an issue
in practice.

> > > > The most natural implementation would be for the callback routine to 
> > > > set 
> > > > a flag; at various strategic points the kthread would check the flag 
> > > > and 
> > > > if it was set, call a routine that sits around and waits for the 
> > > > suspend 
> > > > to be over.  
> > > 
> > > Could you name at least some existing kthreads that would actually *need* 
> > > such complex handling, instead of just waiting in schedule() until 
> > > suspend-resume cycle is over, given that PM callbacks do all the 
> > > necessary 
> > > cleanup (putting HW to sleep, cancelling timers, etc) anyway?
> > > 
> > > PM callback can always explicitly do kthread_stop() on a particular 
> > > kthread if really necessary.
> > 
> > Runtime PM uses a freezable workqueue, allocated in pm_start_workqueue().
> 
> Yeah, agreed, as I said in the original mail, freezable workqueues I am 
> not covering for the moment, there might be a additional issues with them 
> compared to kthreads. That needs to be further looked into.

OK

> > > The main point of freezer is to reach quiescent state wrt. filesystems 
> > > (metadata in memory need to be absolutely in sync with what's on disk). 
> > > That's no different between hibernation and s2ram, is it?
> > > 
> > > BTW, a quite some of this has been already "pre-discussed" in 
> > > Documentation/power/freezing-of-tasks.txt (which has BTW been written 
> > > before we've had the possibility to freeze filesystems, and this fact is 
> > > even point there out).
> > 
> > That is somewhat outdated in my view.  At least my list of reasons for using
> > the freezer is now somewhat different from the one given in that file.
> 
> Well, so what is the list then, and how do you know that kthread_run() 
> users are behaving according to that list? That's basically exactly what 
> this effort is about -- making some sense out of current situation.

Currently, the #1 reason for using the freezer is to prevent user space
from interacting with devices during system suspend/resume.  BTW, that also
covers devices with runtime PM support, because the handling of user space
access to them in the runtime PM case may be different.

For example, if user space does a "read" or "write" on a character device
which is runtime-suspended at that point, the driver may want to resume the
device temporarily, carry out the operation and suspend it again, but that
generally won't work for the system suspend case.

Note that this applies to both system suspend (all variants of it for that
matter) and hibernation.

The #2 reason in my view is that essentially the freezer is what makes a
difference between runtime idle and system suspend on platforms without
firmware/hardware suspend support (or the lightweight variants of suspend
in general).  Namely, it effectively prevents user space from setting up
timers in the future and helps to reduce interrupt noise causing the CPUs
to leave deep low-power states too often (at least in some cases).

The #3 reason is to provide a way for things that might touch persistent
storage after a hibernation image had been created to prevent themselves from
running during that time.  Note that it also is a good idea to prevent such
things from running during system suspend/resume in general so they don't
try to access devices at wrong times.

BTW, the freezing of filesystems during system suspend (not hibernation) makes
sense too, because it will help to address the long-standing issue with storage
devices that go away while the system is suspended.

I guess it may also helps to address the case when a device is removed from a
suspended system, written to on another system in the meantime and inserted
back into the (still suspended) original system which then is resumed.  Today
this is an almost guaranteed data corruption scenario, but if the filesystem in
question is properly frozen during suspend, the driver should be able to detect

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Rafael J. Wysocki]

On Saturday, October 31, 2015 09:19:33 AM Jiri Kosina wrote:
> On Sat, 31 Oct 2015, Rafael J. Wysocki wrote:
> 
> > > > > > > I would say instead "no I/O is allowed from now on".  Maybe 
> > > > > > > that's an 
> > > > > > > overstatement, but I think it comes closer to the truth.
> > > > > 
> > > > > But that's what PM callbacks are for.
> > 
> > Not really.  In fact, PM callbacks may not be suitable for some use cases 
> > even
> > in theory (that's if you want something to stop running before any PM 
> > callbacks
> > are executed during suspend).
> 
> If that's the case, wouldn't then it be better to have .pre_suspend 
> callback as well, instead of having to do this kind of polling?

If that's an individual driver's kthread, I guess it should be sufficient to
stop it from the .prepare callback.  If it is something more generic, creating
a device for it just in order to be able to execute a PM callback from there
may be slightly overkill.

But again, I'm not aware of any real cases like that, so it may not be an issue
in practice.

> > > > The most natural implementation would be for the callback routine to 
> > > > set 
> > > > a flag; at various strategic points the kthread would check the flag 
> > > > and 
> > > > if it was set, call a routine that sits around and waits for the 
> > > > suspend 
> > > > to be over.  
> > > 
> > > Could you name at least some existing kthreads that would actually *need* 
> > > such complex handling, instead of just waiting in schedule() until 
> > > suspend-resume cycle is over, given that PM callbacks do all the 
> > > necessary 
> > > cleanup (putting HW to sleep, cancelling timers, etc) anyway?
> > > 
> > > PM callback can always explicitly do kthread_stop() on a particular 
> > > kthread if really necessary.
> > 
> > Runtime PM uses a freezable workqueue, allocated in pm_start_workqueue().
> 
> Yeah, agreed, as I said in the original mail, freezable workqueues I am 
> not covering for the moment, there might be a additional issues with them 
> compared to kthreads. That needs to be further looked into.

OK

> > > The main point of freezer is to reach quiescent state wrt. filesystems 
> > > (metadata in memory need to be absolutely in sync with what's on disk). 
> > > That's no different between hibernation and s2ram, is it?
> > > 
> > > BTW, a quite some of this has been already "pre-discussed" in 
> > > Documentation/power/freezing-of-tasks.txt (which has BTW been written 
> > > before we've had the possibility to freeze filesystems, and this fact is 
> > > even point there out).
> > 
> > That is somewhat outdated in my view.  At least my list of reasons for using
> > the freezer is now somewhat different from the one given in that file.
> 
> Well, so what is the list then, and how do you know that kthread_run() 
> users are behaving according to that list? That's basically exactly what 
> this effort is about -- making some sense out of current situation.

Currently, the #1 reason for using the freezer is to prevent user space
from interacting with devices during system suspend/resume.  BTW, that also
covers devices with runtime PM support, because the handling of user space
access to them in the runtime PM case may be different.

For example, if user space does a "read" or "write" on a character device
which is runtime-suspended at that point, the driver may want to resume the
device temporarily, carry out the operation and suspend it again, but that
generally won't work for the system suspend case.

Note that this applies to both system suspend (all variants of it for that
matter) and hibernation.

The #2 reason in my view is that essentially the freezer is what makes a
difference between runtime idle and system suspend on platforms without
firmware/hardware suspend support (or the lightweight variants of suspend
in general).  Namely, it effectively prevents user space from setting up
timers in the future and helps to reduce interrupt noise causing the CPUs
to leave deep low-power states too often (at least in some cases).

The #3 reason is to provide a way for things that might touch persistent
storage after a hibernation image had been created to prevent themselves from
running during that time.  Note that it also is a good idea to prevent such
things from running during system suspend/resume in general so they don't
try to access devices at wrong times.

BTW, the freezing of filesystems during system suspend (not hibernation) makes
sense too, because it will help to address the long-standing issue with storage
devices that go away while the system is suspended.

I guess it may also helps to address the case when a device is removed from a
suspended system, written to on another system in the meantime and inserted
back into the (still suspended) original system which then is resumed.  Today
this is an almost guaranteed data corruption scenario, but if the filesystem in
question is properly frozen during suspend, the driver should be able to detect

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Alan Stern]

On Sat, 31 Oct 2015, Rafael J. Wysocki wrote:

> Runtime PM uses a freezable workqueue, allocated in pm_start_workqueue().
> 
> That's because we don't want async runtime PM to happen during system
> suspend/resume and for good reasons, so if you want to remove the freezing
> mechanism, you need to stop that workqueue at the beginning of dpm_prepare
> and start it again at the end of dpm_complete().

The same sort of thing is true for the USB hub driver's workqueue.  
Since it registers new devices (as a result of hotplugs), it must stop
running at the beginning of the dpm_prepare stage.  Of course, that's
also a workqueue and not a kthread.

Alan Stern

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

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Alan Stern]

On Fri, 30 Oct 2015, Jiri Kosina wrote:

> On Fri, 30 Oct 2015, Alan Stern wrote:
> 
> > > > > I would say instead "no I/O is allowed from now on".  Maybe that's an 
> > > > > overstatement, but I think it comes closer to the truth.
> > > 
> > > But that's what PM callbacks are for.
> > 
> > Why are PM callbacks any more suitable than the freezer?  
> 
> Once the PM callback triggers, you know that you are really actually 
> undergoing suspend and have to do whatever is necessary.
> 
> OTOH, try_to_freeze() is a kind of "are we there yet?" polling, and the 
> whole state needs to be prepared pro-actively for suspend already when you 
> call it, each and every time, even if you are not going through suspend at 
> all.
> 
> That's sub-optimal, and very easy to get wrong over gradual code changes.

I think we are talking at cross purposes.  Your view of how a kthread 
works appears to be very different from mine.

Here's how I see it: A kthread performs some service, generally in a
big loop.  At certain points in that loop (perhaps only at the head),
the kthread will be in a suitable state for suspend: sufficiently
quiescent, no locks held, no I/O in progress, and so on.  At other
points, the kthread is not ready for suspend.

Therefore the fact that a PM callback tells you exactly when a supsend 
is about to occur is of no use.  The kthread can't act on that 
information directly, because most of the time it isn't ready for a 
suspend.  Only when it reaches one of its quiescent points will it be 
ready to do whatever is necessary -- which usually is nothing at all.

Given this picture, I don't see any alternative to a polling approach
of one kind or another.  At various quiescent points the kthread checks
to see if a suspend is imminent before moving forward.  At other times
the kthread can't handle suspend anyway, so it ignores the issue.  
This approach is exactly what try_to_freeze and friends support.

> > The most natural implementation would be for the callback routine to set 
> > a flag; at various strategic points the kthread would check the flag and 
> > if it was set, call a routine that sits around and waits for the suspend 
> > to be over.  
> 
> Could you name at least some existing kthreads that would actually *need* 
> such complex handling, instead of just waiting in schedule() until 
> suspend-resume cycle is over, given that PM callbacks do all the necessary 
> cleanup (putting HW to sleep, cancelling timers, etc) anyway?

Out of all the kthreads you modified in your patch 2/3, the only one
I'm familiar with is the one in f_mass_storage.c (the USB mass-storage
gadget driver).  That's kind of a special case, and I don't mind you
ripping out all the freezer stuff from it because its approach was
almost completely arbitrary all along.  AFAIK, we have never settled on
the right way for a USB gadget to handle system sleep.

Other cases I don't know about.  My argument is based on the general
analysis given above.  But consider one point: You said "instead of
just waiting in schedule() until suspend-resume cycle is over".  What
if, at the time of the PM callback, the kthread happens to be holding a
mutex that some driver needs to acquire while suspending?  If the
kthread just hops into schedule() and waits there, the suspend will
fail or deadlock.  This example shows that the situation is more
complicated than you make it appear.

> PM callback can always explicitly do kthread_stop() on a particular 
> kthread if really necessary.

Don't kthreads have to poll to find out when they need to stop?  How is 
that different from (or better than) polling to see when they need to 
freeze?

> > Also, you never replied to my question about suspend vs. hibernation.
> 
> The main point of freezer is to reach quiescent state wrt. filesystems 
> (metadata in memory need to be absolutely in sync with what's on disk). 
> That's no different between hibernation and s2ram, is it?

That was my point.  Since there's no difference, why did your patch 
talk about hibernation only, and not s2ram?

(Of course, some people like Len Brown have been arguing that for 
s2ram, the metadata in memory does _not_ need to be in sync with what's 
on disk.  I'm ignoring that for now, but such things would have to be 
taken into account when the final patch is written.)

Alan Stern

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

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Jiri Kosina]

On Sat, 31 Oct 2015, Rafael J. Wysocki wrote:

> > > > > > I would say instead "no I/O is allowed from now on".  Maybe that's 
> > > > > > an 
> > > > > > overstatement, but I think it comes closer to the truth.
> > > > 
> > > > But that's what PM callbacks are for.
> 
> Not really.  In fact, PM callbacks may not be suitable for some use cases even
> in theory (that's if you want something to stop running before any PM 
> callbacks
> are executed during suspend).

If that's the case, wouldn't then it be better to have .pre_suspend 
callback as well, instead of having to do this kind of polling?

> > > The most natural implementation would be for the callback routine to set 
> > > a flag; at various strategic points the kthread would check the flag and 
> > > if it was set, call a routine that sits around and waits for the suspend 
> > > to be over.  
> > 
> > Could you name at least some existing kthreads that would actually *need* 
> > such complex handling, instead of just waiting in schedule() until 
> > suspend-resume cycle is over, given that PM callbacks do all the necessary 
> > cleanup (putting HW to sleep, cancelling timers, etc) anyway?
> > 
> > PM callback can always explicitly do kthread_stop() on a particular 
> > kthread if really necessary.
> 
> Runtime PM uses a freezable workqueue, allocated in pm_start_workqueue().

Yeah, agreed, as I said in the original mail, freezable workqueues I am 
not covering for the moment, there might be a additional issues with them 
compared to kthreads. That needs to be further looked into.

> > The main point of freezer is to reach quiescent state wrt. filesystems 
> > (metadata in memory need to be absolutely in sync with what's on disk). 
> > That's no different between hibernation and s2ram, is it?
> > 
> > BTW, a quite some of this has been already "pre-discussed" in 
> > Documentation/power/freezing-of-tasks.txt (which has BTW been written 
> > before we've had the possibility to freeze filesystems, and this fact is 
> > even point there out).
> 
> That is somewhat outdated in my view.  At least my list of reasons for using
> the freezer is now somewhat different from the one given in that file.

Well, so what is the list then, and how do you know that kthread_run() 
users are behaving according to that list? That's basically exactly what 
this effort is about -- making some sense out of current situation.

Thanks,

-- 
Jiri Kosina
SUSE Labs

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

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Jiri Kosina]

On Sat, 31 Oct 2015, Rafael J. Wysocki wrote:

> > > > > > I would say instead "no I/O is allowed from now on".  Maybe that's 
> > > > > > an 
> > > > > > overstatement, but I think it comes closer to the truth.
> > > > 
> > > > But that's what PM callbacks are for.
> 
> Not really.  In fact, PM callbacks may not be suitable for some use cases even
> in theory (that's if you want something to stop running before any PM 
> callbacks
> are executed during suspend).

If that's the case, wouldn't then it be better to have .pre_suspend 
callback as well, instead of having to do this kind of polling?

> > > The most natural implementation would be for the callback routine to set 
> > > a flag; at various strategic points the kthread would check the flag and 
> > > if it was set, call a routine that sits around and waits for the suspend 
> > > to be over.  
> > 
> > Could you name at least some existing kthreads that would actually *need* 
> > such complex handling, instead of just waiting in schedule() until 
> > suspend-resume cycle is over, given that PM callbacks do all the necessary 
> > cleanup (putting HW to sleep, cancelling timers, etc) anyway?
> > 
> > PM callback can always explicitly do kthread_stop() on a particular 
> > kthread if really necessary.
> 
> Runtime PM uses a freezable workqueue, allocated in pm_start_workqueue().

Yeah, agreed, as I said in the original mail, freezable workqueues I am 
not covering for the moment, there might be a additional issues with them 
compared to kthreads. That needs to be further looked into.

> > The main point of freezer is to reach quiescent state wrt. filesystems 
> > (metadata in memory need to be absolutely in sync with what's on disk). 
> > That's no different between hibernation and s2ram, is it?
> > 
> > BTW, a quite some of this has been already "pre-discussed" in 
> > Documentation/power/freezing-of-tasks.txt (which has BTW been written 
> > before we've had the possibility to freeze filesystems, and this fact is 
> > even point there out).
> 
> That is somewhat outdated in my view.  At least my list of reasons for using
> the freezer is now somewhat different from the one given in that file.

Well, so what is the list then, and how do you know that kthread_run() 
users are behaving according to that list? That's basically exactly what 
this effort is about -- making some sense out of current situation.

Thanks,

-- 
Jiri Kosina
SUSE Labs

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

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Alan Stern]

On Sat, 31 Oct 2015, Rafael J. Wysocki wrote:

> Runtime PM uses a freezable workqueue, allocated in pm_start_workqueue().
> 
> That's because we don't want async runtime PM to happen during system
> suspend/resume and for good reasons, so if you want to remove the freezing
> mechanism, you need to stop that workqueue at the beginning of dpm_prepare
> and start it again at the end of dpm_complete().

The same sort of thing is true for the USB hub driver's workqueue.  
Since it registers new devices (as a result of hotplugs), it must stop
running at the beginning of the dpm_prepare stage.  Of course, that's
also a workqueue and not a kthread.

Alan Stern

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

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Alan Stern]

On Fri, 30 Oct 2015, Jiri Kosina wrote:

> On Fri, 30 Oct 2015, Alan Stern wrote:
> 
> > > > > I would say instead "no I/O is allowed from now on".  Maybe that's an 
> > > > > overstatement, but I think it comes closer to the truth.
> > > 
> > > But that's what PM callbacks are for.
> > 
> > Why are PM callbacks any more suitable than the freezer?  
> 
> Once the PM callback triggers, you know that you are really actually 
> undergoing suspend and have to do whatever is necessary.
> 
> OTOH, try_to_freeze() is a kind of "are we there yet?" polling, and the 
> whole state needs to be prepared pro-actively for suspend already when you 
> call it, each and every time, even if you are not going through suspend at 
> all.
> 
> That's sub-optimal, and very easy to get wrong over gradual code changes.

I think we are talking at cross purposes.  Your view of how a kthread 
works appears to be very different from mine.

Here's how I see it: A kthread performs some service, generally in a
big loop.  At certain points in that loop (perhaps only at the head),
the kthread will be in a suitable state for suspend: sufficiently
quiescent, no locks held, no I/O in progress, and so on.  At other
points, the kthread is not ready for suspend.

Therefore the fact that a PM callback tells you exactly when a supsend 
is about to occur is of no use.  The kthread can't act on that 
information directly, because most of the time it isn't ready for a 
suspend.  Only when it reaches one of its quiescent points will it be 
ready to do whatever is necessary -- which usually is nothing at all.

Given this picture, I don't see any alternative to a polling approach
of one kind or another.  At various quiescent points the kthread checks
to see if a suspend is imminent before moving forward.  At other times
the kthread can't handle suspend anyway, so it ignores the issue.  
This approach is exactly what try_to_freeze and friends support.

> > The most natural implementation would be for the callback routine to set 
> > a flag; at various strategic points the kthread would check the flag and 
> > if it was set, call a routine that sits around and waits for the suspend 
> > to be over.  
> 
> Could you name at least some existing kthreads that would actually *need* 
> such complex handling, instead of just waiting in schedule() until 
> suspend-resume cycle is over, given that PM callbacks do all the necessary 
> cleanup (putting HW to sleep, cancelling timers, etc) anyway?

Out of all the kthreads you modified in your patch 2/3, the only one
I'm familiar with is the one in f_mass_storage.c (the USB mass-storage
gadget driver).  That's kind of a special case, and I don't mind you
ripping out all the freezer stuff from it because its approach was
almost completely arbitrary all along.  AFAIK, we have never settled on
the right way for a USB gadget to handle system sleep.

Other cases I don't know about.  My argument is based on the general
analysis given above.  But consider one point: You said "instead of
just waiting in schedule() until suspend-resume cycle is over".  What
if, at the time of the PM callback, the kthread happens to be holding a
mutex that some driver needs to acquire while suspending?  If the
kthread just hops into schedule() and waits there, the suspend will
fail or deadlock.  This example shows that the situation is more
complicated than you make it appear.

> PM callback can always explicitly do kthread_stop() on a particular 
> kthread if really necessary.

Don't kthreads have to poll to find out when they need to stop?  How is 
that different from (or better than) polling to see when they need to 
freeze?

> > Also, you never replied to my question about suspend vs. hibernation.
> 
> The main point of freezer is to reach quiescent state wrt. filesystems 
> (metadata in memory need to be absolutely in sync with what's on disk). 
> That's no different between hibernation and s2ram, is it?

That was my point.  Since there's no difference, why did your patch 
talk about hibernation only, and not s2ram?

(Of course, some people like Len Brown have been arguing that for 
s2ram, the metadata in memory does _not_ need to be in sync with what's 
on disk.  I'm ignoring that for now, but such things would have to be 
taken into account when the final patch is written.)

Alan Stern

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

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Rafael J. Wysocki]

On Friday, October 30, 2015 10:17:54 PM Jiri Kosina wrote:
> On Fri, 30 Oct 2015, Alan Stern wrote:
> 
> > > > > I would say instead "no I/O is allowed from now on".  Maybe that's an 
> > > > > overstatement, but I think it comes closer to the truth.
> > > 
> > > But that's what PM callbacks are for.

Not really.  In fact, PM callbacks may not be suitable for some use cases even
in theory (that's if you want something to stop running before any PM callbacks
are executed during suspend).

> > Why are PM callbacks any more suitable than the freezer?  
> 
> Once the PM callback triggers, you know that you are really actually 
> undergoing suspend and have to do whatever is necessary.
> 
> OTOH, try_to_freeze() is a kind of "are we there yet?" polling, and the 
> whole state needs to be prepared pro-actively for suspend already when you 
> call it, each and every time, even if you are not going through suspend at 
> all.
> 
> That's sub-optimal, and very easy to get wrong over gradual code changes.
> 
> > The most natural implementation would be for the callback routine to set 
> > a flag; at various strategic points the kthread would check the flag and 
> > if it was set, call a routine that sits around and waits for the suspend 
> > to be over.  
> 
> Could you name at least some existing kthreads that would actually *need* 
> such complex handling, instead of just waiting in schedule() until 
> suspend-resume cycle is over, given that PM callbacks do all the necessary 
> cleanup (putting HW to sleep, cancelling timers, etc) anyway?
> 
> PM callback can always explicitly do kthread_stop() on a particular 
> kthread if really necessary.

Runtime PM uses a freezable workqueue, allocated in pm_start_workqueue().

That's because we don't want async runtime PM to happen during system
suspend/resume and for good reasons, so if you want to remove the freezing
mechanism, you need to stop that workqueue at the beginning of dpm_prepare
and start it again at the end of dpm_complete().

> > Also, you never replied to my question about suspend vs. hibernation.
> 
> The main point of freezer is to reach quiescent state wrt. filesystems 
> (metadata in memory need to be absolutely in sync with what's on disk). 
> That's no different between hibernation and s2ram, is it?
> 
> BTW, a quite some of this has been already "pre-discussed" in 
> Documentation/power/freezing-of-tasks.txt (which has BTW been written 
> before we've had the possibility to freeze filesystems, and this fact is 
> even point there out).

That is somewhat outdated in my view.  At least my list of reasons for using
the freezer is now somewhat different from the one given in that file.

In any case, there may be legitimate reasons for kernel threads to be
freezable, so you need to be careful about the wholesale removal of that
feature.

The above is the only one I can recall ATM, but that doesn't mean there
aren't any other.

Thanks,
Rafael

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

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Jiri Kosina]

On Fri, 30 Oct 2015, Alan Stern wrote:

> > > > I would say instead "no I/O is allowed from now on".  Maybe that's an 
> > > > overstatement, but I think it comes closer to the truth.
> > 
> > But that's what PM callbacks are for.
> 
> Why are PM callbacks any more suitable than the freezer?  

Once the PM callback triggers, you know that you are really actually 
undergoing suspend and have to do whatever is necessary.

OTOH, try_to_freeze() is a kind of "are we there yet?" polling, and the 
whole state needs to be prepared pro-actively for suspend already when you 
call it, each and every time, even if you are not going through suspend at 
all.

That's sub-optimal, and very easy to get wrong over gradual code changes.

> The most natural implementation would be for the callback routine to set 
> a flag; at various strategic points the kthread would check the flag and 
> if it was set, call a routine that sits around and waits for the suspend 
> to be over.  

Could you name at least some existing kthreads that would actually *need* 
such complex handling, instead of just waiting in schedule() until 
suspend-resume cycle is over, given that PM callbacks do all the necessary 
cleanup (putting HW to sleep, cancelling timers, etc) anyway?

PM callback can always explicitly do kthread_stop() on a particular 
kthread if really necessary.

> Also, you never replied to my question about suspend vs. hibernation.

The main point of freezer is to reach quiescent state wrt. filesystems 
(metadata in memory need to be absolutely in sync with what's on disk). 
That's no different between hibernation and s2ram, is it?

BTW, a quite some of this has been already "pre-discussed" in 
Documentation/power/freezing-of-tasks.txt (which has BTW been written 
before we've had the possibility to freeze filesystems, and this fact is 
even point there out).

Thanks,

-- 
Jiri Kosina
SUSE Labs

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

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Alan Stern]

On Fri, 30 Oct 2015, Jiri Kosina wrote:

> On Fri, 30 Oct 2015, Pavel Machek wrote:
> 
> > > I would say instead "no I/O is allowed from now on".  Maybe that's an 
> > > overstatement, but I think it comes closer to the truth.
> 
> But that's what PM callbacks are for.

Why are PM callbacks any more suitable than the freezer?  The most 
natural implementation would be for the callback routine to set a flag; 
at various strategic points the kthread would check the flag and if it 
was set, call a routine that sits around and waits for the suspend to 
be over.  How does that differ from using the freezer, apart from being 
more cumbersome and involving more code?

Also, you never replied to my question about suspend vs. hibernation.

Alan Stern

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

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Jiri Kosina]

On Fri, 30 Oct 2015, Pavel Machek wrote:

> > I would say instead "no I/O is allowed from now on".  Maybe that's an 
> > overstatement, but I think it comes closer to the truth.

But that's what PM callbacks are for.

> Exactly. And I'm pretty sure hardware drivers do use kernel threads,
> and do I/O from them.
> 
> LEDs are just one example

And why is that relevant? First, I don't see any freezable kthread in leds 
class implementation whatsoever. Second, I am pretty sure that it's quite 
unlikely to participate in filesystem I/O.

Sure, you need to suspend and resume the devices when going through 
suspend. That's why led_suspend() exists. How would try_to_freeze() help 
at all? That's basically just a fancy schedule() called at certain points.

-- 
Jiri Kosina
SUSE Labs

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

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Pavel Machek]

On Fri 2015-10-30 11:29:08, Alan Stern wrote:
> On Fri, 30 Oct 2015, Jiri Kosina wrote:
> 
> > This series is a followup to my proposal I brought up on Kernel Summit in 
> > Seoul. Noone seemed to had any principal objections, so let's have wider 
> > audience look into it.
> > 
> > In a nuthsell: freezing of kernel threads is horrible interface with 
> > unclear semantics and guarantees, and I am surprised it ever worked 
> > properly. Plus there are a lot of places that simply use it in a 
> > completely wrong way (which is not suprising, given the lack of defined 
> > semantics and requirements).
> > 
> > I've tested this over a series of suspend/resume cycles on several 
> > machines with at least ext4, btrfs and xfs, and it survived the testing 
> > without any harm.
> > 
> > Patch 1/3   implements the actual change, and has a more detailed 
> > explanation on "why?" and "how?" questions in the changelog
> 
> This patch talks about freezing in relation to hibernation.  What about 
> other forms of suspend?
> 
> Also, it replaces kthread freezing with filesystem freezing.  What 
> about kthreads performing I/O that doesn't go through a filesystem?  
> You write:
> 
> > the only facility that is needed during suspend: "no persistent fs
> > changes are allowed from now on"
> 
> I would say instead "no I/O is allowed from now on".  Maybe that's an 
> overstatement, but I think it comes closer to the truth.

Exactly. And I'm pretty sure hardware drivers do use kernel threads,
and do I/O from them.

LEDs are just one example

Best regards,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Alan Stern]

On Fri, 30 Oct 2015, Jiri Kosina wrote:

> This series is a followup to my proposal I brought up on Kernel Summit in 
> Seoul. Noone seemed to had any principal objections, so let's have wider 
> audience look into it.
> 
> In a nuthsell: freezing of kernel threads is horrible interface with 
> unclear semantics and guarantees, and I am surprised it ever worked 
> properly. Plus there are a lot of places that simply use it in a 
> completely wrong way (which is not suprising, given the lack of defined 
> semantics and requirements).
> 
> I've tested this over a series of suspend/resume cycles on several 
> machines with at least ext4, btrfs and xfs, and it survived the testing 
> without any harm.
> 
> Patch 1/3 implements the actual change, and has a more detailed 
>   explanation on "why?" and "how?" questions in the changelog

This patch talks about freezing in relation to hibernation.  What about 
other forms of suspend?

Also, it replaces kthread freezing with filesystem freezing.  What 
about kthreads performing I/O that doesn't go through a filesystem?  
You write:

> the only facility that is needed during suspend: "no persistent fs
> changes are allowed from now on"

I would say instead "no I/O is allowed from now on".  Maybe that's an 
overstatement, but I think it comes closer to the truth.

> Patch 2/3 nukes all (hopefully) the calls to freezer from kthreads 
>   in Linus' tree (as of 858e904bd71)
> 
> Patch 3/3 introduces WARN_ON() if anyone is trying to make use of 
>   this again
> 
> Open questions / discussion points:
> 
> - is the way I am traversing list of superblocks backwards enough to 
>   guarantee correct ordering? Especially: does this work as intended for 
>   FUSE?
> 
> - should freezable workqueues be dealt with the same way? I haven't even 
>   started to look into them in a serious way, but it seems like the 
>   drivers that are making use of them would actually like to use proper
>   PM callbacks instead

In the examples of freezable workqueues that I'm familiar with, what we 
really want is for the queue to stop running before the system goes any 
further into suspend.  This could be implemented with PM callbacks, but 
using the freezer instead seems a lot simpler.

Alan Stern

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

[PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Jiri Kosina]

This series is a followup to my proposal I brought up on Kernel Summit in 
Seoul. Noone seemed to had any principal objections, so let's have wider 
audience look into it.

In a nuthsell: freezing of kernel threads is horrible interface with 
unclear semantics and guarantees, and I am surprised it ever worked 
properly. Plus there are a lot of places that simply use it in a 
completely wrong way (which is not suprising, given the lack of defined 
semantics and requirements).

I've tested this over a series of suspend/resume cycles on several 
machines with at least ext4, btrfs and xfs, and it survived the testing 
without any harm.

Patch 1/3   implements the actual change, and has a more detailed 
explanation on "why?" and "how?" questions in the changelog

Patch 2/3   nukes all (hopefully) the calls to freezer from kthreads 
in Linus' tree (as of 858e904bd71)

Patch 3/3   introduces WARN_ON() if anyone is trying to make use of 
this again

Open questions / discussion points:

- is the way I am traversing list of superblocks backwards enough to 
  guarantee correct ordering? Especially: does this work as intended for 
  FUSE?

- should freezable workqueues be dealt with the same way? I haven't even 
  started to look into them in a serious way, but it seems like the 
  drivers that are making use of them would actually like to use proper
  PM callbacks instead

-- 
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Jiri Kosina]

On Fri, 30 Oct 2015, Alan Stern wrote:

> > > > I would say instead "no I/O is allowed from now on".  Maybe that's an 
> > > > overstatement, but I think it comes closer to the truth.
> > 
> > But that's what PM callbacks are for.
> 
> Why are PM callbacks any more suitable than the freezer?  

Once the PM callback triggers, you know that you are really actually 
undergoing suspend and have to do whatever is necessary.

OTOH, try_to_freeze() is a kind of "are we there yet?" polling, and the 
whole state needs to be prepared pro-actively for suspend already when you 
call it, each and every time, even if you are not going through suspend at 
all.

That's sub-optimal, and very easy to get wrong over gradual code changes.

> The most natural implementation would be for the callback routine to set 
> a flag; at various strategic points the kthread would check the flag and 
> if it was set, call a routine that sits around and waits for the suspend 
> to be over.  

Could you name at least some existing kthreads that would actually *need* 
such complex handling, instead of just waiting in schedule() until 
suspend-resume cycle is over, given that PM callbacks do all the necessary 
cleanup (putting HW to sleep, cancelling timers, etc) anyway?

PM callback can always explicitly do kthread_stop() on a particular 
kthread if really necessary.

> Also, you never replied to my question about suspend vs. hibernation.

The main point of freezer is to reach quiescent state wrt. filesystems 
(metadata in memory need to be absolutely in sync with what's on disk). 
That's no different between hibernation and s2ram, is it?

BTW, a quite some of this has been already "pre-discussed" in 
Documentation/power/freezing-of-tasks.txt (which has BTW been written 
before we've had the possibility to freeze filesystems, and this fact is 
even point there out).

Thanks,

-- 
Jiri Kosina
SUSE Labs

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

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Alan Stern]

On Fri, 30 Oct 2015, Jiri Kosina wrote:

> On Fri, 30 Oct 2015, Pavel Machek wrote:
> 
> > > I would say instead "no I/O is allowed from now on".  Maybe that's an 
> > > overstatement, but I think it comes closer to the truth.
> 
> But that's what PM callbacks are for.

Why are PM callbacks any more suitable than the freezer?  The most 
natural implementation would be for the callback routine to set a flag; 
at various strategic points the kthread would check the flag and if it 
was set, call a routine that sits around and waits for the suspend to 
be over.  How does that differ from using the freezer, apart from being 
more cumbersome and involving more code?

Also, you never replied to my question about suspend vs. hibernation.

Alan Stern

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

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Rafael J. Wysocki]

On Friday, October 30, 2015 10:17:54 PM Jiri Kosina wrote:
> On Fri, 30 Oct 2015, Alan Stern wrote:
> 
> > > > > I would say instead "no I/O is allowed from now on".  Maybe that's an 
> > > > > overstatement, but I think it comes closer to the truth.
> > > 
> > > But that's what PM callbacks are for.

Not really.  In fact, PM callbacks may not be suitable for some use cases even
in theory (that's if you want something to stop running before any PM callbacks
are executed during suspend).

> > Why are PM callbacks any more suitable than the freezer?  
> 
> Once the PM callback triggers, you know that you are really actually 
> undergoing suspend and have to do whatever is necessary.
> 
> OTOH, try_to_freeze() is a kind of "are we there yet?" polling, and the 
> whole state needs to be prepared pro-actively for suspend already when you 
> call it, each and every time, even if you are not going through suspend at 
> all.
> 
> That's sub-optimal, and very easy to get wrong over gradual code changes.
> 
> > The most natural implementation would be for the callback routine to set 
> > a flag; at various strategic points the kthread would check the flag and 
> > if it was set, call a routine that sits around and waits for the suspend 
> > to be over.  
> 
> Could you name at least some existing kthreads that would actually *need* 
> such complex handling, instead of just waiting in schedule() until 
> suspend-resume cycle is over, given that PM callbacks do all the necessary 
> cleanup (putting HW to sleep, cancelling timers, etc) anyway?
> 
> PM callback can always explicitly do kthread_stop() on a particular 
> kthread if really necessary.

Runtime PM uses a freezable workqueue, allocated in pm_start_workqueue().

That's because we don't want async runtime PM to happen during system
suspend/resume and for good reasons, so if you want to remove the freezing
mechanism, you need to stop that workqueue at the beginning of dpm_prepare
and start it again at the end of dpm_complete().

> > Also, you never replied to my question about suspend vs. hibernation.
> 
> The main point of freezer is to reach quiescent state wrt. filesystems 
> (metadata in memory need to be absolutely in sync with what's on disk). 
> That's no different between hibernation and s2ram, is it?
> 
> BTW, a quite some of this has been already "pre-discussed" in 
> Documentation/power/freezing-of-tasks.txt (which has BTW been written 
> before we've had the possibility to freeze filesystems, and this fact is 
> even point there out).

That is somewhat outdated in my view.  At least my list of reasons for using
the freezer is now somewhat different from the one given in that file.

In any case, there may be legitimate reasons for kernel threads to be
freezable, so you need to be careful about the wholesale removal of that
feature.

The above is the only one I can recall ATM, but that doesn't mean there
aren't any other.

Thanks,
Rafael

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

[PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Jiri Kosina]

This series is a followup to my proposal I brought up on Kernel Summit in 
Seoul. Noone seemed to had any principal objections, so let's have wider 
audience look into it.

In a nuthsell: freezing of kernel threads is horrible interface with 
unclear semantics and guarantees, and I am surprised it ever worked 
properly. Plus there are a lot of places that simply use it in a 
completely wrong way (which is not suprising, given the lack of defined 
semantics and requirements).

I've tested this over a series of suspend/resume cycles on several 
machines with at least ext4, btrfs and xfs, and it survived the testing 
without any harm.

Patch 1/3   implements the actual change, and has a more detailed 
explanation on "why?" and "how?" questions in the changelog

Patch 2/3   nukes all (hopefully) the calls to freezer from kthreads 
in Linus' tree (as of 858e904bd71)

Patch 3/3   introduces WARN_ON() if anyone is trying to make use of 
this again

Open questions / discussion points:

- is the way I am traversing list of superblocks backwards enough to 
  guarantee correct ordering? Especially: does this work as intended for 
  FUSE?

- should freezable workqueues be dealt with the same way? I haven't even 
  started to look into them in a serious way, but it seems like the 
  drivers that are making use of them would actually like to use proper
  PM callbacks instead

-- 
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Alan Stern]

On Fri, 30 Oct 2015, Jiri Kosina wrote:

> This series is a followup to my proposal I brought up on Kernel Summit in 
> Seoul. Noone seemed to had any principal objections, so let's have wider 
> audience look into it.
> 
> In a nuthsell: freezing of kernel threads is horrible interface with 
> unclear semantics and guarantees, and I am surprised it ever worked 
> properly. Plus there are a lot of places that simply use it in a 
> completely wrong way (which is not suprising, given the lack of defined 
> semantics and requirements).
> 
> I've tested this over a series of suspend/resume cycles on several 
> machines with at least ext4, btrfs and xfs, and it survived the testing 
> without any harm.
> 
> Patch 1/3 implements the actual change, and has a more detailed 
>   explanation on "why?" and "how?" questions in the changelog

This patch talks about freezing in relation to hibernation.  What about 
other forms of suspend?

Also, it replaces kthread freezing with filesystem freezing.  What 
about kthreads performing I/O that doesn't go through a filesystem?  
You write:

> the only facility that is needed during suspend: "no persistent fs
> changes are allowed from now on"

I would say instead "no I/O is allowed from now on".  Maybe that's an 
overstatement, but I think it comes closer to the truth.

> Patch 2/3 nukes all (hopefully) the calls to freezer from kthreads 
>   in Linus' tree (as of 858e904bd71)
> 
> Patch 3/3 introduces WARN_ON() if anyone is trying to make use of 
>   this again
> 
> Open questions / discussion points:
> 
> - is the way I am traversing list of superblocks backwards enough to 
>   guarantee correct ordering? Especially: does this work as intended for 
>   FUSE?
> 
> - should freezable workqueues be dealt with the same way? I haven't even 
>   started to look into them in a serious way, but it seems like the 
>   drivers that are making use of them would actually like to use proper
>   PM callbacks instead

In the examples of freezable workqueues that I'm familiar with, what we 
really want is for the queue to stop running before the system goes any 
further into suspend.  This could be implemented with PM callbacks, but 
using the freezer instead seems a lot simpler.

Alan Stern

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

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Pavel Machek]

On Fri 2015-10-30 11:29:08, Alan Stern wrote:
> On Fri, 30 Oct 2015, Jiri Kosina wrote:
> 
> > This series is a followup to my proposal I brought up on Kernel Summit in 
> > Seoul. Noone seemed to had any principal objections, so let's have wider 
> > audience look into it.
> > 
> > In a nuthsell: freezing of kernel threads is horrible interface with 
> > unclear semantics and guarantees, and I am surprised it ever worked 
> > properly. Plus there are a lot of places that simply use it in a 
> > completely wrong way (which is not suprising, given the lack of defined 
> > semantics and requirements).
> > 
> > I've tested this over a series of suspend/resume cycles on several 
> > machines with at least ext4, btrfs and xfs, and it survived the testing 
> > without any harm.
> > 
> > Patch 1/3   implements the actual change, and has a more detailed 
> > explanation on "why?" and "how?" questions in the changelog
> 
> This patch talks about freezing in relation to hibernation.  What about 
> other forms of suspend?
> 
> Also, it replaces kthread freezing with filesystem freezing.  What 
> about kthreads performing I/O that doesn't go through a filesystem?  
> You write:
> 
> > the only facility that is needed during suspend: "no persistent fs
> > changes are allowed from now on"
> 
> I would say instead "no I/O is allowed from now on".  Maybe that's an 
> overstatement, but I think it comes closer to the truth.

Exactly. And I'm pretty sure hardware drivers do use kernel threads,
and do I/O from them.

LEDs are just one example

Best regards,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Jiri Kosina]

On Fri, 30 Oct 2015, Pavel Machek wrote:

> > I would say instead "no I/O is allowed from now on".  Maybe that's an 
> > overstatement, but I think it comes closer to the truth.

But that's what PM callbacks are for.

> Exactly. And I'm pretty sure hardware drivers do use kernel threads,
> and do I/O from them.
> 
> LEDs are just one example

And why is that relevant? First, I don't see any freezable kthread in leds 
class implementation whatsoever. Second, I am pretty sure that it's quite 
unlikely to participate in filesystem I/O.

Sure, you need to suspend and resume the devices when going through 
suspend. That's why led_suspend() exists. How would try_to_freeze() help 
at all? That's basically just a fancy schedule() called at certain points.

-- 
Jiri Kosina
SUSE Labs

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