From mboxrd@z Thu Jan 1 00:00:00 1970 From: Doug Goldstein Subject: Re: [PATCH RFC 1/4] tmem: Move global stats in a tmem_statistics structure Date: Wed, 18 May 2016 11:30:21 -0500 Message-ID: References: <1463414311-10677-1-git-send-email-konrad.wilk@oracle.com> <1463414311-10677-2-git-send-email-konrad.wilk@oracle.com> <0de0fdf1-b0b8-ece5-643b-b5880db14d5e@cardoe.com> <20160518143417.GB10092@char.us.oracle.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0603060088909074957==" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1b34Mi-0005q1-6z for xen-devel@lists.xenproject.org; Wed, 18 May 2016 16:30:32 +0000 Received: by mail-yw0-f193.google.com with SMTP id y6so7267937ywe.0 for ; Wed, 18 May 2016 09:30:30 -0700 (PDT) In-Reply-To: <20160518143417.GB10092@char.us.oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Konrad Rzeszutek Wilk Cc: xen-devel@lists.xenproject.org, wei.liu2@citrix.com, jbeulich@suse.com, andrew.cooper3@citrix.com List-Id: xen-devel@lists.xenproject.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --===============0603060088909074957== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="qvguNKewUePfNEquQXFTjAoXSWGSqGgFF" This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --qvguNKewUePfNEquQXFTjAoXSWGSqGgFF Content-Type: multipart/mixed; boundary="aAVlOjxvhEK0ekiTpLWN6s3KVPBvC7URR" From: Doug Goldstein To: Konrad Rzeszutek Wilk Cc: xen-devel@lists.xenproject.org, andrew.cooper3@citrix.com, wei.liu2@citrix.com, jbeulich@suse.com Message-ID: Subject: Re: [Xen-devel] [PATCH RFC 1/4] tmem: Move global stats in a tmem_statistics structure References: <1463414311-10677-1-git-send-email-konrad.wilk@oracle.com> <1463414311-10677-2-git-send-email-konrad.wilk@oracle.com> <0de0fdf1-b0b8-ece5-643b-b5880db14d5e@cardoe.com> <20160518143417.GB10092@char.us.oracle.com> In-Reply-To: <20160518143417.GB10092@char.us.oracle.com> --aAVlOjxvhEK0ekiTpLWN6s3KVPBvC7URR Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 5/18/16 9:34 AM, Konrad Rzeszutek Wilk wrote: > On Tue, May 17, 2016 at 09:01:00PM -0500, Doug Goldstein wrote: >> On 5/17/16 8:57 PM, Doug Goldstein wrote: >>> On 5/16/16 10:58 AM, Konrad Rzeszutek Wilk wrote: >>>> And adjust the macro: atomic_inc_and_max to update the structure. >>>> >>>> Sadly one entry: pool->pgp_count cannot use this macro anymore >>>> so unroll the macro for this instance. >>>> >>>> No functional change. The name has the 'tmem_stats' as it will >>>> be eventually non-local. >>>> >>>> Signed-off-by: Konrad Rzeszutek Wilk >>>> --- >>>> xen/common/tmem.c | 135 +++++++++++++++++++++++++++++--------------= ----------- >>>> 1 file changed, 73 insertions(+), 62 deletions(-) >>>> >>>> diff --git a/xen/common/tmem.c b/xen/common/tmem.c >>>> index 16e249a..d362eae 100644 >>>> --- a/xen/common/tmem.c >>>> +++ b/xen/common/tmem.c >>>> @@ -28,25 +28,32 @@ >>>> #define TMEM_SPEC_VERSION 1 >>>> =20 >>>> /* Global statistics (none need to be locked). */ >>>> -static unsigned long total_tmem_ops =3D 0; >>>> -static unsigned long errored_tmem_ops =3D 0; >>>> -static unsigned long total_flush_pool =3D 0; >>>> -static unsigned long alloc_failed =3D 0, alloc_page_failed =3D 0; >>>> -static unsigned long evicted_pgs =3D 0, evict_attempts =3D 0; >>>> -static unsigned long relinq_pgs =3D 0, relinq_attempts =3D 0; >>>> -static unsigned long max_evicts_per_relinq =3D 0; >>>> -static unsigned long low_on_memory =3D 0; >>>> -static unsigned long deduped_puts =3D 0; >>>> -static unsigned long tot_good_eph_puts =3D 0; >>>> -static int global_obj_count_max =3D 0; >>>> -static int global_pgp_count_max =3D 0; >>>> -static int global_pcd_count_max =3D 0; >>>> -static int global_page_count_max =3D 0; >>>> -static int global_rtree_node_count_max =3D 0; >>>> -static long global_eph_count_max =3D 0; >>>> -static unsigned long failed_copies; >>>> -static unsigned long pcd_tot_tze_size =3D 0; >>>> -static unsigned long pcd_tot_csize =3D 0; >>>> +struct tmem_statistics { >>>> + unsigned long total_tmem_ops; >>>> + unsigned long errored_tmem_ops; >>>> + unsigned long total_flush_pool; >>>> + unsigned long alloc_failed; >>>> + unsigned long alloc_page_failed; >>>> + unsigned long evicted_pgs; >>>> + unsigned long evict_attempts; >>>> + unsigned long relinq_pgs; >>>> + unsigned long relinq_attempts; >>>> + unsigned long max_evicts_per_relinq; >>>> + unsigned long low_on_memory; >>>> + unsigned long deduped_puts; >>>> + unsigned long tot_good_eph_puts; >>>> + int global_obj_count_max; >>>> + int global_pgp_count_max; >>>> + int global_pcd_count_max; >>>> + int global_page_count_max; >>>> + int global_rtree_node_count_max; >>>> + long global_eph_count_max; >>>> + unsigned long failed_copies; >>>> + unsigned long pcd_tot_tze_size; >>>> + unsigned long pcd_tot_csize; >>>> +}; >>>> + >>>> +static struct tmem_statistics tmem_stats; >>>> =20 >>>> /************ CORE DATA STRUCTURES ********************************= ****/ >>>> =20 >>>> @@ -225,8 +232,8 @@ static atomic_t global_rtree_node_count =3D ATOM= IC_INIT(0); >>>> =20 >>>> #define atomic_inc_and_max(_c) do { \ >>>> atomic_inc(&_c); \ >>>> - if ( _atomic_read(_c) > _c##_max ) \ >>>> - _c##_max =3D _atomic_read(_c); \ >>>> + if ( _atomic_read(_c) > tmem_stats._c##_max ) \ >>>> + tmem_stats._c##_max =3D _atomic_read(_c); \ >>>> } while (0) >>>> =20 >>>> #define atomic_dec_and_assert(_c) do { \ >>>> @@ -273,7 +280,7 @@ static void *tmem_malloc(size_t size, struct tme= m_pool *pool) >>>> v =3D xmem_pool_alloc(size, tmem_mempool); >>>> } >>>> if ( v =3D=3D NULL ) >>>> - alloc_failed++; >>>> + tmem_stats.alloc_failed++; >>>> return v; >>>> } >>>> =20 >>>> @@ -300,7 +307,7 @@ static struct page_info *tmem_alloc_page(struct = tmem_pool *pool) >>>> else >>>> pfp =3D __tmem_alloc_page(); >>>> if ( pfp =3D=3D NULL ) >>>> - alloc_page_failed++; >>>> + tmem_stats.alloc_page_failed++; >>>> else >>>> atomic_inc_and_max(global_page_count); >>> >>> So the code was previously like this but this change made me notice >>> this. How come tmem_stats.global_page_count needs to be atomically >>> incremented but not tmem_stats.alloc_page_failed? Its also a little >>> weird that global_page_count is in the struct now but not really visi= ble >>> here while alloc_page_count is in the struct but has to be explicitly= >>> called out. >>> >>> >> >> Actually I just realized "global_page_count" but this patch actually >> deals with "global_page_count_max". So ignore the original email. >> >> But does this patch compile (for bisect-ability) due to the change to >> atomic_inc_and_max() but not moving "global_page_count" into the struc= ture? >=20 > Yup: >=20 > #define atomic_inc_and_max(_c) do { \ > atomic_inc(&_c); \ > - if ( _atomic_read(_c) > _c##_max ) \ > - _c##_max =3D _atomic_read(_c); \ > + if ( _atomic_read(_c) > tmem_stats._c##_max ) \ > + tmem_stats._c##_max =3D _atomic_read(_c); \ > } while (0) > =20 > As the global_page_count is still an atomic outside the structure. > And the global_page_count_max is inside the structure. >=20 >> >> --=20 >> Doug Goldstein >> >=20 >=20 >=20 Gah. You're right. Reviewed-by: Doug Goldstein --=20 Doug Goldstein --aAVlOjxvhEK0ekiTpLWN6s3KVPBvC7URR-- --qvguNKewUePfNEquQXFTjAoXSWGSqGgFF Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG/MacGPG2 v2.0 iQJ8BAEBCgBmBQJXPJiiXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXRBNTM5MEQ2RTNFMTkyNzlCNzVDMzIwOTVB MkJDMDNEQzg3RUQxQkQ0AAoJEKK8A9yH7RvUk34P/2AskICecGknZEr5bOyuEnlW JxLAA2Kwvo6CP5QgewFc9FIvgxYMEus/h0NX1kJ82joNZVKvrbfYdLhLVF23vwJ1 ID+mDFkkGI2qxMwKLlOa84cyBP0EoEWa1/wCcfgBN8mDr67Gi37MPzT1RvUQE7xz JVmVswDTa8qXrHI37R5OP3jQXAxQxpoDggGrs4xkrRKEJrWQwTA9bbNBUVpUTZAl bNePK9lMsK0nDBCtDABJOCKxH++Dj723rx8RmLrWtNGu7zLKj3QBnY8J3GpProtE SY/82bpHufdwv+WXFB+9Zqs3HEn8EfLePFN2wPmYR5td6OraN+dOUwTGwD0KBrjx bCMBAqjXYoEnEEwJU0DhWGpiE5bIMhAMiFADqQnFC2M8SDeR8dwgVUiIrUjkko7H cFNXrzx7zgzbFZwWMHYSwLYbaJe4AMnMQJB7g+oGyJIb+htbAL55xev+D9vCjSPi /5BbYguCgEfk1Cmqr/HfKbY1Ng8QjnqVcwp9ovHgsnzd8morNL+xjoP3qWBL5WjK O7G54hfLHU/6stH16h4TMuNS9lui3BynsAgq2LXaJ7H1N+0qJLVzLcSQcMhSDttP jVg/NEBbmMGh5+G/c74YwwoNrrjaTEmhkESyh7slpMc6M7tMit2PttCS0tLKj9YX 1kGOw5lDj6ztmv3AU/qG =aHpe -----END PGP SIGNATURE----- --qvguNKewUePfNEquQXFTjAoXSWGSqGgFF-- --===============0603060088909074957== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --===============0603060088909074957==--