[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 5/5] migration: Move the yank unregister of channel_close
Dr. David Alan Gilbert
Re: [PATCH v2 5/5] migration: Move the yank unregister of channel_close out
Thu, 22 Jul 2021 18:09:03 +0100
* Peter Xu (email@example.com) wrote:
> On Thu, Jul 22, 2021 at 04:27:35PM +0100, Dr. David Alan Gilbert wrote:
> > > @@ -3352,6 +3355,8 @@ static MigThrError postcopy_pause(MigrationState *s)
> > >
> > > /* Current channel is possibly broken. Release it. */
> > > assert(s->to_dst_file);
> > > + /* Unregister yank for current channel */
> > > + migration_ioc_unregister_yank_from_file(s->to_dst_file);
> > Should this go inside the lock?
> Shouldn't need to; as we've got the assert() right above so otherwise we'll
> abrt otherwise :)
Hmm OK; although with out always having to think about it then you worry
about something getting in after the assert.
> The mutex lock/unlock right below this one is not protecting us from someone
> changing it but really for being able to wait until someone finished using it
> then we won't crash someone else.
> I think the rational is to_dst_file is managed by migration thread while
> from_dst_file is managed by rp_thread.
> Maybe I add a comment above?
OK, I almost pushed this further, but then I started to get worried that
we'd trace off a race with ordering on two locks with yank_lock; so yeh
lets just add a comment.
> Peter Xu
Dr. David Alan Gilbert / firstname.lastname@example.org / Manchester, UK
- [PATCH v2 2/5] migration: Make from_dst_file accesses thread-safe, (continued)