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 20599E77188 for ; Fri, 10 Jan 2025 12:16:50 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A9F5A6B0095; Fri, 10 Jan 2025 07:16:44 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id A4E0F6B00A8; Fri, 10 Jan 2025 07:16:44 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 916016B00AB; Fri, 10 Jan 2025 07:16:44 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id C8A216B0095 for ; Fri, 10 Jan 2025 07:16:42 -0500 (EST) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 6AAE214090F for ; Fri, 10 Jan 2025 12:16:42 +0000 (UTC) X-FDA: 82991440644.05.0220D84 Received: from mail-lj1-f182.google.com (mail-lj1-f182.google.com [209.85.208.182]) by imf08.hostedemail.com (Postfix) with ESMTP id 732F1160024 for ; Fri, 10 Jan 2025 12:16:40 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=iHANSAEU; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf08.hostedemail.com: domain of dvyukov@google.com designates 209.85.208.182 as permitted sender) smtp.mailfrom=dvyukov@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1736511400; a=rsa-sha256; cv=none; b=lwxLj24Qrs/5a8PUXftHvlNRzZdkqN8oZiiFBei8zsiFiOWO57gl9h5wR9dH+8X5HX1fqA jawCH8eXLxR1Juq49WBijxvuxYj1d82aUmhO3bNZnuBMeO70+7WAYYm2G++zd/YLAQOZDo tEVDUHWXvdO2Opas+eY/XuOaWQp0TH8= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=iHANSAEU; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf08.hostedemail.com: domain of dvyukov@google.com designates 209.85.208.182 as permitted sender) smtp.mailfrom=dvyukov@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1736511400; 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=dx6LfbWoOZVo4WZlNr29Yr02kRZ3p+X0xl0ciBlTxio=; b=i2RRQNi5Wy7n3ose/dUlAlp3BLd32qK1goNJ4SZ/JZkq7FO3DNVEV2eWMJho9vLoAtOjQ0 dYs/QkGx54lTnnNPr/gMU3iYIwYT0MzQzrbj82NGjFv3NxXuIw9hk1VzWoku1cXBVxc2+Q 4F5Lnk1DTVsDwYXbvhfLyrEl4AlsPmw= Received: by mail-lj1-f182.google.com with SMTP id 38308e7fff4ca-30229d5b21cso17622291fa.1 for ; Fri, 10 Jan 2025 04:16:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1736511398; x=1737116198; darn=kvack.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=dx6LfbWoOZVo4WZlNr29Yr02kRZ3p+X0xl0ciBlTxio=; b=iHANSAEUfh+dr5w5qRIcaldctzJj8J8Hurt9NR3blSIc8YpEORbx0kB5k595LiWhXj YSUw3NECjWLSjIxwjPpMpRDykbl/F4FI6i2S9P1U+Eax9ujjRNpniQU85eoH4wMl6jbG Q3a6BpMAtABnJs/1JnrXSul5xiz6fHit9UZdNrK3H4LHuH46FeP2Eeh6C4tVRd0u+L+k LRE0F58QmOmIBuYWaHlAFQ/8EBytiwgga6jBhIzoo3yhriYkBeYowNUywusCul7KXjQg v+rWIyWq26j7+5EiIKWMT5bUXmiN9HG8tYHEnDhSsXpFQeISHboAIIbNltJtpligfIKz u2EQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736511398; x=1737116198; h=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=dx6LfbWoOZVo4WZlNr29Yr02kRZ3p+X0xl0ciBlTxio=; b=sDi8JGmWha+c47YsPJmYuzUENTkSjwmgz7kk5JEIH4wlDubtFniDdIogLAsL5DWPAP Z1shIkeSf4cTSrINRZrtSf2zTpmqxlkb2mdWqvdEdX6p0LM7xvaHVjbncNkO7mgopx5k iFLdQkxPZp+0lWdD7l19L70/aIdB+mqT+gTinrR31UQdZoYKMDKSV3MWzZrH3trAXkHa oTwmFrFNCcTFZO7kh48EgVvj/YfWrekwIxwkGSfx0MxJZNBSbXj0X9nPRTcCAO36yqXQ 08dnyVKUQu8WeySp10poKWDWoVoSZL7QTOf49j4QgxuOWDcNA3eLxpQPEVcdh2XUnJak KnAw== X-Forwarded-Encrypted: i=1; AJvYcCX0ZsUgrC/QIHtuXZHjHqr9F0UZ8wu65hPk3f5xwUAOFQOXjSEcgyMW8eqU5EUBUB4yiE0uGQkhmw==@kvack.org X-Gm-Message-State: AOJu0Yyfk5ycN4QqtzWz9+ZVpx9yFP06UvSL4BIChGwxRtk/HgbLijWA W6kY9lcXmd/1RtY4ajGQIBUdZGXgLc4OlI1ZkzAOzH3KdQfS2KWIGgT2yYjMfemSIpk/Oqz72Q/ WY2uqO4G2YqQdYbeOXGm1pypVEw8R8ks4thLP X-Gm-Gg: ASbGnctx2Ws2o1gSLEAbP1MNmGgzHWCb+4MXPjcWTCIgzPxnJh8a5cr7S8BChUf8MHK fSe683oz9sHXcqhcx1p8Az0IJYiOcOxI4erX2gBA4fnEdKc3LTHeaBMW6mF+T1PGGMYGmuv0= X-Google-Smtp-Source: AGHT+IGVz7X+fbBVIzBdInnx2bkV3sGGjpZi0hhX8DWvZ9b1BNEo4hsq0wEuqhU72LBjprO2Rqcz30cGAQVOyHHn4rc= X-Received: by 2002:a05:651c:19a6:b0:300:2464:c0c2 with SMTP id 38308e7fff4ca-305f453158amr29422851fa.8.1736511398361; Fri, 10 Jan 2025 04:16:38 -0800 (PST) MIME-Version: 1.0 References: <20250110073056.2594638-1-quic_jiangenj@quicinc.com> In-Reply-To: From: Dmitry Vyukov Date: Fri, 10 Jan 2025 13:16:27 +0100 X-Gm-Features: AbW1kvYg6Yj75R_YQTMDFKYPIK4IFfqRlUkZXLVrw7rnxTU_BdbrBR450FQtMuY Message-ID: Subject: Re: [PATCH] kcov: add unique cover, edge, and cmp modes To: Marco Elver Cc: Joey Jiao , andreyknvl@gmail.com, corbet@lwn.net, akpm@linux-foundation.org, gregkh@linuxfoundation.org, nogikh@google.com, pierre.gondois@arm.com, cmllamas@google.com, quic_zijuhu@quicinc.com, richard.weiyang@gmail.com, tglx@linutronix.de, arnd@arndb.de, catalin.marinas@arm.com, will@kernel.org, dennis@kernel.org, tj@kernel.org, cl@linux.com, ruanjinjie@huawei.com, colyli@suse.de, andriy.shevchenko@linux.intel.com, kernel@quicinc.com, quic_likaid@quicinc.com, kasan-dev@googlegroups.com, workflows@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" X-Rspam-User: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 732F1160024 X-Stat-Signature: 738tz1zqnfus51pimkmyz988gk1xj9ez X-HE-Tag: 1736511400-678643 X-HE-Meta: U2FsdGVkX18ingXLYmftKoHVLxIs0FtocZVy/SJ0bvc2DfgXTWIJRXJ+RhPBtkLf9qGNB4nDyzeBdJyukGwWxT/ULYv1Z3Zmb3mjpcNZLyc7TeZA7FPuva2dI1exdk3vDHJZHN+zvONb8+jRYx1kvdq+zGPsw+/VPv7fdGFmsrXsHnV1I7FaZTcdtp2rRTOnT/vBA1doQwBsHcTlCo1vFyZu86NTlucAQirZWOK4inKfAdHUvfc6nkbEApGH553NWuMa9XTQVnykm0vbKzd4OXqsjdXRFuXNMcnW1+SEDQu5ZUb7MVT8COXvWNqey14bIA7S3+pky8b4eDdU00VQczAu4HDoPY2k5drNaAiLiOIfOV/3v7iaflm35GyyXScxJRTivJXc+492RDpzDhj+5dZHoCrkiE8udAGsTRgc8hgwRE2/gkshjZv6mF07a/bLrEvR3xxeUc0u7Y7ngZ+q+G7BXQyJJVlbEC4C53J7zk7JKLHFbtCd0vkeHrtCaVIbTxRpGpT7L23KCWCy814s60+R9GXmhnTO5JMk0sR6YEroQWR/Nwbbxtbl81IRQnvQhyLnLWVGaydU1s5QAHnF2QYeWJO/g3euS048CD0TaXiutk0D523wd9uGZfPM5Bol2NwmjDQTZkKnkHqGZ3xRu+JZcsyPMgM3XrEYXOfGcEnMqe6DO5bkFq/my+axsdGkBp6lhCWDq7jmsFfgUAArJvQnBDjwTutT09g7zLzzAFr4N9fPgcFjEOJVsEbB6erhbumpXF921sgjr79QuvbS7DAOHskeYsgYScJ6+1qMtpCTcrJEQap3udki3LyKExqNQ2y61BtkFS2sSGyl6ClW529ruDn3xSItz5FRwBqnsfxvid+e0xmj+MiOOt3t+JVRa6ImMOkf2C5CNXj3rn5h4MxXtT54Bgn3dJfErcQT9VnGCHrkVqBhw47UWEHfaAFjWvWoLyB3qTPrf4YY+49 Nnk8Qur7 krkPEOEg7NusI6wPny8J6K9Fi7NlKwvoQZomKrtME4vNFOjgnKMy2lt2RhOMnqH26ervD86l5bhZ8LVUWwAm9HAY8vHM1cb07HyIdfUVU9GYUciQdwlG/1q7LGZWaVCuhN1J2NfZ8IltgmDK/FM97D4wN4++QA/yAk2RdGCYf1srjOrdvp/+TfCYj0Bk8a49I/5+SYUgFZLpjj8HVyktdHczIr5Tte1iesL3j+W43Hhx17YFL9r6e6ru0rfjNArPWyqOebS4mi79rLcH4YNBfVC17QZ8PLpr6P7eeYTg6lfwv0HDU4ds0EqfRzl+6AKVIrwBHMaPDkLeiuspbat4meuXEvEoEdGNDGcc6frkLUgeV2c6wKTP4aoZyEVpzLbiUpbvHutwZGjkURl80fBvti6bdeQ== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000181, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Fri, 10 Jan 2025 at 10:23, Marco Elver wrote: > > From: "Jiao, Joey" > > > > The current design of KCOV risks frequent buffer overflows. To mitigate > > this, new modes are introduced: KCOV_TRACE_UNIQ_PC, KCOV_TRACE_UNIQ_EDGE, > > and KCOV_TRACE_UNIQ_CMP. These modes allow for the recording of unique > > PCs, edges, and comparison operands (CMP). > > There ought to be a cover letter explaining the motivation for this, > and explaining why the new modes would help. Ultimately, what are you > using KCOV for where you encountered this problem? > > > Key changes include: > > - KCOV_TRACE_UNIQ_[PC|EDGE] can be used together to replace KCOV_TRACE_PC. > > - KCOV_TRACE_UNIQ_CMP can be used to replace KCOV_TRACE_CMP mode. > > - Introduction of hashmaps to store unique coverage data. > > - Pre-allocated entries in kcov_map_init during KCOV_INIT_TRACE to avoid > > performance issues with kmalloc. > > - New structs and functions for managing memory and unique coverage data. > > - Example program demonstrating the usage of the new modes. > > This should be a patch series, carefully splitting each change into a > separate patch. > https://docs.kernel.org/process/submitting-patches.html#split-changes > > > With the new hashmap and pre-alloced memory pool added, cover size can't > > be set to higher value like 1MB in KCOV_TRACE_PC or KCOV_TRACE_CMP modes > > in 2GB device with 8 procs, otherwise it causes frequent oom. > > > > For KCOV_TRACE_UNIQ_[PC|EDGE|CMP] modes, smaller cover size like 8KB can > > be used. > > > > Signed-off-by: Jiao, Joey > > As-is it's hard to review, and the motivation is unclear. A lot of > code was moved and changed, and reviewers need to understand why that > was done besides your brief explanation above. > > Generally, KCOV has very tricky constraints, due to being callable > from any context, including NMI. This means adding new dependencies > need to be carefully reviewed. For one, we can see this in genalloc's > header: > > > * The lockless operation only works if there is enough memory > > * available. If new memory is added to the pool a lock has to be > > * still taken. So any user relying on locklessness has to ensure > > * that sufficient memory is preallocated. > > * > > * The basic atomic operation of this allocator is cmpxchg on long. > > * On architectures that don't have NMI-safe cmpxchg implementation, > > * the allocator can NOT be used in NMI handler. So code uses the > > * allocator in NMI handler should depend on > > * CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG. > > And you are calling gen_pool_alloc() from __sanitizer_cov_trace_pc. > Which means this implementation is likely broken on > !CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG architectures (do we have > architectures like that, that support KCOV?). > > There are probably other sharp corners due to the contexts KCOV can > run in, but would simply ask you to carefully reason about why each > new dependency is safe. I am also concerned about the performance effect. Does it add a stack frame to __sanitizer_cov_trace_pc()? Please show disassm of the function before/after. Also, I have concerns about interrupts and reentrancy. We are still getting some reentrant calls from interrupts (not all of them are filtered by in_task() check). I am afraid these complex hashmaps will corrupt.