On 31.01.23 г. 18:27 ч., Alexander Atanasov wrote:
Hi,


On 31.01.23 14:23, Nikolay Borisov wrote:
Replace all shift rights/shift lefts by divide and multiply operations.

What's wrong with shifts?

I've checked that this doesn't generate worse assembly and that shifts
are indeed continued to be used. PAGE_SHIFT + 3 is actually
BITS_PER_PAGE and is a constant integer expression so the compiler is
perfectly capable of figuring that out and generatin optimal assembly.

Replace an if/else construct with just an if which handles the error
condition. This makes the code somewhat easier to follow in a linear
fashion.

Remove a superfluous continue and make off2 variable local the sole
context it's being used.

Signed-off-by: Nikolay Borisov <nikolay.bori...@virtuozzo.com>
---
  block/blk-cbt.c | 21 +++++++++++----------
  1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/block/blk-cbt.c b/block/blk-cbt.c
index 967533a2a7a3..8baeb2804a61 100644
--- a/block/blk-cbt.c
+++ b/block/blk-cbt.c
@@ -758,14 +758,14 @@ static void cbt_flush_cache(struct cbt_info *cbt)
  static void cbt_find_next_extent(struct cbt_info *cbt, blkcnt_t block, struct cbt_extent *ex)
  {
-    unsigned long off, off2, idx;
+    unsigned long off, idx;
      struct page *page;
      bool found = 0;
      ex->start = cbt->block_max;
      ex->len = 0;
-    idx = block >> (PAGE_SHIFT + 3);
+    idx = block / BITS_PER_PAGE;

As we discussed about the previous series is that this is valid only when working with powers of two - what guarantees that BITS_PER_PAGE is power of two ? Same bellow.

BITS_PER_PAGE is PAGE_SIZE * 8, so long as PAGE_SIZE Is power of 2, which at this point is pretty much guaranteed for every architecture, let alone for every arch VHI is used on (x86_64).


- Assembly generation is compiler dependant

Sure, but the macro is a guaranteed integer constant expression which is power of 2 so shifts are pretty much "guaranteed".

- It is well known that division must be avoided inside the kernel

      while (block < cbt->block_max) {
          off = block & (BITS_PER_PAGE -1);
          page = CBT_PAGE(cbt, idx);
@@ -778,17 +778,19 @@ static void cbt_find_next_extent(struct cbt_info *cbt, blkcnt_t block, struct cb
          /* Find extent start */
          if (!found) {
              ex->start = find_next_bit(page_address(page), BITS_PER_PAGE, off);
-            if (ex->start != BITS_PER_PAGE) {
-                off = ex->start;
-                ex->start += idx << (PAGE_SHIFT + 3);
-                found = 1;
-            } else {
+            /* No set bits in current page, skip to next one */
+            if (ex->start == BITS_PER_PAGE) {
                  unlock_page(page);
                  goto next;
              }
+
+            off = ex->start;
+            ex->start += idx / BITS_PER_PAGE;
+            found = 1;
          }
          if (found) {
-            off2 = find_next_zero_bit(page_address(page), BITS_PER_PAGE, off);
+            unsigned long off2 = find_next_zero_bit(page_address(page),
+                                BITS_PER_PAGE, off);
              ex->len += off2 - off;
              if (off2 != BITS_PER_PAGE) {
                  unlock_page(page);
@@ -798,8 +800,7 @@ static void cbt_find_next_extent(struct cbt_info *cbt, blkcnt_t block, struct cb
          unlock_page(page);
      next:
          idx++;
-        block = idx << (PAGE_SHIFT + 3);
-        continue;
+        block = idx * BITS_PER_PAGE;
      }
  }


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to