From: NeilBrown <neilb@suse.de>
To: "Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
"Christian König" <ckoenig.leichtzumerken@gmail.com>
Cc: mojha@codeaurora.org, jkosina@suse.cz, cezary.rojewski@intel.com,
neilb@suse.com, b00073877@aus.edu, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] list: add more extensive double add check
Date: Tue, 02 Feb 2021 09:16:10 +1100 [thread overview]
Message-ID: <8735yffn85.fsf@notabene.neil.brown.name> (raw)
In-Reply-To: <YBgnY8FXpHJdoDos@smile.fi.intel.com>
[-- Attachment #1: Type: text/plain, Size: 2243 bytes --]
On Mon, Feb 01 2021, Andy Shevchenko wrote:
> On Mon, Feb 01, 2021 at 02:52:51PM +0100, Christian König wrote:
>> Adding the same element to a linked list multiple times
>> seems to be a rather common programming mistake. To debug
>> those I've more than once written some code to check a
>> linked list for duplicates.
>>
>> Since re-inventing the wheel over and over again is a bad
>> idea this patch tries to add some common code which allows
>> to check linked lists for duplicates while adding new
>> elements.
>>
>> When list debugging is enabled we currently already check
>> the previous and next element if they are identical to the
>> new one. This patch now adds a configuration option to
>> check N elements before and after the desired position.
>>
>> By default we still only test one item since testing more
>> means quite a large CPU overhead. This can be overwritten
>> on a per C file bases by defining DEBUG_LIST_DOUBLE_ADD
>> before including list.h.
>
> I'm not sure it is a good idea. Currently the implementation is *generic*.
> You are customizing it w/o letting caller know.
>
> Create a derivative implementation and name it exlist (exclusive list) and use
> whenever it makes sense.
>
> And I think if you are still pushing to modify generic one the default must be
> 0 in order not altering current behaviour.
I don't understand your complaint.
The extra checks are also completely *generic*. It can never make sense
to add sometime to a list if it is already on the list. All lists are
exclusive lists.
The code ALREADY tests if the inserted object is already present either
side of the insert side of the insertion point. This patch just extends
it somewhat.
I myself have never had, or heard of, a bug due to double insertion so
I'm no strongly in favour of this patch for that reason.
But I *am* in favour of making the platform more resilient in general,
and if others have experienced this sort of bug, then I'm in favour of
make that easier to detect in future.
NeilBrown
>
>> A new kunit test is also added to the existing list tests
>> which intentionally triggers the debug functionality.
>
> --
> With Best Regards,
> Andy Shevchenko
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 853 bytes --]
next prev parent reply other threads:[~2021-02-01 22:17 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-01 13:52 [PATCH] list: add more extensive double add check Christian König
2021-02-01 16:08 ` Andy Shevchenko
2021-02-01 16:09 ` Andy Shevchenko
2021-02-01 22:16 ` NeilBrown [this message]
2021-02-02 12:59 ` Christian König
2021-02-04 7:25 ` [list] b9dc2e0952: WARNING:at_lib/list_debug.c:#__list_add_valid kernel test robot
2021-02-04 7:25 ` kernel test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8735yffn85.fsf@notabene.neil.brown.name \
--to=neilb@suse.de \
--cc=andriy.shevchenko@linux.intel.com \
--cc=b00073877@aus.edu \
--cc=cezary.rojewski@intel.com \
--cc=ckoenig.leichtzumerken@gmail.com \
--cc=jkosina@suse.cz \
--cc=linux-kernel@vger.kernel.org \
--cc=mojha@codeaurora.org \
--cc=neilb@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.