[dm-devel] staged dm_internal_{suspend, resume} related changes for wider review

Mikulas Patocka mpatocka at redhat.com
Fri Jan 2 22:56:44 UTC 2015


Hi, I'm back.

I looked at current internal suspend code in 3.19.

One thing that seems strange is this:

static void __dm_internal_suspend(struct mapped_device *md, unsigned suspend_flags)
{
        struct dm_table *map = NULL;

        if (dm_suspended_internally_md(md))
                return; /* nested internal suspend */
....
static void __dm_internal_resume(struct mapped_device *md)
{
        if (!dm_suspended_internally_md(md))
                return; /* resume from nested internal suspend */

- the comments imply that the code supports nested internal suspend, while 
in fact it doesn't. If the caller calls:
* dm_internal_suspend_noflush
* dm_internal_suspend_noflush
* dm_internal_resume
the device is not suspended while it should be.

The "if" branch in __dm_internal_resume has nothing to do with nested 
suspend, it's called if the caller calls dm_internal_resume without 
dm_internal_suspend_noflush - that shouldn't happen, so there should be 
WARN or BUG.

If you want to support multiple internal suspends (which is needed if one 
table can have multiple dm-thin targets), you need a counter that counts 
how many times the device was suspended - the counter doesn't have to be 
atomic because the code already holds md->suspend_lock. If you don't want 
to support multiple internal suspends, there should be at least WARN when 
the caller attempts to do it.

Mikulas




More information about the dm-devel mailing list