Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   David Laight <David.Laight@ACULAB.COM>
To:     'Saurabh Singh Sengar' <ssengar@linux.microsoft.com>
CC:     'Praveen Kumar' <kumarpraveen@linux.microsoft.com>,
        "kys@microsoft.com" <kys@microsoft.com>,
        "haiyangz@microsoft.com" <haiyangz@microsoft.com>,
        "sthemmin@microsoft.com" <sthemmin@microsoft.com>,
        "wei.liu@kernel.org" <wei.liu@kernel.org>,
        "decui@microsoft.com" <decui@microsoft.com>,
        "jejb@linux.ibm.com" <jejb@linux.ibm.com>,
        "martin.petersen@oracle.com" <martin.petersen@oracle.com>,
        "linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
        "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "ssengar@microsoft.com" <ssengar@microsoft.com>,
        "mikelley@microsoft.com" <mikelley@microsoft.com>
Subject: RE: [PATCH] scsi: storvsc: Prevent running tasklet for long
Thread-Topic: [PATCH] scsi: storvsc: Prevent running tasklet for long
Thread-Index: AQHYkRjhlVfTnLUZGEKVRk7w2SiyFK1xLt+AgAMM44CAAC7FkA==
Date:   Fri, 8 Jul 2022 12:41:17 +0000
Message-ID: <2638aa8409104c41a560548be5628173@AcuMS.aculab.com>
References: <1657035141-2132-1-git-send-email-ssengar@linux.microsoft.com>
 <b4fea161-41c5-a03e-747b-316c74eb986c@linux.microsoft.com>
 <a9af8d8d5ee24d19a87c3353a4e8941d@AcuMS.aculab.com>
 <20220708104203.GA10366@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
In-Reply-To: <20220708104203.GA10366@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
Accept-Language: en-GB, en-US
MIME-Version: 1.0
Content-Language: en-US
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT
Precedence: bulk

From: Saurabh Singh Sengar
> Sent: 08 July 2022 11:42
> 
> On Wed, Jul 06, 2022 at 11:09:43AM +0000, David Laight wrote:
> > From: Praveen Kumar
> > > Sent: 06 July 2022 10:15
> > >
> > > On 05-07-2022 21:02, Saurabh Sengar wrote:
> > > > There can be scenarios where packets in ring buffer are continuously
> > > > getting queued from upper layer and dequeued from storvsc interrupt
> > > > handler, such scenarios can hold the foreach_vmbus_pkt loop (which is
> > > > executing as a tasklet) for a long duration. Theoretically its possible
> > > > that this loop executes forever. Add a condition to limit execution of
> > > > this tasklet for finite amount of time to avoid such hazardous scenarios.
> >
> > Does this really make much difference?
> >
> > I'd guess the tasklet gets immediately rescheduled as soon as
> > the upper layer queues another packet?
> >
> > Or do you get a different 'bug' where it is never woken again
> > because the ring is stuck full?
> >
> > 	David
> 
> My initial understanding was that staying in a tasklet for "too long" may not be a
> good idea, however I was not sure what the "too long" value be, thus we are thinking
> to provide this parameter as a configurable sysfs entry. I couldn't find any linux
> doc justifying this, so please correct me here if I am mistaken.
> We have also considered the networking drivers NAPI budget feature while deciding
> this approach, where softirq exits once the budget is crossed. This budget feature
> act as a performance tuning parameter for driver, and also can help with ring buffer
> overflow. I believe similar reasons are true for scsi softirq as well.
> 
> NAPI budget Ref : https://wiki.linuxfoundation.org/networking/napi.

The NAPI 'budget' just changes where the system loops.
Instead of looping inside the ethernet driver rx processing
it first loops through the other functions of that 'napi' and
then loops in the softirq code.
All that just allows other 'softint' functions to run.
The softint code itself will defer to a kernel thread.
The softint code got patched (by Eric) to make it defer to
the thread less often (to avoid dropping packets), but that
change got reverted (well the original check was added back)
so the problem Eric was fixing got re-introduced.

If you are trying to stop rx ring buffer overflow then you
do need it to loop. If the softint code ever defers to a thread
you lose big-time.

The only way I've managed to receive 500,000 packets/sec is
using threaded napi and setting the napi thread to run under
the RT scheduler.

If you are looping from the softint scheduler you probably need to
return within a few microseconds - otherwise the ethernet
receive will start dropping packets at moderate loads.

OTOH I don't know where 'tasklet' get scheduler from.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)