Re: [PATCH v3 0/1] Add KUnit tests for llist

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/19/24 1:01 PM, Shuah Khan wrote:
On 9/16/24 18:51, Artur Alves wrote:
Hi all,

This is part of a hackathon organized by LKCAMP[1], focused on writing
tests using KUnit. We reached out a while ago asking for advice on what
would be a useful contribution[2] and ended up choosing data structures
that did not yet have tests.

This patch adds tests for the llist data structure, defined in
include/linux/llist.h, and is inspired by the KUnit tests for the doubly
linked list in lib/list-test.c[3].

It is important to note that this patch depends on the patch referenced
in [4], as it utilizes the newly created lib/tests/ subdirectory.

[1] https://lkcamp.dev/about/
[2] https://lore.kernel.org/all/Zktnt7rjKryTh9-N@arch/
[3] https://elixir.bootlin.com/linux/latest/source/lib/list-test.c
[4] https://lore.kernel.org/all/20240720181025.work.002-kees@xxxxxxxxxx/

---
Changes in v3:
     - Resolved checkpatch warnings:
         - Renamed tests for macros starting with 'for_each'

Shouldn't this a separate patch to make it easy to review?

         - Removed link from commit message
     - Replaced hardcoded constants with ENTRIES_SIZE

Shouldn't this a separate patch to make it easy to review?

     - Updated initialization of llist_node array
     - Fixed typos
     - Update Kconfig.debug message for llist_kunit

Are these changes to existing code or warnings on your added code?

Changes in v2:
     - Add MODULE_DESCRIPTION()
     - Move the tests from lib/llist_kunit.c to lib/tests/llist_kunit.c
     - Change the license from "GPL v2" to "GPL"

Artur Alves (1):
   lib/llist_kunit.c: add KUnit tests for llist

  lib/Kconfig.debug       |  11 ++
  lib/tests/Makefile      |   1 +
  lib/tests/llist_kunit.c | 358 ++++++++++++++++++++++++++++++++++++++++
  3 files changed, 370 insertions(+)
  create mode 100644 lib/tests/llist_kunit.c


You are combining lot of changes in one single patch. Each change as a separate
patch will help reviewers.

Adding new test should be a separate patch.

- renaming as a separate patch

thanks,
-- Shuah

Hi, thanks for the reply!

I'm not sure if I understood your concerns ...

In this patch, I'm adding the entire test suite for the lock-less list data structure, which is the primary reason for its larger size. The changes in V2 and V3 were made in response to code review suggestions from previous iterations.

However, as a big patch I see how this cause an annoyance to review. I'm open to any suggestions on how I can reduce its size or make the review process more manageable.

Best regards,
- Artur




[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux