So I'm going to list all the things that are wrong with this in the off chance 
that you will actually improve and then go back to ignoring your emails.

1) You test your patches before you send them.  Xfstests is how you test.

2) You seem to think page_cache_release releases the page cache.  It doesn't, 
it releases a ref on the page, we take a ref while we are doing IO and then 
drop it when we are done.  It is not evicted from cache until the VM decides to 
do it at some point in the future, so all your patch does is make us leak page 
cache.

3) You don't free the compressed pages.  This leaks memory.  These pages are 
only used for writing out, there is absolutely no reason to keep them around 
after the IO is complete, we have the actual page with the real data still in 
cache.

4) You also made it so we don't free the the struct we use to track the 
compressed IO, which is just as useless as the compressed pages now, causing 
another memory leak.

Had you tested this patch it would have killed the box pretty quickly and you 
would have known all of this.  But you didn't, and wasted a lot of much smarter 
peoples time.  This is not OK, this gets you ignored.  Do not do it again.  
Thanks,

Josef

Nick Krause <xerofo...@gmail.com> wrote:


On Wed, Jul 30, 2014 at 4:51 PM, Zach Brown <z...@zabbo.net> wrote:
> On Wed, Jul 30, 2014 at 04:47:12PM -0400, Josef Bacik wrote:
>> On 07/30/2014 04:42 PM, Nicholas Krause wrote:
>> >This patch removes the lines for releasing the page cache in certain
>> >files as this may aid in perfomance with writes in the compression
>> >rountines of btrfs. Please note that this patch has not been tested
>> >on my own hardware due to no compression based btrfs volumes of my
>> >own.
>> >
>>
>> https://urldefense.proofpoint.com/v1/url?u=http://37.media.tumblr.com/tumblr_m44v95k9Y11qjhgo6o1_250.gif&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=cKCbChRKsMpTX8ybrSkonQ%3D%3D%0A&m=epKcfiwbd4AA3VbfORegZjsqgl8dEZjg%2B3XKikkWVBM%3D%0A&s=5dd9a127c17ca09d584d86694d71f37cd336eab3f17658f8c0dccbfe2a53e2d8
>
> https://urldefense.proofpoint.com/v1/url?u=https://www.youtube.com/watch?v%3DjVfkYZmXHAg&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=cKCbChRKsMpTX8ybrSkonQ%3D%3D%0A&m=epKcfiwbd4AA3VbfORegZjsqgl8dEZjg%2B3XKikkWVBM%3D%0A&s=5402110ffa4beccf257af0fc266e8cab20264119fe9bdb2e4734c8ba3e49a211
>
> - z


Zach,
Is there a reason for freeing the page cache as I seem to be missing why?
Regards Nick
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to