Re: [PATCH v4 00/13] "Task_isolation" mode

Thomas Gleixner
2020-07-24T16:08:20Z

Alex,

Alex Belits <abelits@marvell.com> writes:
> On Thu, 2020-07-23 at 23:44 +0200, Thomas Gleixner wrote:
>> 1) That inline function can be put out of line by the compiler and
>> placed into the regular text section which makes it subject to
>> instrumentation
>>
>> 2) That inline function invokes local_irq_save() which is subject to
>> instrumentation _before_ the entry state for the instrumentation
>> mechanisms is established.
>>
>> 3) That inline function invokes sync_core() before important state
>> has
>> been established, which is especially interesting in NMI like
>> exceptions.
>>
>> As you clearly documented why all of the above is safe and does not
>> cause any problems, it's just me and Peter being silly, right?
>>
>> Try again.
>
> I don't think, accusations and mockery are really necessary here.

Let's get some context to this.

I told you in my first mail, that this breaks noinstr and that
building with full debug would have told you.

Peter gave you a clear hint where to look.

Now it might be expected that you investigate that or at least ask
questions before making the bold claim:

>> > Unless something else is involved, those operations are safe, so I
>> > am not adding anything that can break those.

Surely I could have avoided the snide remark, but after you demonstrably
ignored technically valid concerns and suggestions in your other reply,
I was surely not in the mood to be overly careful in my choice of words.

> The result of this may be not a "design" per se, but an understanding
> of how things are implemented, and what rules are being followed, so I
> could add my code in a manner consistent with what is done, and
> document the whole thing.

Every other big and technically complex project which has to change the
very inner workings of the kernel started the same way. I'm not aware of
any of them getting accepted as is or in a big code dump.

What you have now qualifies as proof of concept and the big challenge is
to turn it into something which is acceptable and maintainable.

You talk in great length about how inconsistent stuff is all over the
place. Yes, it is indeed. You even call that inconsistency an existing
design:

> My patches reflect what is already in code and in its design.

I agree that you just work with the code as is, but you might have
noticed that quite some of this stuff is clearly not designed at all or
designed badly.

The solution is not to pile on top of the inconsistency, the solution is
to make it consistent in the first place.

You are surely going to say, that's beyond the scope of your project. I
can tell you that it is in the scope of your project simply because just
proliferating the status quo and piling new stuff on top is not an
option. And no, there are no people waiting in a row to mop up after
you either.

Quite some of the big technology projects have spent and still spend
considerable amount of time to do exactly this kind of consolidation
work upfront in order to make their features acceptable in a
maintainable form.

All of these projects have been merged or are still being merged
piecewise in reviewable chunks.

We are talking about intrusive technology which requires a very careful
integration to prevent it from becoming a roadblock or a maintenaince
headache. The approach and implementation has to be _agreed_ on by the
involved parties, i.e. submitters, reviewers and maintainers.

> While I understand that this is an unusual feature and by its nature
> it affects kernel in multiple places, it does not deserve to be called
> a "mess" and other synonyms of "mess".

The feature is perfectly fine and I completely understand why you want
it. Guess who started to lay the grounds for NOHZ_FULL more than a
decade ago and why?

The implementation is not acceptable on technical grounds,
maintainability reasons, lack of design and proper consolidated
integration.

> Another issue that you have asked me to defend is the existence and
> scope of task isolation itself.

I have not asked you to defend the existance. I asked you for coherent
explanations how the implementation works and why the chosen approach is
correct and valid. That's a completely different thing.

> It's an attempt to introduce a feature that turns Linux userspace into
> superior replacement of RTOS.....

Can you please spare me the advertising and marketing? I'm very well
aware what an RTOS is and I'm also very well aware that there is no such
thing like a 'superior replacement' for RTOS in general.

If your view of RTOS is limited to this particular feature, then I have
to tell you that this particular feature is only useful for a very small
portion of the overall RTOS use cases.

> However most definitely this is not a "mess", and it I do not believe
> that I have to defend the validity of this direction of development, or
> be accused of general incompetence every time someone finds a
> frustrating mistake in my code.

Nobody accuses you of incompetence, but you will have to defend the
validity of your approach and implementation and accept that things
might not be as shiny as you think they are. That's not hostility,
that's just how Linux kernel development works whether you like it or
not.

I surely can understand your frustration over my view of this series,
but you might have noticed that aside of criticism I gave you very clear
technical arguments and suggestions how to proceed.

It's your decision what you make of that.

Thanks,

tglx