lkml.org 
[lkml]   [2022]   [Mar]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [EXTERNAL] [PATCH 1/2] PCI: hv: Use IDR to generate transaction IDs for VMBus hardening
Date


> -----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

\
 
 \ /
  Last update: 2022-03-20 06:54    [W:0.077 / U:0.392 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site