Re: [PATCH] eal: fix alignment of RISCV xmm vector type

[Thomas Monjalon <thomas@monjalon.net>]

17/11/2023 12:18, Morten Brørup:
> +CC Thomas, this patch is ready to be applied to 23.11.
> 
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > Sent: Friday, 17 November 2023 11.55
> > 
> > On Thu, Nov 16, 2023 at 08:45:35AM +0100, Morten Brørup wrote:
> > > > From: Stanisław Kardach [mailto:kda@semihalf.com]
> > > > Sent: Thursday, 16 November 2023 00.21
> > > >
> > > > On Wed, Nov 15, 2023 at 10:31 PM Morten Brørup
> > > > <mb@smartsharesystems.com> wrote:
> > > > >
> > > > > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > > > > Sent: Wednesday, 15 November 2023 22.17
> > > > > >
> > > > > > Fix the alignment for rte_xmm_t it should be 16 instead of 8
> > bytes.
> > > > > >
> > > > > > Fixes: f22e705ebf12 ("eal/riscv: support RISC-V architecture")
> > > > > > Cc: maz@semihalf.com
> > > > > > Cc: stable@dpdk.org
> > > > > > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > > > > > ---
> > > > >
> > > > > Reviewed-by: Morten Brørup <mb@smartsharesystems.com>
> > > > >
> > > > > As mentioned in the other thread:
> > > > >
> > > > > We need to urgently decide if this bug should live on in DPDK
> > 23.11,
> > > > or if the fix should be included although we are very late in the
> > > > release process.
> > > > >
> > > > > Stanislaw, what do you think?
> > > > Good catch! As for backporting I'm not sure of the urgency given
> > that
> > > > our examples still use scalar instructions for handling xmm_t. The
> > > > question is whether there is a platform in use which has vector
> > > > extensions enabled and that utilizes DPDK. I'm not that sure of it
> > > > though I'd be happy to be proven wrong.
> > >
> > > Can we extrapolate this, and also conclude that postponing this
> > bugfix until the next ABI/API breaking release, DPDK 24.11, is not
> > really going to hurt anyone?
> > >
> > > Stanislaw, please confirm?
> > >
> > > Bruce, I don't feel 100 % confident in making this postponement
> > recommendation. Could you please provide a second opinion regarding the
> > timing of fixing this bug? Or rather: do you have any strong arguments
> > *against* postponing it to DPDK 24.11?
> > >
> > 
> > Not sure I'm any better placed to make an argument either way!
> 
> Bruce, I picked you because of your experience with vector code.
> 
> > However, I
> > would very much tend to say that we should include this in 23.11, on
> > the
> > basis that if it turns out to be important we can't backport it later
> > without affecting ABI. Right now, the code looks broken to me, and I'm
> > also
> > struggling to find circumstances where increasing the alignment will
> > actually stop something from working. There could well be performance
> > implications of having extra padding, but things should still work,
> > AFAIK.
> > On the other hand, if we don't include the fix, it is possible that a
> > system (possibly a future one) could break and segfault due to
> > incorrect
> > vector code. I'd take a possible slowdown over a segfault!
> 
> The risk of slowdown isn't a factor for me at this stage.
> 
> I'm trying to balance the risk of fixing the bug vs. breaking something this late in the 23.11 release cycle.
> 
> You have a strong point that we also need to consider the bugfix in the context of the total lifetime of DPDK 23.11 in the wild.
> With RISC-V's current traction, that certainly speaks in favor of including it in 23.11.
> 
> > 
> > Is my assessment correct, or perhaps I'm missing some detail.
> 
> Thank you for your valuable feedback, Bruce.
> 
> I was just being overly cautious... After all, 23.11 is still at a stage where bug fixes are accepted!
> 
> New conclusion: Let's get it into 23.11.

Applied, thanks.

I've kept CC:stable as it is a real fix.
I don't see a problem in breaking API/ABI for RISC-V which is quite new.
Anyway stable maintainers will choose what to do.