[v4,04/12] raw/ioat: add explicit padding to descriptor struct

  Add an explicit padding field to the end of the descriptor structure so
that when the batch descriptor is defined on the stack for perform-ops, the
unused space is all zeroed appropriately.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/raw/ioat/rte_ioat_rawdev_fns.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
  

30/04/2021 17:06, Bruce Richardson:
> Add an explicit padding field to the end of the descriptor structure so
> that when the batch descriptor is defined on the stack for perform-ops, the
> unused space is all zeroed appropriately.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> --- a/drivers/raw/ioat/rte_ioat_rawdev_fns.h
> +++ b/drivers/raw/ioat/rte_ioat_rawdev_fns.h
> -	/* 28 bytes of padding here */
> +	uint16_t intr_handle; /* completion interrupt handle */

This is more than padding.
Does it deserve its own commit?

> +
> +	/* remaining 26 bytes are reserved */
> +	uint16_t __reserved[13];
>  } __rte_aligned(64);
  

On Mon, May 03, 2021 at 11:20:40PM +0200, Thomas Monjalon wrote:
> 30/04/2021 17:06, Bruce Richardson:
> > Add an explicit padding field to the end of the descriptor structure so
> > that when the batch descriptor is defined on the stack for perform-ops, the
> > unused space is all zeroed appropriately.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> > --- a/drivers/raw/ioat/rte_ioat_rawdev_fns.h
> > +++ b/drivers/raw/ioat/rte_ioat_rawdev_fns.h
> > -	/* 28 bytes of padding here */
> > +	uint16_t intr_handle; /* completion interrupt handle */
> 
> This is more than padding.
> Does it deserve its own commit?
> 

This field is unused by the driver, since we don't use any interrupts, so I
consider it as padding in that regard. However, I agree that in reality
it's not padding, but I think rather than having it's own commit, I can
just reword the patch commit log to cover it. Something like:

raw/ioat: expand descriptor struct to full 64 bytes

Although it's unused by the driver, add the interrupt handle field in the
descriptor to the descriptor structure for completeness, and explicitly add
the reserved padding field on the end of the structure too. This means that
when a descriptor is defined on the stack, or initialized by the compiler,
the unused/reserved space will be zeroed appropriately.

> > +
> > +	/* remaining 26 bytes are reserved */
> > +	uint16_t __reserved[13];
> >  } __rte_aligned(64);
> 
> 
>
  

04/05/2021 14:04, Bruce Richardson:
> On Mon, May 03, 2021 at 11:20:40PM +0200, Thomas Monjalon wrote:
> > 30/04/2021 17:06, Bruce Richardson:
> > > Add an explicit padding field to the end of the descriptor structure so
> > > that when the batch descriptor is defined on the stack for perform-ops, the
> > > unused space is all zeroed appropriately.
> > > 
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > ---
> > > --- a/drivers/raw/ioat/rte_ioat_rawdev_fns.h
> > > +++ b/drivers/raw/ioat/rte_ioat_rawdev_fns.h
> > > -	/* 28 bytes of padding here */
> > > +	uint16_t intr_handle; /* completion interrupt handle */
> > 
> > This is more than padding.
> > Does it deserve its own commit?
> > 
> 
> This field is unused by the driver, since we don't use any interrupts, so I
> consider it as padding in that regard. However, I agree that in reality
> it's not padding, but I think rather than having it's own commit, I can
> just reword the patch commit log to cover it. Something like:
> 
> raw/ioat: expand descriptor struct to full 64 bytes
> 
> Although it's unused by the driver, add the interrupt handle field in the
> descriptor to the descriptor structure for completeness, and explicitly add
> the reserved padding field on the end of the structure too. This means that
> when a descriptor is defined on the stack, or initialized by the compiler,
> the unused/reserved space will be zeroed appropriately.

OK, thanks for adding full explanation in the git history.