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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 89475C7EE25 for ; Sat, 10 Jun 2023 01:45:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C78516B0072; Fri, 9 Jun 2023 21:45:26 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C28266B0074; Fri, 9 Jun 2023 21:45:26 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AC8A38E0002; Fri, 9 Jun 2023 21:45:26 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 9A73C6B0072 for ; Fri, 9 Jun 2023 21:45:26 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 6A5A01C7B4A for ; Sat, 10 Jun 2023 01:45:26 +0000 (UTC) X-FDA: 80885145852.17.24160ED Received: from mail-lf1-f53.google.com (mail-lf1-f53.google.com [209.85.167.53]) by imf25.hostedemail.com (Postfix) with ESMTP id 8A107A0005 for ; Sat, 10 Jun 2023 01:45:24 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=cu0m1MUv; dmarc=pass (policy=none) header.from=chromium.org; spf=pass (imf25.hostedemail.com: domain of stevensd@chromium.org designates 209.85.167.53 as permitted sender) smtp.mailfrom=stevensd@chromium.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1686361524; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Y+PqyIB5ZqDwlmZ1up6rkpqNAc9gKklwSUdrIkGBt/E=; b=OahgEPT3Kk3zX0YVMn2BDydxLlSPr790uZXVJzYtjy78gPs62+/7an1tCCpACcxxV/5yH+ JpL/ycJWgcUKLpvg5IIlR53Zv624bCsi82d3WY9XNL+2ALcGMNHeL5/9jdEzEgMm26/cm/ Nlo1/FLBeupySByVGhGx7AZ7k2VsFEk= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=cu0m1MUv; dmarc=pass (policy=none) header.from=chromium.org; spf=pass (imf25.hostedemail.com: domain of stevensd@chromium.org designates 209.85.167.53 as permitted sender) smtp.mailfrom=stevensd@chromium.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1686361524; a=rsa-sha256; cv=none; b=tcd21nkjTWzSwqlecGDafdk04f+89VIuBUloz9pmL2UukLzvad4+7NS9BGPSasVYPpsXOP DgNBnw2IIYBP0S3hD/jR0nl89A9hhvNqxUVyAv+db87qH3K3hzuTq0ajZtd3eODoen0gjn g/7Fanlpitu5Yn/z5KfJOJoYHHyLJTM= Received: by mail-lf1-f53.google.com with SMTP id 2adb3069b0e04-4effb818c37so2995189e87.3 for ; Fri, 09 Jun 2023 18:45:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1686361522; x=1688953522; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=Y+PqyIB5ZqDwlmZ1up6rkpqNAc9gKklwSUdrIkGBt/E=; b=cu0m1MUv/HXrUmSOm4jb525dJ3bd+mgyvDKFXtB+yAmRSQvoMPoei1e63p5TuksRUQ CIDLjtLkiqfA4HB7tGzdN9ELMJ9bAKLJbSH3om9TvSip61Ua4Kr/sCORSAicmie+lhex Ku1DndiaervlNRJ2mIs5rEmsH4te5Sq8Ej47k= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686361522; x=1688953522; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Y+PqyIB5ZqDwlmZ1up6rkpqNAc9gKklwSUdrIkGBt/E=; b=IacXe0mWHW2sfiHNf/g0/1CKTysor+WIzm9B5lu9If1m+8MoCLKuPwzHWfuzzU/28u kuYuJeqjXX8/OttBqP2r9kxUyiqWk1Beax5y3Acm5P9bCShNvpzKOoTYkk9xMpsBTjrz DzypgD839vvro+8wRwqdEzNowE3UMz5aSyF4M5y7A6fDL3npdPBLMlmJkvIkq2NiHJk6 QOUAMB3yDCUk5E88T3V8NWyOgXvG9F4nBirOCcexb5ildogUxRMetzxtX6bOB4t7B1le UEuukuq0YQq13qMKgWuqR46syifmCc0ZxpOt8ewX3CBDXlFud5VSydl7JO1GSFMsYzYG tznA== X-Gm-Message-State: AC+VfDy+hB4ECm89oCEdkcYlPJQUvPJDHDUtt+JmYwrWvjtE22paFqvt AabnxK0rNY76PT7p8NfxXuH5PCg17nF9uZES1KEXQw== X-Google-Smtp-Source: ACHHUZ4Z86AS8lMjHUd1LPTyasItm1dhaX6DQFHJjKsZR+sQdizE7s6pOdBkHPuVtetFopzJo9oBh1QgA8xrmyfYVKY= X-Received: by 2002:ac2:5dc9:0:b0:4ec:8816:f4fc with SMTP id x9-20020ac25dc9000000b004ec8816f4fcmr1842267lfq.6.1686361522539; Fri, 09 Jun 2023 18:45:22 -0700 (PDT) MIME-Version: 1.0 References: <20230607053135.2087354-1-stevensd@google.com> In-Reply-To: From: David Stevens Date: Sat, 10 Jun 2023 10:45:11 +0900 Message-ID: Subject: Re: [PATCH] mm/khugepaged: fix iteration in collapse_file To: Hugh Dickins Cc: linux-mm@kvack.org, Andrew Morton , Peter Xu , Matthew Wilcox , "Kirill A . Shutemov" , Yang Shi , David Hildenbrand , Jiaqi Yan , linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 8A107A0005 X-Stat-Signature: zoczgzmdrir1rfwyyp8xg9o9trtbr4ep X-Rspam-User: X-HE-Tag: 1686361524-801664 X-HE-Meta: U2FsdGVkX1+yFUHf6tHDgdSHGK2Txvt3tkaUOqfzvDZNCs0EseRLqo/4FkAfTlt1NnlOE4kuW7gacPcUwynwqN8CsZMpFgyg6y0JjcWaYWDrKB7+DB8bw+wzezKG9q6NAkr8ZSXDpJpm6j1EEDsVFAtTi3LbtqUkxb6bCjUeMdg1V2EXsgZfnn7bQrmV+gqZ3PqhF0E1XNvktVWuP0j7irO4mjoaBgJ+CdGeWDsKAZe3U+jLQ9oeCdvJQfqKvAPxft+CnKVit4yZgIM4a+o9fHx2r95IZ9G8YUe/FGEJClVm3aNbUW2/JwhzfCk3xQILZQUIffo7ZoQ8ZKCz1Xp3/65gQ7/0Tlee8tt/BRHdWzUIQevyTrb+ws+/uQY+d6rpy7StqLB47zgUvFe6qAw5NbJeJN0SV4TnE7WFzg4LNWekevhlpdtUgQmKLIveThxBE+I3zOMSM6hvhBoI+JAJX0aedWlyOwBuDwTnZQb8usvrADVB57V0RCXSRGQjzXxxPIFc5sqOBHkWWHFcp66FebZbOgdKXs9G3VfdfXsZhEWM4Cqj1E8gULG7qdLuc5+8tYHcbvfvjr5ui0G2hj/zbVtm5yZsXvXjHqSw6EmcjKaOFiFRDY06UQ4lY1yd4tbgJQzWVdDC3VJu6lkkvWtR0nI0FDIdGARd0RZ9OkYJVEljo4OEPpf/TU5lpyMazuvfB9RbVyoNrhoPszDJYEFSUwaci+dzE9v2dbjMi1gvFJh4fQmCof7humTAXtf6EWdVkNZ8boSskBiS6XgUeKU6UPeGtpebrIf1QkAdDzu2KrNyrevHwJItJg3YOKtvwqg2yQHzhad+dI3pYwZDsnZCGGm5OZaNA1LP5Cd6fJZ4h9ekRTJCqYJGTo7Dcw2Fh8eJKvpalPJ9B536awE6vJGm82osiRgE7D9nAQOLxrbkPATcVE0paa3cH0xefRkH6F5zLRDmokuc5k4nkUV+dGT yQAB7DaK LfIud4DCZSb8YfyArdVgdhxg48mKjeFpPL8noTAVSYgM4WV/fou3WSGG18YV/qTMg5h1c2v2NzX1gYAsUNI8ba6n8iJQUgFkxKtYufWpIgmGdKvI+qyUCJO1P/q0rmkOTLEIe8FxTr5DFR8I39XlCNRzkSzQ9EeTuBpEU6IhO6jvY994QaLMLhoulQKxfOeXy/nDMlsz+NKebNYdLVrQ1Yx75ec4zAPMjtD4mDmXjbZw++rDaHyZBV4q74Rx4FkkVmC4tN9+/0bb1L/L1VUPjdW1AhcDW/scJBNGaqDWFHewiGR9vLQZ0nistRnjk6y/cmEfvnlK+vTU0Ucde+mpmHKB9jhci0qIKbRnJQdJKvGqITfc= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Sat, Jun 10, 2023 at 5:03=E2=80=AFAM Hugh Dickins wro= te: > > On Wed, 7 Jun 2023, David Stevens wrote: > > > > Remove an unnecessary call to xas_set(index) when iterating over the > > target range in collapse_file. The extra call to xas_set reset the xas > > cursor to the top of the tree, causing the xas_next call on the next > > iteration to walk the tree to index instead of advancing to index+1. > > This returned the same page again, which would cause collapse_file to > > fail because the page is already locked. > > > > This bug was hidden when CONFIG_DEBUG_VM was set. When that config was > > used, the xas_load in a subsequent VM_BUG_ON assert would walk xas from > > the top of the tree to index, causing the xas_next call on the next loo= p > > iteration to advance the cursor as expected. > > > > Fixes: a2e17cc2efc7 ("mm/khugepaged: maintain page cache uptodate flag"= ) > > Signed-off-by: David Stevens > > This patch seems to be wrong, but I have not investigated why. > > It's certainly an interesting and worrying observation, > if a CONFIG_DEBUG_VM=3Dy kernel goes a significantly different way. > > I almost always do have CONFIG_DEBUG_VM=3Dy, so you won't be surprised th= at > I never saw the issue. But once I ran an mm-everything with this patch i= n, > I hit that VM_BUG_ON_PAGE(page !=3D xas_load(&xas), page) for the first t= ime > (after about 2 hours of huge tmpfs swapping load). Is the particular workload one you can share? I haven't hit that assert so far with my tests. Also, I'm a little surprised that this is the assert which is being hit. My patch series didn't make that many changes to the first loop of the function, and the changes it did make were mostly about missing pages, not present pages. > As if you have just transferred the problem from DEBUG_VM=3Dn to DEBUG_VM= =3Dy. > But I then tried a CONFIG_DEBUG_VM off 6.4-rc1 kernel (including the fixe= e > but not this fixer) under similar load, and saw no problem in 14 hours. > So I can't even reproduce the bug that is being fixed here: only hit a > bug that it introduces. The bug this fixes isn't a crash - it's the fact that khugepage becomes nearly unable to collapse pages. Specifically, it can only collapse if there is exactly one present page, and that page is at index 511. Since MADV_COLLAPSE uses the same code path, it's easy to reproduce the bug I'm trying to fix with this program: #define _GNU_SOURCE #include #include #include #include #include #define THP_SIZE (2 * 1024 * 1024) #ifndef MADV_COLLAPSE #define MADV_COLLAPSE 25 #endif int main() { int memfd =3D memfd_create("memfd", MFD_CLOEXEC); assert(memfd >=3D 0); int ret =3D ftruncate(memfd, THP_SIZE); assert(ret >=3D 0); char *addr =3D mmap(NULL, THP_SIZE, PROT_READ|PROT_WRITE, MAP_SHARED, mem= fd, 0); assert(addr !=3D MAP_FAILED); addr[0] =3D 0xff; ret =3D madvise(addr, THP_SIZE, MADV_COLLAPSE); assert(ret =3D=3D 0); } If DEBUG_VM isn't set, then the test will trigger the assert. If DEBUG_VM is set or if this fix is included, then the test will pass. -David > Hugh > > > --- > > mm/khugepaged.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > > index 6b9d39d65b73..2d0d58fb4e7f 100644 > > --- a/mm/khugepaged.c > > +++ b/mm/khugepaged.c > > @@ -2070,7 +2070,6 @@ static int collapse_file(struct mm_struct *mm, un= signed long addr, > > TTU_IGNORE_MLOCK | TTU_BATCH_FLUS= H); > > > > xas_lock_irq(&xas); > > - xas_set(&xas, index); > > > > VM_BUG_ON_PAGE(page !=3D xas_load(&xas), page); > > > > -- > > 2.41.0.rc2.161.g9c6817b8e7-goog