On Wed, Jul 06, 2022 at 02:44:42PM +0530, Praveen Kumar wrote:
> 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.
> >
> > Signed-off-by: Saurabh Sengar <ssengar@xxxxxxxxxxxxxxxxxxx>
> > ---
> > drivers/scsi/storvsc_drv.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> > index fe000da..0c428cb 100644
> > --- a/drivers/scsi/storvsc_drv.c
> > +++ b/drivers/scsi/storvsc_drv.c
> > @@ -60,6 +60,9 @@
> > #define VMSTOR_PROTO_VERSION_WIN8_1 VMSTOR_PROTO_VERSION(6, 0)
> > #define VMSTOR_PROTO_VERSION_WIN10 VMSTOR_PROTO_VERSION(6, 2)
> >
> > +/* channel callback timeout in ms */
> > +#define CALLBACK_TIMEOUT 5
>
> If I may, it would be good if we have the CALLBACK_TIMEOUT configurable based upon user's requirement with default value to '5'.
> I assume, this value '5' fits best to the use-case which we are trying to resolve here. Thanks.
Agree, how about adding a sysfs entry for this parameter
>
> > +
> > /* Packet structure describing virtual storage requests. */
> > enum vstor_packet_operation {
> > VSTOR_OPERATION_COMPLETE_IO = 1,
> > @@ -1204,6 +1207,7 @@ static void storvsc_on_channel_callback(void *context)
> > struct hv_device *device;
> > struct storvsc_device *stor_device;
> > struct Scsi_Host *shost;
> > + unsigned long expire = jiffies + msecs_to_jiffies(CALLBACK_TIMEOUT);
> >
> > if (channel->primary_channel != NULL)
> > device = channel->primary_channel->device_obj;
> > @@ -1224,6 +1228,9 @@ static void storvsc_on_channel_callback(void *context)
> > u32 minlen = rqst_id ? sizeof(struct vstor_packet) :
> > sizeof(enum vstor_packet_operation);
> >
> > + if (time_after(jiffies, expire))
> > + break;
> > +
> > if (pktlen < minlen) {
> > dev_err(&device->device,
> > "Invalid pkt: id=%llu, len=%u, minlen=%u\n",
>
> Regards,
>
> ~Praveen.
