Re: [Xen-devel] [PATCH 8/8] gnttab: drop struct active_grant_entry's gfn field for release builds

[Andrew Cooper]

On 15/08/17 15:42, Jan Beulich wrote:
> This shrinks the size from 48 to 40 bytes bytes on 64-bit builds.
>
> Signed-off-by: Jan Beulich 
>
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -185,7 +185,12 @@ struct active_grant_entry {
>  grant_ref_t   trans_gref;
>  struct domain *trans_domain;
>  unsigned long frame;  /* Frame being granted. */
> +#ifndef NDEBUG
>  unsigned long gfn;/* Guest's idea of the frame being granted. */
> +# define act_set_gfn(act, val) ((act)->gfn = (val))
> +#else
> +# define act_set_gfn(act, gfn)
> +#endif
>  spinlock_tlock;  /* lock to protect access of this entry.
>  see docs/misc/grant-tables.txt for
>  locking protocol  */

IMO, this would be cleaner as

static void act_set_gfn(struct active_grant_entry *act, unsigned long gfn)
{
#ifndef NDEBUG
act->gfn = gfn;
#endif
}

Which both moves the function out of the struct definition, and takes
care of side effect evaluation differences.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 8/8] gnttab: drop struct active_grant_entry's gfn field for release builds

[Jan Beulich]

This shrinks the size from 48 to 40 bytes bytes on 64-bit builds.

Signed-off-by: Jan Beulich 

--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -185,7 +185,12 @@ struct active_grant_entry {
 grant_ref_t   trans_gref;
 struct domain *trans_domain;
 unsigned long frame;  /* Frame being granted. */
+#ifndef NDEBUG
 unsigned long gfn;/* Guest's idea of the frame being granted. */
+# define act_set_gfn(act, val) ((act)->gfn = (val))
+#else
+# define act_set_gfn(act, gfn)
+#endif
 spinlock_tlock;  /* lock to protect access of this entry.
 see docs/misc/grant-tables.txt for
 locking protocol  */
@@ -893,7 +898,7 @@ map_grant_ref(
  op->flags & GNTMAP_readonly, rd);
 if ( rc != GNTST_okay )
 goto unlock_out_clear;
-act->gfn = gfn;
+act_set_gfn(act, gfn);
 act->domid = ld->domain_id;
 act->frame = frame;
 act->start = 0;
@@ -2286,7 +2291,7 @@ acquire_grant_for_copy(
 act->trans_domain = td;
 act->trans_gref = trans_gref;
 act->frame = grant_frame;
-act->gfn = gfn_x(INVALID_GFN);
+act_set_gfn(act, gfn_x(INVALID_GFN));
 /*
  * The actual remote remote grant may or may not be a sub-page,
  * but we always treat it as one because that blocks mappings of
@@ -2312,7 +2317,7 @@ acquire_grant_for_copy(
 rc = get_paged_frame(gfn, _frame, page, readonly, rd);
 if ( rc != GNTST_okay )
 goto unlock_out_clear;
-act->gfn = gfn;
+act_set_gfn(act, gfn);
 is_sub_page = 0;
 trans_page_off = 0;
 trans_length = PAGE_SIZE;
@@ -2323,7 +2328,7 @@ acquire_grant_for_copy(
  readonly, rd);
 if ( rc != GNTST_okay )
 goto unlock_out_clear;
-act->gfn = sha2->full_page.frame;
+act_set_gfn(act, sha2->full_page.frame);
 is_sub_page = 0;
 trans_page_off = 0;
 trans_length = PAGE_SIZE;
@@ -2334,7 +2339,7 @@ acquire_grant_for_copy(
  readonly, rd);
 if ( rc != GNTST_okay )
 goto unlock_out_clear;
-act->gfn = sha2->sub_page.frame;
+act_set_gfn(act, sha2->sub_page.frame);
 is_sub_page = 1;
 trans_page_off = sha2->sub_page.page_off;
 trans_length = sha2->sub_page.length;
@@ -3496,8 +3501,16 @@ void grant_table_warn_active_grants(stru
 
 nr_active++;
 if ( nr_active <= WARN_GRANT_MAX )
-printk(XENLOG_G_DEBUG "Dom%d has an active grant: GFN: %lx (MFN: 
%lx)\n",
-   d->domain_id, act->gfn, act->frame);
+printk(XENLOG_G_DEBUG "Dom%d has active grant %x ("
+#ifndef NDEBUG
+   "GFN %lx, "
+#endif
+   "MFN: %lx)\n",
+   d->domain_id, ref,
+#ifndef NDEBUG
+   act->gfn,
+#endif
+   act->frame);
 active_entry_release(act);
 }
 




___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel