Re: Memory leak in 2.6.11-rc1?

[Dave Hansen <haveblue@us.ibm.com>]

On Wed, 2005-02-02 at 10:27 -0800, Linus Torvalds wrote:
> How many of these pages do you see? It's normal for a single pipe to be 
> associated with up to 16 pages (although that would only happen if there 
> is no reader or a slow reader, which is obviously not very common). 

Strangely enough, it seems to be one single, persistent page.  

> Now, if your memory freeing code depends on the fact that all HIGHMEM
> pages are always "freeable" (page cache + VM mappings only), then yes, the
> new pipe code introduces highmem pages that weren't highmem before.  But
> such long-lived and unfreeable pages have been there before too:  kernel
> modules (or any other vmalloc() user, for that matter) also do the same
> thing.

That might be it.  For now, I just change the GFP masks for vmalloc() so
that I don't have to deal with it, yet.  But, I certainly can see that
how this is a new user of highmem.

I did go around killing processes like mad to see if any of them still
had a hold of the pipe, but the shotgun approach didn't seem to help.

> Now, there _is_ another possibility here: we might have had a pipe leak
> before, and the new pipe code would potentially make it a lot more
> noticeable, with up to sixteen times as many pages lost if somebody freed
> a pipe inode without calling "free_pipe_info()". I don't see where that 
> would happen - all the normal "release" functions seem fine.
> 
> Hmm.. Adding a 
> 
> 	WARN_ON(inode->i_pipe);
> 
> to "iput_final()" might be a good idea - showing if somebody is releasing 
> an inode while it still associated with a pipe-info data structure.
> 
> Also, while I don't see how a write could leak, but maybe you could you
> add a
> 
> 	WARN_ON(buf->ops);
> 
> to the pipe_writev() case just before we insert a new buffer (ie to just
> after the comment that says "Insert it into the buffer array"). Just to
> see if the circular buffer handling might overwrite an old entry (although
> I _really_ don't see that - it's not like the code is complex, and it
> would also be accompanied by data-loss in the pipe, so we'd have seen
> that, methinks).

I'll put the warnings in, and see if anything comes up.

-- Dave