| Message ID | 20210901000812.120968-7-sukadev@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ibmvnic: Reuse ltb, rx, tx pools | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 7 maintainers not CCed: mpe@ellerman.id.au paulus@samba.org tlfalcon@linux.ibm.com davem@davemloft.net linuxppc-dev@lists.ozlabs.org kuba@kernel.org benh@kernel.crashing.org |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 4 this patch: 4 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 45 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On 2021-08-31 17:08, Sukadev Bhattiprolu wrote: > In a follow-on patch, we will reuse long term buffers when possible. > When doing so we have to be careful to properly assign map ids. We > can no longer assign them sequentially because a lower map id may be > available and we could wrap at 255 and collide with an in-use map id. > > Instead, use a bitmap to track active map ids and to find a free map > id. > Don't need to take locks here since the map_id only changes during > reset > and at that time only the reset worker thread should be using the > adapter. > > Noticed this when analyzing an error Dany Madden ran into with the > patch set. > > Reported-by: Dany Madden <drt@linux.ibm.com> > Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com> Reviewed-by: Dany Madden <drt@linux.ibm.com> > --- > drivers/net/ethernet/ibm/ibmvnic.c | 12 ++++++++---- > drivers/net/ethernet/ibm/ibmvnic.h | 3 ++- > 2 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/ethernet/ibm/ibmvnic.c > b/drivers/net/ethernet/ibm/ibmvnic.c > index 8894afdb3cb3..30153a8bb5ec 100644 > --- a/drivers/net/ethernet/ibm/ibmvnic.c > +++ b/drivers/net/ethernet/ibm/ibmvnic.c > @@ -228,8 +228,9 @@ static int alloc_long_term_buff(struct > ibmvnic_adapter *adapter, > dev_err(dev, "Couldn't alloc long term buffer\n"); > return -ENOMEM; > } > - ltb->map_id = adapter->map_id; > - adapter->map_id++; > + ltb->map_id = find_first_zero_bit(adapter->map_ids, > + MAX_MAP_ID); > + bitmap_set(adapter->map_ids, ltb->map_id, 1); > > mutex_lock(&adapter->fw_lock); > adapter->fw_done_rc = 0; > @@ -284,6 +285,8 @@ static void free_long_term_buff(struct > ibmvnic_adapter *adapter, > dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr); > > ltb->buff = NULL; > + /* mark this map_id free */ > + bitmap_clear(adapter->map_ids, ltb->map_id, 1); > ltb->map_id = 0; > } > > @@ -1231,8 +1234,6 @@ static int init_resources(struct ibmvnic_adapter > *adapter) > return rc; > } > > - adapter->map_id = 1; > - > rc = init_napi(adapter); > if (rc) > return rc; > @@ -5553,6 +5554,9 @@ static int ibmvnic_probe(struct vio_dev *dev, > const struct vio_device_id *id) > adapter->vdev = dev; > adapter->netdev = netdev; > adapter->login_pending = false; > + memset(&adapter->map_ids, 0, sizeof(adapter->map_ids)); > + /* map_ids start at 1, so ensure map_id 0 is always "in-use" */ > + bitmap_set(adapter->map_ids, 0, 1); > > ether_addr_copy(adapter->mac_addr, mac_addr_p); > ether_addr_copy(netdev->dev_addr, adapter->mac_addr); > diff --git a/drivers/net/ethernet/ibm/ibmvnic.h > b/drivers/net/ethernet/ibm/ibmvnic.h > index 5652566818fb..e97f1aa98c05 100644 > --- a/drivers/net/ethernet/ibm/ibmvnic.h > +++ b/drivers/net/ethernet/ibm/ibmvnic.h > @@ -979,7 +979,8 @@ struct ibmvnic_adapter { > u64 opt_tx_entries_per_subcrq; > u64 opt_rxba_entries_per_subcrq; > __be64 tx_rx_desc_req; > - u8 map_id; > +#define MAX_MAP_ID 255 > + DECLARE_BITMAP(map_ids, MAX_MAP_ID); > u32 num_active_rx_scrqs; > u32 num_active_rx_pools; > u32 num_active_rx_napi;
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 8894afdb3cb3..30153a8bb5ec 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -228,8 +228,9 @@ static int alloc_long_term_buff(struct ibmvnic_adapter *adapter, dev_err(dev, "Couldn't alloc long term buffer\n"); return -ENOMEM; } - ltb->map_id = adapter->map_id; - adapter->map_id++; + ltb->map_id = find_first_zero_bit(adapter->map_ids, + MAX_MAP_ID); + bitmap_set(adapter->map_ids, ltb->map_id, 1); mutex_lock(&adapter->fw_lock); adapter->fw_done_rc = 0; @@ -284,6 +285,8 @@ static void free_long_term_buff(struct ibmvnic_adapter *adapter, dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr); ltb->buff = NULL; + /* mark this map_id free */ + bitmap_clear(adapter->map_ids, ltb->map_id, 1); ltb->map_id = 0; } @@ -1231,8 +1234,6 @@ static int init_resources(struct ibmvnic_adapter *adapter) return rc; } - adapter->map_id = 1; - rc = init_napi(adapter); if (rc) return rc; @@ -5553,6 +5554,9 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id) adapter->vdev = dev; adapter->netdev = netdev; adapter->login_pending = false; + memset(&adapter->map_ids, 0, sizeof(adapter->map_ids)); + /* map_ids start at 1, so ensure map_id 0 is always "in-use" */ + bitmap_set(adapter->map_ids, 0, 1); ether_addr_copy(adapter->mac_addr, mac_addr_p); ether_addr_copy(netdev->dev_addr, adapter->mac_addr); diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h index 5652566818fb..e97f1aa98c05 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.h +++ b/drivers/net/ethernet/ibm/ibmvnic.h @@ -979,7 +979,8 @@ struct ibmvnic_adapter { u64 opt_tx_entries_per_subcrq; u64 opt_rxba_entries_per_subcrq; __be64 tx_rx_desc_req; - u8 map_id; +#define MAX_MAP_ID 255 + DECLARE_BITMAP(map_ids, MAX_MAP_ID); u32 num_active_rx_scrqs; u32 num_active_rx_pools; u32 num_active_rx_napi;
In a follow-on patch, we will reuse long term buffers when possible. When doing so we have to be careful to properly assign map ids. We can no longer assign them sequentially because a lower map id may be available and we could wrap at 255 and collide with an in-use map id. Instead, use a bitmap to track active map ids and to find a free map id. Don't need to take locks here since the map_id only changes during reset and at that time only the reset worker thread should be using the adapter. Noticed this when analyzing an error Dany Madden ran into with the patch set. Reported-by: Dany Madden <drt@linux.ibm.com> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com> --- drivers/net/ethernet/ibm/ibmvnic.c | 12 ++++++++---- drivers/net/ethernet/ibm/ibmvnic.h | 3 ++- 2 files changed, 10 insertions(+), 5 deletions(-)