Re: staged dm_internal_{suspend, resume} related changes for wider review

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel



[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux