From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-io1-f51.google.com (mail-io1-f51.google.com [209.85.166.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 25E261B5837 for ; Tue, 30 Jul 2024 15:55:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.166.51 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722354926; cv=none; b=um+YRPgcJTDyouEoVQKbYuozbzyQShvGiNaa9znQCKLp0MIJbzWN3zIuy94qaVP8GYlVeE94fwVa5Bf1zU1ktZTGj4ON0Fn7awLS/bhO2lKBAss8uAaodOVBMQA4VyFhxV/gAGlRjHNRGB0oHcVH7/PYemaXuVRUHTlWjvTsHgc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722354926; c=relaxed/simple; bh=vWmqf+DZ6yy7uefW9kB1OXmHEFwi0VcF0uleWNg69dU=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=sa3kQ+WL+lYoHbQ/8qDQe1vheqCmKvxQJMFTMmBduoMgLGvYtUhQRgWlgObsLdLCpqzD5ghGRCAR7lfXUafenkuv+YPmARKOIwOd5hJloUJ1MgRBnzjoQ61m8+G3B94kxALq/TRSlS07wM1XwhMn3Pl8zTwmCIyioI+pX/Y+rxo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linuxfoundation.org; spf=pass smtp.mailfrom=linuxfoundation.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b=BTMYePbX; arc=none smtp.client-ip=209.85.166.51 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linuxfoundation.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linuxfoundation.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b="BTMYePbX" Received: by mail-io1-f51.google.com with SMTP id ca18e2360f4ac-81f8add99b6so24345739f.2 for ; Tue, 30 Jul 2024 08:55:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linuxfoundation.org; s=google; t=1722354924; x=1722959724; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=PzCaghtWJkWwHrbcMiszDi+qmsX7TBq8ffoDMqJhL1o=; b=BTMYePbXq0tPGbhaW4vRXZ1gc/9bf8MtYyrdJZO7wVBIcf7EzMPy2tuE9Q/onuiAfR L1WHSKI8jfkMMnXtmJklEnEY6+/rzrK/L4t/G4NMaNhnxPK547CQNyt0s4NbL2Kf3nfF PuWGGgAh3hxYV4FPl3fKmxpcaphg3HBdyt0EM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722354924; x=1722959724; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=PzCaghtWJkWwHrbcMiszDi+qmsX7TBq8ffoDMqJhL1o=; b=fVniKrm0QSWl3rihAo05uCLwHXLEVfTtWFAAXwSDDysfs4FF0rhz/wCUL0Yu4RarKP bvBTvZyhqe8sTxWIUEtVAo1xrcSMCX8v2iHQQWNZNa+wdKHDY9IevRoNQNhkpnohAvrI VSFxnFiVWHnv3+5/i8mYaViGpj/8We47sJPcK+C8ZoEerOke0DWzrljvofcc5e8uP71t X4l7YxRcY6Kf2rJIOAsLFQV49XWSqEeQtJLIh++XPVRQQWi1siQPps0Q68MYGs1DqZ4v Efo/xzT/rxiT4zgO+MFIsr+dscgKL7FmiVz9dNGuDB0yEm/bn13Cg5yRXkmwR8/cXrXa Bxww== X-Forwarded-Encrypted: i=1; AJvYcCWOYwxGnlIsusfLMOiLY7/p7qeagV4pdVI/u6MxUY46Oz84wSQrlumhy0jf2ZU9E+EjLfa9DRZnbzEkWXM=@vger.kernel.org X-Gm-Message-State: AOJu0Yy1DF5dDBkSqsigqZrB/W1pzqVzBoSke78LAdcOnxvmyCQNT3Jm fT5ZaZPZuT9HOsVuKkTSrJdEzA9MGwto7wEw01BW5U8AyZRf4lCEwqQfQ6ROVQE= X-Google-Smtp-Source: AGHT+IGuQMM6gxKKpjFLk13+wgKSCeu7d/MaJ7gUsdzsTKePVhPCSVaQL3WB7bTmQo5YaG+Bs9e4GA== X-Received: by 2002:a05:6602:1ccb:b0:81f:bf02:fd0a with SMTP id ca18e2360f4ac-81fbf0301ccmr96363439f.3.1722354924184; Tue, 30 Jul 2024 08:55:24 -0700 (PDT) Received: from [192.168.1.128] ([38.175.170.29]) by smtp.gmail.com with ESMTPSA id 8926c6da1cb9f-4c29fc42b48sm2721593173.157.2024.07.30.08.55.23 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 30 Jul 2024 08:55:23 -0700 (PDT) Message-ID: <714e7642-6f92-4e41-aa36-c854668e0bb0@linuxfoundation.org> Date: Tue, 30 Jul 2024 09:55:22 -0600 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 2/3] bitmap: Rename module To: David Gow , Randy Dunlap , kees@kernel.org, Muhammad Usama Anjum Cc: Yury Norov , Andrew Morton , Rasmus Villemoes , Shuah Khan , linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, John Hubbard , kernel@collabora.com, Shuah Khan References: <20240726110658.2281070-1-usama.anjum@collabora.com> <20240726110658.2281070-3-usama.anjum@collabora.com> <85f575b4-4842-4189-9bba-9ee1085a5e80@collabora.com> Content-Language: en-US From: Shuah Khan In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 7/30/24 04:10, David Gow wrote: > On Mon, 29 Jul 2024 at 22:09, Randy Dunlap wrote: >> >> >> >> On 7/29/24 1:07 AM, Muhammad Usama Anjum wrote: >>> On 7/27/24 10:35 PM, Yury Norov wrote: >>>> On Fri, Jul 26, 2024 at 04:06:57PM +0500, Muhammad Usama Anjum wrote: >>>>> Rename module to bitmap_kunit and rename the configuration option >>>>> compliant with kunit framework. >>>> >>>> ... , so those enabling bitmaps testing in their configs by setting >>>> "CONFIG_TEST_BITMAP=y" will suddenly get it broken, and will likely >>>> not realize it until something nasty will happen. >>> CONFIG_TEST_BITMAP was being enabled by the kselftest suite lib. The bitmap >>> test and its config option would disappear. The same test can be run by >>> just enabling KUNIT default config option: >>> >>> KUNIT_ALL_TESTS=y enables this bitmap config by default. >>> >>>> >>>> Sorry, NAK for config rename. >>>> >> >> I agree with Yury. Using KUNIT takes away test coverage for people who >> are willing to run selftests but not use KUNIT. This is incorrect. There are selftest that are used to - regression test a subsystem or a abi during boot - spot check on a running system to debug and test by loading a test module. > > I can see the point that renaming the config option is just churn, but > is there a reason people would run the bitmap selftest but be unable > or unwilling to use KUnit? > > Beyond a brief period of adjustment (which could probably be made > quite minimal with a wrapper script or something), there shouldn't > really be any fundamental difference: KUnit tests can already run at > boot, be configured with a config option, and write output to the > kernel log. There's nothing really being taken away here, and the > bonus of having easier access to run the tests with KUnit's tooling > (or have them automatically run by systems which run KUnit tests) > would seem worthwhile to me, especially since it's optional. And > CONFIG_KUNIT shouldn't be heavy enough to cause problems. > > Obviously I can't force people to use KUnit, but this is exactly the > sort of test which would fit KUnit well, and -- forgive me if I'm > missing something -- the only real argument against it I'm hearing is > "it's different". And while that's valid (as I said, churn for churn's > sake isn't great), none of the "people who are willing to run > selftests but not use KUnit" have given reasons why. Especially since > this is the sort of test (testing in-kernel functions) we're > encouraging people to write with KUnit in > Documentation/dev-tools/testing-overview.rst -- if there are good > reasons people are refusing to run these, maybe we need to fix those > or change the recommendation. It isn't about kunit vs. kselftest. It is about overall kernel validation and ability to test effectively by developers and users. KUnit isn't ideal for cases where people would want to check a subsystem on a running kernel - KUnit covers some use-cases and kselftest covers others. What happens if we are debugging a problem that requires us to debug on a running system? Please don't go converting kselftest into kunit without understanding how these frameworks are intended to be used. Yes kselftest results need to be looked at. Write a parser which can be improved. What you are doing is reducing the coverage and talking away the ability to debug and test on running system. Fix what needs to be fixed instead of deleting tests. Think through the use-cases before advocating KUnit is suitable for all testing needs and talking about being able to force or not force people to use one or the other. Reports aren't everything. The primary reason we have these tests is for developers to be able to test. Reports can be improved and shouldn't come at the expense of coverage and testing. Any patch that does that will be NACKed. thanks, -- Shuah