DPDK patches and discussions
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: "Morten Brørup" <mb@smartsharesystems.com>
Cc: "Stanisław Kardach" <kda@semihalf.com>,
	"Tyler Retzlaff" <roretzla@linux.microsoft.com>,
	dev@dpdk.org, "Michal Mazurek" <maz@semihalf.com>,
	stable@dpdk.org
Subject: Re: [PATCH] eal: fix alignment of RISCV xmm vector type
Date: Fri, 17 Nov 2023 10:54:33 +0000	[thread overview]
Message-ID: <ZVdGaQ78T9accA+G@bricha3-MOBL.ger.corp.intel.com> (raw)
In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35E9F033@smartserver.smartshare.dk>

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! 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!

Is my assessment correct, or perhaps I'm missing some detail.

/Bruce

  reply	other threads:[~2023-11-17 10:54 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-15 21:16 Tyler Retzlaff
2023-11-15 21:31 ` Morten Brørup
2023-11-15 23:20   ` Stanisław Kardach
2023-11-16  7:45     ` Morten Brørup
2023-11-17 10:54       ` Bruce Richardson [this message]
2023-11-17 11:18         ` Morten Brørup
2023-11-18  8:04           ` Thomas Monjalon

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=ZVdGaQ78T9accA+G@bricha3-MOBL.ger.corp.intel.com \
    --to=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=kda@semihalf.com \
    --cc=maz@semihalf.com \
    --cc=mb@smartsharesystems.com \
    --cc=roretzla@linux.microsoft.com \
    --cc=stable@dpdk.org \
    /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).