Re: [PATCH v1 01/25] net: core: device_rename: Use rwsem instead of a seqcount

From: Thomas Gleixner
Date: Tue May 19 2020 - 21:56:17 EST


Stephen Hemminger <stephen@xxxxxxxxxxxxxxxxxx> writes:
> On Wed, 20 May 2020 01:42:30 +0200
> Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
>
>> Stephen Hemminger <stephen@xxxxxxxxxxxxxxxxxx> writes:
>> > On Wed, 20 May 2020 00:23:48 +0200
>> > Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
>> >> No. We did not. -ENOTESTCASE
>> >
>> > Please try, it isn't that hard..
>> >
>> > # time for ((i=0;i<1000;i++)); do ip li add dev dummy$i type dummy; done
>> >
>> > real 0m17.002s
>> > user 0m1.064s
>> > sys 0m0.375s
>>
>> And that solves the incorrectness of the current code in which way?
>
> Agree that the current code is has evolved over time to a state where it is not
> correct in the case of Preempt-RT.

That's not a RT problem as explained in great length in the changelog
and as I pointed out in my previous reply.

Realtime scheduling classes are available on stock kernels and all
those attempts to "fix" the livelock problem are ignoring that fact.

Just because you or whoever involved are not using them or do not care
is not making the code more correct.

> The motivation for the changes to seqcount goes back many years when
> there were ISP's that were concerned about scaling of tunnels, vlans
> etc.

I completely understand where this comes from, but that is not a
justification for incorrect code at all.

> Is it too much to ask for a simple before/after test of your patch as part
> of the submission. You probably measure latency changes to the
> nanosecond.

It's not too much to ask and I'm happy to provide the numbers.

But before I waste my time and produce them, can you please explain how
any numbers provided are going to change the fact that the code is
incorrect?

A bug, is a bug no matter what the numbers are.

I don't have a insta reproducer at hand for the problem which made that
code go belly up, but the net result is simply:

Before: After:
real INFINTE 0mxx.yyys

And the 'Before' comes with the extra benefit of stall warnings (if
enabled in the config).

If you insist I surely can go the extra mile and write up the insta
reproducer and stick it into a bugzilla for you.

Thanks,

tglx