linux-kernel - Re: [PATCH 3/3] mm/hugetlb: Assert hugetlb_lock in __hugetlb_cgroup_commit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Fri, 19 Apr 2024 11:21:58 -0400
From: Peter Xu <peterx@...hat.com>
To: Mina Almasry <almasrymina@...gle.com>
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org,
	David Hildenbrand <david@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Muchun Song <muchun.song@...ux.dev>,
	David Rientjes <rientjes@...gle.com>
Subject: Re: [PATCH 3/3] mm/hugetlb: Assert hugetlb_lock in
 __hugetlb_cgroup_commit_charge

On Fri, Apr 19, 2024 at 08:03:08AM -0700, Mina Almasry wrote:
> On Wed, Apr 17, 2024 at 2:18 PM Peter Xu <peterx@...hat.com> wrote:
> >
> > This is similar to __hugetlb_cgroup_uncharge_folio() where it relies on
> > holding hugetlb_lock.  Add the similar assertion like the other one, since
> > it looks like such things may help some day.
> >
> > Signed-off-by: Peter Xu <peterx@...hat.com>
> 
> Reviewed-by: Mina Almasry <almasrymina@...gle.com>

Thanks.

> 
> > ---
> >  mm/hugetlb_cgroup.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
> > index aa4486bd3904..e20339a346b9 100644
> > --- a/mm/hugetlb_cgroup.c
> > +++ b/mm/hugetlb_cgroup.c
> > @@ -308,7 +308,7 @@ static void __hugetlb_cgroup_commit_charge(int idx, unsigned long nr_pages,
> >  {
> >         if (hugetlb_cgroup_disabled() || !h_cg)
> >                 return;
> > -
> > +       lockdep_assert_held(&hugetlb_lock);
> 
> Maybe also remove the comment on the top of the function:
> 
> /* Should be called with hugetlb_lock held */
> 
> Now that the function asserts, the comment seems redundant, but up to you.

IMHO there's no harm to be verbose in this case, so I'll just keep it
simple to be as-is.

Thanks,

-- 
Peter Xu