On Tue, Mar 19, 2019 at 01:14:20PM +0100, Greg KH wrote:
> On Mon, Mar 18, 2019 at 02:17:11PM -0700, Matthew Wilcox wrote:
> > Signed-off-by: Matthew Wilcox <willy@xxxxxxxxxxxxx>
>
> Also something here saying acm_minors was an idr structure that is being
> converted to xarray.
ACK.
> > @@ -86,20 +86,15 @@ static struct acm *acm_get_by_minor(unsigned int minor)
> > */
> > static int acm_alloc_minor(struct acm *acm)
> > {
> > - int minor;
> > -
> > - mutex_lock(&acm_minors_lock);
> > - minor = idr_alloc(&acm_minors, acm, 0, ACM_TTY_MINORS, GFP_KERNEL);
> > - mutex_unlock(&acm_minors_lock);
> > -
> > - return minor;
> > + return xa_alloc(&acm_minors, &acm->minor, acm,
> > + XA_LIMIT(0, ACM_TTY_MINORS - 1), GFP_KERNEL);
>
> So, the only thing "better" here is that you don't need a lock anymore?
As far as this driver is concerned, I think that's the only advantage.
Other drivers see more advantages.
> Why not just replace the idr api "underneath" with xarray and then drop
> all of the locks over time?
>
> idr was just a wrapper on top of ida, what's one more :)
umm, IDR was never a wrapper over IDA. Originally, it was its own data
structure, then IDA was grafted on top of IDR, then I merged the IDR
and radix tree, now I'm replacing both the IDR and the radix tree.
> Anyway, no objection from me, I like tree-wide changes, and appreciate
> the effort that goes into it. Thanks for doing it. If you fix up the
> changelog entries for the ones that have none, I'll be glad to queue
> these up.
Thanks.
