Messages in this thread |  | | From | Saurabh Singh Sengar <> | Subject | RE: [EXTERNAL] [PATCH 1/2] PCI: hv: Use IDR to generate transaction IDs for VMBus hardening | Date | Sun, 20 Mar 2022 05:53:29 +0000 |
| |
> -----Original Message----- > From: Andrea Parri <parri.andrea@gmail.com> > Sent: 19 March 2022 21:29 > To: Saurabh Singh Sengar <ssengar@microsoft.com> > Cc: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang > <haiyangz@microsoft.com>; Stephen Hemminger > <sthemmin@microsoft.com>; Wei Liu <wei.liu@kernel.org>; Dexuan Cui > <decui@microsoft.com>; Michael Kelley (LINUX) <mikelley@microsoft.com>; > Wei Hu <weh@microsoft.com>; Lorenzo Pieralisi > <lorenzo.pieralisi@arm.com>; Rob Herring <robh@kernel.org>; Krzysztof > Wilczynski <kw@linux.com>; Bjorn Helgaas <bhelgaas@google.com>; linux- > pci@vger.kernel.org; linux-hyperv@vger.kernel.org; linux- > kernel@vger.kernel.org > 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@microsoft.com>
> > Thanks, > Andrea
|  |