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 0DF87C43334 for ; Wed, 22 Jun 2022 19:14:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7B9408E00D8; Wed, 22 Jun 2022 15:14:33 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 768786B0087; Wed, 22 Jun 2022 15:14:33 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 62F428E00D8; Wed, 22 Jun 2022 15:14:33 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 50AD36B0085 for ; Wed, 22 Jun 2022 15:14:33 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 1DFFC7D0 for ; Wed, 22 Jun 2022 19:14:28 +0000 (UTC) X-FDA: 79606823016.23.2304320 Received: from mail-vs1-f50.google.com (mail-vs1-f50.google.com [209.85.217.50]) by imf21.hostedemail.com (Postfix) with ESMTP id 428111C0006 for ; Wed, 22 Jun 2022 19:14:25 +0000 (UTC) Received: by mail-vs1-f50.google.com with SMTP id 184so2962468vsz.2 for ; Wed, 22 Jun 2022 12:14:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=NoREZ7M189g66OG8lXhmFj3468Kg13OaxkXYT1yDuy0=; b=SM1g3aqxVrUaMGMy35p/7jhCtE+8kqqQRHT0rNEpJZWkxmz4NQfVQ1vS4wsE5PXkOm BEZzyiYQ9qTsNlIKMkPD0oHcL2GUWxwBBHLoynI5Ds2O+cAiVGROk0Fp7TiczG9v5gtA 1SL3+zTzEYdIBq9lYISUPU6KPgjBhrWz7rTOjN5jkLLGjYUECl0BLXZnl1kQlj2bs6wh ZQUkcj9kA6Vn7kEhQ5dlzhgBd4Punbj32qMSkl9RtN2V+81sL0ilnVR/INN094vIJfMW aA5cI2rG+Ilvpy47D1tla5NCzP8r2O5D4gzSXUj6MGR/wp85LYyBAWPUZhug62RQQbv3 A8tw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=NoREZ7M189g66OG8lXhmFj3468Kg13OaxkXYT1yDuy0=; b=OOJPsjof/ZOXR5mBJSJk/HuNwAVU1RjkGXy1hUju2J//gAZopDxhPWYD4ldryA7yIP MaTa0v2DFlrOiiMKIcRlc/EHC0OU11Eh7ibK5OJCvXnp8srD76yyNYXPrb0ojQQKLoAc 9owzyScKhNa0ApjFVp0+LA/RhfhDDmsBqi5n1DxICwnfPqsUl8ampztoHDzmfANpVcEA cNAkYo0PwQaVheheAPjVHHcukPceM6aLGzvKYQAZa/lLn2NE5JWrhYZqUciUz29OW7aO CICeShVXg+3hqyvh6S/dAI1SfbZ75goyGOYbuZZKKVtHmarSjuBvf7DhWTfP64zbmcHM ZeqA== X-Gm-Message-State: AJIora9ya1hG3e5c2b70sMSS1d+r6UqwoU+/ZtzN6TDH3l/NeTNa2VfV AA5QWXWiMEB45oT53EyU9OZO4IweiXEFty32D4dpQw== X-Google-Smtp-Source: AGRyM1tqzfn/eGooe7rVyqLakJqYPnRIyJ7e5Hcfdor2BRQlRrkwrn6OyhG0ItwgwFDnmhbwVkV/LdlAgMONSUwHn8Q= X-Received: by 2002:a67:fa01:0:b0:354:3ce9:4537 with SMTP id i1-20020a67fa01000000b003543ce94537mr6735666vsq.50.1655925255299; Wed, 22 Jun 2022 12:14:15 -0700 (PDT) MIME-Version: 1.0 References: <20220614071650.206064-1-yuzhao@google.com> <20220614071650.206064-13-yuzhao@google.com> <214db251-827c-715c-54cf-9c0e9bb5fe30@bytedance.com> In-Reply-To: <214db251-827c-715c-54cf-9c0e9bb5fe30@bytedance.com> From: Yu Zhao Date: Wed, 22 Jun 2022 13:13:39 -0600 Message-ID: Subject: Re: [PATCH v12 12/14] mm: multi-gen LRU: debugfs interface To: Qi Zheng Cc: Andrew Morton , Andi Kleen , Aneesh Kumar , Catalin Marinas , Dave Hansen , Hillf Danton , Jens Axboe , Johannes Weiner , Jonathan Corbet , Linus Torvalds , Matthew Wilcox , Mel Gorman , Michael Larabel , Michal Hocko , Mike Rapoport , Peter Zijlstra , Tejun Heo , Vlastimil Babka , Will Deacon , Linux ARM , "open list:DOCUMENTATION" , linux-kernel , Linux-MM , "the arch/x86 maintainers" , Kernel Page Reclaim v2 , Brian Geffon , Jan Alexander Steffens , Oleksandr Natalenko , Steven Barrett , Suleiman Souhlal , Daniel Byrne , Donald Carr , =?UTF-8?Q?Holger_Hoffst=C3=A4tte?= , Konstantin Kharlamov , Shuang Zhai , Sofia Trinh , Vaibhav Jain Content-Type: text/plain; charset="UTF-8" ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1655925265; a=rsa-sha256; cv=none; b=SXgEozEdM5zUwwkguIB39qqmqs34VU/NocbwbdmMBNpTsHm6oJqyfyZ2j75mRtwGyWAa1Z J0Cr6zdf/qTti4ETjDLwHgqvvIhQhhoEzFYmdpSv/5X6R2yzYSfg6SIUkE7P+JjsYBwK28 eLmbj1qkmkbvY5eGHiwFYWWWw/wgQ+s= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=SM1g3aqx; spf=pass (imf21.hostedemail.com: domain of yuzhao@google.com designates 209.85.217.50 as permitted sender) smtp.mailfrom=yuzhao@google.com; dmarc=temperror reason="server fail" header.from=google.com (policy=temperror) ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1655925265; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=NoREZ7M189g66OG8lXhmFj3468Kg13OaxkXYT1yDuy0=; b=5d0wwsxQjkN5o3FQq7jHzC8stkOKzD92NdFMCDmBTh/ipseY47idVrxTzzIJa9U6Y7ihcQ 3HLy+kbZf4mLSK62PwMz3AdFMn0hJarbtK0t3YNC4XHjGRBQf+PCwd7GAefS/2Rj+1pgMD +OIqeSgPEO6RCC+OyibTLk/f0lmh/jw= X-Stat-Signature: c3jmc31wcnw8tgpqbe8pinbho14go71n X-Rspamd-Server: rspam06 Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=SM1g3aqx; spf=pass (imf21.hostedemail.com: domain of yuzhao@google.com designates 209.85.217.50 as permitted sender) smtp.mailfrom=yuzhao@google.com; dmarc=temperror reason="server fail" header.from=google.com (policy=temperror) X-Rspam-User: X-Rspamd-Queue-Id: 428111C0006 X-HE-Tag: 1655925265-635776 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 Wed, Jun 22, 2022 at 3:16 AM Qi Zheng wrote: > > +static ssize_t lru_gen_seq_write(struct file *file, const char __user *src, > > + size_t len, loff_t *pos) > > +{ > > + void *buf; > > + char *cur, *next; > > + unsigned int flags; > > + struct blk_plug plug; > > + int err = -EINVAL; > > + struct scan_control sc = { > > + .may_writepage = true, > > + .may_unmap = true, > > + .may_swap = true, > > + .reclaim_idx = MAX_NR_ZONES - 1, > > + .gfp_mask = GFP_KERNEL, > > + }; > > + > > + buf = kvmalloc(len + 1, GFP_KERNEL); > > + if (!buf) > > + return -ENOMEM; > > + > > + if (copy_from_user(buf, src, len)) { > > + kvfree(buf); > > + return -EFAULT; > > + } > > + > > + if (!set_mm_walk(NULL)) { > > The current->reclaim_state will be dereferenced in set_mm_walk(), so > calling set_mm_walk() before set_task_reclaim_state(current, > &sc.reclaim_state) will cause panic: > > [ 1861.154916] BUG: kernel NULL pointer dereference, address: > 0000000000000008 Thanks. Apparently I shot myself in the foot by one of the nits between v11 and v12. > > + kvfree(buf); > > + return -ENOMEM; > > + } > > + > > + set_task_reclaim_state(current, &sc.reclaim_state); > > + flags = memalloc_noreclaim_save(); > > + blk_start_plug(&plug); > > + > > + next = buf; > > + next[len] = '\0'; > > + > > + while ((cur = strsep(&next, ",;\n"))) { > > + int n; > > + int end; > > + char cmd; > > + unsigned int memcg_id; > > + unsigned int nid; > > + unsigned long seq; > > + unsigned int swappiness = -1; > > + unsigned long opt = -1; > > + > > + cur = skip_spaces(cur); > > + if (!*cur) > > + continue; > > + > > + n = sscanf(cur, "%c %u %u %lu %n %u %n %lu %n", &cmd, &memcg_id, &nid, > > + &seq, &end, &swappiness, &end, &opt, &end); > > + if (n < 4 || cur[end]) { > > + err = -EINVAL; > > + break; > > + } > > + > > + err = run_cmd(cmd, memcg_id, nid, seq, &sc, swappiness, opt); > > + if (err) > > + break; > > + } > > + > > + blk_finish_plug(&plug); > > + memalloc_noreclaim_restore(flags); > > + set_task_reclaim_state(current, NULL); > > + > > + clear_mm_walk(); > > Ditto, we can't call clear_mm_walk() after > set_task_reclaim_state(current, NULL). > > Maybe it can be modified as follows: > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index 2422edc786eb..552e6ae5243e 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -5569,12 +5569,12 @@ static ssize_t lru_gen_seq_write(struct file > *file, const char __user *src, > return -EFAULT; > } > > + set_task_reclaim_state(current, &sc.reclaim_state); > if (!set_mm_walk(NULL)) { > kvfree(buf); > return -ENOMEM; > } > > - set_task_reclaim_state(current, &sc.reclaim_state); We need a `goto` because otherwise we leave a dangling `current->reclaim_state`. (I swear I had one.)