mbox series

[0/5] Fix memory leak on failed cache-writes

Message ID 20200211151023.16060-1-johannes.thumshirn@wdc.com
Headers show
Series Fix memory leak on failed cache-writes | expand

Message

Johannes Thumshirn Feb. 11, 2020, 3:10 p.m. UTC
Fstests' test case generic/475 reliably leaks the btrfs_io_ctl::pages
allocated in __btrfs_write_out_cache().

The first four patches are small things I noticed while hunting down the
problem and are independant of the last patch in this series freeing the pages
when we throw away a dirty block group.

Johannes Thumshirn (5):
  btrfs: use inode from io_ctl in io_ctl_prepare_pages
  btrfs: make the uptodate argument of io_ctl_add_pages() boolean.
  btrfs: use standard debug config option to enable free-space-cache
    debug prints
  btrfs: simplify error handling in __btrfs_write_out_cache()
  btrfs: free allocated pages jon failed cache write-out

 fs/btrfs/disk-io.c          |  6 ++++++
 fs/btrfs/free-space-cache.c | 44 ++++++++++++++++++++++++--------------------
 fs/btrfs/free-space-cache.h |  1 +
 3 files changed, 31 insertions(+), 20 deletions(-)

Comments

David Sterba Feb. 11, 2020, 5:59 p.m. UTC | #1
On Wed, Feb 12, 2020 at 12:10:18AM +0900, Johannes Thumshirn wrote:
> Fstests' test case generic/475 reliably leaks the btrfs_io_ctl::pages
> allocated in __btrfs_write_out_cache().
> 
> The first four patches are small things I noticed while hunting down the
> problem and are independant of the last patch in this series freeing the pages
> when we throw away a dirty block group.

Thanks, just a note about the patch ordering: the fix 5/5 should go
first so it applies cleanly without the unrelated cleanups. As it's a
leak fix it'll go to some rc and then to stable.
Johannes Thumshirn Feb. 11, 2020, 6:47 p.m. UTC | #2
On 11/02/2020 19:45, David Sterba wrote:
> On Wed, Feb 12, 2020 at 12:10:18AM +0900, Johannes Thumshirn wrote:
>> Fstests' test case generic/475 reliably leaks the btrfs_io_ctl::pages
>> allocated in __btrfs_write_out_cache().
>>
>> The first four patches are small things I noticed while hunting down the
>> problem and are independant of the last patch in this series freeing the pages
>> when we throw away a dirty block group.
> 
> Thanks, just a note about the patch ordering: the fix 5/5 should go
> first so it applies cleanly without the unrelated cleanups. As it's a
> leak fix it'll go to some rc and then to stable.
> 


OK I can reorder if I need to resend.
Josef Bacik Feb. 13, 2020, 8:10 p.m. UTC | #3
On 2/11/20 10:10 AM, Johannes Thumshirn wrote:
> Fstests' test case generic/475 reliably leaks the btrfs_io_ctl::pages
> allocated in __btrfs_write_out_cache().
> 
> The first four patches are small things I noticed while hunting down the
> problem and are independant of the last patch in this series freeing the pages
> when we throw away a dirty block group.
> 
> Johannes Thumshirn (5):
>    btrfs: use inode from io_ctl in io_ctl_prepare_pages
>    btrfs: make the uptodate argument of io_ctl_add_pages() boolean.
>    btrfs: use standard debug config option to enable free-space-cache
>      debug prints
>    btrfs: simplify error handling in __btrfs_write_out_cache()
>    btrfs: free allocated pages jon failed cache write-out
> 
>   fs/btrfs/disk-io.c          |  6 ++++++
>   fs/btrfs/free-space-cache.c | 44 ++++++++++++++++++++++++--------------------
>   fs/btrfs/free-space-cache.h |  1 +
>   3 files changed, 31 insertions(+), 20 deletions(-)
> 

Weird, I swear I reviewed these yesterday, but I don't see any replies from me. 
You can add

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

to the whole series.  Thanks,

Josef