Re: [PATCH 02/34] drm: Convert aux_idr to XArray

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 02/34] drm: Convert aux_idr to XArray
Date: Mon, 25 Feb 2019 20:50:56 +0200	[thread overview]
Message-ID: <20190225185056.GJ20097@intel.com> (raw)
In-Reply-To: <20190225184246.GG2496@bombadil.infradead.org>

On Mon, Feb 25, 2019 at 10:42:46AM -0800, Matthew Wilcox wrote:
> On Mon, Feb 25, 2019 at 07:57:33PM +0200, Ville Syrjälä wrote:
> > On Thu, Feb 21, 2019 at 10:41:23AM -0800, Matthew Wilcox wrote:
> > > @@ -49,8 +49,7 @@ struct drm_dp_aux_dev {
> > >  
> > >  #define DRM_AUX_MINORS	256
> > >  #define AUX_MAX_OFFSET	(1 << 20)
> > > -static DEFINE_IDR(aux_idr);
> > > -static DEFINE_MUTEX(aux_idr_mutex);
> > > +static DEFINE_XARRAY_ALLOC(aux_xa);
> > >  static struct class *drm_dp_aux_dev_class;
> > >  static int drm_dev_major = -1;
> [...]
> > >  static struct drm_dp_aux_dev *alloc_drm_dp_aux_dev(struct drm_dp_aux *aux)
> > >  {
> > > +	static u32 next;
> > 
> > Is there a particular reason for that being static?
> 
> It needs to maintain its value between function calls so that we know
> where to start allocating from the next time we call xa_alloc_cyclic().
> It can either live here with a short name, or we could put it at file
> scope with a descriptive name (probably aux_xa_next).  It's up to you
> which you think is better; it's your driver.

File scope would seem a bit more clear to me. I'm slightly worried
someone might do some cleanup here and drop the static without
thinking. Alterntively a short comment would probably suffice.

> 
> The IDR embedded the 'next' value in the struct idr, but that was
> overhead paid by all users of the IDR rather than the very few that
> called idr_alloc_cyclic().
> 
> > > +	err = xa_alloc_cyclic(&aux_xa, &aux_dev->index, aux_dev,
> > > +			XA_LIMIT(0, DRM_AUX_MINORS), &next, GFP_KERNEL);
> > > +	if (err < 0) {
> > >  		kfree(aux_dev);

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel