RE: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-13 Thread Wang, Wei W
On Tuesday, July 13, 2021 11:59 PM, Peter Xu wrote: > On Tue, Jul 13, 2021 at 08:40:21AM +, Wang, Wei W wrote: > > Didn't get a chance to document it as it's in a pull now; but as long as > you're okay > with no-per-page lock (which I still don't agree with), I can follow this up. > > Are

RE: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-13 Thread Wang, Wei W
On Tuesday, July 13, 2021 6:22 PM, David Hildenbrand wrote: > Can you send an official patch for the free page hinting clean_bmap handling I > reported? > > I can then give both tests in combination a quick test (before/after this > patch > here). > Yes, I'll send, thanks! Best, Wei

Re: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-13 Thread Peter Xu
On Tue, Jul 13, 2021 at 08:40:21AM +, Wang, Wei W wrote: > On Thursday, July 1, 2021 4:08 AM, Peter Xu wrote: > > Taking the mutex every time for each dirty bit to clear is too slow, > > especially we'll > > take/release even if the dirty bit is cleared. So far it's only used to > > sync

Re: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-13 Thread David Hildenbrand
On 13.07.21 10:40, Wang, Wei W wrote: On Thursday, July 1, 2021 4:08 AM, Peter Xu wrote: Taking the mutex every time for each dirty bit to clear is too slow, especially we'll take/release even if the dirty bit is cleared. So far it's only used to sync with special cases with

RE: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-13 Thread Wang, Wei W
On Thursday, July 1, 2021 4:08 AM, Peter Xu wrote: > Taking the mutex every time for each dirty bit to clear is too slow, > especially we'll > take/release even if the dirty bit is cleared. So far it's only used to sync > with > special cases with qemu_guest_free_page_hint() against migration

RE: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-13 Thread Wang, Wei W
On Friday, July 9, 2021 10:48 PM, Peter Xu wrote: > On Fri, Jul 09, 2021 at 08:58:08AM +, Wang, Wei W wrote: > > On Friday, July 9, 2021 2:31 AM, Peter Xu wrote: > > > > > Yes I think this is the place I didn't make myself clear. It's > > > > > not about sleeping, it's about the cmpxchg being

Re: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-09 Thread Peter Xu
On Fri, Jul 09, 2021 at 08:58:08AM +, Wang, Wei W wrote: > On Friday, July 9, 2021 2:31 AM, Peter Xu wrote: > > > > Yes I think this is the place I didn't make myself clear. It's not > > > > about sleeping, it's about the cmpxchg being expensive already when the > > > > vm > > is huge. > > >

RE: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-09 Thread Wang, Wei W
On Friday, July 9, 2021 2:31 AM, Peter Xu wrote: > > > Yes I think this is the place I didn't make myself clear. It's not > > > about sleeping, it's about the cmpxchg being expensive already when the vm > is huge. > > > > OK. > > How did you root cause that it's caused by cmpxchg, instead of lock

Re: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-08 Thread Peter Xu
On Thu, Jul 08, 2021 at 02:55:23AM +, Wang, Wei W wrote: > On Thursday, July 8, 2021 12:55 AM, Peter Xu wrote: > > On Wed, Jul 07, 2021 at 08:34:50AM +, Wang, Wei W wrote: > > > On Wednesday, July 7, 2021 1:47 AM, Peter Xu wrote: > > > > On Sat, Jul 03, 2021 at 02:53:27AM +, Wang, Wei

Re: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-08 Thread Peter Xu
On Thu, Jul 08, 2021 at 02:49:51AM +, Wang, Wei W wrote: > On Thursday, July 8, 2021 12:44 AM, Peter Xu wrote: > > > > Not to mention the hard migration issues are mostly with non-idle > > > > guest, in that case having the balloon in the guest will be > > > > disastrous from this pov since

RE: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-07 Thread Wang, Wei W
On Thursday, July 8, 2021 12:55 AM, Peter Xu wrote: > On Wed, Jul 07, 2021 at 08:34:50AM +, Wang, Wei W wrote: > > On Wednesday, July 7, 2021 1:47 AM, Peter Xu wrote: > > > On Sat, Jul 03, 2021 at 02:53:27AM +, Wang, Wei W wrote: > > > > + do { > > > > +page_to_clear =

RE: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-07 Thread Wang, Wei W
On Thursday, July 8, 2021 12:44 AM, Peter Xu wrote: > > > Not to mention the hard migration issues are mostly with non-idle > > > guest, in that case having the balloon in the guest will be > > > disastrous from this pov since it'll start to take mutex for each > > > page, while balloon would

Re: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-07 Thread Peter Xu
On Wed, Jul 07, 2021 at 11:25:50PM +, Wang, Wei W wrote: > On Thursday, July 8, 2021 12:45 AM, Peter Xu wrote: > > On Wed, Jul 07, 2021 at 12:45:32PM +, Wang, Wei W wrote: > > > Btw, what would you think if we change mutex to QemuSpin? It will also > > > reduce > > the overhead, I think.

RE: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-07 Thread Wang, Wei W
On Thursday, July 8, 2021 12:45 AM, Peter Xu wrote: > On Wed, Jul 07, 2021 at 12:45:32PM +, Wang, Wei W wrote: > > Btw, what would you think if we change mutex to QemuSpin? It will also > > reduce > the overhead, I think. > > As I replied at the other place, the bottleneck we encountered is

Re: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-07 Thread Peter Xu
On Wed, Jul 07, 2021 at 08:34:50AM +, Wang, Wei W wrote: > On Wednesday, July 7, 2021 1:47 AM, Peter Xu wrote: > > On Sat, Jul 03, 2021 at 02:53:27AM +, Wang, Wei W wrote: > > > + do { > > > +page_to_clear = start + (i++ << block->clear_bmap_shift); > > > > Why "i" needs

Re: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-07 Thread Peter Xu
On Wed, Jul 07, 2021 at 12:45:32PM +, Wang, Wei W wrote: > Btw, what would you think if we change mutex to QemuSpin? It will also reduce > the overhead, I think. As I replied at the other place, the bottleneck we encountered is the lock taking not sleeping, so I'm afraid spinlock will have

Re: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-07 Thread Peter Xu
On Wed, Jul 07, 2021 at 08:33:21AM +, Wang, Wei W wrote: > On Wednesday, July 7, 2021 2:00 AM, Peter Xu wrote: > > On Fri, Jul 02, 2021 at 02:29:41AM +, Wang, Wei W wrote: > > > With that, if free page opt is off, the mutex is skipped, isn't it? > > > > Yes, but when free page is on,

RE: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-07 Thread Wang, Wei W
On Wednesday, July 7, 2021 1:40 AM, Peter Xu wrote: > On Tue, Jul 06, 2021 at 12:05:49PM +0200, David Hildenbrand wrote: > > On 06.07.21 11:41, Wang, Wei W wrote: > > > On Monday, July 5, 2021 9:42 PM, David Hildenbrand wrote: > > > > On 03.07.21 04:53, Wang, Wei W wrote: > > > > > On Friday, July

RE: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-07 Thread Wang, Wei W
On Wednesday, July 7, 2021 2:00 AM, Peter Xu wrote: > On Fri, Jul 02, 2021 at 02:29:41AM +, Wang, Wei W wrote: > > With that, if free page opt is off, the mutex is skipped, isn't it? > > Yes, but when free page is on, it'll check once per page. As I mentioned I > still > don't think it's

RE: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-07 Thread Wang, Wei W
On Wednesday, July 7, 2021 1:47 AM, Peter Xu wrote: > On Sat, Jul 03, 2021 at 02:53:27AM +, Wang, Wei W wrote: > > + do { > > +page_to_clear = start + (i++ << block->clear_bmap_shift); > > Why "i" needs to be shifted? Just move to the next clear chunk, no? For example, (1

Re: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-06 Thread Peter Xu
On Sun, Jul 04, 2021 at 04:14:57PM +0200, Lukas Straub wrote: > On Sat, 3 Jul 2021 18:31:15 +0200 > Lukas Straub wrote: > > > On Wed, 30 Jun 2021 16:08:05 -0400 > > Peter Xu wrote: > > > > > Taking the mutex every time for each dirty bit to clear is too slow, > > > especially > > > we'll

Re: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-06 Thread Peter Xu
On Fri, Jul 02, 2021 at 02:29:41AM +, Wang, Wei W wrote: > On Thursday, July 1, 2021 8:51 PM, Peter Xu wrote: > > On Thu, Jul 01, 2021 at 04:42:38AM +, Wang, Wei W wrote: > > > On Thursday, July 1, 2021 4:08 AM, Peter Xu wrote: > > > > Taking the mutex every time for each dirty bit to

Re: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-06 Thread Peter Xu
On Sat, Jul 03, 2021 at 02:53:27AM +, Wang, Wei W wrote: > On Friday, July 2, 2021 3:07 PM, David Hildenbrand wrote: > > On 02.07.21 04:48, Wang, Wei W wrote: > > > On Thursday, July 1, 2021 10:22 PM, David Hildenbrand wrote: > > >> On 01.07.21 14:51, Peter Xu wrote: > > > > I think that

Re: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-06 Thread Peter Xu
On Tue, Jul 06, 2021 at 12:05:49PM +0200, David Hildenbrand wrote: > On 06.07.21 11:41, Wang, Wei W wrote: > > On Monday, July 5, 2021 9:42 PM, David Hildenbrand wrote: > > > On 03.07.21 04:53, Wang, Wei W wrote: > > > > On Friday, July 2, 2021 3:07 PM, David Hildenbrand wrote: > > > > > On

Re: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-06 Thread David Hildenbrand
On 06.07.21 11:41, Wang, Wei W wrote: On Monday, July 5, 2021 9:42 PM, David Hildenbrand wrote: On 03.07.21 04:53, Wang, Wei W wrote: On Friday, July 2, 2021 3:07 PM, David Hildenbrand wrote: On 02.07.21 04:48, Wang, Wei W wrote: On Thursday, July 1, 2021 10:22 PM, David Hildenbrand wrote:

RE: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-06 Thread Wang, Wei W
On Monday, July 5, 2021 9:42 PM, David Hildenbrand wrote: > On 03.07.21 04:53, Wang, Wei W wrote: > > On Friday, July 2, 2021 3:07 PM, David Hildenbrand wrote: > >> On 02.07.21 04:48, Wang, Wei W wrote: > >>> On Thursday, July 1, 2021 10:22 PM, David Hildenbrand wrote: > On 01.07.21 14:51,

Re: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-05 Thread David Hildenbrand
On 03.07.21 04:53, Wang, Wei W wrote: On Friday, July 2, 2021 3:07 PM, David Hildenbrand wrote: On 02.07.21 04:48, Wang, Wei W wrote: On Thursday, July 1, 2021 10:22 PM, David Hildenbrand wrote: On 01.07.21 14:51, Peter Xu wrote: I think that clearly shows the issue. My theory I did not

Re: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-04 Thread Lukas Straub
On Sat, 3 Jul 2021 18:31:15 +0200 Lukas Straub wrote: > On Wed, 30 Jun 2021 16:08:05 -0400 > Peter Xu wrote: > > > Taking the mutex every time for each dirty bit to clear is too slow, > > especially > > we'll take/release even if the dirty bit is cleared. So far it's only used > > to > >

Re: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-03 Thread Lukas Straub
On Wed, 30 Jun 2021 16:08:05 -0400 Peter Xu wrote: > Taking the mutex every time for each dirty bit to clear is too slow, > especially > we'll take/release even if the dirty bit is cleared. So far it's only used to > sync with special cases with qemu_guest_free_page_hint() against migration >

RE: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-02 Thread Wang, Wei W
On Friday, July 2, 2021 3:07 PM, David Hildenbrand wrote: > On 02.07.21 04:48, Wang, Wei W wrote: > > On Thursday, July 1, 2021 10:22 PM, David Hildenbrand wrote: > >> On 01.07.21 14:51, Peter Xu wrote: > > I think that clearly shows the issue. > > My theory I did not verify yet: Assume we have

Re: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-02 Thread David Hildenbrand
On 02.07.21 04:48, Wang, Wei W wrote: On Thursday, July 1, 2021 10:22 PM, David Hildenbrand wrote: On 01.07.21 14:51, Peter Xu wrote: Spoiler alert: the introduction of clean bitmaps partially broke free page hinting already (as clearing happens deferred -- and might never happen if we don't

RE: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-01 Thread Wang, Wei W
On Thursday, July 1, 2021 10:22 PM, David Hildenbrand wrote: > On 01.07.21 14:51, Peter Xu wrote: > Spoiler alert: the introduction of clean bitmaps partially broke free page > hinting > already (as clearing happens deferred -- and might never happen if we don't > migrate *any* page within a

RE: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-01 Thread Wang, Wei W
On Thursday, July 1, 2021 8:51 PM, Peter Xu wrote: > On Thu, Jul 01, 2021 at 04:42:38AM +, Wang, Wei W wrote: > > On Thursday, July 1, 2021 4:08 AM, Peter Xu wrote: > > > Taking the mutex every time for each dirty bit to clear is too slow, > > > especially we'll take/release even if the dirty

Re: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-01 Thread David Hildenbrand
On 01.07.21 14:51, Peter Xu wrote: On Thu, Jul 01, 2021 at 04:42:38AM +, Wang, Wei W wrote: On Thursday, July 1, 2021 4:08 AM, Peter Xu wrote: Taking the mutex every time for each dirty bit to clear is too slow, especially we'll take/release even if the dirty bit is cleared. So far it's

Re: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-07-01 Thread Peter Xu
On Thu, Jul 01, 2021 at 04:42:38AM +, Wang, Wei W wrote: > On Thursday, July 1, 2021 4:08 AM, Peter Xu wrote: > > Taking the mutex every time for each dirty bit to clear is too slow, > > especially we'll > > take/release even if the dirty bit is cleared. So far it's only used to > > sync

RE: [PATCH] migration: Move bitmap_mutex out of migration_bitmap_clear_dirty()

2021-06-30 Thread Wang, Wei W
On Thursday, July 1, 2021 4:08 AM, Peter Xu wrote: > Taking the mutex every time for each dirty bit to clear is too slow, > especially we'll > take/release even if the dirty bit is cleared. So far it's only used to sync > with > special cases with qemu_guest_free_page_hint() against migration