Re: [PATCH 01/13] genirq/msi: Add cpumask allocation to alloc_msi_entry

2016-09-20 Thread Thomas Gleixner 
On Tue, 20 Sep 2016, Alexander Gordeev wrote:
> On Mon, Sep 19, 2016 at 03:50:07PM +0200, Christoph Hellwig wrote:
> > On Mon, Sep 19, 2016 at 09:30:58AM +0200, Alexander Gordeev wrote:
> > > > INIT_LIST_HEAD(&desc->list);
> > > > desc->dev = dev;
> > > > +   desc->nvec_used = nvec;
> 
> (*)
> 
> > > > +   if (affinity) {
> > > > +   desc->affinity = kmemdup(affinity,
> > > > +   nvec * sizeof(*desc->affinity), GFP_KERNEL);
> > > > +   if (!desc->affinity) {
> > > > +   kfree(desc);
> > > > +   return NULL;
> > > > +   }
> > > > +   }
> > > 
> > > nit - should not "desc" initialization follow "desc->affinity" allocation?
> > 
> > I can't parse that sentence.  Do you mean the desc->nvec_used setup?
> 
> Yes, the inits above (*) would be useless if desc->affinity allocation failed.

And that matters how?

Thanks,

tglx
 
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/13] genirq/msi: Add cpumask allocation to alloc_msi_entry

2016-09-20 Thread Alexander Gordeev 
On Mon, Sep 19, 2016 at 03:50:07PM +0200, Christoph Hellwig wrote:
> On Mon, Sep 19, 2016 at 09:30:58AM +0200, Alexander Gordeev wrote:
> > >   INIT_LIST_HEAD(&desc->list);
> > >   desc->dev = dev;
> > > + desc->nvec_used = nvec;

(*)

> > > + if (affinity) {
> > > + desc->affinity = kmemdup(affinity,
> > > + nvec * sizeof(*desc->affinity), GFP_KERNEL);
> > > + if (!desc->affinity) {
> > > + kfree(desc);
> > > + return NULL;
> > > + }
> > > + }
> > 
> > nit - should not "desc" initialization follow "desc->affinity" allocation?
> 
> I can't parse that sentence.  Do you mean the desc->nvec_used setup?

Yes, the inits above (*) would be useless if desc->affinity allocation failed.
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/13] genirq/msi: Add cpumask allocation to alloc_msi_entry

2016-09-19 Thread Christoph Hellwig 
On Mon, Sep 19, 2016 at 09:30:58AM +0200, Alexander Gordeev wrote:
> > INIT_LIST_HEAD(&desc->list);
> > desc->dev = dev;
> > +   desc->nvec_used = nvec;
> > +   if (affinity) {
> > +   desc->affinity = kmemdup(affinity,
> > +   nvec * sizeof(*desc->affinity), GFP_KERNEL);
> > +   if (!desc->affinity) {
> > +   kfree(desc);
> > +   return NULL;
> > +   }
> > +   }
> 
> nit - should not "desc" initialization follow "desc->affinity" allocation?

I can't parse that sentence.  Do you mean the desc->nvec_used setup?
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/13] genirq/msi: Add cpumask allocation to alloc_msi_entry

2016-09-19 Thread Alexander Gordeev 
On Wed, Sep 14, 2016 at 04:18:47PM +0200, Christoph Hellwig wrote:
> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
> index 19e9dfb..8a3e8727 100644
> --- a/kernel/irq/msi.c
> +++ b/kernel/irq/msi.c
> @@ -18,20 +18,42 @@
>  /* Temparory solution for building, will be removed later */
>  #include 
>  
> -struct msi_desc *alloc_msi_entry(struct device *dev)
> +/**
> + * alloc_msi_entry - Allocate an initialize msi_entry
> + * @dev: Pointer to the device for which this is allocated
> + * @nvec:The number of vectors used in this entry
> + * @affinity:Optional pointer to an affinity mask array size of @nvec
> + *
> + * If @affinity is not NULL then a an affinity array[@nvec] is allocated
> + * and the affinity masks from @affinity are copied.
> + */
> +struct msi_desc *
> +alloc_msi_entry(struct device *dev, int nvec, const struct cpumask *affinity)
>  {
> - struct msi_desc *desc = kzalloc(sizeof(*desc), GFP_KERNEL);
> + struct msi_desc *desc;
> +
> + desc = kzalloc(sizeof(*desc), GFP_KERNEL);
>   if (!desc)
>   return NULL;
>  
>   INIT_LIST_HEAD(&desc->list);
>   desc->dev = dev;
> + desc->nvec_used = nvec;
> + if (affinity) {
> + desc->affinity = kmemdup(affinity,
> + nvec * sizeof(*desc->affinity), GFP_KERNEL);
> + if (!desc->affinity) {
> + kfree(desc);
> + return NULL;
> + }
> + }

nit - should not "desc" initialization follow "desc->affinity" allocation?

>   return desc;
>  }
>  
>  void free_msi_entry(struct msi_desc *entry)
>  {
> + kfree(entry->affinity);
>   kfree(entry);
>  }
>  
> -- 
> 2.1.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html