From: Alex Lyakas <alex@zadara.com>
To: David Sterba <dsterba@suse.cz>, Nikolay Borisov <nborisov@suse.com>
Cc: linux-btrfs <linux-btrfs@vger.kernel.org>, robbieko@synology.com
Subject: Re: [PATCH 0/7] eb reference count cleanups
Date: Wed, 6 Feb 2019 16:26:15 +0200 [thread overview]
Message-ID: <CAOcd+r3CzY2w94Y--9Nb+Fawv_JGFLnK3AgZdYWjNOsKth_aeQ@mail.gmail.com> (raw)
In-Reply-To: <20181106143024.GC24115@twin.jikos.cz>
Hi Nikolay, David,
Isn't patch 5 (btrfs: Remove extra reference count bumps in
btrfs_compare_trees) fixing a memory leak, and hence should be tagged
as "stable"? I am specifically interested in 4.14.x.
The comment says "remove redundant calls to extent_buffer_get since
they don't really add any value". But with the extra ref-count, the
extent buffer will not be properly freed and will cause a memory leak,
won't it?
Thanks,
Alex.
On Tue, Nov 6, 2018 at 4:30 PM David Sterba <dsterba@suse.cz> wrote:
>
> On Wed, Aug 15, 2018 at 06:26:50PM +0300, Nikolay Borisov wrote:
> > Here is a series which simplifies the way eb are used in EXTENT_BUFFER_UNMAPPED
> > context. The end goal was to remove the special "if we have ref count of 2 and
> > EXTENT_BUFFER_UNMAPPED flag then act as if this is the last ref and free the
> > buffer" case. To enable this the first 6 patches modify call sites which
> > needlessly bump the reference count.
> >
> > Patch 1 & 2 remove some btree locking when we are operating on unmapped extent
> > buffers. Each patch's changelog explains why this is safe to do .
> >
> > Patch 3,4,5 and 6 remove redundant calls to extent_buffer_get since they don't
> > really add any value. In all 3 cases having a reference count of 1 is sufficient
> > for the eb to be freed via btrfs_release_path.
> >
> > Patch 7 removes the special handling of EXTENT_BUFFER_UNMAPPED flag in
> > free_extent_buffer. Also adjust the selftest code to account for this change
> > by calling one extra time free_extent_buffer. Also document which references
> > are being dropped. All in all this shouldn't have any functional bearing.
> >
> > This was tested with multiple full xfstest runs as well as unloading the btrfs
> > module after each one to trigger the leak check and ensure no eb's are leaked.
> > I've also run it through btrfs' selftests multiple times with no problems.
> >
> > With this set applied EXTENT_BUFFER_UNMAPPED seems to be relevant only for
> > selftest which leads me to believe it can be removed altogether. I will
> > investigate this next but in the meantime this series should be good to go.
>
> Besides the 8/7 patch, the rest was in for-next for a long time so I'm
> merging that to misc-next, targeting 4.21. I'll do one last pass
> thhrough fstests with the full set and then upddate and push the branch
> so there might be some delay before it appears in the public repo.
> Thanks for the cleanup.
next prev parent reply other threads:[~2019-02-06 14:26 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-15 15:26 [PATCH 0/7] eb reference count cleanups Nikolay Borisov
2018-08-15 15:26 ` [PATCH 1/7] btrfs: Remove needless locking in iterate_inode_refs Nikolay Borisov
2018-08-15 15:26 ` [PATCH 2/7] btrfs: Remove needless locking in iterate_inode_extrefs Nikolay Borisov
2018-08-15 15:26 ` [PATCH 3/7] btrfs: Remove redundant extent_buffer_get in get_old_root Nikolay Borisov
2018-08-15 15:26 ` [PATCH 4/7] btrfs: Remove extraneous extent_buffer_get from tree_mod_log_rewind Nikolay Borisov
2018-08-15 15:26 ` [PATCH 5/7] btrfs: Remove extra reference count bumps in btrfs_compare_trees Nikolay Borisov
2018-08-15 15:26 ` [PATCH 6/7] btrfs: Remove unnecessary locking code in qgroup_rescan_leaf Nikolay Borisov
2018-08-15 15:26 ` [PATCH 7/7] btrfs: Remove special handling of EXTENT_BUFFER_UNMAPPED while freeing Nikolay Borisov
2018-09-27 12:40 ` [PATCH 0/7] eb reference count cleanups David Sterba
2018-10-15 14:04 ` [PATCH] btrfs: Adjust loop in free_extent_buffer Nikolay Borisov
2018-11-06 14:30 ` [PATCH 0/7] eb reference count cleanups David Sterba
2019-02-06 14:26 ` Alex Lyakas [this message]
2019-02-06 14:36 ` Nikolay Borisov
2019-02-06 15:33 ` Alex Lyakas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAOcd+r3CzY2w94Y--9Nb+Fawv_JGFLnK3AgZdYWjNOsKth_aeQ@mail.gmail.com \
--to=alex@zadara.com \
--cc=dsterba@suse.cz \
--cc=linux-btrfs@vger.kernel.org \
--cc=nborisov@suse.com \
--cc=robbieko@synology.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).