Re: [PATCH] spec: Add NBD_OPT_EXTENDED_HEADERS

[Vladimir Sementsov-Ogievskiy]

07.12.2021 12:08, Vladimir Sementsov-Ogievskiy wrote:
> 07.12.2021 02:00, Eric Blake wrote:
> > On Mon, Dec 06, 2021 at 02:40:45PM +0300, Vladimir Sementsov-Ogievskiy wrote:

[..]

> > > > +S: 64 bits, padding (MUST be zero)
> > >
> > > Hmm. Extra 8 bytes to be power-of-2. Does 32 bytes really perform better than 
> > > 24 bytes?
> >
> > Consider:
> > struct header[100];
> >
> > if sizeof(header[0]) is a power of 2 <= the cache line size (and the
> > compiler prefers to start arrays aligned to the cache line) then we
> > are guaranteed that all array members each reside in a single cache
> > line.  But if it is not a power of 2, some of the array members
> > straddle two cache lines.
> >
> > Will there be code that wants to create an array of headers?  Perhaps
> > so, because that is a logical way (along with scatter/gather to
> > combine the header with variable-sized payloads) of tracking the
> > headers for multiple commands issued in parallel.
> >
> > Do I have actual performance numbers?  No. But there's plenty of
> > google hits for why sizing structs to a power of 2 is a good idea.

I have a thought:

If client stores headers in separate, nothing prevents make this padding in 
RAM. So you can define header struct with padding. But what a reason to make 
the padding in the stream? You can have and array of good-aligned structures, 
but fill only part of header structure reading from the socket. Note, that you 
can read only one header in one read() call anyway, as you have to analyze, 
does it have payload or not.

So, if we want to improve performance by padding the structures in RAM, it's 
not a reason for padding them in the wire, keeping in mind that we can't read 
more then one structure at once.


--
Best regards,
Vladimir