[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH RESEND v6 0/5] migration: reduce time of loading non-iterable
From: |
Peter Xu |
Subject: |
Re: [PATCH RESEND v6 0/5] migration: reduce time of loading non-iterable vmstate |
Date: |
Tue, 7 Mar 2023 12:04:36 -0500 |
On Tue, Mar 07, 2023 at 09:24:31PM +0800, Chuang Xu wrote:
> > Why do we need address_space_get_flatview_rcu()? I'm not sure whether you
>
> address_space_cahce_init() uses address_space_get_flatview() to acquire
> a ref-ed flatview. If we want to use address_space_to_flatview_rcu() and
> make the flatview ref-ed, maybe we need to add
> address_space_get_flatview_rcu()?
> Or if we use address_space_to_flatview_rcu() directly, then we'll get a
> unref-ed flatview, I'm not sure wheather it's safe in other cases.. At least
> I don't want the assertion to be triggered one day.
No we can't touch that, afaict. It was a real fix (447b0d0b9e) to a real
bug.
What I meant is we make address_space_get_flatview() always use
address_space_to_flatview_rcu().
I think it's safe, if you see the current callers of it (after my patch 1
fixed version applied):
memory_region_sync_dirty_bitmap[2249] view = address_space_get_flatview(as);
memory_region_update_coalesced_range[2457] view =
address_space_get_flatview(as);
memory_region_clear_dirty_bitmap[2285] view = address_space_get_flatview(as);
mtree_info_flatview[3418] view = address_space_get_flatview(as);
address_space_cache_init[3350] cache->fv = address_space_get_flatview(as);
Case 5 is what we're targeting for which I think it's fine. Logically any
commit() hook should just use address_space_get_flatview_raw() to reference
the flatview, but at least address_space_cache_init() is also called in
non-BQL sections, so _rcu is the only thing we can use here, iiuc..
Case 1-3 is never called during vm load, so I think it's safe.
Case 4 can be accessing stalled flatview but I assume fine since it's just
for debugging. E.g. if someone tries "info mtree -f" during vm loading
after your patch applied, then one can see stalled info. I don't think it
can even happen because so far "info mtree" should also take BQL.. so it'll
be blocked until vm load completes.
The whole thing is still tricky. I didn't yet make my mind through on how
to make it very clean, I think it's slightly beyond what this series does
and I need some more thinkings (or suggestions from others).
--
Peter Xu
- [PATCH RESEND v6 0/5] migration: reduce time of loading non-iterable vmstate, Chuang Xu, 2023/03/03
- [PATCH RESEND v6 2/5] rcu: Introduce rcu_read_is_locked(), Chuang Xu, 2023/03/03
- [PATCH RESEND v6 4/5] memory: Add sanity check in address_space_to_flatview, Chuang Xu, 2023/03/03
- [PATCH RESEND v6 1/5] memory: Reference as->current_map directly in memory commit, Chuang Xu, 2023/03/03
- [PATCH RESEND v6 3/5] memory: Introduce memory_region_transaction_do_commit(), Chuang Xu, 2023/03/03
- [PATCH RESEND v6 5/5] migration: Reduce time of loading non-iterable vmstate, Chuang Xu, 2023/03/03
- Re: [PATCH RESEND v6 0/5] migration: reduce time of loading non-iterable vmstate, Peter Xu, 2023/03/05
- Re: [PATCH RESEND v6 0/5] migration: reduce time of loading non-iterable vmstate, Chuang Xu, 2023/03/06
- Re: [PATCH RESEND v6 0/5] migration: reduce time of loading non-iterable vmstate, Peter Xu, 2023/03/06
- Re: [PATCH RESEND v6 0/5] migration: reduce time of loading non-iterable vmstate, Chuang Xu, 2023/03/07
- Re: [PATCH RESEND v6 0/5] migration: reduce time of loading non-iterable vmstate,
Peter Xu <=
- Re: [PATCH RESEND v6 0/5] migration: reduce time of loading non-iterable vmstate, Chuang Xu, 2023/03/08
- Re: [PATCH RESEND v6 0/5] migration: reduce time of loading non-iterable vmstate, Peter Xu, 2023/03/08
- Re: [PATCH RESEND v6 0/5] migration: reduce time of loading non-iterable vmstate, Chuang Xu, 2023/03/08
- Re: [PATCH RESEND v6 0/5] migration: reduce time of loading non-iterable vmstate, Peter Xu, 2023/03/08
- Re: [PATCH RESEND v6 0/5] migration: reduce time of loading non-iterable vmstate, Chuang Xu, 2023/03/09