Re: [PATCH v3 07/15] argo: implement the register op

From: Christopher Clark <christopher.w.clark@gmail.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Tim Deegan <tim@xen.org>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Ross Philipson <ross.philipson@gmail.com>,
	Jason Andryuk <jandryuk@gmail.com>,
	Daniel Smith <dpsmith@apertussolutions.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>,
	Rich Persaud <persaur@gmail.com>,
	James McKenzie <james@bromium.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Julien Grall <julien.grall@arm.com>,
	Paul Durrant <paul.durrant@citrix.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	eric chanudet <eric.chanudet@gmail.com>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [PATCH v3 07/15] argo: implement the register op
Date: Tue, 15 Jan 2019 00:46:16 -0800	[thread overview]
Message-ID: <CACMJ4GYpOwSx2bQFsSpi10XKKDNTKdPeKLd5LRXmP88SpeZvFA@mail.gmail.com> (raw)
In-Reply-To: <5C3D9B97020000780020DA23@prv1-mh.provo.novell.com>

On Tue, Jan 15, 2019 at 12:36 AM Jan Beulich <JBeulich@suse.com> wrote:
>
> >>> On 15.01.19 at 08:56, <christopher.w.clark@gmail.com> wrote:
> > On Mon, Jan 14, 2019 at 6:19 AM Jan Beulich <JBeulich@suse.com> wrote:
> >> >>> On 07.01.19 at 08:42, <christopher.w.clark@gmail.com> wrote:
> >> > +    /*
> >> > +     * A ring must be large enough to transmit messages, so requires space for:
> >> > +     * * 1 message header, plus
> >> > +     * * 1 payload slot (payload is always rounded to a multiple of 16 bytes)
> >> > +     *   for the message payload to be written into, plus
> >> > +     * * 1 more slot, so that the ring cannot be filled to capacity with a
> >> > +     *   single message -- see the logic in ringbuf_insert -- allowing for this
> >> > +     *   ensures that there can be space remaining when a message is present.
> >> > +     * The above determines the minimum acceptable ring size.
> >> > +     */
> >> > +    if ( (reg.len < (sizeof(struct xen_argo_ring_message_header)
> >> > +                      + ROUNDUP_MESSAGE(1) + ROUNDUP_MESSAGE(1))) ||
> >>
> >> These two summands don't look to fulfill the "cannot be filled to
> >> capacity" constraint the comment describes, as (aiui) messages
> >> can be larger than 16 bytes. What's the deal?
> >
> > This is intended to be about bound checking reg.len against a minimum
> > size: the smallest ring that you can fit a message onto, as determined
> > by the logic in ringbuf_insert. The smallest message you can send is:
> > sizeof(struct xen_argo_ring_message_header) + ROUNDUP_MESSAGE(1)
> >
> > and then ringbuf_insert won't accept a message unless there is (at
> > least) ROUNDUP_MESSAGE(1) space remaining, so that, plus the smallest
> > message size, is the smallest viable ring. There's no point accepting
> > registration of a ring smaller than that.
> >
> > You're right that messages can be larger than 16 bytes, but they can
> > only be sent to rings that are larger than that minimum - on a minimum
> > sized ring, they'll be rejected by sendv.
>
> So perhaps the comment wants to say "... cannot be filled to capacity
> with a single, minimum size message"?

ack, done.

Christopher

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel