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.
> ---
> drivers/usb/class/cdc-acm.c | 33 ++++++++++++++-------------------
> 1 file changed, 14 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
> index 739f8960811a..28eb9a898b4a 100644
> --- a/drivers/usb/class/cdc-acm.c
> +++ b/drivers/usb/class/cdc-acm.c
> @@ -47,7 +47,7 @@
> static struct usb_driver acm_driver;
> static struct tty_driver *acm_tty_driver;
>
> -static DEFINE_IDR(acm_minors);
> +static DEFINE_XARRAY_ALLOC(acm_minors);
> static DEFINE_MUTEX(acm_minors_lock);
>
> static void acm_tty_set_termios(struct tty_struct *tty,
> @@ -66,7 +66,7 @@ static struct acm *acm_get_by_minor(unsigned int minor)
> struct acm *acm;
>
> mutex_lock(&acm_minors_lock);
> - acm = idr_find(&acm_minors, minor);
> + acm = xa_load(&acm_minors, minor);
> if (acm) {
> mutex_lock(&acm->mutex);
> if (acm->disconnected) {
> @@ -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?
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 :)
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,
greg k-h
