Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 13 Aug 2022 12:09:52 +0200
From:   Borislav Petkov <bp@alien8.de>
To:     Mateusz =?utf-8?Q?Jo=C5=84czyk?= <mat.jonczyk@o2.pl>
Cc:     Yazen Ghannam <yazen.ghannam@amd.com>, linux-edac@vger.kernel.org,
        linux-kernel@vger.kernel.org, tony.luck@intel.com, x86@kernel.org,
        Smita.KoralahalliChannabasappa@amd.com, mpatocka@redhat.com,
        gregkh@linuxfoundation.org
Subject: Re: [PATCH] x86/MCE/AMD: Decrement threshold_bank refcount when
 removing threshold blocks
Message-ID: <Yvd4cCW4ng2cG96p@zn.tnic>
References: <20220614174346.3648305-1-yazen.ghannam@amd.com>
 <7c8e34c8-f12d-e7e4-b6bc-4867824865ea@o2.pl>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <7c8e34c8-f12d-e7e4-b6bc-4867824865ea@o2.pl>
Precedence: bulk

On Fri, Aug 12, 2022 at 11:14:44PM +0200, Mateusz Jończyk wrote:
> Shouldn't there be "kobject_put(&pos->kobj)" here instead?

Yes, it should.

> Also, it seems to me that "kobject_put(b->kobj);" before the loop
> may be relocated after the loop - so that the refcounts on the child
> objects are decreased first, then the refcount on the parent object.

Yes, I guess we can do that.

> Additionally, shouldn't there be a call to
> "kobject_put(&b->blocks->kobj);" in __threshold_remove_blocks()?

Makes sense, we do

	kobject_add(&b->blocks->kobj, ...

in __threshold_add_blocks().

> From what I understand, b->blocks is a list head, so we need to
> decrease the refcount on it too.

Not list_heads - we modify the refcount of kobjects. See what the arg of
kobject_put() is.

> After these changes, the __threshold_remove_blocks() function looks
> very similar to deallocate_threshold_blocks() function just above it.

Yes, minus the list_del(&pos->miscj); But that can be made conditional
with a bool arg to deallocate_threshold_blocks() and then remove
__threshold_remove_blocks().

Care to take Yazen's patch, fix it up, test it thoroughly (you should
enable KASAN to catch any potential memory leaks) and send it?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette