Re: [PATCH] console: generalize the ability for domU access

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

On 8/4/23 03:49, Jan Beulich wrote:
> On 03.08.2023 18:31, Daniel P. Smith wrote:
> > On 8/3/23 11:56, Jan Beulich wrote:
> > > On 03.08.2023 14:56, Daniel P. Smith wrote:
> > > > On 8/2/23 07:01, Jan Beulich wrote:
> > > > > On 01.08.2023 18:06, Daniel P. Smith wrote:
> > > > > > +        {
> > > > > > +            for_each_domain(next)
> > > > > What guarantees that the list won't change behind your back? You don't
> > > > > hold domlist_read_lock here afaict. It might be that you're safe because
> > > > > that lock is an RCU one and this function is only invoked at init time
> > > > > or from some form of interrupt handler. But that's far from obvious and
> > > > > will hence need both properly confirming and stating in a comment. (It
> > > > > is actually this concern, iirc, which so far had us avoid iterating the
> > > > > domain list here.)
> > > > It is better to error on the side of caution instead of assuming this
> > > > will always be invoked in a safe manner. I will add a read lock for the
> > > > domain list.
> > > I'm not firm enough in RCU to be certain whether acquiring that lock is
> > > permissible here.
> > Same and I took your statements to suggest that I should.
> Actually I wasn't paying close enough attention here: The code already
> uses rcu_lock_domain_by_id(), which acquires domlist_read_lock.
Right, it grabs the lock while iterating through domain_hash[], I 
thought your concern was with regard to the iterating with 
for_each_domain and the embedded open coded version. Because of your 
inquiry, I have been thinking about it and I should be grabbing the lock 
as I iterate to be sure that I don't get deceived into believing the end 
of list was hit because a domain was being removed as I walked the list. 
And if it so happens that the context is always safe, then there should 
be no contention on grabbing the lock. Do you disagree?
v/r,
dps