Re: [PATCH V2] Free map to avoid resource leak issues

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

 




On 07/17/2018 10:17 AM, Mariusz Tkaczyk wrote:
>
>
> On 07/17/2018 08:48 AM, Guoqing Jiang wrote:
>>
>>
>> On 07/13/2018 09:36 PM, Tkaczyk, Mariusz wrote:
>>> Try this:
>>> mdadm --zero-super /dev/sd*
>>> mdadm -C /dev/md/imsm -n2 -eimsm /dev/sdb /dev/sdc --run
>>> mdadm -C /dev/md/r1 -n2 -z15G -eimsm /dev/sdb /dev/sdc -l1 --run 
>>> --assume-clean
>>> mdadm -f /dev/md126 /dev/sdb
>>> mdadm -Ss
>>>
>>> Remember about exporting IMSM_NO_PLATFORM and IMSM_DEVNAME_AS_SERIAL.
>>> The problem occurs when map file is freeing.
>>> The previous reproduction steps works for me, because I had old, 
>>> invalid entry in /var/run/mdadm/map.
>>> This issue seems to be related with this file.
>>> I didn't reproduce it on native metadata without outdated entry in 
>>> this file.
>>
>> Thanks, since Manage_stop calls map_remove and map_unlock, but *mapp 
>> is not set to NULL after
>> map_remove -> map_free, so map_unlock will call map_free again. Pls 
>> try below to see it works for
>> you or not.
>>
>> diff --git a/mapfile.c b/mapfile.c
>> index a50255632d28..8d7acb3cc389 100644
>> --- a/mapfile.c
>> +++ b/mapfile.c
>> @@ -268,6 +268,7 @@ void map_remove(struct map_ent **mapp, char *devnm)
>>         map_delete(mapp, devnm);
>>         map_write(*mapp);
>>         map_free(*mapp);
>> +       *mapp = NULL;
>>  }
>>
>>
>> In current code, after call map_free(), some places set the map 
>> pointer to NULL while some not.
>> Jes, do you think it is necessary to change all the place? Thanks.
>>
>> gqjiang@linux-8mgw:~/source-tree/mdadm> grep "map_free(" *.c -r -A 1
>> config.c:            map_free(map);
>> config.c-            map = NULL;
>> -- 
>> Detail.c:            map_free(map);
>> Detail.c-        } else {
>> -- 
>> Detail.c:            map_free(map);
>> Detail.c-        }
>> -- 
>> Incremental.c:    map_free(mapl);
>> Incremental.c-    return rv;
>> -- 
>> Incremental.c:        map_free(map);
>> Incremental.c-    }
>> -- 
>> Incremental.c:        map_free(map);
>> Incremental.c-        map = NULL;
>> -- 
>> Incremental.c:    map_free(map);
>> Incremental.c-    return 0;
>> -- 
>> Incremental.c:        map_free(map);
>> Incremental.c-    }
>> -- 
>> mapfile.c:        map_free(*melp);
>> mapfile.c-    map_read(melp);
>> -- 
>> mapfile.c:        map_free(*melp);
>> mapfile.c-    lf = NULL;
>> -- 
>> mapfile.c:void map_free(struct map_ent *map)
>> mapfile.c-{
>> -- 
>> mapfile.c:    map_free(map);
>> mapfile.c-    return rv;
>> -- 
>> mapfile.c:    map_free(*mapp);
>> mapfile.c-}
>> -- 
>> mapfile.c:    map_free(map);
>> mapfile.c-    free_mdstat(mdstat);
>> -- 
>> mdadm.c:            map_free(map);
>> mdadm.c-            map = NULL;
>>
>>
>> Regards,
>> Guoqing
>
> First tests looks promising but I want to run full scope of our 
> internal tests. It should take one day.
> I will inform you about results.
>
> Thanks,
> Mariusz
>
>
All tests passed, the issue in IMSM is fixed.

Thanks,
Mariusz
��.n��������+%������w��{.n�����{����w��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f




[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux