From: Wojciech Drewek <wojciech.drewek@intel.com>
To: Ivan Vecera <ivecera@redhat.com>, <netdev@vger.kernel.org>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>,
Tony Nguyen <anthony.l.nguyen@intel.com>,
"David S. Miller" <davem@davemloft.net>,
"Eric Dumazet" <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
<intel-wired-lan@lists.osuosl.org>,
<linux-kernel@vger.kernel.org>,
Jacob Keller <jacob.e.keller@intel.com>
Subject: Re: [PATCH iwl-next] i40e: Delete unused and useless i40e_pf fields
Date: Thu, 26 Oct 2023 11:40:23 +0200 [thread overview]
Message-ID: <b3e2e8ca-eec6-45fd-bc67-53fe0d8daffd@intel.com> (raw)
In-Reply-To: <20231025145937.1873040-1-ivecera@redhat.com>
On 25.10.2023 16:59, Ivan Vecera wrote:
> Removed fields:
> .fc_autoneg_status
> Since commit c56999f94876 ("i40e/i40evf: Add set_fc and init of
> FC settings") write-only and otherwise unused
> .eeprom_version
> Write-only and otherwise unused
> .atr_sample_rate
> Has only one possible value (I40E_DEFAULT_ATR_SAMPLE_RATE). Remove
> it and replace its occurrences by I40E_DEFAULT_ATR_SAMPLE_RATE
> .adminq_work_limit
> Has only one possible value (I40E_AQ_WORK_LIMIT). Remove it and
> replace its occurrences by I40E_AQ_WORK_LIMIT
> .tx_sluggish_count
> Unused, never written
> .pf_seid
> Used to store VSI downlink seid and it is referenced only once
> in the same codepath. There is no need to save it into i40e_pf.
> Remove it and use downlink_seid directly in the mentioned log
> message.
> .instance
> Write only. Remove it as well as ugly static local variable
> 'pfs_found' in i40e_probe.
> .int_policy
> .switch_kobj
> .ptp_pps_work
> .ptp_extts1_work
> .ptp_pps_start
> .pps_delay
> .ptp_pin
> .override_q_count
> All these unused at all
>
> Prior the patch:
> pahole -Ci40e_pf drivers/net/ethernet/intel/i40e/i40e.ko | tail -5
> /* size: 5368, cachelines: 84, members: 127 */
> /* sum members: 5297, holes: 20, sum holes: 71 */
> /* paddings: 6, sum paddings: 19 */
> /* last cacheline: 56 bytes */
> };
>
> After the patch:
> pahole -Ci40e_pf drivers/net/ethernet/intel/i40e/i40e.ko | tail -5
> /* size: 4976, cachelines: 78, members: 112 */
> /* sum members: 4905, holes: 17, sum holes: 71 */
> /* paddings: 6, sum paddings: 19 */
> /* last cacheline: 48 bytes */
> };
>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
Nice cleanup! Only one nit.
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
> drivers/net/ethernet/intel/i40e/i40e.h | 16 ----------------
> drivers/net/ethernet/intel/i40e/i40e_debugfs.c | 3 ---
> drivers/net/ethernet/intel/i40e/i40e_main.c | 18 ++++--------------
> 3 files changed, 4 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
> index 5926bcc76141..f1627ab211cd 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
> @@ -468,9 +468,7 @@ struct i40e_pf {
> struct i40e_hw hw;
> DECLARE_BITMAP(state, __I40E_STATE_SIZE__);
> struct msix_entry *msix_entries;
> - bool fc_autoneg_status;
>
> - u16 eeprom_version;
> u16 num_vmdq_vsis; /* num vmdq vsis this PF has set up */
> u16 num_vmdq_qps; /* num queue pairs per vmdq pool */
> u16 num_vmdq_msix; /* num queue vectors per vmdq pool */
> @@ -486,7 +484,6 @@ struct i40e_pf {
> u16 rss_size_max; /* HW defined max RSS queues */
> u16 fdir_pf_filter_count; /* num of guaranteed filters for this PF */
> u16 num_alloc_vsi; /* num VSIs this driver supports */
> - u8 atr_sample_rate;
> bool wol_en;
>
> struct hlist_head fdir_filter_list;
> @@ -524,12 +521,10 @@ struct i40e_pf {
> struct hlist_head cloud_filter_list;
> u16 num_cloud_filters;
>
> - enum i40e_interrupt_policy int_policy;
> u16 rx_itr_default;
> u16 tx_itr_default;
> u32 msg_enable;
> char int_name[I40E_INT_NAME_STR_LEN];
> - u16 adminq_work_limit; /* num of admin receive queue desc to process */
> unsigned long service_timer_period;
> unsigned long service_timer_previous;
> struct timer_list service_timer;
> @@ -543,7 +538,6 @@ struct i40e_pf {
> u32 tx_timeout_count;
> u32 tx_timeout_recovery_level;
> unsigned long tx_timeout_last_recovery;
> - u32 tx_sluggish_count;
> u32 hw_csum_rx_error;
> u32 led_status;
> u16 corer_count; /* Core reset count */
> @@ -565,17 +559,13 @@ struct i40e_pf {
> struct i40e_lump_tracking *irq_pile;
>
> /* switch config info */
> - u16 pf_seid;
> u16 main_vsi_seid;
> u16 mac_seid;
> - struct kobject *switch_kobj;
> #ifdef CONFIG_DEBUG_FS
> struct dentry *i40e_dbg_pf;
> #endif /* CONFIG_DEBUG_FS */
> bool cur_promisc;
>
> - u16 instance; /* A unique number per i40e_pf instance in the system */
> -
> /* sr-iov config info */
> struct i40e_vf *vf;
> int num_alloc_vfs; /* actual number of VFs allocated */
> @@ -669,9 +659,7 @@ struct i40e_pf {
> unsigned long ptp_tx_start;
> struct hwtstamp_config tstamp_config;
> struct timespec64 ptp_prev_hw_time;
> - struct work_struct ptp_pps_work;
> struct work_struct ptp_extts0_work;
> - struct work_struct ptp_extts1_work;
> ktime_t ptp_reset_start;
> struct mutex tmreg_lock; /* Used to protect the SYSTIME registers. */
> u32 ptp_adj_mult;
> @@ -679,10 +667,7 @@ struct i40e_pf {
> u32 tx_hwtstamp_skipped;
> u32 rx_hwtstamp_cleared;
> u32 latch_event_flags;
> - u64 ptp_pps_start;
> - u32 pps_delay;
> spinlock_t ptp_rx_lock; /* Used to protect Rx timestamp registers. */
> - struct ptp_pin_desc ptp_pin[3];
> unsigned long latch_events[4];
> bool ptp_tx;
> bool ptp_rx;
> @@ -695,7 +680,6 @@ struct i40e_pf {
> u32 fd_inv;
> u16 phy_led_val;
>
> - u16 override_q_count;
> u16 last_sw_conf_flags;
> u16 last_sw_conf_valid_flags;
> /* List to keep previous DDP profiles to be rolled back in the future */
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
> index e0849f0c9c27..6f289237a8b0 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
> @@ -1029,9 +1029,6 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
> "emp reset count: %d\n", pf->empr_count);
> dev_info(&pf->pdev->dev,
> "pf reset count: %d\n", pf->pfr_count);
> - dev_info(&pf->pdev->dev,
> - "pf tx sluggish count: %d\n",
> - pf->tx_sluggish_count);
> } else if (strncmp(&cmd_buf[5], "port", 4) == 0) {
> struct i40e_aqc_query_port_ets_config_resp *bw_data;
> struct i40e_dcbx_config *cfg =
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 7169ceb6b757..69dafdecb243 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -3465,7 +3465,7 @@ static int i40e_configure_tx_ring(struct i40e_ring *ring)
>
> /* some ATR related tx ring init */
> if (test_bit(I40E_FLAG_FD_ATR_ENA, vsi->back->flags)) {
> - ring->atr_sample_rate = vsi->back->atr_sample_rate;
> + ring->atr_sample_rate = I40E_DEFAULT_ATR_SAMPLE_RATE;
> ring->atr_count = 0;
> } else {
> ring->atr_sample_rate = 0;
> @@ -10242,9 +10242,9 @@ static void i40e_clean_adminq_subtask(struct i40e_pf *pf)
> opcode);
> break;
> }
> - } while (i++ < pf->adminq_work_limit);
> + } while (i++ < I40E_AQ_WORK_LIMIT);
>
> - if (i < pf->adminq_work_limit)
> + if (i < I40E_AQ_WORK_LIMIT)
> clear_bit(__I40E_ADMINQ_EVENT_PENDING, pf->state);
>
> /* re-enable Admin queue interrupt cause */
> @@ -12785,7 +12785,6 @@ static int i40e_sw_init(struct i40e_pf *pf)
> if ((pf->hw.func_caps.fd_filters_guaranteed > 0) ||
> (pf->hw.func_caps.fd_filters_best_effort > 0)) {
> set_bit(I40E_FLAG_FD_ATR_ENA, pf->flags);
> - pf->atr_sample_rate = I40E_DEFAULT_ATR_SAMPLE_RATE;
> if (test_bit(I40E_FLAG_MFP_ENA, pf->flags) &&
> pf->hw.num_partitions > 1)
> dev_info(&pf->pdev->dev,
> @@ -12831,7 +12830,6 @@ static int i40e_sw_init(struct i40e_pf *pf)
> I40E_MAX_VF_COUNT);
> }
> #endif /* CONFIG_PCI_IOV */
> - pf->eeprom_version = 0xDEAD;
> pf->lan_veb = I40E_NO_VEB;
> pf->lan_vsi = I40E_NO_VSI;
>
> @@ -14992,12 +14990,11 @@ static void i40e_setup_pf_switch_element(struct i40e_pf *pf,
> * the PF's VSI
> */
> pf->mac_seid = uplink_seid;
> - pf->pf_seid = downlink_seid;
> pf->main_vsi_seid = seid;
> if (printconfig)
> dev_info(&pf->pdev->dev,
> "pf_seid=%d main_vsi_seid=%d\n",
Change the log msg to downlink_seid as well?
> - pf->pf_seid, pf->main_vsi_seid);
> + downlink_seid, pf->main_vsi_seid);
> break;
> case I40E_SWITCH_ELEMENT_TYPE_PF:
> case I40E_SWITCH_ELEMENT_TYPE_VF:
> @@ -15176,10 +15173,6 @@ static int i40e_setup_pf_switch(struct i40e_pf *pf, bool reinit, bool lock_acqui
> /* fill in link information and enable LSE reporting */
> i40e_link_event(pf);
>
> - /* Initialize user-specific link properties */
> - pf->fc_autoneg_status = ((pf->hw.phy.link_info.an_info &
> - I40E_AQ_AN_COMPLETED) ? true : false);
> -
> i40e_ptp_init(pf);
>
> if (!lock_acquired)
> @@ -15654,7 +15647,6 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> #endif /* CONFIG_I40E_DCB */
> struct i40e_pf *pf;
> struct i40e_hw *hw;
> - static u16 pfs_found;
> u16 wol_nvm_bits;
> char nvm_ver[32];
> u16 link_status;
> @@ -15732,7 +15724,6 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> hw->bus.device = PCI_SLOT(pdev->devfn);
> hw->bus.func = PCI_FUNC(pdev->devfn);
> hw->bus.bus_id = pdev->bus->number;
> - pf->instance = pfs_found;
>
> /* Select something other than the 802.1ad ethertype for the
> * switch to use internally and drop on ingress.
> @@ -15794,7 +15785,6 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> }
> hw->aq.arq_buf_size = I40E_MAX_AQ_BUF_SIZE;
> hw->aq.asq_buf_size = I40E_MAX_AQ_BUF_SIZE;
> - pf->adminq_work_limit = I40E_AQ_WORK_LIMIT;
>
> snprintf(pf->int_name, sizeof(pf->int_name) - 1,
> "%s-%s:misc",
next prev parent reply other threads:[~2023-10-26 9:40 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-25 14:59 [PATCH iwl-next] i40e: Delete unused and useless i40e_pf fields Ivan Vecera
2023-10-26 9:33 ` Przemek Kitszel
2023-10-26 9:40 ` Wojciech Drewek [this message]
2023-10-30 7:04 ` [Intel-wired-lan] " Pucha, HimasekharX Reddy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b3e2e8ca-eec6-45fd-bc67-53fe0d8daffd@intel.com \
--to=wojciech.drewek@intel.com \
--cc=anthony.l.nguyen@intel.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=ivecera@redhat.com \
--cc=jacob.e.keller@intel.com \
--cc=jesse.brandeburg@intel.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).