Re: [Findutils-patches] [PATCH] find memory leak

[Goffredo Baroncelli]

On 2017-02-02 23:31, Bernhard Voelker wrote:
> Hi Goffredo,
> 
> On 02/02/2017 08:48 PM, Goffredo Baroncelli wrote:
>> On 2017-02-02 00:34, Bernhard Voelker wrote:
>>> On 12/15/2016 10:19 PM, Goffredo Baroncelli wrote:
>>>> Hi All,
>> [...]
>>>>
>>>> # Without the patch, this command requires up to 602MB of memory !!!
>>>> $ /usr/bin/time ./find /lib/modules/ -ignore_readdir_race      \( -fstype 
>>>> NFS -o -fstype nfs -o -fstype nfs4 -o -fstype afs -o -fstype binfmt_misc 
>>>> -o -fstype proc -o -fstype smbfs -o -fstype autofs -o -fstype iso9660 -o 
>>>> -fstype ncpfs -o -fstype coda -o -fstype devpts -o -fstype ftpfs -o 
>>>> -fstype devfs -o -fstype mfs -o -fstype shfs -o -fstype sysfs -o -fstype 
>>>> cifs -o -fstype lustre_lite -o -fstype tmpfs -o -fstype usbfs -o -fstype 
>>>> udf -o -fstype ocfs2 -o      -type d -false  \) -prune -o -print0  | wc -l
>>>> 31.59user 27.17system 0:58.82elapsed 99%CPU (0avgtext+0avgdata 
>>>> 602476maxresident)k
>>>> 0inputs+0outputs (0major+150129minor)pagefaults 0swaps
>>>>
>>>> # With the patch, the same command as above requires only 2MB of memory !!!
>>>> $ /usr/bin/time ./find /lib/modules/ -ignore_readdir_race      \( -fstype 
>>>> NFS -o -fstype nfs -o -fstype nfs4 -o -fstype afs -o -fstype binfmt_misc 
>>>> -o -fstype proc -o -fstype smbfs -o -fstype autofs -o -fstype iso9660 -o 
>>>> -fstype ncpfs -o -fstype coda -o -fstype devpts -o -fstype ftpfs -o 
>>>> -fstype devfs -o -fstype mfs -o -fstype shfs -o -fstype sysfs -o -fstype 
>>>> cifs -o -fstype lustre_lite -o -fstype tmpfs -o -fstype usbfs -o -fstype 
>>>> udf -o -fstype ocfs2 -o      -type d -false  \) -prune -o -print0  | wc -l
>>>> 30.94user 26.14system 0:57.14elapsed 99%CPU (0avgtext+0avgdata 
>>>> 2764maxresident)k
>>>> 0inputs+0outputs (0major+165minor)pagefaults 0swaps
>>>>
>>>>
>>>> # for curiosity my /lib/modules contains
>>>> $ find /lib/modules/ | wc -l
>>>> 68101
>>>>
>>>> Finally, what about caching the value of read_file_system_list() instead 
>>>> of recomputing it every time ? On the basis of my tests I found an 
>>>> impressive gain in terms of cpu usage. But what are the cons ?
>>>>
>>>>
>>>> BR
>>>> G.Baroncelli
>>>
>>> I cannot reproduce here - i.e. I do not see such a huge memory leak here 
>>> ... even
>>> if I add some 8 bind-mounts so that /proc/self/mountinfo has 30k.
>>> How does your test system look like? Is there anything special about your 
>>> directory
>>> structure?  
>>
>> I don't think that my directory structure is special in any way. 
>>
>> $ ls /lib/mnodules
>> 3.16.0-4-amd64  4.3.0-trunk-amd64  4.6.0-1-amd64  4.8.15  4.9.5
>> 3.16-3-amd64    4.4.0-rc8-amd64    4.8.0-2-amd64  4.9.3
>>
>>
>> In your testa, have you used all the "-fstype ...." arguments ? IIRC the 
>> leak size depends by the number of "-fstype ...."
> 
> yes, I did.  But my /lib/modules only has  9470 files, 

my one has 12000 files; so the numbers are comparables.



> so the memory usage without
> the patches was only
>       Maximum resident set size (kbytes): 4136
> and with the patches it's down to
>       Maximum resident set size (kbytes): 2556

Which tools are you using to get these info ?

> 
> So I'm wondering why your find uses 110M.  Even if I try for '/' (which has
> 1281671 files output by that find command), then only 16-18M (without/with 
> the patch)
> are used here:
>       Maximum resident set size (kbytes): 17892
>       Maximum resident set size (kbytes): 15800
> 
> Thus said, 110M sound excessive.
> 
>>> What does 'valgrind --leak-check=full --show-leak-kinds=all ./find ...'
>>> tell you about the leak(s)? How many lost blocks do you have?
>>
>> Below the output:
>>
>>
>>
>> $ valgrind --leak-check=full --show-leak-kinds=all  /usr/bin/find 
>> /lib/modules/ -ignore_readdir_race      \( -fstype NFS -o -fstype nfs -o 
>> -fstype nfs4 -o -fstype afs -o -fstype binfmt_misc -o -fstype proc -o 
>> -fstype smbfs -o -fstype autofs -o -fstype iso9660 -o -fstype ncpfs -o 
>> -fstype coda -o -fstype devpts -o -fstype ftpfs -o -fstype devfs -o -fstype 
>> mfs -o -fstype shfs -o -fstype sysfs -o -fstype cifs -o -fstype lustre_lite 
>> -o -fstype tmpfs -o -fstype usbfs -o -fstype udf -o -fstype ocfs2 -o      
>> -type d -false  \) -prune -o -print0  | wc -l                     
>> ==4520== Memcheck, a memory error detector                                   
>>                        
>> ==4520== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.     
>>                        
>> ==4520== Using Valgrind-3.12.0 and LibVEX; rerun with -h for copyright info  
>>                        
>> ==4520== Command: /usr/bin/find /lib/modules/ -ignore_readdir_race ( -fstype 
>> NFS -o -fstype nfs -o -fstype nfs4 -o -fstype afs -o -fstype binfmt_misc -o 
>> -fstype proc -o -fstype smbfs -o -fstype autofs -o -fstype iso9660 -o 
>> -fstype ncpfs -o -fstype coda -o -fstype devpts -o -fstype ftpfs -o -fstype 
>> devfs -o -fstype mfs -o -fstype shfs -o -fstype sysfs -o -fstype cifs -o 
>> -fstype lustre_lite -o -fstype tmpfs -o -fstype usbfs -o -fstype udf -o 
>> -fstype ocfs2 -o -type d -false ) -prune -o -print0
>> ==4520== 
>>
>>
>>
>> 0
>> ==4520== 
>> ==4520== HEAP SUMMARY:
>> ==4520==     in use at exit: 7,625,040 bytes in 3,340,960 blocks
>> ==4520==   total heap usage: 17,220,046 allocs, 13,879,086 frees, 
>> 550,821,122 bytes allocated
[...]
>> ==4520== 
>> ==4520== LEAK SUMMARY:
>> ==4520==    definitely lost: 7,529,984 bytes in 3,336,628 blocks
>> ==4520==    indirectly lost: 75,944 bytes in 4,268 blocks
>> ==4520==      possibly lost: 0 bytes in 0 blocks
>> ==4520==    still reachable: 19,112 bytes in 64 blocks
>> ==4520==         suppressed: 0 bytes in 0 blocks
>> ==4520== 
>> ==4520== For counts of detected and suppressed errors, rerun with: -v
>> ==4520== ERROR SUMMARY: 5 errors from 5 contexts (suppressed: 0 from 0)
>>
>>
>> Pay attention that I removed some kernel modules directories from 
>> /lib/modules after my email, so now the memory usage is lower
>>
>> /usr/bin/time /usr/bin/find /lib/modules/ -ignore_readdir_race      \( 
>> -fstype NFS -o -fstype nfs -o -fstype nfs4 -o -fstype afs -o -fstype 
>> binfmt_misc -o -fstype proc -o -fstype smbfs -o -fstype autofs -o -fstype 
>> iso9660 -o -fstype ncpfs -o -fstype coda -o -fstype devpts -o -fstype ftpfs 
>> -o -fstype devfs -o -fstype mfs -o -fstype shfs -o -fstype sysfs -o -fstype 
>> cifs -o -fstype lustre_lite -o -fstype tmpfs -o -fstype usbfs -o -fstype udf 
>> -o -fstype ocfs2 -o      -type d -false  \) -prune -o -print0  | wc -l
>> 5.62user 5.00system 0:10.63elapsed 99%CPU (0avgtext+0avgdata 
>> 110204maxresident)k
>> 0inputs+0outputs (0major+26952minor)pagefaults 0swaps
>>
>> (only 110MB used, but before the memory usage was ~600MB)
> 
> Did you try with your patch (or incl. my patches) as well?
The test above is without the patche.

$ dpkg --list | grep findutils
ii  findutils                                     4.6.0+git+20161106-1          
       amd64        utilities for finding files--find, xargs


> 
> The above valgrind output doesn't look too bad, actually.  It seems
> that it is not (only) a memory leak, but instead regular use of memory which
> is correctly free()d: only those ~8M are lost (and parts of that would be 
> freed
> with the patches).

This is not clear to me: "time" states that the maximum memory occupation is 
near 110MB (see above). With the patches "time" states that it requires only 
2.7MB

/usr/bin/time ./find /lib/modules/ -ignore_readdir_race      \( -fstype NFS -o 
-fstype nfs -o -fstype nfs4 -o -fstype afs -o -fstype binfmt_misc -o -fstype 
proc -o -fstype smbfs -o -fstype autofs -o -fstype iso9660 -o -fstype ncpfs -o 
-fstype coda -o -fstype devpts -o -fstype ftpfs -o -fstype devfs -o -fstype mfs 
-o -fstype shfs -o -fstype sysfs -o -fstype cifs -o -fstype lustre_lite -o 
-fstype tmpfs -o -fstype usbfs -o -fstype udf -o -fstype ocfs2 -o      -type d 
-false  \) -prune -o -print0  | wc -l
0.09user 0.09system 0:00.23elapsed 79%CPU (0avgtext+0avgdata 2728maxresident)k
1088inputs+0outputs (1major+162minor)pagefaults 0swaps


the differences should be the leak....
are we looking to the same info ?

Which find version are you testing, maybe that there are other variables which 
we aren't considering.

> 
> Have a nice day,
> Berny
> 

BR
G.Baroncelli