add(function): modify find_first_bit function to support building on … #292

wangzihao3 · 2019-11-19T01:45:01Z

…arm64

Modify the find_first_bit function in istgt_lu_disk.c, which is
implemented by using asm and cannot run on arm64. And add the
implementation by using c language.

Signed-off-by: wangzihao [email protected]

…arm64 Related to: openebs/openebs#1295 Modify the find_first_bit function in istgt_lu_disk.c, which is implemented by using asm and cannot run on arm64. And add the implementation by using c language. Signed-off-by: wangzihao <[email protected]>

mynktl · 2019-11-19T11:50:21Z

Hi @Wangzihao18
Thanks for the changes.

I think few more changes are required to build istgt for ARM architecture..

need to add rte files for ARM. openebs-archive/libcstor@a534beb
need update config.sub, config.guess. (automake --add-missing --copy --force-missing will generate the latest files)

diff --git a/src/istgt_lu.h b/src/istgt_lu.h
index a4144ec..690b7cc 100644
--- a/src/istgt_lu.h
+++ b/src/istgt_lu.h
@@ -57,7 +57,9 @@
 #include "ring_mempool.h"
 #endif
 
-#ifdef __linux__
+#if defined(__linux__) && (defined(__aarch64__) || defined(__arm__))
+#include <aarch64-linux-gnu/sys/queue.h>
+#else
 #include <x86_64-linux-gnu/sys/queue.h>
 #endif
 #include <stdbool.h>

Details of the system used for compilation :

System Information
	Manufacturer: FOXCONN
	Product Name: R2-1221R-A4
	Version: 1A21MT100-600-G

Linux 4.15.0-36-generic #39~16.04.1-Ubuntu SMP aarch64 aarch64 aarch64 GNU/Linux

wangzihao3 · 2019-11-19T12:39:10Z

Hi @Wangzihao18
Thanks for the changes.

I think few more changes are required to build istgt for ARM architecture..

need to add rte files for ARM. openebs/libcstor@a534beb

need update config.sub, config.guess. (automake --add-missing --copy --force-missing will generate the latest files)
diff --git a/src/istgt_lu.h b/src/istgt_lu.h
index a4144ec..690b7cc 100644
--- a/src/istgt_lu.h
+++ b/src/istgt_lu.h
@@ -57,7 +57,9 @@
 #include "ring_mempool.h"
 #endif
 
-#ifdef __linux__
+#if defined(__linux__) && (defined(__aarch64__) || defined(__arm__))
+#include <aarch64-linux-gnu/sys/queue.h>
+#else
 #include <x86_64-linux-gnu/sys/queue.h>
 #endif
 #include <stdbool.h>
Details of the system used for compilation :
System Information
	Manufacturer: FOXCONN
	Product Name: R2-1221R-A4
	Version: 1A21MT100-600-G
Linux 4.15.0-36-generic #39~16.04.1-Ubuntu SMP aarch64 aarch64 aarch64 GNU/Linux

Hi, @mynktl .Yes, thanks for your tips. Some changes can be seen in this commit wangzihao3@166463b. Because the modifications are little too much. I am not sure the changes are all right, So I split them up. I will continue update the other changes

mynktl · 2019-11-19T13:58:54Z

Ok @Wangzihao18 . Thanks for the information!

pawanpraka1 · 2019-11-19T16:37:25Z

src/istgt_lu_disk.c

-		res_final +=res;
-		if((unsigned long)res != nbits)
+		res_final += res;
+		if ((unsigned long)res != nbits)


can we fix it for this case also as you did for aarch64? Can we put a check if ((unsigned long)res != nbits << 6) here also.

pawanpraka1 · 2019-11-19T16:38:30Z

src/istgt_lu_disk.c

+				res += 64;
+			}
+		}
+		res_final += res;


lets have these 3 lines common for both the case and put it out of #ifdef.

pawanpraka1 · 2019-11-19T16:40:04Z

src/istgt_lu_disk.c

+				res += __builtin_ffsl(addr[idx]) - 1;
+				break;
+			} else {
+				res += 64;


use sizeof (addr[0]) or sizeof (*addr) instead of constant.

yeah, thanks @pawanpraka1 . These changes are great. I will update them.

One question is, the value of sizeof(*addr) is 8, but we should plus 64 here.

Should I use sizeof(*addr) * 8or other methods ?

good point. sizeof returns bytes, we should convert that to bits here.

we can do sizeof(*addr) << 3 and put some comments saying we need bits.

OK, I will update it now ^^

pawanpraka1 · 2019-11-20T06:13:25Z

src/istgt_lu_disk.c

 			return res_final;
+#elif defined(__aarch64__)


Also there is no aarch64 specific code here. Lets put this in #else section so that it works for all other architecture.

Oh yes , I am working on these codes.

ifdef. Signed-off-by: wangzihao <[email protected]>

pawanpraka1

looks good.

mynktl · 2019-11-21T06:50:53Z

@Wangzihao18
Can you raise the next PR for rte header files and remaining changes?
Thanks!

wangzihao3 · 2019-11-21T07:02:32Z

@Wangzihao18
Can you raise the next PR for rte header files and remaining changes?
Thanks!

Sure @mynktl . I am working on it now. I can build it successfully, but I don't know how to test it. I will raise the PR now.

mynktl · 2019-11-21T11:07:15Z

Ok @Wangzihao18 . We will test it in our lab.

mynktl approved these changes Nov 19, 2019

View reviewed changes

pawanpraka1 reviewed Nov 19, 2019

View reviewed changes

pawanpraka1 reviewed Nov 20, 2019

View reviewed changes

fix the error of check the value of res and put some common part out of

8b1c721

ifdef. Signed-off-by: wangzihao <[email protected]>

wangzihao3 requested review from pawanpraka1 and mynktl November 20, 2019 06:59

pawanpraka1 approved these changes Nov 20, 2019

View reviewed changes

mynktl approved these changes Nov 20, 2019

View reviewed changes

vishnuitta merged commit c3fa155 into openebs-archive:replication Nov 20, 2019

kmova modified the milestones: 1.6.0, 1.5.0 Jan 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add(function): modify find_first_bit function to support building on … #292

add(function): modify find_first_bit function to support building on … #292

wangzihao3 commented Nov 19, 2019

mynktl commented Nov 19, 2019 •

edited

Loading

wangzihao3 commented Nov 19, 2019

mynktl commented Nov 19, 2019

pawanpraka1 Nov 19, 2019

pawanpraka1 Nov 19, 2019 •

edited

Loading

pawanpraka1 Nov 19, 2019 •

edited

Loading

wangzihao3 Nov 20, 2019

wangzihao3 Nov 20, 2019

wangzihao3 Nov 20, 2019

pawanpraka1 Nov 20, 2019

pawanpraka1 Nov 20, 2019

wangzihao3 Nov 20, 2019

pawanpraka1 Nov 20, 2019

wangzihao3 Nov 20, 2019

pawanpraka1 left a comment

mynktl commented Nov 21, 2019

wangzihao3 commented Nov 21, 2019

mynktl commented Nov 21, 2019

add(function): modify find_first_bit function to support building on … #292

add(function): modify find_first_bit function to support building on … #292

Conversation

wangzihao3 commented Nov 19, 2019

mynktl commented Nov 19, 2019 • edited Loading

wangzihao3 commented Nov 19, 2019

mynktl commented Nov 19, 2019

Choose a reason for hiding this comment

pawanpraka1 Nov 19, 2019 • edited Loading

Choose a reason for hiding this comment

pawanpraka1 Nov 19, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pawanpraka1 left a comment

Choose a reason for hiding this comment

mynktl commented Nov 21, 2019

wangzihao3 commented Nov 21, 2019

mynktl commented Nov 21, 2019

mynktl commented Nov 19, 2019 •

edited

Loading

pawanpraka1 Nov 19, 2019 •

edited

Loading

pawanpraka1 Nov 19, 2019 •

edited

Loading