[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 0/7] Rework vhost memory region updates
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH v6 0/7] Rework vhost memory region updates |
Date: |
Fri, 19 Jan 2018 10:41:26 +0000 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
* Michael S. Tsirkin (address@hidden) wrote:
> On Thu, Jan 18, 2018 at 07:59:35PM +0000, Dr. David Alan Gilbert wrote:
> > * Michael S. Tsirkin (address@hidden) wrote:
> > > On Tue, Jan 16, 2018 at 06:04:01PM +0000, Dr. David Alan Gilbert (git)
> > > wrote:
> > > > From: "Dr. David Alan Gilbert" <address@hidden>
> > > >
> > > > Hi,
> > > > This patch set reworks the way the vhost code handles changes in
> > > > physical address space layout that came from a discussion with Igor.
> > > >
> > > > Its intention is to simplify a lot of the update code,
> > > > and to make it easier for the postcopy+shared code to
> > > > do the hugepage alignments that are needed.
> > > >
> > > > Instead of inserting/removing each section during the add/del
> > > > callbacks of the listener, we start afresh and build a list
> > > > from the add and nop callbacks, then at the end compare the list
> > > > we've built with the exisiting list.
> > > >
> > > > v6
> > > > Tidy ups from Igor
> > > > The biggest change is moving the 'Move log_dirty check' to be
> > > > the last patch in the set.
> > > >
> > > > Dr. David Alan Gilbert (7):
> > > > vhost: Build temporary section list and deref after commit
> > > > vhost: Simplify ring verification checks
> > > > vhost: Merge sections added to temporary list
> > > > vhost: Regenerate region list from changed sections list
> > > > vhost: Clean out old vhost_set_memory and friends
> > > > vhost: Merge and delete unused callbacks
> > > > vhost: Move log_dirty check
> > > >
> > > > hw/virtio/trace-events | 6 +
> > > > hw/virtio/vhost.c | 497
> > > > ++++++++++++++++------------------------------
> > > > include/hw/virtio/vhost.h | 5 +-
> > > > 3 files changed, 180 insertions(+), 328 deletions(-)
> > >
> > >
> > > Seems to trigger errors with clang runtime sanitizer:
> > >
> > > /scm/qemu/hw/virtio/vhost.c:425:26: runtime error: null pointer passed as
> > > argument 1, which is declared to never be null
> > > /usr/include/string.h:64:33: note: nonnull attribute specified here
> > > /scm/qemu/hw/virtio/vhost.c:425:45: runtime error: null pointer passed as
> > > argument 2, which is declared to never be null
> > > /usr/include/string.h:64:33: note: nonnull attribute specified here
> > > /scm/qemu/hw/virtio/vhost.c:425:26: runtime error: null pointer passed as
> > > argument 1, which is declared to never be null
> > > /usr/include/string.h:64:33: note: nonnull attribute specified here
> > > /scm/qemu/hw/virtio/vhost.c:425:45: runtime error: null pointer passed as
> > > argument 2, which is declared to never be null
> > > /usr/include/string.h:64:33: note: nonnull attribute specified here
> > > /scm/qemu/hw/virtio/vhost.c:425:26: runtime error: null pointer passed as
> > > argument 1, which is declared to never be null
> > > /usr/include/string.h:64:33: note: nonnull attribute specified here
> > > /scm/qemu/hw/virtio/vhost.c:425:45: runtime error: null pointer passed as
> > > argument 2, which is declared to never be null
> > > /usr/include/string.h:64:33: note: nonnull attribute specified here
> > > /scm/qemu/hw/virtio/vhost.c:425:26: runtime error: null pointer passed as
> > > argument 1, which is declared to never be null
> > > /usr/include/string.h:64:33: note: nonnull attribute specified here
> > > /scm/qemu/hw/virtio/vhost.c:425:45: runtime error: null pointer passed as
> > > argument 2, which is declared to never be null
> > > /usr/include/string.h:64:33: note: nonnull attribute specified here
> >
> >
> > How are you running that test?
>
> One Fedora:
> dnf install clang
>
> '/scm/qemu-clang/../qemu/configure' '--cc=clang' '--cxx=clang++'
> '--extra-cflags=-fsanitize=undefined -ferror-limit=10000
> -fno-sanitize=shift-base'
> make
> make check.
Thanks.
> > Can you add this printf and tell me what
> > it's seeing?
> >
> > /* Same size, lets check the contents */
> > fprintf(stderr, "%s: %p %p %d\n", __func__, dev->mem_sections,
> > old_sections, n_old_sections);
> > changed = memcmp(dev->mem_sections, old_sections,
> > n_old_sections * sizeof(old_sections[0])) != 0;
> >
> > I'm seeing a bunch of calls where both pointers are NULL, but
> > n_old_sections is 0, which feels legal to me.
>
> https://stackoverflow.com/questions/16362925/can-i-pass-a-null-pointer-to-memcmp
>
> says it's invalid.
Posted v7 with trivial fix for it.
(It seems rather ungenerous of the standard to be fussy about that).
Dave
> > I guess we could make it:
> > changed = n_old_sections ? memcmp(....) : false;
> > just to shut clang up.
> >
> > Dave
> >
> > >
> > >
> > > > --
> > > > 2.14.3
> > --
> > Dr. David Alan Gilbert / address@hidden / Manchester, UK
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- [Qemu-devel] [PATCH v6 1/7] vhost: Build temporary section list and deref after commit, (continued)
- [Qemu-devel] [PATCH v6 1/7] vhost: Build temporary section list and deref after commit, Dr. David Alan Gilbert (git), 2018/01/16
- [Qemu-devel] [PATCH v6 2/7] vhost: Simplify ring verification checks, Dr. David Alan Gilbert (git), 2018/01/16
- [Qemu-devel] [PATCH v6 5/7] vhost: Clean out old vhost_set_memory and friends, Dr. David Alan Gilbert (git), 2018/01/16
- [Qemu-devel] [PATCH v6 3/7] vhost: Merge sections added to temporary list, Dr. David Alan Gilbert (git), 2018/01/16
- [Qemu-devel] [PATCH v6 4/7] vhost: Regenerate region list from changed sections list, Dr. David Alan Gilbert (git), 2018/01/16
- [Qemu-devel] [PATCH v6 6/7] vhost: Merge and delete unused callbacks, Dr. David Alan Gilbert (git), 2018/01/16
- [Qemu-devel] [PATCH v6 7/7] vhost: Move log_dirty check, Dr. David Alan Gilbert (git), 2018/01/16
- Re: [Qemu-devel] [PATCH v6 0/7] Rework vhost memory region updates, Michael S. Tsirkin, 2018/01/18