All of lore.kernel.org
 help / color / mirror / Atom feed
From: Juan Quintela <quintela@redhat.com>
To: marcandre.lureau@redhat.com
Cc: qemu-devel@nongnu.org, "Cameron Esfahani" <dirty@apple.com>,
	"Roman Bolshakov" <r.bolshakov@yadro.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"David Hildenbrand" <david@redhat.com>,
	"Igor Mammedov" <imammedo@redhat.com>,
	"Kevin Wolf" <kwolf@redhat.com>,
	"Hanna Reitz" <hreitz@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Fam Zheng" <fam@euphon.net>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Denis V. Lunev" <den@openvz.org>,
	"Vladimir Sementsov-Ogievskiy" <v.sementsov-og@mail.ru>,
	"Warner Losh" <imp@bsdimp.com>, "Kyle Evans" <kevans@freebsd.org>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Eduardo Habkost" <eduardo@habkost.net>,
	"Marcel Apfelbaum" <marcel.apfelbaum@gmail.com>,
	"Yanan Wang" <wangyanan55@huawei.com>,
	"Cornelia Huck" <cohuck@redhat.com>,
	"Thomas Huth" <thuth@redhat.com>,
	"Halil Pasic" <pasic@linux.ibm.com>,
	"Christian Borntraeger" <borntraeger@linux.ibm.com>,
	"Mark Cave-Ayland" <mark.cave-ayland@ilande.co.uk>,
	"Cédric Le Goater" <clg@kaod.org>,
	"Daniel Henrique Barboza" <danielhb413@gmail.com>,
	"David Gibson" <david@gibson.dropbear.id.au>,
	"Greg Kurz" <groug@kaod.org>,
	"Yuval Shaia" <yuval.shaia.ml@gmail.com>,
	"Stefan Berger" <stefanb@linux.vnet.ibm.com>,
	"Alex Williamson" <alex.williamson@redhat.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Peter Xu" <peterx@redhat.com>,
	"Laurent Vivier" <laurent@vivier.eu>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Wenchao Wang" <wenchao.wang@intel.com>,
	"Colin Xu" <colin.xu@intel.com>,
	"Kamil Rytarowski" <kamil@netbsd.org>,
	"Reinoud Zandijk" <reinoud@netbsd.org>,
	"Sunil Muthuswamy" <sunilmut@microsoft.com>,
	"Stefan Weil" <sw@weilnetz.de>,
	kvm@vger.kernel.org (open list:Overall KVM CPUs),
	qemu-block@nongnu.org (open list:Block layer core),
	qemu-s390x@nongnu.org (open list:S390 general arch...),
	qemu-ppc@nongnu.org (open list:New World (mac99)),
	haxm-team@intel.com (open list:X86 HAXM CPUs)
Subject: Re: [PATCH 11/32] Replace qemu_real_host_page variables with inlined functions
Date: Fri, 25 Mar 2022 09:34:33 +0100	[thread overview]
Message-ID: <87k0cie8h2.fsf@secure.mitica> (raw)
In-Reply-To: <20220323155743.1585078-12-marcandre.lureau@redhat.com> (marcandre lureau's message of "Wed, 23 Mar 2022 19:57:22 +0400")

marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Replace the global variables with inlined helper functions. getpagesize() is very
> likely annotated with a "const" function attribute (at least with glibc), and thus
> optimization should apply even better.
>
> This avoids the need for a constructor initialization too.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

I see what you are tyring to do here.

But once here, why aren't we just calling getpagesize() and call it a
day?  I can't see what advantage has to have this other name for this
function, and as you are already changing all its uses anyways.

Once there, I can see that qemu_real_host_page_mask is used (almost)
everywhere as (vfio code use the real value):

    foo & qemu_real_host_page_mask()

or

    foo & ~qemu_real_host_page_mask()

And no, I don't have any good names for this macros.

Anyways, if you don't like the suggestion, your changes are better that
we already have, so ...

Reviewed-by: Juan Quintela <quintela@redhat.com>


WARNING: multiple messages have this Message-ID (diff)
From: Juan Quintela <quintela@redhat.com>
To: marcandre.lureau@redhat.com
Cc: "Fam Zheng" <fam@euphon.net>,
	"Daniel Henrique Barboza" <danielhb413@gmail.com>,
	"open list:Overall KVM CPUs" <kvm@vger.kernel.org>,
	"David Hildenbrand" <david@redhat.com>,
	"Mark Cave-Ayland" <mark.cave-ayland@ilande.co.uk>,
	qemu-devel@nongnu.org, "Peter Xu" <peterx@redhat.com>,
	"Laurent Vivier" <laurent@vivier.eu>,
	"Yanan Wang" <wangyanan55@huawei.com>,
	"open list:X86 HAXM CPUs" <haxm-team@intel.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Denis V. Lunev" <den@openvz.org>, "Warner Losh" <imp@bsdimp.com>,
	"Alex Williamson" <alex.williamson@redhat.com>,
	qemu-block@nongnu.org,
	"Stefan Berger" <stefanb@linux.vnet.ibm.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Halil Pasic" <pasic@linux.ibm.com>,
	"Kamil Rytarowski" <kamil@netbsd.org>,
	"Reinoud Zandijk" <reinoud@netbsd.org>,
	"Stefan Weil" <sw@weilnetz.de>,
	"Christian Borntraeger" <borntraeger@linux.ibm.com>,
	"Eduardo Habkost" <eduardo@habkost.net>,
	"Thomas Huth" <thuth@redhat.com>,
	"Vladimir Sementsov-Ogievskiy" <v.sementsov-og@mail.ru>,
	"Kyle Evans" <kevans@freebsd.org>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Greg Kurz" <groug@kaod.org>,
	"Cameron Esfahani" <dirty@apple.com>,
	"Yuval Shaia" <yuval.shaia.ml@gmail.com>,
	"Roman Bolshakov" <r.bolshakov@yadro.com>,
	"Cédric Le Goater" <clg@kaod.org>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Sunil Muthuswamy" <sunilmut@microsoft.com>,
	"David Gibson" <david@gibson.dropbear.id.au>,
	"Kevin Wolf" <kwolf@redhat.com>,
	"open list:S390 general arch..." <qemu-s390x@nongnu.org>,
	"Cornelia Huck" <cohuck@redhat.com>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Hanna Reitz" <hreitz@redhat.com>,
	"open list:New World mac99" <qemu-ppc@nongnu.org>,
	"Wenchao Wang" <wenchao.wang@intel.com>,
	"Igor Mammedov" <imammedo@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	"Colin Xu" <colin.xu@intel.com>
Subject: Re: [PATCH 11/32] Replace qemu_real_host_page variables with inlined functions
Date: Fri, 25 Mar 2022 09:34:33 +0100	[thread overview]
Message-ID: <87k0cie8h2.fsf@secure.mitica> (raw)
In-Reply-To: <20220323155743.1585078-12-marcandre.lureau@redhat.com> (marcandre lureau's message of "Wed, 23 Mar 2022 19:57:22 +0400")

marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Replace the global variables with inlined helper functions. getpagesize() is very
> likely annotated with a "const" function attribute (at least with glibc), and thus
> optimization should apply even better.
>
> This avoids the need for a constructor initialization too.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

I see what you are tyring to do here.

But once here, why aren't we just calling getpagesize() and call it a
day?  I can't see what advantage has to have this other name for this
function, and as you are already changing all its uses anyways.

Once there, I can see that qemu_real_host_page_mask is used (almost)
everywhere as (vfio code use the real value):

    foo & qemu_real_host_page_mask()

or

    foo & ~qemu_real_host_page_mask()

And no, I don't have any good names for this macros.

Anyways, if you don't like the suggestion, your changes are better that
we already have, so ...

Reviewed-by: Juan Quintela <quintela@redhat.com>



  reply	other threads:[~2022-03-25  8:34 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-23 15:57 [PATCH 00/32] Misc cleanups marcandre.lureau
2022-03-23 15:57 ` [PATCH 01/32] meson: use chardev_ss dependencies marcandre.lureau
2022-03-23 15:57 ` [PATCH 02/32] meson: add util dependency for oslib-posix on freebsd marcandre.lureau
2022-03-23 15:57 ` [PATCH 03/32] meson: remove unneeded py3 marcandre.lureau
2022-03-23 15:57 ` [PATCH 04/32] meson: remove test-qdev-global-props dependency on testqapi marcandre.lureau
2022-03-23 15:57 ` [PATCH 05/32] char: move qemu_openpty_raw from util/ to char/ marcandre.lureau
2022-03-23 15:57 ` [PATCH 06/32] Replace config-time define HOST_WORDS_BIGENDIAN marcandre.lureau
2022-03-23 15:57   ` marcandre.lureau
2022-03-23 17:10   ` Richard Henderson
2022-03-23 17:16   ` Richard Henderson
2022-03-23 15:57 ` [PATCH 07/32] Replace TARGET_WORDS_BIGENDIAN marcandre.lureau
2022-03-23 15:57   ` marcandre.lureau
2022-03-23 17:12   ` Richard Henderson
2022-03-23 15:57 ` [PATCH 08/32] osdep: poison {HOST,TARGET}_WORDS_BIGENDIAN marcandre.lureau
2022-03-23 17:14   ` Richard Henderson
2022-03-23 15:57 ` [PATCH 09/32] include/qapi: add g_autoptr support for qobject types marcandre.lureau
2022-03-23 18:11   ` Richard Henderson
2022-03-24 16:00   ` Markus Armbruster
2022-03-23 15:57 ` [PATCH 10/32] tests: replace free_all() usage with g_auto marcandre.lureau
2022-03-23 18:13   ` Richard Henderson
2022-03-23 15:57 ` [PATCH 11/32] Replace qemu_real_host_page variables with inlined functions marcandre.lureau
2022-03-23 15:57   ` marcandre.lureau
2022-03-25  8:34   ` Juan Quintela [this message]
2022-03-25  8:34     ` Juan Quintela
2022-03-23 15:57 ` [PATCH 12/32] qga: replace deprecated g_get_current_time() marcandre.lureau
2022-04-06 14:53   ` Damien Hedde
2022-04-07  5:46   ` Markus Armbruster
2022-04-07 11:19     ` Marc-André Lureau
2022-03-23 15:57 ` [PATCH 13/32] error-report: replace deprecated g_get_current_time() with glib >= 2.62 marcandre.lureau
2022-04-06  9:07   ` Markus Armbruster
2022-04-06  9:35     ` Marc-André Lureau
2022-04-06  9:40       ` Marc-André Lureau
2022-04-06 10:38         ` Markus Armbruster
2022-03-23 15:57 ` [PATCH 14/32] util: rename qemu-error.c to match its header name marcandre.lureau
2022-03-23 18:23   ` Richard Henderson
2022-04-07  5:49   ` Markus Armbruster
2022-03-23 15:57 ` [PATCH 15/32] error-report: use error_printf() for program prefix marcandre.lureau
2022-04-07  5:50   ` Markus Armbruster
2022-03-23 15:57 ` [PATCH 16/32] include: move TFR to osdep.h marcandre.lureau
2022-03-23 15:57 ` [PATCH 17/32] include: move qemu_write_full() declaration " marcandre.lureau
2022-03-23 15:57 ` [PATCH 18/32] include: move qemu_pipe() " marcandre.lureau
2022-03-23 15:57 ` [PATCH 19/32] include: move coroutine IO functions to coroutine.h marcandre.lureau
2022-03-23 15:57 ` [PATCH 20/32] include: move dump_in_progress() to runstate.h marcandre.lureau
2022-04-06  8:45   ` Markus Armbruster
2022-03-23 15:57 ` [PATCH 21/32] include: move C/util-related declarations to cutils.h marcandre.lureau
2022-03-23 15:57 ` [PATCH 22/32] include: move cpu_exec* declarations to cpu-common.h marcandre.lureau
2022-03-23 15:57 ` [PATCH 23/32] include: move target page bits declaration to page-vary.h marcandre.lureau
2022-03-23 18:29   ` Richard Henderson
2022-03-23 15:57 ` [PATCH 24/32] include: move progress API to qemu-progress.h marcandre.lureau
2022-03-23 18:30   ` Richard Henderson
2022-03-23 15:57 ` [PATCH 25/32] include: move qemu_get_vm_name() to sysemu.h marcandre.lureau
2022-03-23 15:57 ` [PATCH 26/32] include: move os_*() to os-foo.h marcandre.lureau
2022-03-23 18:31   ` Richard Henderson
2022-03-23 21:11   ` Philippe Mathieu-Daudé
2022-03-23 15:57 ` [PATCH 27/32] include: move page_size_init() to include/hw/core/cpu.h marcandre.lureau
2022-03-23 15:57 ` [PATCH 28/32] Move CPU softfloat unions to cpu-float.h marcandre.lureau
2022-03-23 15:57   ` marcandre.lureau
2022-03-25  8:16   ` Juan Quintela
2022-03-25  8:16     ` Juan Quintela
2022-03-23 15:57 ` [PATCH 29/32] Move fcntl_setfl() to oslib-posix marcandre.lureau
2022-03-23 18:34   ` Richard Henderson
2022-03-23 15:57 ` [PATCH 30/32] qga: remove explicit environ argument from exec/spawn marcandre.lureau
2022-03-23 18:36   ` Richard Henderson
2022-03-23 15:57 ` [PATCH 31/32] RFC: Simplify softmmu/main.c marcandre.lureau
2022-03-23 20:56   ` Akihiko Odaki
2022-03-24  7:51   ` Paolo Bonzini
2022-04-20  7:57     ` Marc-André Lureau
2022-04-20 10:48       ` Akihiko Odaki
2022-03-23 15:57 ` [PATCH 32/32] Remove qemu-common.h include from most units marcandre.lureau
2022-03-23 15:57   ` marcandre.lureau
2022-04-05 15:02   ` Ani Sinha
2022-04-05 15:02     ` Ani Sinha
2022-04-06 10:45     ` Markus Armbruster
2022-04-06 10:45       ` Markus Armbruster
2022-04-06 11:14       ` Peter Maydell
2022-04-06 11:14         ` Peter Maydell
2022-04-05 15:24   ` Warner Losh
2022-04-05 15:24     ` Warner Losh
2022-03-24  9:26 ` [PATCH 00/32] Misc cleanups Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87k0cie8h2.fsf@secure.mitica \
    --to=quintela@redhat.com \
    --cc=alex.williamson@redhat.com \
    --cc=armbru@redhat.com \
    --cc=borntraeger@linux.ibm.com \
    --cc=clg@kaod.org \
    --cc=cohuck@redhat.com \
    --cc=colin.xu@intel.com \
    --cc=danielhb413@gmail.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=david@redhat.com \
    --cc=den@openvz.org \
    --cc=dgilbert@redhat.com \
    --cc=dirty@apple.com \
    --cc=eduardo@habkost.net \
    --cc=f4bug@amsat.org \
    --cc=fam@euphon.net \
    --cc=groug@kaod.org \
    --cc=haxm-team@intel.com \
    --cc=hreitz@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=imp@bsdimp.com \
    --cc=kamil@netbsd.org \
    --cc=kevans@freebsd.org \
    --cc=kraxel@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=kwolf@redhat.com \
    --cc=laurent@vivier.eu \
    --cc=marcandre.lureau@redhat.com \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=mark.cave-ayland@ilande.co.uk \
    --cc=mst@redhat.com \
    --cc=pasic@linux.ibm.com \
    --cc=pbonzini@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --cc=qemu-s390x@nongnu.org \
    --cc=r.bolshakov@yadro.com \
    --cc=reinoud@netbsd.org \
    --cc=richard.henderson@linaro.org \
    --cc=stefanb@linux.vnet.ibm.com \
    --cc=stefanha@redhat.com \
    --cc=sunilmut@microsoft.com \
    --cc=sw@weilnetz.de \
    --cc=thuth@redhat.com \
    --cc=v.sementsov-og@mail.ru \
    --cc=wangyanan55@huawei.com \
    --cc=wenchao.wang@intel.com \
    --cc=yuval.shaia.ml@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.