> -----Original Message-----
> From: Andrea Parri <parri.andrea@xxxxxxxxx>
> Sent: 19 March 2022 21:29
> To: Saurabh Singh Sengar <ssengar@xxxxxxxxxxxxx>
> Cc: KY Srinivasan <kys@xxxxxxxxxxxxx>; Haiyang Zhang
> <haiyangz@xxxxxxxxxxxxx>; Stephen Hemminger
> <sthemmin@xxxxxxxxxxxxx>; Wei Liu <wei.liu@xxxxxxxxxx>; Dexuan Cui
> <decui@xxxxxxxxxxxxx>; Michael Kelley (LINUX) <mikelley@xxxxxxxxxxxxx>;
> Wei Hu <weh@xxxxxxxxxxxxx>; Lorenzo Pieralisi
> <lorenzo.pieralisi@xxxxxxx>; Rob Herring <robh@xxxxxxxxxx>; Krzysztof
> Wilczynski <kw@xxxxxxxxx>; Bjorn Helgaas <bhelgaas@xxxxxxxxxx>; linux-
> pci@xxxxxxxxxxxxxxx; linux-hyperv@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: Re: [EXTERNAL] [PATCH 1/2] PCI: hv: Use IDR to generate transaction
> IDs for VMBus hardening
>
> > > @@ -1208,6 +1211,27 @@ static void hv_pci_read_config_compl(void
> > > *context, struct pci_response *resp,
> > > complete(&comp->comp_pkt.host_event);
> > > }
> > >
> > > +static inline int alloc_request_id(struct hv_pcibus_device *hbus,
> > > + void *ptr, gfp_t gfp)
> > > +{
> > > + unsigned long flags;
> > > + int req_id;
> > > +
> > > + spin_lock_irqsave(&hbus->idr_lock, flags);
> > > + req_id = idr_alloc(&hbus->idr, ptr, 1, 0, gfp);
> >
> > [Saurabh Singh Sengar] Many a place we are using alloc_request_id with
> GFP_KERNEL, which results this allocation inside of spin lock with
> GFP_KERNEL.
>
> That's a bug.
>
>
> > Is this a good opportunity to use idr_preload ?
>
> I'd rather fix (and 'simplify' a bit the interface) by doing:
>
> static inline int alloc_request_id(struct hv_pcibus_device *hbus, void *ptr)
> {
> unsigned long flags;
> int req_id;
>
> spin_lock_irqsave(&hbus->idr_lock, flags);
> req_id = idr_alloc(&hbus->idr, ptr, 1, 0, GFP_ATOMIC);
> spin_unlock_irqrestore(&hbus->idr_lock, flags);
> return req_id;
> }
>
> Thoughts?
[Saurabh Sengar] Yes, if we are fine to use GFP_ATOMIC, this makes perfect sense.
Once fixed, please add: Reviewed-by: Saurabh Sengar <ssengar@xxxxxxxxxxxxx>
>
> Thanks,
> Andrea
