| Message ID | 20240930120402.3468-4-michal.swiatkowski@linux.intel.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Anthony Nguyen |
| Headers | show |
| Series | ice: managing MSI-X in driver | expand |
+ David Laight On Mon, Sep 30, 2024 at 02:03:57PM +0200, Michal Swiatkowski wrote: > Remove the field to allow having more queues than MSI-X on VSI. As > default the number will be the same, but if there won't be more MSI-X > available VSI can run with at least one MSI-X. > > Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com> > Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> > --- > drivers/net/ethernet/intel/ice/ice.h | 1 - > drivers/net/ethernet/intel/ice/ice_base.c | 10 +++----- > drivers/net/ethernet/intel/ice/ice_ethtool.c | 8 +++--- > drivers/net/ethernet/intel/ice/ice_irq.c | 11 +++------ > drivers/net/ethernet/intel/ice/ice_lib.c | 26 +++++++++++--------- > 5 files changed, 27 insertions(+), 29 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h > index cf824d041d5a..1e23aec2634f 100644 > --- a/drivers/net/ethernet/intel/ice/ice.h > +++ b/drivers/net/ethernet/intel/ice/ice.h > @@ -622,7 +622,6 @@ struct ice_pf { > u16 max_pf_txqs; /* Total Tx queues PF wide */ > u16 max_pf_rxqs; /* Total Rx queues PF wide */ > struct ice_pf_msix msix; > - u16 num_lan_msix; /* Total MSIX vectors for base driver */ > u16 num_lan_tx; /* num LAN Tx queues setup */ > u16 num_lan_rx; /* num LAN Rx queues setup */ > u16 next_vsi; /* Next free slot in pf->vsi[] - 0-based! */ ... > diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c > index 85a3b2326e7b..e5c56ec8bbda 100644 > --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c > +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c > @@ -3811,8 +3811,8 @@ ice_get_ts_info(struct net_device *dev, struct kernel_ethtool_ts_info *info) > */ > static int ice_get_max_txq(struct ice_pf *pf) > { > - return min3(pf->num_lan_msix, (u16)num_online_cpus(), > - (u16)pf->hw.func_caps.common_cap.num_txq); > + return min_t(u16, num_online_cpus(), > + pf->hw.func_caps.common_cap.num_txq); It is unclear why min_t() is used here or elsewhere in this patch instead of min() as it seems that all the entities being compared are unsigned. Are you concerned about overflowing u16? If so, perhaps clamp, or some error handling, is a better approach. I am concerned that the casting that min_t() brings will hide any problems that may exist. > } > > /** > @@ -3821,8 +3821,8 @@ static int ice_get_max_txq(struct ice_pf *pf) > */ > static int ice_get_max_rxq(struct ice_pf *pf) > { > - return min3(pf->num_lan_msix, (u16)num_online_cpus(), > - (u16)pf->hw.func_caps.common_cap.num_rxq); > + return min_t(u16, num_online_cpus(), > + pf->hw.func_caps.common_cap.num_rxq); > } > > /** ... > diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c > index d4e74f96a8ad..26cfb4b67972 100644 > --- a/drivers/net/ethernet/intel/ice/ice_lib.c > +++ b/drivers/net/ethernet/intel/ice/ice_lib.c > @@ -157,6 +157,16 @@ static void ice_vsi_set_num_desc(struct ice_vsi *vsi) > } > } > > +static u16 ice_get_rxq_count(struct ice_pf *pf) > +{ > + return min_t(u16, ice_get_avail_rxq_count(pf), num_online_cpus()); > +} > + > +static u16 ice_get_txq_count(struct ice_pf *pf) > +{ > + return min_t(u16, ice_get_avail_txq_count(pf), num_online_cpus()); > +} > + > /** > * ice_vsi_set_num_qs - Set number of queues, descriptors and vectors for a VSI > * @vsi: the VSI being configured > @@ -178,9 +188,7 @@ static void ice_vsi_set_num_qs(struct ice_vsi *vsi) > vsi->alloc_txq = vsi->req_txq; > vsi->num_txq = vsi->req_txq; > } else { > - vsi->alloc_txq = min3(pf->num_lan_msix, > - ice_get_avail_txq_count(pf), > - (u16)num_online_cpus()); > + vsi->alloc_txq = ice_get_txq_count(pf); > } > > pf->num_lan_tx = vsi->alloc_txq; > @@ -193,17 +201,14 @@ static void ice_vsi_set_num_qs(struct ice_vsi *vsi) > vsi->alloc_rxq = vsi->req_rxq; > vsi->num_rxq = vsi->req_rxq; > } else { > - vsi->alloc_rxq = min3(pf->num_lan_msix, > - ice_get_avail_rxq_count(pf), > - (u16)num_online_cpus()); > + vsi->alloc_rxq = ice_get_rxq_count(pf); > } > } > > pf->num_lan_rx = vsi->alloc_rxq; > > - vsi->num_q_vectors = min_t(int, pf->num_lan_msix, > - max_t(int, vsi->alloc_rxq, > - vsi->alloc_txq)); > + vsi->num_q_vectors = max_t(int, vsi->alloc_rxq, > + vsi->alloc_txq); > break; > case ICE_VSI_SF: > vsi->alloc_txq = 1; > @@ -1173,12 +1178,11 @@ static void ice_set_rss_vsi_ctx(struct ice_vsi_ctx *ctxt, struct ice_vsi *vsi) > static void > ice_chnl_vsi_setup_q_map(struct ice_vsi *vsi, struct ice_vsi_ctx *ctxt) > { > - struct ice_pf *pf = vsi->back; > u16 qcount, qmap; > u8 offset = 0; > int pow; > > - qcount = min_t(int, vsi->num_rxq, pf->num_lan_msix); > + qcount = vsi->num_rxq; > > pow = order_base_2(qcount); > qmap = FIELD_PREP(ICE_AQ_VSI_TC_Q_OFFSET_M, offset); > -- > 2.42.0 > >
On 10/12/2024 8:13 AM, Simon Horman wrote: > + David Laight > > On Mon, Sep 30, 2024 at 02:03:57PM +0200, Michal Swiatkowski wrote: >> Remove the field to allow having more queues than MSI-X on VSI. As >> default the number will be the same, but if there won't be more MSI-X >> available VSI can run with at least one MSI-X. >> >> Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com> >> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> >> --- >> drivers/net/ethernet/intel/ice/ice.h | 1 - >> drivers/net/ethernet/intel/ice/ice_base.c | 10 +++----- >> drivers/net/ethernet/intel/ice/ice_ethtool.c | 8 +++--- >> drivers/net/ethernet/intel/ice/ice_irq.c | 11 +++------ >> drivers/net/ethernet/intel/ice/ice_lib.c | 26 +++++++++++--------- >> 5 files changed, 27 insertions(+), 29 deletions(-) >> >> diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h >> index cf824d041d5a..1e23aec2634f 100644 >> --- a/drivers/net/ethernet/intel/ice/ice.h >> +++ b/drivers/net/ethernet/intel/ice/ice.h >> @@ -622,7 +622,6 @@ struct ice_pf { >> u16 max_pf_txqs; /* Total Tx queues PF wide */ >> u16 max_pf_rxqs; /* Total Rx queues PF wide */ >> struct ice_pf_msix msix; >> - u16 num_lan_msix; /* Total MSIX vectors for base driver */ >> u16 num_lan_tx; /* num LAN Tx queues setup */ >> u16 num_lan_rx; /* num LAN Rx queues setup */ >> u16 next_vsi; /* Next free slot in pf->vsi[] - 0-based! */ > > ... > >> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c >> index 85a3b2326e7b..e5c56ec8bbda 100644 >> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c >> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c >> @@ -3811,8 +3811,8 @@ ice_get_ts_info(struct net_device *dev, struct kernel_ethtool_ts_info *info) >> */ >> static int ice_get_max_txq(struct ice_pf *pf) >> { >> - return min3(pf->num_lan_msix, (u16)num_online_cpus(), >> - (u16)pf->hw.func_caps.common_cap.num_txq); >> + return min_t(u16, num_online_cpus(), >> + pf->hw.func_caps.common_cap.num_txq); > > It is unclear why min_t() is used here or elsewhere in this patch > instead of min() as it seems that all the entities being compared > are unsigned. Are you concerned about overflowing u16? If so, perhaps > clamp, or some error handling, is a better approach. > > I am concerned that the casting that min_t() brings will hide > any problems that may exist. > Ya, I think min makes more sense. min_t was likely selected out of habit or looking at other examples in the driver.
From: Jacob Keller > Sent: 14 October 2024 19:51 > > On 10/12/2024 8:13 AM, Simon Horman wrote: > > + David Laight > > > > On Mon, Sep 30, 2024 at 02:03:57PM +0200, Michal Swiatkowski wrote: > >> Remove the field to allow having more queues than MSI-X on VSI. As > >> default the number will be the same, but if there won't be more MSI-X > >> available VSI can run with at least one MSI-X. > >> > >> Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com> > >> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> > >> --- > >> drivers/net/ethernet/intel/ice/ice.h | 1 - > >> drivers/net/ethernet/intel/ice/ice_base.c | 10 +++----- > >> drivers/net/ethernet/intel/ice/ice_ethtool.c | 8 +++--- > >> drivers/net/ethernet/intel/ice/ice_irq.c | 11 +++------ > >> drivers/net/ethernet/intel/ice/ice_lib.c | 26 +++++++++++--------- > >> 5 files changed, 27 insertions(+), 29 deletions(-) > >> > >> diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h > >> index cf824d041d5a..1e23aec2634f 100644 > >> --- a/drivers/net/ethernet/intel/ice/ice.h > >> +++ b/drivers/net/ethernet/intel/ice/ice.h > >> @@ -622,7 +622,6 @@ struct ice_pf { > >> u16 max_pf_txqs; /* Total Tx queues PF wide */ > >> u16 max_pf_rxqs; /* Total Rx queues PF wide */ > >> struct ice_pf_msix msix; > >> - u16 num_lan_msix; /* Total MSIX vectors for base driver */ > >> u16 num_lan_tx; /* num LAN Tx queues setup */ > >> u16 num_lan_rx; /* num LAN Rx queues setup */ > >> u16 next_vsi; /* Next free slot in pf->vsi[] - 0-based! */ > > > > ... > > > >> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c > b/drivers/net/ethernet/intel/ice/ice_ethtool.c > >> index 85a3b2326e7b..e5c56ec8bbda 100644 > >> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c > >> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c > >> @@ -3811,8 +3811,8 @@ ice_get_ts_info(struct net_device *dev, struct kernel_ethtool_ts_info *info) > >> */ > >> static int ice_get_max_txq(struct ice_pf *pf) > >> { > >> - return min3(pf->num_lan_msix, (u16)num_online_cpus(), > >> - (u16)pf->hw.func_caps.common_cap.num_txq); > >> + return min_t(u16, num_online_cpus(), > >> + pf->hw.func_caps.common_cap.num_txq); > > > > It is unclear why min_t() is used here or elsewhere in this patch > > instead of min() as it seems that all the entities being compared > > are unsigned. Are you concerned about overflowing u16? If so, perhaps > > clamp, or some error handling, is a better approach. > > > > I am concerned that the casting that min_t() brings will hide > > any problems that may exist. > > > Ya, I think min makes more sense. min_t was likely selected out of habit > or looking at other examples in the driver. My 'spot patches that use min_t()' failed to spot that one. But it is just plain wrong - and always was. You want a result that is 16bits, casting the inputs is wrong. Consider a system with 64k cpus! Pretty much all the min_t() that specify u8 or u16 are likely to be actually broken. Most of the rest specify u32 or u64 in order to compare (usually) unsigned values of different sizes. But I found some that might be using 'long' on 64bit values on 32bit (and as disk sector numbers!). In the current min() bleats, the code is almost certainly awry. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On 10/14/2024 12:04 PM, David Laight wrote: > From: Jacob Keller >> Sent: 14 October 2024 19:51 >> >> On 10/12/2024 8:13 AM, Simon Horman wrote: >>> + David Laight >>> >>> On Mon, Sep 30, 2024 at 02:03:57PM +0200, Michal Swiatkowski wrote: >>>> Remove the field to allow having more queues than MSI-X on VSI. As >>>> default the number will be the same, but if there won't be more MSI-X >>>> available VSI can run with at least one MSI-X. >>>> >>>> Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com> >>>> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> >>>> --- >>>> drivers/net/ethernet/intel/ice/ice.h | 1 - >>>> drivers/net/ethernet/intel/ice/ice_base.c | 10 +++----- >>>> drivers/net/ethernet/intel/ice/ice_ethtool.c | 8 +++--- >>>> drivers/net/ethernet/intel/ice/ice_irq.c | 11 +++------ >>>> drivers/net/ethernet/intel/ice/ice_lib.c | 26 +++++++++++--------- >>>> 5 files changed, 27 insertions(+), 29 deletions(-) >>>> >>>> diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h >>>> index cf824d041d5a..1e23aec2634f 100644 >>>> --- a/drivers/net/ethernet/intel/ice/ice.h >>>> +++ b/drivers/net/ethernet/intel/ice/ice.h >>>> @@ -622,7 +622,6 @@ struct ice_pf { >>>> u16 max_pf_txqs; /* Total Tx queues PF wide */ >>>> u16 max_pf_rxqs; /* Total Rx queues PF wide */ >>>> struct ice_pf_msix msix; >>>> - u16 num_lan_msix; /* Total MSIX vectors for base driver */ >>>> u16 num_lan_tx; /* num LAN Tx queues setup */ >>>> u16 num_lan_rx; /* num LAN Rx queues setup */ >>>> u16 next_vsi; /* Next free slot in pf->vsi[] - 0-based! */ >>> >>> ... >>> >>>> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c >> b/drivers/net/ethernet/intel/ice/ice_ethtool.c >>>> index 85a3b2326e7b..e5c56ec8bbda 100644 >>>> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c >>>> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c >>>> @@ -3811,8 +3811,8 @@ ice_get_ts_info(struct net_device *dev, struct kernel_ethtool_ts_info *info) >>>> */ >>>> static int ice_get_max_txq(struct ice_pf *pf) >>>> { >>>> - return min3(pf->num_lan_msix, (u16)num_online_cpus(), >>>> - (u16)pf->hw.func_caps.common_cap.num_txq); >>>> + return min_t(u16, num_online_cpus(), >>>> + pf->hw.func_caps.common_cap.num_txq); >>> >>> It is unclear why min_t() is used here or elsewhere in this patch >>> instead of min() as it seems that all the entities being compared >>> are unsigned. Are you concerned about overflowing u16? If so, perhaps >>> clamp, or some error handling, is a better approach. >>> >>> I am concerned that the casting that min_t() brings will hide >>> any problems that may exist. >>> >> Ya, I think min makes more sense. min_t was likely selected out of habit >> or looking at other examples in the driver. > > My 'spot patches that use min_t()' failed to spot that one. > > But it is just plain wrong - and always was. > You want a result that is 16bits, casting the inputs is wrong. > Consider a system with 64k cpus! > Yea, that makes sense. This is definitely not going to behave well in the event that one of the values is above 16-bit. > Pretty much all the min_t() that specify u8 or u16 are likely to > be actually broken. > Most of the rest specify u32 or u64 in order to compare (usually) > unsigned values of different sizes. > But I found some that might be using 'long' on 64bit values > on 32bit (and as disk sector numbers!). > > In the current min() bleats, the code is almost certainly awry. > > David
diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h index cf824d041d5a..1e23aec2634f 100644 --- a/drivers/net/ethernet/intel/ice/ice.h +++ b/drivers/net/ethernet/intel/ice/ice.h @@ -622,7 +622,6 @@ struct ice_pf { u16 max_pf_txqs; /* Total Tx queues PF wide */ u16 max_pf_rxqs; /* Total Rx queues PF wide */ struct ice_pf_msix msix; - u16 num_lan_msix; /* Total MSIX vectors for base driver */ u16 num_lan_tx; /* num LAN Tx queues setup */ u16 num_lan_rx; /* num LAN Rx queues setup */ u16 next_vsi; /* Next free slot in pf->vsi[] - 0-based! */ diff --git a/drivers/net/ethernet/intel/ice/ice_base.c b/drivers/net/ethernet/intel/ice/ice_base.c index 98c3764fed39..35a8721cb110 100644 --- a/drivers/net/ethernet/intel/ice/ice_base.c +++ b/drivers/net/ethernet/intel/ice/ice_base.c @@ -795,13 +795,11 @@ int ice_vsi_alloc_q_vectors(struct ice_vsi *vsi) return 0; err_out: - while (v_idx--) - ice_free_q_vector(vsi, v_idx); - dev_err(dev, "Failed to allocate %d q_vector for VSI %d, ret=%d\n", - vsi->num_q_vectors, vsi->vsi_num, err); - vsi->num_q_vectors = 0; - return err; + dev_info(dev, "Failed to allocate %d q_vectors for VSI %d, new value %d", + vsi->num_q_vectors, vsi->vsi_num, v_idx); + vsi->num_q_vectors = v_idx; + return v_idx ? 0 : err; } /** diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c index 85a3b2326e7b..e5c56ec8bbda 100644 --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c @@ -3811,8 +3811,8 @@ ice_get_ts_info(struct net_device *dev, struct kernel_ethtool_ts_info *info) */ static int ice_get_max_txq(struct ice_pf *pf) { - return min3(pf->num_lan_msix, (u16)num_online_cpus(), - (u16)pf->hw.func_caps.common_cap.num_txq); + return min_t(u16, num_online_cpus(), + pf->hw.func_caps.common_cap.num_txq); } /** @@ -3821,8 +3821,8 @@ static int ice_get_max_txq(struct ice_pf *pf) */ static int ice_get_max_rxq(struct ice_pf *pf) { - return min3(pf->num_lan_msix, (u16)num_online_cpus(), - (u16)pf->hw.func_caps.common_cap.num_rxq); + return min_t(u16, num_online_cpus(), + pf->hw.func_caps.common_cap.num_rxq); } /** diff --git a/drivers/net/ethernet/intel/ice/ice_irq.c b/drivers/net/ethernet/intel/ice/ice_irq.c index 3d89d5303cd1..f1d5e43251ad 100644 --- a/drivers/net/ethernet/intel/ice/ice_irq.c +++ b/drivers/net/ethernet/intel/ice/ice_irq.c @@ -101,7 +101,7 @@ void ice_clear_interrupt_scheme(struct ice_pf *pf) int ice_init_interrupt_scheme(struct ice_pf *pf) { int total_vectors = pf->hw.func_caps.common_cap.num_msix_vectors; - int vectors, max_vectors; + int vectors; /* load default PF MSI-X range */ if (!pf->msix.min) @@ -110,20 +110,17 @@ int ice_init_interrupt_scheme(struct ice_pf *pf) if (!pf->msix.max) pf->msix.max = total_vectors / 2; - if (pci_msix_can_alloc_dyn(pf->pdev)) { + if (pci_msix_can_alloc_dyn(pf->pdev)) vectors = pf->msix.min; - max_vectors = total_vectors; - } else { + else vectors = pf->msix.max; - max_vectors = vectors; - } vectors = pci_alloc_irq_vectors(pf->pdev, pf->msix.min, vectors, PCI_IRQ_MSIX); if (vectors < pf->msix.min) return -ENOMEM; - ice_init_irq_tracker(pf, max_vectors, vectors); + ice_init_irq_tracker(pf, pf->msix.max, vectors); return 0; } diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c index d4e74f96a8ad..26cfb4b67972 100644 --- a/drivers/net/ethernet/intel/ice/ice_lib.c +++ b/drivers/net/ethernet/intel/ice/ice_lib.c @@ -157,6 +157,16 @@ static void ice_vsi_set_num_desc(struct ice_vsi *vsi) } } +static u16 ice_get_rxq_count(struct ice_pf *pf) +{ + return min_t(u16, ice_get_avail_rxq_count(pf), num_online_cpus()); +} + +static u16 ice_get_txq_count(struct ice_pf *pf) +{ + return min_t(u16, ice_get_avail_txq_count(pf), num_online_cpus()); +} + /** * ice_vsi_set_num_qs - Set number of queues, descriptors and vectors for a VSI * @vsi: the VSI being configured @@ -178,9 +188,7 @@ static void ice_vsi_set_num_qs(struct ice_vsi *vsi) vsi->alloc_txq = vsi->req_txq; vsi->num_txq = vsi->req_txq; } else { - vsi->alloc_txq = min3(pf->num_lan_msix, - ice_get_avail_txq_count(pf), - (u16)num_online_cpus()); + vsi->alloc_txq = ice_get_txq_count(pf); } pf->num_lan_tx = vsi->alloc_txq; @@ -193,17 +201,14 @@ static void ice_vsi_set_num_qs(struct ice_vsi *vsi) vsi->alloc_rxq = vsi->req_rxq; vsi->num_rxq = vsi->req_rxq; } else { - vsi->alloc_rxq = min3(pf->num_lan_msix, - ice_get_avail_rxq_count(pf), - (u16)num_online_cpus()); + vsi->alloc_rxq = ice_get_rxq_count(pf); } } pf->num_lan_rx = vsi->alloc_rxq; - vsi->num_q_vectors = min_t(int, pf->num_lan_msix, - max_t(int, vsi->alloc_rxq, - vsi->alloc_txq)); + vsi->num_q_vectors = max_t(int, vsi->alloc_rxq, + vsi->alloc_txq); break; case ICE_VSI_SF: vsi->alloc_txq = 1; @@ -1173,12 +1178,11 @@ static void ice_set_rss_vsi_ctx(struct ice_vsi_ctx *ctxt, struct ice_vsi *vsi) static void ice_chnl_vsi_setup_q_map(struct ice_vsi *vsi, struct ice_vsi_ctx *ctxt) { - struct ice_pf *pf = vsi->back; u16 qcount, qmap; u8 offset = 0; int pow; - qcount = min_t(int, vsi->num_rxq, pf->num_lan_msix); + qcount = vsi->num_rxq; pow = order_base_2(qcount); qmap = FIELD_PREP(ICE_AQ_VSI_TC_Q_OFFSET_M, offset);