Re: [Xen-devel] [PATCH v3 04/15] argo: init, destroy and soft-reset, with enable command line opt

[Christopher Clark <christopher.w.clark@xxxxxxxxx>]

On Wed, Jan 9, 2019 at 6:16 AM Jason Andryuk <jandryuk@xxxxxxxxx> wrote:
> On Wed, Jan 9, 2019 at 1:48 AM Christopher Clark 
> <christopher.w.clark@xxxxxxxxx> wrote:
> > On Tue, Jan 8, 2019 at 2:54 PM Jason Andryuk <jandryuk@xxxxxxxxx> wrote:
> > > On Mon, Jan 7, 2019 at 2:43 AM Christopher Clark 
> > > <christopher.w.clark@xxxxxxxxx> wrote:
>
> > > > +
> > > > +/* A space-available notification that is awaiting sufficient space */
> > > > +struct pending_ent
> > > > +{
> > > > +    /*
> > > > +     * Pointer to the ring_info that this ent pertains to. Used to 
> > > > ensure that
> > > > +     * ring_info->npending is decremented when ents for wildcard rings 
> > > > are
> > > > +     * cancelled for domain destroy.
> > > > +     * Caution: Must hold the correct locks before accessing ring_info 
> > > > via this.
> > >
> > > It would be clearer if this stated the correct locks.
> >
> > ok - it would mean duplicating the statement about which locks are
> > needed though, since it is explained elsewhere in the file, which means
> > it will need updating in two places if the locking requirements change.
> > That was why I worded it that way, as an indicator to go and find where
> > it is already described, to avoid that.
>
> "Caution" made me think *ring_info points from domain A's pending_ent
> to domain B's ring_info.  Reading patch 10 (notify op) I see that it
> really just points back to domain A's ring_info.  So the "Caution" is
> just that you still have to lock ring_info (L3) even though you can
> get to the pointer via L2.  Is that correct?

Yes, exactly.

> I agree a single location for the locking documentation is better than
> splitting or duplicating.  As long as no cross-domain locking is
> required, this is fine.

OK - thanks.

Christopher

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