Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     jun qian <qianjun.kernel@gmail.com>
Cc:     peterz@infradead.org, will@kernel.org, luto@kernel.org,
        linux-kernel@vger.kernel.org, Yafang Shao <laoar.shao@gmail.com>,
        Uladzislau Rezki <urezki@gmail.com>
Subject: Re: [PATCH V4] Softirq:avoid large sched delay from the pending softirqs
In-Reply-To: <CAKc596+vF4eYa4h55P2cssQbRKqBV_-9c_v35SXVMdonP3HBHA@mail.gmail.com>
References: <1595601083-10183-1-git-send-email-qianjun.kernel@gmail.com> <87sgddaru7.fsf@nanos.tec.linutronix.de> <CAKc596+vF4eYa4h55P2cssQbRKqBV_-9c_v35SXVMdonP3HBHA@mail.gmail.com>
Date:   Wed, 29 Jul 2020 14:16:44 +0200
Message-ID: <87v9i6a53n.fsf@nanos.tec.linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Qian,

jun qian <qianjun.kernel@gmail.com> writes:
> On Mon, Jul 27, 2020 at 11:41 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>> > +                     or_softirq_pending(pending << (vec_nr + 1));
>>
>> To or the value interrupts need to be disabled because otherwise you can
>> lose a bit when an interrupt happens in the middle of the RMW operation
>> and raises a softirq which is not in @pending and not in the per CPU
>> local softirq pending storage.
>>
> I can't understand the problem described above, could you explain in
> detail.

Oring a value to a memory location is a Read Modify Write (RMW)
operation.

        x |= a;

translates to pseudo code:

        R1 =  x      // Load content of memory location x into register R1
        R1 |= a      // Or value a on R1
        x = R1       // Write back result
        
So assume:

   x = 0
   a = 1

   R1 = x  --> R1 == 0
   R1 |= a --> R1 == 1

interrupt sets bit 1 in x       --> x == 0x02

   x = R1  --> x == 1

So you lost the set bit 1, right? Not really what you want.

>> There is another problem. Assume bit 0 and 1 are pending when the
>> processing starts. Now it breaks out after bit 0 has been handled and
>> stores back bit 1 as pending. Before ksoftirqd runs bit 0 gets raised
>> again. ksoftirqd runs and handles bit 0, which takes more than the
>> timeout. As a result the bit 0 processing can starve all other softirqs.
>>
> May I use a percpu val to record the order of processing softirq, by the order
> val, when it is in ksoftirqd we can process the pending softirq in order. In the
> scenario you described above, before ksoftirqd runs, bit 0 gets raised again,
> ksoftirqd runs and handles bit 1 by the percpu val. just like a ring.

Yes, you need something to save information about the not-processed
bits. Keeping track of which bit to process next works, but whether
that's going to result in efficient and simple code is a different
question.

Thanks,

        tglx