| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2352d03eea2267df1c95328caea214c1@imap.linux.ibm.com>
Date: Tue, 31 Aug 2021 18:34:21 -0700
From: Dany Madden <drt@...ux.ibm.com>
To: Sukadev Bhattiprolu <sukadev@...ux.ibm.com>
Cc: netdev@...r.kernel.org, Brian King <brking@...ux.ibm.com>,
cforno12@...ux.ibm.com, Rick Lindsley <ricklind@...ux.ibm.com>
Subject: Re: [PATCH net-next 7/9] ibmvnic: Reuse LTB when possible
On 2021-08-31 17:08, Sukadev Bhattiprolu wrote:
> Reuse the long term buffer during a reset as long as its size has
> not changed. If the size has changed, free it and allocate a new
> one of the appropriate size.
>
> When we do this, alloc_long_term_buff() and reset_long_term_buff()
> become identical. Drop reset_long_term_buff().
>
> Signed-off-by: Sukadev Bhattiprolu <sukadev@...ux.ibm.com>
Reviewed-by: Dany Madden <drt@...ux.ibm.com>
> ---
> drivers/net/ethernet/ibm/ibmvnic.c | 122 ++++++++++++++---------------
> 1 file changed, 59 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
> b/drivers/net/ethernet/ibm/ibmvnic.c
> index 30153a8bb5ec..1bb5996c4313 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -108,6 +108,8 @@ static int init_crq_queue(struct ibmvnic_adapter
> *adapter);
> static int send_query_phys_parms(struct ibmvnic_adapter *adapter);
> static void ibmvnic_tx_scrq_clean_buffer(struct ibmvnic_adapter
> *adapter,
> struct ibmvnic_sub_crq_queue *tx_scrq);
> +static void free_long_term_buff(struct ibmvnic_adapter *adapter,
> + struct ibmvnic_long_term_buff *ltb);
>
> struct ibmvnic_stat {
> char name[ETH_GSTRING_LEN];
> @@ -214,23 +216,62 @@ static int ibmvnic_wait_for_completion(struct
> ibmvnic_adapter *adapter,
> return -ETIMEDOUT;
> }
>
> +/**
> + * Reuse long term buffer unless size has changed.
> + */
> +static bool reuse_ltb(struct ibmvnic_long_term_buff *ltb, int size)
> +{
> + return (ltb->buff && ltb->size == size);
> +}
> +
> +/**
> + * Allocate a long term buffer of the specified size and notify VIOS.
> + *
> + * If the given @ltb already has the correct size, reuse it. Otherwise
> if
> + * its non-NULL, free it. Then allocate a new one of the correct size.
> + * Notify the VIOS either way since we may now be working with a new
> VIOS.
> + *
> + * Allocating larger chunks of memory during resets, specially LPM or
> under
> + * low memory situations can cause resets to fail/timeout and for LPAR
> to
> + * lose connectivity. So hold onto the LTB even if we fail to
> communicate
> + * with the VIOS and reuse it on next open. Free LTB when adapter is
> closed.
> + */
> static int alloc_long_term_buff(struct ibmvnic_adapter *adapter,
> struct ibmvnic_long_term_buff *ltb, int size)
> {
> struct device *dev = &adapter->vdev->dev;
> int rc;
>
> - ltb->size = size;
> - ltb->buff = dma_alloc_coherent(dev, ltb->size, <b->addr,
> - GFP_KERNEL);
> + if (!reuse_ltb(ltb, size)) {
> + dev_dbg(dev,
> + "LTB size changed from 0x%llx to 0x%x, reallocating\n",
> + ltb->size, size);
> + free_long_term_buff(adapter, ltb);
> + }
>
> - if (!ltb->buff) {
> - dev_err(dev, "Couldn't alloc long term buffer\n");
> - return -ENOMEM;
> + if (ltb->buff) {
> + dev_dbg(dev, "Reusing LTB [map %d, size 0x%llx]\n",
> + ltb->map_id, ltb->size);
> + } else {
> + ltb->buff = dma_alloc_coherent(dev, size, <b->addr,
> + GFP_KERNEL);
> + if (!ltb->buff) {
> + dev_err(dev, "Couldn't alloc long term buffer\n");
> + return -ENOMEM;
> + }
> + ltb->size = size;
> +
> + ltb->map_id = find_first_zero_bit(adapter->map_ids,
> + MAX_MAP_ID);
> + bitmap_set(adapter->map_ids, ltb->map_id, 1);
> +
> + dev_dbg(dev,
> + "Allocated new LTB [map %d, size 0x%llx]\n",
> + ltb->map_id, ltb->size);
> }
> - ltb->map_id = find_first_zero_bit(adapter->map_ids,
> - MAX_MAP_ID);
> - bitmap_set(adapter->map_ids, ltb->map_id, 1);
> +
> + /* Ensure ltb is zeroed - specially when reusing it. */
> + memset(ltb->buff, 0, ltb->size);
>
> mutex_lock(&adapter->fw_lock);
> adapter->fw_done_rc = 0;
> @@ -257,10 +298,7 @@ static int alloc_long_term_buff(struct
> ibmvnic_adapter *adapter,
> }
> rc = 0;
> out:
> - if (rc) {
> - dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr);
> - ltb->buff = NULL;
> - }
> + /* don't free LTB on communication error - see function header */
> mutex_unlock(&adapter->fw_lock);
> return rc;
> }
> @@ -290,43 +328,6 @@ static void free_long_term_buff(struct
> ibmvnic_adapter *adapter,
> ltb->map_id = 0;
> }
>
> -static int reset_long_term_buff(struct ibmvnic_adapter *adapter,
> - struct ibmvnic_long_term_buff *ltb)
> -{
> - struct device *dev = &adapter->vdev->dev;
> - int rc;
> -
> - memset(ltb->buff, 0, ltb->size);
> -
> - mutex_lock(&adapter->fw_lock);
> - adapter->fw_done_rc = 0;
> -
> - reinit_completion(&adapter->fw_done);
> - rc = send_request_map(adapter, ltb->addr, ltb->size, ltb->map_id);
> - if (rc) {
> - mutex_unlock(&adapter->fw_lock);
> - return rc;
> - }
> -
> - rc = ibmvnic_wait_for_completion(adapter, &adapter->fw_done, 10000);
> - if (rc) {
> - dev_info(dev,
> - "Reset failed, long term map request timed out or aborted\n");
> - mutex_unlock(&adapter->fw_lock);
> - return rc;
> - }
> -
> - if (adapter->fw_done_rc) {
> - dev_info(dev,
> - "Reset failed, attempting to free and reallocate buffer\n");
> - free_long_term_buff(adapter, ltb);
> - mutex_unlock(&adapter->fw_lock);
> - return alloc_long_term_buff(adapter, ltb, ltb->size);
> - }
> - mutex_unlock(&adapter->fw_lock);
> - return 0;
> -}
> -
> static void deactivate_rx_pools(struct ibmvnic_adapter *adapter)
> {
> int i;
> @@ -548,18 +549,10 @@ static int reset_rx_pools(struct ibmvnic_adapter
> *adapter)
>
> netdev_dbg(adapter->netdev, "Re-setting rx_pool[%d]\n", i);
>
> - if (rx_pool->buff_size != buff_size) {
> - free_long_term_buff(adapter, &rx_pool->long_term_buff);
> - rx_pool->buff_size = ALIGN(buff_size, L1_CACHE_BYTES);
> - rc = alloc_long_term_buff(adapter,
> - &rx_pool->long_term_buff,
> - rx_pool->size *
> - rx_pool->buff_size);
> - } else {
> - rc = reset_long_term_buff(adapter,
> - &rx_pool->long_term_buff);
> - }
> -
> + rx_pool->buff_size = ALIGN(buff_size, L1_CACHE_BYTES);
> + rc = alloc_long_term_buff(adapter,
> + &rx_pool->long_term_buff,
> + rx_pool->size * rx_pool->buff_size);
> if (rc)
> return rc;
>
> @@ -692,9 +685,12 @@ static int init_rx_pools(struct net_device
> *netdev)
> static int reset_one_tx_pool(struct ibmvnic_adapter *adapter,
> struct ibmvnic_tx_pool *tx_pool)
> {
> + struct ibmvnic_long_term_buff *ltb;
> int rc, i;
>
> - rc = reset_long_term_buff(adapter, &tx_pool->long_term_buff);
> + ltb = &tx_pool->long_term_buff;
> +
> + rc = alloc_long_term_buff(adapter, ltb, ltb->size);
> if (rc)
> return rc;
Powered by blists - more mailing lists