All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: Stefano Stabellini <stefano.stabellini@amd.com>,
	xen-devel@lists.xenproject.org
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Julien Grall" <julien@xen.org>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Anthony PERARD" <anthony.perard@vates.tech>,
	"Michal Orzel" <michal.orzel@amd.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: [PATCH v5] xen: simplify bitmap_to_xenctl_bitmap for little endian
Date: Tue, 1 Apr 2025 12:09:08 +0200	[thread overview]
Message-ID: <694eac75-e872-4ba0-80ed-95c14cd11f5e@suse.com> (raw)
In-Reply-To: <20250401005008.2787764-1-stefano.stabellini@amd.com>

From: Stefano Stabellini <stefano.stabellini@amd.com>

The little endian implementation of bitmap_to_xenctl_bitmap leads to
unnecessary xmallocs and xfrees. Given that Xen only supports little
endian architectures, it is worth optimizing.

This patch removes the need for the xmalloc on little endian
architectures.

Remove clamp_last_byte as it is only called once and only needs to
modify one byte. Inline it.

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v5: Fix IS_ENABLED() use. Keep more code common. Move comment.
    Convert LE bitmap_long_to_byte() to just a declaration.

--- a/xen/common/bitmap.c
+++ b/xen/common/bitmap.c
@@ -40,21 +40,6 @@
  * for the best explanations of this ordering.
  */
 
-/*
- * If a bitmap has a number of bits which is not a multiple of 8 then
- * the last few bits of the last byte of the bitmap can be
- * unexpectedly set which can confuse consumers (e.g. in the tools)
- * who also round up their loops to 8 bits. Ensure we clear those left
- * over bits so as to prevent surprises.
- */
-static void clamp_last_byte(uint8_t *bp, unsigned int nbits)
-{
-	unsigned int remainder = nbits % 8;
-
-	if (remainder)
-		bp[nbits/8] &= (1U << remainder) - 1;
-}
-
 int __bitmap_empty(const unsigned long *bitmap, unsigned int bits)
 {
 	int k, lim = bits/BITS_PER_LONG;
@@ -338,7 +323,6 @@ static void bitmap_long_to_byte(uint8_t
 			nbits -= 8;
 		}
 	}
-	clamp_last_byte(bp, nbits);
 }
 
 static void bitmap_byte_to_long(unsigned long *lp, const uint8_t *bp,
@@ -359,12 +343,11 @@ static void bitmap_byte_to_long(unsigned
 
 #elif defined(__LITTLE_ENDIAN)
 
-static void bitmap_long_to_byte(uint8_t *bp, const unsigned long *lp,
-				unsigned int nbits)
-{
-	memcpy(bp, lp, DIV_ROUND_UP(nbits, BITS_PER_BYTE));
-	clamp_last_byte(bp, nbits);
-}
+#define LITTLE_ENDIAN 1 /* For IS_ENABLED(). */
+
+/* Unused function, but a declaration is needed. */
+void bitmap_long_to_byte(uint8_t *bp, const unsigned long *lp,
+			 unsigned int nbits);
 
 static void bitmap_byte_to_long(unsigned long *lp, const uint8_t *bp,
 				unsigned int nbits)
@@ -384,22 +367,46 @@ int bitmap_to_xenctl_bitmap(struct xenct
     uint8_t zero = 0;
     int err = 0;
     unsigned int xen_bytes = DIV_ROUND_UP(nbits, BITS_PER_BYTE);
-    uint8_t *bytemap = xmalloc_array(uint8_t, xen_bytes);
+    const uint8_t *bytemap;
+    uint8_t last, *buf = NULL;
 
-    if ( !bytemap )
-        return -ENOMEM;
+    if ( !IS_ENABLED(LITTLE_ENDIAN) )
+    {
+        buf = xmalloc_array(uint8_t, xen_bytes);
+        if ( !buf )
+            return -ENOMEM;
+
+        bitmap_long_to_byte(buf, bitmap, nbits);
+
+        bytemap = buf;
+    }
+    else
+        bytemap = (const uint8_t *)bitmap;
 
     guest_bytes = DIV_ROUND_UP(xenctl_bitmap->nr_bits, BITS_PER_BYTE);
     copy_bytes  = min(guest_bytes, xen_bytes);
 
-    bitmap_long_to_byte(bytemap, bitmap, nbits);
+    if ( copy_bytes > 1 &&
+         copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes - 1) )
+        err = -EFAULT;
+
+    /*
+     * If a bitmap has a number of bits which is not a multiple of 8 then the
+     * last few bits of the last byte of the bitmap can be unexpectedly set,
+     * which can confuse consumers (e.g. in the tools), who also may round up
+     * their loops to 8 bits. Ensure we clear those left over bits so as to
+     * prevent surprises.
+     */
+    last = bytemap[nbits / 8];
+    if ( nbits % 8 )
+        last &= (1U << (nbits % 8)) - 1;
+
+    xfree(buf);
 
     if ( copy_bytes &&
-         copy_to_guest(xenctl_bitmap->bitmap, bytemap, copy_bytes) )
+         copy_to_guest_offset(xenctl_bitmap->bitmap, copy_bytes - 1, &last, 1) )
         err = -EFAULT;
 
-    xfree(bytemap);
-
     for ( i = copy_bytes; !err && i < guest_bytes; i++ )
         if ( copy_to_guest_offset(xenctl_bitmap->bitmap, i, &zero, 1) )
             err = -EFAULT;



  reply	other threads:[~2025-04-01 10:09 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-01  0:50 [PATCH v4] xen: simplify bitmap_to_xenctl_bitmap for little endian Stefano Stabellini
2025-04-01 10:09 ` Jan Beulich [this message]
2025-04-02  0:14   ` [PATCH v5] " Stefano Stabellini
2025-04-02  9:18     ` Jan Beulich
2025-04-02  9:21   ` Andrew Cooper
2025-04-02  9:30     ` Jan Beulich
2025-04-02  9:36       ` Andrew Cooper
2025-04-02  9:42         ` Jan Beulich

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=694eac75-e872-4ba0-80ed-95c14cd11f5e@suse.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=anthony.perard@vates.tech \
    --cc=julien@xen.org \
    --cc=michal.orzel@amd.com \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=stefano.stabellini@amd.com \
    --cc=xen-devel@lists.xenproject.org \
    /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.