From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.1 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D5726C432BE for ; Sat, 28 Aug 2021 19:37:21 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 3087960EB5 for ; Sat, 28 Aug 2021 19:37:21 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 3087960EB5 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=morinfr.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id AA5356B006C; Sat, 28 Aug 2021 15:37:20 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A54B06B0071; Sat, 28 Aug 2021 15:37:20 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 942DD8D0001; Sat, 28 Aug 2021 15:37:20 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0163.hostedemail.com [216.40.44.163]) by kanga.kvack.org (Postfix) with ESMTP id 783656B006C for ; Sat, 28 Aug 2021 15:37:20 -0400 (EDT) Received: from smtpin29.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 13900182D5C84 for ; Sat, 28 Aug 2021 19:37:20 +0000 (UTC) X-FDA: 78525498240.29.80D9A5F Received: from smtp4-g21.free.fr (smtp4-g21.free.fr [212.27.42.4]) by imf30.hostedemail.com (Postfix) with ESMTP id 89F2AE0016BB for ; Sat, 28 Aug 2021 19:37:19 +0000 (UTC) Received: from bender.morinfr.org (unknown [82.64.86.27]) by smtp4-g21.free.fr (Postfix) with ESMTPS id DDA7219F58A; Sat, 28 Aug 2021 21:37:17 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=morinfr.org ; s=20170427; h=In-Reply-To:Content-Type:MIME-Version:References:Message-ID: Subject:To:From:Date:Sender:Reply-To:Cc:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=Eg7jIAyWdnvSe9BizhWpgYxEQwOfxC+45v1t71BBM2Q=; b=FIrTWsfqFMjx6IsmX3sH67wt/S vrvMIZYejksNVNuxGibFxvlSZWDrxZBNEwb87X5iSIWPcWYF7oI7JA58Glx/pwiIlu3d56yrnAgJt tfnGq0tRMzjjO1IbbrI6v+kZ3KXaWAjHbPne5Aee8A2zi6JNwgJDXliGbP7DvDPJdGS8=; Received: from guillaum by bender.morinfr.org with local (Exim 4.92) (envelope-from ) id 1mK48j-0005iQ-KD; Sat, 28 Aug 2021 21:37:17 +0200 Date: Sat, 28 Aug 2021 21:37:17 +0200 From: Guillaume Morin To: almasrymina@google.com, mike.kravetz@oracle.com, cgroups@vger.kernel.org, guillaume@morinfr.org, linux-mm@kvack.org Subject: Re: [BUG] potential hugetlb css refcounting issues Message-ID: <20210828193716.GA21491@bender.morinfr.org> Mail-Followup-To: almasrymina@google.com, mike.kravetz@oracle.com, cgroups@vger.kernel.org, guillaume@morinfr.org, linux-mm@kvack.org References: <8a4f2fbc-76e8-b67b-f110-30beff2228f5@oracle-com> <20210827225841.GA30891@bender.morinfr.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210827225841.GA30891@bender.morinfr.org> User-Agent: Mutt/1.10.1 (2018-07-13) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=morinfr.org header.s=20170427 header.b=FIrTWsfq; dmarc=none; spf=none (imf30.hostedemail.com: domain of guillaume@morinfr.org has no SPF policy when checking 212.27.42.4) smtp.mailfrom=guillaume@morinfr.org X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 89F2AE0016BB X-Stat-Signature: gpaohmfhek67iujoimntu4jiia5sdce1 X-HE-Tag: 1630179439-789947 X-Bogosity: Ham, tests=bogofilter, spamicity=0.012037, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 28 Aug 0:58, Guillaume Morin wrote: > > I am not sure about the above analysis. It is true that > > hugetlb_cgroup_uncharge_page_rsvd is called unconditionally in > > free_huge_page. However, IIUC hugetlb_cgroup_uncharge_page_rsvd will > > only decrement the css refcount if there is a non-NULL hugetlb_cgroup > > pointer in the page. And, the pointer in the page would only be set > > in the 'deferred_reserve' path of alloc_huge_page. Unless I am > > missing something, they seem to balance. > > Now that you explain, I am pretty sure that you're right and I was > wrong. > > I'll confirm that I can't reproduce without my change for 2. Confirmed. With the patch for the first issue, the issue is indeed fixed. I must have messed up something during my testing... Anyway, this is the change for 1: diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 8ea35ba6699f..00ad4af0399b 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -4033,8 +4033,11 @@ static void hugetlb_vm_op_open(struct vm_area_struct *vma) * after this open call completes. It is therefore safe to take a * new reference here without additional locking. */ - if (resv && is_vma_resv_set(vma, HPAGE_RESV_OWNER)) + if (resv && is_vma_resv_set(vma, HPAGE_RESV_OWNER)) { + if (resv->css) + css_get(resv->css); kref_get(&resv->refs); + } } static void hugetlb_vm_op_close(struct vm_area_struct *vma) -- Guillaume Morin From mboxrd@z Thu Jan 1 00:00:00 1970 From: Guillaume Morin Subject: Re: [BUG] potential hugetlb css refcounting issues Date: Sat, 28 Aug 2021 21:37:17 +0200 Message-ID: <20210828193716.GA21491@bender.morinfr.org> References: <8a4f2fbc-76e8-b67b-f110-30beff2228f5@oracle-com> <20210827225841.GA30891@bender.morinfr.org> Mime-Version: 1.0 Return-path: DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=morinfr.org ; s=20170427; h=In-Reply-To:Content-Type:MIME-Version:References:Message-ID: Subject:To:From:Date:Sender:Reply-To:Cc:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=Eg7jIAyWdnvSe9BizhWpgYxEQwOfxC+45v1t71BBM2Q=; b=FIrTWsfqFMjx6IsmX3sH67wt/S vrvMIZYejksNVNuxGibFxvlSZWDrxZBNEwb87X5iSIWPcWYF7oI7JA58Glx/pwiIlu3d56yrnAgJt tfnGq0tRMzjjO1IbbrI6v+kZ3KXaWAjHbPne5Aee8A2zi6JNwgJDXliGbP7DvDPJdGS8=; Content-Disposition: inline In-Reply-To: <20210827225841.GA30891-iHhE99jIcVYJQ5r9O9UB6R2eb7JE58TQ@public.gmane.org> List-ID: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: almasrymina-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org, mike.kravetz-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org, cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, guillaume-/FyPzM6KSZdAfugRpC6u6w@public.gmane.org, linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org On 28 Aug 0:58, Guillaume Morin wrote: > > I am not sure about the above analysis. It is true that > > hugetlb_cgroup_uncharge_page_rsvd is called unconditionally in > > free_huge_page. However, IIUC hugetlb_cgroup_uncharge_page_rsvd will > > only decrement the css refcount if there is a non-NULL hugetlb_cgroup > > pointer in the page. And, the pointer in the page would only be set > > in the 'deferred_reserve' path of alloc_huge_page. Unless I am > > missing something, they seem to balance. > > Now that you explain, I am pretty sure that you're right and I was > wrong. > > I'll confirm that I can't reproduce without my change for 2. Confirmed. With the patch for the first issue, the issue is indeed fixed. I must have messed up something during my testing... Anyway, this is the change for 1: diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 8ea35ba6699f..00ad4af0399b 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -4033,8 +4033,11 @@ static void hugetlb_vm_op_open(struct vm_area_struct *vma) * after this open call completes. It is therefore safe to take a * new reference here without additional locking. */ - if (resv && is_vma_resv_set(vma, HPAGE_RESV_OWNER)) + if (resv && is_vma_resv_set(vma, HPAGE_RESV_OWNER)) { + if (resv->css) + css_get(resv->css); kref_get(&resv->refs); + } } static void hugetlb_vm_op_close(struct vm_area_struct *vma) -- Guillaume Morin