On Fri, Jun 29, 2018 at 09:07:12PM +0800, Qu Wenruo wrote: > >> @@ -5117,7 +5117,7 @@ void free_extent_buffer(struct extent_buffer *eb) > >> > >> spin_lock(&eb->refs_lock); > >> if (atomic_read(&eb->refs) == 2 && > >> - test_bit(EXTENT_BUFFER_DUMMY, &eb->bflags)) > >> + test_bit(EXTENT_BUFFER_PRIVATE, &eb->bflags)) > >> atomic_dec(&eb->refs); > > Also discussed in off list mail, this extra atomic_dec for cloned eb > looks confusing. > (That also explains why after cloning the eb, we call > extent_buffer_get() and only frees it once, and still no eb leaking) > What about just removing such special handling?
The extent_buffer_get can be moved to the cloning function, all callers increase the reference but the missing dec is indeed confusing. However I don't think we can remove it completely. We need to keep the eb::refs at the same level for all eb types (regular, STALE, DUMMY), so we can use eg. free_extent_buffer. There may be other way how to clean it that I don't see now, so if you think you can improve the reference handling, then go for it. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
