On 9/22/2016 12:05 AM, Thomas Gleixner wrote:
B1;2802;0cOn Wed, 21 Sep 2016, Santosh Shilimkar wrote:
I requested you to include this patch but now am not sure anymore.
Looks like there are almost 30 more users which are directly
tweaking 'tasklet_struct' fields and calling other APIs. Hunting them
and fixing them probably would be an exercise and also those changes
needs those changed drivers to be tested.

What do you suggest ? At least this patch needs to be dropped as of now
till we can have complete coverage for those bad users.

Yes, it needs to be dropped. Stephen, can you please revert it from next?

How to fix this: The only way is to review all tasklet usage sites for
creative abuse and then fix them one by one. This needs to be done anyway
because those are ticking timebombs even without changes in the core
code. I looked at one of the offenders and it's broken today, it's just
protected by the extremly low probablity to hit the wreckage case.

What you can do to coerce the developers/maintainers of offending code into
looking at the mess they created/merged is to implement accessors for the
tasklet struct fields and replace the open coded fiddling with them.

Once that is done, rename the struct fields to something which is absurd
enough to type.  But don't worry, you will find people doing that. I
catched a few brainwrecks who actually used:

 irqdesc->core_internal_state__do_not_mess_with_it

in their code.

Now after having everything converted to accessors, you can add sanity
checks into the accessors and emit WARN_ONCE() when they are used in the
wrong context. That'll make them look and explain why they think that
fiddling in the internals is a good idea.

Thanks Thomas for suggestion and looking into it. Sounds a good plan
to me to tackle this mess. I will give a try and hopefully with help
of those maintainers come up with a series first to fix the
existing bad users. As you said, fixing core will be simple after that.

Regards,
Santosh

Reply via email to