[Skiboot] [RFC PATCH 4/6] core/flash: Make opal_flash_op() actually asynchronous

[Vasant Hegde]

On 03/01/2019 06:02 AM, Stewart Smith wrote:
> Vasant Hegde <hegdevasant at linux.vnet.ibm.com> writes:
>> On 02/28/2019 11:48 AM, Stewart Smith wrote:
>>> From: Cyril Bur <cyril.bur at au1.ibm.com>
>>>
>>
>> .../...
>>
>>
>>> -err:
>>> +	if (size - len) {
>>> +		/* Work remains */
>>> +		schedule_timer(&flash->async.poller, 0);
>>
>> Why are we calling schedule_time with 0?
> 
> The idea is to ask linux to get back to us "immediately", so we can
> effectively spin reading things from LPC, but still give the kernel time
> to do anything critical before calling back into OPAL.
> 
> If we delay, then we end up impacting performance of reading from flash.

Got it.  If we schedule with `zero` we may endup in continuous loop. We need to 
cross check this:
   - __check_timers() -> schedule_timer(.., 0) -> insert to top of `timer_list` 
-> __check_timers() see message expired.


> 
>>> diff --git a/core/timer.c b/core/timer.c
>>> index 1c539517839e..c68f978ccb07 100644
>>> --- a/core/timer.c
>>> +++ b/core/timer.c
>>> @@ -199,6 +199,7 @@ static void __check_poll_timers(uint64_t now)
>>>    static void __check_timers(uint64_t now)
>>>    {
>>>    	struct timer *t;
>>> +	uint64_t stop = now + msecs_to_tb(50); /* Run timers for max 5ms */
>>
>> I'm not sure why we need this change. Can you please elaborate?
> 
> it should possibly be a separate commit. The idea was to throw some
> warnings if we spend too long running timers, as well as avoid going
> around in a loop inside OPAL when we have a timer scheduled
> "immediately".
> 
> Otherwise, if we schedule something 0 tb ticks away (or close enough
> that we'd hit it by the time we get to the end of the timer list), we
> just spend ever increasing time in OPAL without giving the kernel a
> chance to do something.

Got it. Makes sense. Yes. this issue an independent issue. May be we should have 
separate patch for this.

-Vasant