| Message ID | 1543925069-8838-11-git-send-email-galpress@amazon.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Elastic Fabric Adapter (EFA) driver | expand |
On Tue, Dec 04, 2018 at 02:04:26PM +0200, Gal Pressman wrote: > Bitmap allocation service is currently used for assigning > Protection Domain (PD) numbers. > > Signed-off-by: Gal Pressman <galpress@amazon.com> > drivers/infiniband/hw/efa/efa_bitmap.c | 76 ++++++++++++++++++++++++++++++++++ > 1 file changed, 76 insertions(+) > create mode 100644 drivers/infiniband/hw/efa/efa_bitmap.c > > diff --git a/drivers/infiniband/hw/efa/efa_bitmap.c b/drivers/infiniband/hw/efa/efa_bitmap.c > new file mode 100644 > index 000000000000..251cc68d25f5 > +++ b/drivers/infiniband/hw/efa/efa_bitmap.c > @@ -0,0 +1,76 @@ > +// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB > +/* > + * Copyright 2006, 2007 Cisco Systems, Inc. All rights reserved. > + * Copyright 2007, 2008 Mellanox Technologies. All rights reserved. > + * Copyright 2018 Amazon.com, Inc. or its affiliates. > + */ > + > +#include <linux/bitmap.h> > + > +#include "efa.h" > + > +u32 efa_bitmap_alloc(struct efa_bitmap *bitmap) > +{ > + u32 obj; > + > + spin_lock(&bitmap->lock); > + > + obj = find_next_zero_bit(bitmap->table, bitmap->max, bitmap->last); > + if (obj >= bitmap->max) > + obj = find_first_zero_bit(bitmap->table, bitmap->max); > + > + if (obj < bitmap->max) { > + set_bit(obj, bitmap->table); > + bitmap->last = obj + 1; > + if (bitmap->last == bitmap->max) > + bitmap->last = 0; > + } else { > + obj = EFA_BITMAP_INVAL; > + } > + > + if (obj != EFA_BITMAP_INVAL) > + --bitmap->avail; > + > + spin_unlock(&bitmap->lock); > + > + return obj; > +} Isn't this just an ida? Jason
On 04-Dec-18 18:03, Jason Gunthorpe wrote: > On Tue, Dec 04, 2018 at 02:04:26PM +0200, Gal Pressman wrote: >> Bitmap allocation service is currently used for assigning >> Protection Domain (PD) numbers. >> >> Signed-off-by: Gal Pressman <galpress@amazon.com> >> drivers/infiniband/hw/efa/efa_bitmap.c | 76 ++++++++++++++++++++++++++++++++++ >> 1 file changed, 76 insertions(+) >> create mode 100644 drivers/infiniband/hw/efa/efa_bitmap.c >> >> diff --git a/drivers/infiniband/hw/efa/efa_bitmap.c b/drivers/infiniband/hw/efa/efa_bitmap.c >> new file mode 100644 >> index 000000000000..251cc68d25f5 >> +++ b/drivers/infiniband/hw/efa/efa_bitmap.c >> @@ -0,0 +1,76 @@ >> +// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB >> +/* >> + * Copyright 2006, 2007 Cisco Systems, Inc. All rights reserved. >> + * Copyright 2007, 2008 Mellanox Technologies. All rights reserved. >> + * Copyright 2018 Amazon.com, Inc. or its affiliates. >> + */ >> + >> +#include <linux/bitmap.h> >> + >> +#include "efa.h" >> + >> +u32 efa_bitmap_alloc(struct efa_bitmap *bitmap) >> +{ >> + u32 obj; >> + >> + spin_lock(&bitmap->lock); >> + >> + obj = find_next_zero_bit(bitmap->table, bitmap->max, bitmap->last); >> + if (obj >= bitmap->max) >> + obj = find_first_zero_bit(bitmap->table, bitmap->max); >> + >> + if (obj < bitmap->max) { >> + set_bit(obj, bitmap->table); >> + bitmap->last = obj + 1; >> + if (bitmap->last == bitmap->max) >> + bitmap->last = 0; >> + } else { >> + obj = EFA_BITMAP_INVAL; >> + } >> + >> + if (obj != EFA_BITMAP_INVAL) >> + --bitmap->avail; >> + >> + spin_unlock(&bitmap->lock); >> + >> + return obj; >> +} > > Isn't this just an ida? > > Jason > Will examine.
On Fri, Dec 07, 2018 at 10:36:12AM -0600, Tom Tucker wrote: > On 12/05/2018 02:56 AM, Gal Pressman wrote: > > On 04-Dec-18 18:03, Jason Gunthorpe wrote: > > On Tue, Dec 04, 2018 at 02:04:26PM +0200, Gal Pressman wrote: > > Bitmap allocation service is currently used for assigning > Protection Domain (PD) numbers. > > Signed-off-by: Gal Pressman [1]<galpress@amazon.com> > drivers/infiniband/hw/efa/efa_bitmap.c | 76 ++++++++++++++++++++++++++++++++++ > 1 file changed, 76 insertions(+) > create mode 100644 drivers/infiniband/hw/efa/efa_bitmap.c > > diff --git a/drivers/infiniband/hw/efa/efa_bitmap.c b/drivers/infiniband/hw/efa/ > efa_bitmap.c > new file mode 100644 > index 000000000000..251cc68d25f5 > +++ b/drivers/infiniband/hw/efa/efa_bitmap.c > @@ -0,0 +1,76 @@ > +// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB > +/* > + * Copyright 2006, 2007 Cisco Systems, Inc. All rights reserved. > + * Copyright 2007, 2008 Mellanox Technologies. All rights reserved. > + * Copyright 2018 Amazon.com, Inc. or its affiliates. > + */ > + > +#include <linux/bitmap.h> > + > +#include "efa.h" > + > +u32 efa_bitmap_alloc(struct efa_bitmap *bitmap) > +{ > + u32 obj; > + > + spin_lock(&bitmap->lock); > + > + obj = find_next_zero_bit(bitmap->table, bitmap->max, bitmap->last); > + if (obj >= bitmap->max) > + obj = find_first_zero_bit(bitmap->table, bitmap->max); > + > + if (obj < bitmap->max) { > + set_bit(obj, bitmap->table); > + bitmap->last = obj + 1; > + if (bitmap->last == bitmap->max) > + bitmap->last = 0; > + } else { > + obj = EFA_BITMAP_INVAL; > + } > + > + if (obj != EFA_BITMAP_INVAL) > + --bitmap->avail; > + > + spin_unlock(&bitmap->lock); > + > + return obj; > +} > > Isn't this just an ida? > > Jason > > > Will examine. > > The idr_alloc() service uses radix trees and is not spin locked. They > would need to wrap the idr calls anyway and not care about the > performance implications. Assiging protection domain numbers is not a performance path - and these days ida has an internal lock that callers can use. For the simple case of < 64*64 the performance will be very similar. A few branches more for the IDA, but the code is also more likely to be in cache. Jason
diff --git a/drivers/infiniband/hw/efa/efa_bitmap.c b/drivers/infiniband/hw/efa/efa_bitmap.c new file mode 100644 index 000000000000..251cc68d25f5 --- /dev/null +++ b/drivers/infiniband/hw/efa/efa_bitmap.c @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB +/* + * Copyright 2006, 2007 Cisco Systems, Inc. All rights reserved. + * Copyright 2007, 2008 Mellanox Technologies. All rights reserved. + * Copyright 2018 Amazon.com, Inc. or its affiliates. + */ + +#include <linux/bitmap.h> + +#include "efa.h" + +u32 efa_bitmap_alloc(struct efa_bitmap *bitmap) +{ + u32 obj; + + spin_lock(&bitmap->lock); + + obj = find_next_zero_bit(bitmap->table, bitmap->max, bitmap->last); + if (obj >= bitmap->max) + obj = find_first_zero_bit(bitmap->table, bitmap->max); + + if (obj < bitmap->max) { + set_bit(obj, bitmap->table); + bitmap->last = obj + 1; + if (bitmap->last == bitmap->max) + bitmap->last = 0; + } else { + obj = EFA_BITMAP_INVAL; + } + + if (obj != EFA_BITMAP_INVAL) + --bitmap->avail; + + spin_unlock(&bitmap->lock); + + return obj; +} + +void efa_bitmap_free(struct efa_bitmap *bitmap, u32 obj) +{ + obj &= bitmap->mask; + + spin_lock(&bitmap->lock); + bitmap_clear(bitmap->table, obj, 1); + bitmap->avail++; + spin_unlock(&bitmap->lock); +} + +u32 efa_bitmap_avail(struct efa_bitmap *bitmap) +{ + return bitmap->avail; +} + +int efa_bitmap_init(struct efa_bitmap *bitmap, u32 num) +{ + /* num must be a power of 2 */ + if (!is_power_of_2(num)) + return -EINVAL; + + bitmap->last = 0; + bitmap->max = num; + bitmap->mask = num - 1; + bitmap->avail = num; + spin_lock_init(&bitmap->lock); + bitmap->table = kcalloc(BITS_TO_LONGS(bitmap->max), + sizeof(long), GFP_KERNEL); + if (!bitmap->table) + return -ENOMEM; + + return 0; +} + +void efa_bitmap_cleanup(struct efa_bitmap *bitmap) +{ + kfree(bitmap->table); +}
Bitmap allocation service is currently used for assigning Protection Domain (PD) numbers. Signed-off-by: Gal Pressman <galpress@amazon.com> --- drivers/infiniband/hw/efa/efa_bitmap.c | 76 ++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 drivers/infiniband/hw/efa/efa_bitmap.c