Fix pointer cast for 32 bits arch

Sparse compalins about casting void * to u64 on i386. Change the cast to resource_size_t.

Signed-off-by Peter Senna Tschudin <[email protected]>

---

Tested by compilation only. Tested for x86 and x86_64.

 drivers/staging/goldfish/goldfish_nand.c | 3 ++-
 include/linux/goldfish.h                 | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

drivers/staging/goldfish/goldfish_nand.c +0 -0

index d68f216..738fdc4 100644

Quoted 12 lines · Peter Senna Tschudin

Sparse compalins about casting void * to u64 on i386. Change the cast to resource_size_t.

Signed-off-by Peter Senna Tschudin <[email protected]>

---

Tested by compilation only. Tested for x86 and x86_64.

 drivers/staging/goldfish/goldfish_nand.c | 3 ++-
 include/linux/goldfish.h                 | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

drivers/staging/goldfish/goldfish_nand.c static u32 goldfish_nand_cmd(struct mtd_info *mtd, enum nand_cmd cmd, · line 91

Quoted 11 lines

drivers/staging/goldfish/goldfish_nand.c +2 -1

index d68f216..738fdc4 100644
@@ -87,7 +87,8 @@ static u32 goldfish_nand_cmd(struct mtd_info *mtd, enum nand_cmd cmd,
                writel((u32)(addr >> 32), base + NAND_ADDR_HIGH);
                writel((u32)addr, base + NAND_ADDR_LOW);
                writel(len, base + NAND_TRANSFER_SIZE);
-               gf_write64((u64)ptr, base + NAND_DATA, base + NAND_DATA_HIGH);
+               gf_write64((resource_size_t)ptr, base + NAND_DATA,
+                          base + NAND_DATA_HIGH);

I guess sparse still complains if CONFIG_X86_PAE=y, which makes resource_size_t u64?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Quoted 14 lines · Geert Uytterhoeven

Quoted 12 lines · Peter Senna Tschudin

Sparse compalins about casting void * to u64 on i386. Change the cast to resource_size_t.

Signed-off-by Peter Senna Tschudin <[email protected]>

---

Tested by compilation only. Tested for x86 and x86_64.

 drivers/staging/goldfish/goldfish_nand.c | 3 ++-
 include/linux/goldfish.h                 | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

Quoted 14 lines

drivers/staging/goldfish/goldfish_nand.c static u32 goldfish_nand_cmd(struct mtd_info *mtd, enum nand_cmd cmd, · line 91

Quoted 11 lines

drivers/staging/goldfish/goldfish_nand.c +2 -1

index d68f216..738fdc4 100644
@@ -87,7 +87,8 @@ static u32 goldfish_nand_cmd(struct mtd_info *mtd, enum nand_cmd cmd,
                writel((u32)(addr >> 32), base + NAND_ADDR_HIGH);
                writel((u32)addr, base + NAND_ADDR_LOW);
                writel(len, base + NAND_TRANSFER_SIZE);
-               gf_write64((u64)ptr, base + NAND_DATA, base + NAND_DATA_HIGH);
+               gf_write64((resource_size_t)ptr, base + NAND_DATA,
+                          base + NAND_DATA_HIGH);

I guess sparse still complains if CONFIG_X86_PAE=y, which makes resource_size_t u64?

Yes, when CONFIG_X86_PAE=y the patch doesn't fix the warning(gcc warning). What is the correct/portable way of fixing this?

Perhaps checkpatch should complain about casts outside header files?

Quoted 1 line · Geert Uytterhoeven

Perhaps checkpatch should complain about casts outside header files?

I think that's not feasible/desirable.

type casting is pretty common and necessary.
There are 50k+ casts in drivers/ alone.

Quoted 31 lines · Peter Senna Tschudin

Quoted 14 lines · Geert Uytterhoeven

Quoted 12 lines · Peter Senna Tschudin

Sparse compalins about casting void * to u64 on i386. Change the cast to resource_size_t.

Signed-off-by Peter Senna Tschudin <[email protected]>

---

Tested by compilation only. Tested for x86 and x86_64.

 drivers/staging/goldfish/goldfish_nand.c | 3 ++-
 include/linux/goldfish.h                 | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

Quoted 14 lines

drivers/staging/goldfish/goldfish_nand.c static u32 goldfish_nand_cmd(struct mtd_info *mtd, enum nand_cmd cmd, · line 91

Quoted 11 lines

drivers/staging/goldfish/goldfish_nand.c +2 -1

index d68f216..738fdc4 100644
@@ -87,7 +87,8 @@ static u32 goldfish_nand_cmd(struct mtd_info *mtd, enum nand_cmd cmd,
                writel((u32)(addr >> 32), base + NAND_ADDR_HIGH);
                writel((u32)addr, base + NAND_ADDR_LOW);
                writel(len, base + NAND_TRANSFER_SIZE);
-               gf_write64((u64)ptr, base + NAND_DATA, base + NAND_DATA_HIGH);
+               gf_write64((resource_size_t)ptr, base + NAND_DATA,
+                          base + NAND_DATA_HIGH);

I guess sparse still complains if CONFIG_X86_PAE=y, which makes resource_size_t u64?

Quoted 2 lines

Yes, when CONFIG_X86_PAE=y the patch doesn't fix the warning(gcc warning). What is the correct/portable way of fixing this?

As gf_write64() takes "unsigned long data" for its first argument, why not simply cast ptr to "unsigned long"? "void *" and "unsigned long" have the same size on all Linux platforms.

Seems like there a several silly casting tricks being done in the goldfish drivers:

$ git grep -w gf_write64drivers/platform/goldfish/goldfish_pipe.c:
 gf_write64((u64)(unsigned long)p
drivers/platform/goldfish/goldfish_pipe.c:      gf_write64((u64)(unsigned long)p
drivers/platform/goldfish/goldfish_pipe.c:                      gf_write64((u64)
drivers/platform/goldfish/goldfish_pipe.c:                      gf_write64(addre
drivers/staging/goldfish/goldfish_audio.c:      (gf_write64((u64)(x), data->reg_
drivers/staging/goldfish/goldfish_nand.c:               gf_write64((u64)ptr, bas
drivers/tty/goldfish.c: gf_write64((u64)buf, base + GOLDFISH_TTY_DATA_PTR,
drivers/tty/goldfish.c: gf_write64((u64)buf, base + GOLDFISH_TTY_DATA_PTR,
include/linux/goldfish.h:static inline void gf_write64(unsigned long data,

with interesting history showing one wrong cast being "fixed" using another wrong cast:

commit f4e131dc38d34469b9fee1840ad237324831bce3

Author: Octavian Purdila <[email protected]>
Date:   Fri May 16 08:24:59 2014 +0300

goldfish: pipe: fix warnings for 32bit builds

(quoted diff) +1 -1

-       gf_write64((u64)pipe, dev->base + PIPE_REG_CHANNEL,
+       gf_write64((u64)(unsigned long)pipe, dev->base + PIPE_REG_CHANNEL,

As in general casts don't belong in drivers[*], what about creating an inline helper gf_set_pipe() that converts the pointer to unsigned long and calls gf_write64()?

Perhaps checkpatch should complain about casts outside header files?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Quoted 1 line

Perhaps checkpatch should complain about casts outside header files?

Cocinelle found 664 double casts:
http://pastebin.com/2bi9Dg7k

and 9 triple casts:
http://pastebin.com/RkJhPTTV

Those are 'not' patches, just the output of Coccinelle for analysis. The .cocci for triple cast:
@@
expression e1, e2;
type t1, t2, t3;
@@
e1 =
- (t1)(t2)(t3)
e2

So if there are cast patterns known to be wrong, it is easy to find and probably fix them with Coccinelle.

Hi Peter,

Quoted 6 lines · Peter Senna Tschudin

Quoted 1 line

Perhaps checkpatch should complain about casts outside header files?

Cocinelle found 664 double casts:
http://pastebin.com/2bi9Dg7k

and 9 triple casts:
http://pastebin.com/RkJhPTTV

I think double casts are only needed when casting between integers and pointers of different sizes:
1. One cast to make the sizes match,
2. One cast to convert between pointer and integer.

I think triple casts can always be simplified.
Or am I missing some use cases?

Quoted 10 lines

Those are 'not' patches, just the output of Coccinelle for analysis. The .cocci for triple cast:
@@
expression e1, e2;
type t1, t2, t3;
@@
e1 =
- (t1)(t2)(t3)
e2

Quoted 2 lines

So if there are cast patterns known to be wrong, it is easy to find and probably fix them with Coccinelle.

It depends on the original type and on the destination type.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Line 4677 of http://pastebin.com/2bi9Dg7k looks like a bug

diff -u -p a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
- regs->nip = (unsigned long) (unsigned long)ksig->ka.sa.sa_handler;

drivers/staging/goldfish/goldfish_audio.c struct goldfish_audio { · line 66

Quoted 8 lines · Peter Senna Tschudin

drivers/staging/goldfish/goldfish_audio.c +1 -1

@@ -63,7 +63,7 @@ struct goldfish_audio {
 #define AUDIO_READ(data, addr)         (readl(data->reg_base + addr))
 #define AUDIO_WRITE(data, addr, x)     (writel(x, data->reg_base + addr))
 #define AUDIO_WRITE64(data, addr, addr2, x)    \
-       (gf_write64((u64)(x), data->reg_base + addr, data->reg_base+addr2))
+       (gf_write_ptr((void *)(x), data->reg_base + addr, data->reg_base+addr2))

This one should not be converted, as all callers pass a dma_addr_t, which may be 64-bit on 32-bit systems, i.e. larger than void *.

Quoted 13 lines · Geert Uytterhoeven

drivers/staging/goldfish/goldfish_audio.c struct goldfish_audio { · line 66

Quoted 8 lines · Peter Senna Tschudin

drivers/staging/goldfish/goldfish_audio.c +1 -1

@@ -63,7 +63,7 @@ struct goldfish_audio {
 #define AUDIO_READ(data, addr)         (readl(data->reg_base + addr))
 #define AUDIO_WRITE(data, addr, x)     (writel(x, data->reg_base + addr))
 #define AUDIO_WRITE64(data, addr, addr2, x)    \
-       (gf_write64((u64)(x), data->reg_base + addr, data->reg_base+addr2))
+       (gf_write_ptr((void *)(x), data->reg_base + addr, data->reg_base+addr2))

This one should not be converted, as all callers pass a dma_addr_t, which may be 64-bit on 32-bit systems, i.e. larger than void *.

Ugh... You're right.

I've been avoiding asking this but I can't any longer. What is gf_write64() actually doing? We are writing dma addresses, user space pointers and kernel space pointers to this hardware?

This stuff doesn't seem to make any kind of sense and I can easily imagine a situation where it wrote a 64 bit pointer. Then we partially write over it with a 32 bit userspace pointer. Then it writes somewhere totally unintended.

This thing doesn't make any sort of sense to me.

Quoted 21 lines · Dan Carpenter

Quoted 13 lines · Geert Uytterhoeven

drivers/staging/goldfish/goldfish_audio.c struct goldfish_audio { · line 66

Quoted 8 lines · Peter Senna Tschudin

drivers/staging/goldfish/goldfish_audio.c +1 -1

@@ -63,7 +63,7 @@ struct goldfish_audio {
 #define AUDIO_READ(data, addr)         (readl(data->reg_base + addr))
 #define AUDIO_WRITE(data, addr, x)     (writel(x, data->reg_base + addr))
 #define AUDIO_WRITE64(data, addr, addr2, x)    \
-       (gf_write64((u64)(x), data->reg_base + addr, data->reg_base+addr2))
+       (gf_write_ptr((void *)(x), data->reg_base + addr, data->reg_base+addr2))

This one should not be converted, as all callers pass a dma_addr_t, which may be 64-bit on 32-bit systems, i.e. larger than void *.

Ugh... You're right.

I've been avoiding asking this but I can't any longer. What is gf_write64() actually doing? We are writing dma addresses, user space pointers and kernel space pointers to this hardware?

Quoted 6 lines

This stuff doesn't seem to make any kind of sense and I can easily imagine a situation where it wrote a 64 bit pointer. Then we partially write over it with a 32 bit userspace pointer. Then it writes somewhere totally unintended.

This thing doesn't make any sort of sense to me.

Its a 64 on 64 or 32 on 32 virtual machine. Goldfish is used for Android emulation for all the system level phone emulation tools. On the emulation side it provides an interface for the emulated OS but makes no effort to emulate it as if it was a real hardware. If you think of it as a funky emulator interface all is good. If you think about it as "hardware" you've got the wrong model and chunks of Goldfish make less sense.

Quoted 29 lines · Alan Cox

Quoted 21 lines · Dan Carpenter

Quoted 13 lines · Geert Uytterhoeven

drivers/staging/goldfish/goldfish_audio.c struct goldfish_audio { · line 66

Quoted 8 lines · Peter Senna Tschudin

drivers/staging/goldfish/goldfish_audio.c +1 -1

@@ -63,7 +63,7 @@ struct goldfish_audio {
 #define AUDIO_READ(data, addr)         (readl(data->reg_base + addr))
 #define AUDIO_WRITE(data, addr, x)     (writel(x, data->reg_base + addr))
 #define AUDIO_WRITE64(data, addr, addr2, x)    \
-       (gf_write64((u64)(x), data->reg_base + addr, data->reg_base+addr2))
+       (gf_write_ptr((void *)(x), data->reg_base + addr, data->reg_base+addr2))

This one should not be converted, as all callers pass a dma_addr_t, which may be 64-bit on 32-bit systems, i.e. larger than void *.

Ugh... You're right.

I've been avoiding asking this but I can't any longer. What is gf_write64() actually doing? We are writing dma addresses, user space pointers and kernel space pointers to this hardware?

Quoted 6 lines

This stuff doesn't seem to make any kind of sense and I can easily imagine a situation where it wrote a 64 bit pointer. Then we partially write over it with a 32 bit userspace pointer. Then it writes somewhere totally unintended.

This thing doesn't make any sort of sense to me.

Quoted 7 lines

Its a 64 on 64 or 32 on 32 virtual machine. Goldfish is used for Android emulation for all the system level phone emulation tools. On the emulation side it provides an interface for the emulated OS but makes no effort to emulate it as if it was a real hardware. If you think of it as a funky emulator interface all is good. If you think about it as "hardware" you've got the wrong model and chunks of Goldfish make less sense.

Is is better to leave the code as is, and ignore the compiler / sparse warnings for i386? Or is the proposal welcome if done correctly? And if so what would be correctly?

As the first argument of gf_write64() was of type unsigned long, and as some calls to gf_write64() were casting the first argument from void * to u64 the compiler and/or sparse were printing warnings for casts of wrong sizes when compiling for i386.

This patch changes the type of the first argument of gf_write64() to void *, and update calls to the function. This change fixed the warnings and allowed to remove casts from 6 calls to gf_write64().

In addition gf_write64() was renamed to gf_write_ptr() as the name was misleading because it only writes 32 bits on 32 bit systems.

gf_write_dma_addr() was added to handle dma_addr_t values which is used at drivers/staging/goldfish/goldfish_audio.c.

Signed-off-by Dan Carpenter <[email protected]>

Signed-off-by Peter Senna Tschudin <[email protected]>

---
Tested by compilation only for x86 and for x86_64.

Its a 64 on 64 or 32 on 32 virtual machine. Goldfish is used for Android emulation for all the system level phone emulation tools. On the emulation side it provides an interface for the emulated OS but makes no effort to emulate it as if it was a real hardware. If you think of it as a funky emulator interface all is good. If you think about it as "hardware" you've got the wrong model and chunks of Goldfish make less sense.

Actually, my patch seems like a good idea to me but it's one of those things that someone should probably test. Unless someone can test goldfish on a 32 bit system with 64 bit dma addresses

Quoted 3 lines · Dan Carpenter

Actually, my patch seems like a good idea to me but it's one of those things that someone should probably test. Unless someone can test goldfish on a 32 bit system with 64 bit dma addresses

No such "system" exists.

Quoted 6 lines · Alan Cox

Quoted 3 lines · Dan Carpenter

Actually, my patch seems like a good idea to me but it's one of those things that someone should probably test. Unless someone can test goldfish on a 32 bit system with 64 bit dma addresses

No such "system" exists.

I don't understand. We definitely can have 64bit dma addresses on x86_32.

Quoted 6 lines · Dan Carpenter

Quoted 5 lines · Alan Cox

Quoted 3 lines · Dan Carpenter

Actually, my patch seems like a good idea to me but it's one of those things that someone should probably test. Unless someone can test goldfish on a 32 bit system with 64 bit dma addresses

Quoted 4 lines

Quoted 1 line

No such "system" exists.

I don't understand. We definitely can have 64bit dma addresses on x86_32.

Yes but no actual Goldfish environment is built that way

Quoted 7 lines · Alan Cox

Quoted 6 lines · Dan Carpenter

Quoted 5 lines · Alan Cox

Quoted 3 lines · Dan Carpenter

Actually, my patch seems like a good idea to me but it's one of those things that someone should probably test. Unless someone can test goldfish on a 32 bit system with 64 bit dma addresses

Quoted 7 lines

Quoted 4 lines

Quoted 1 line

No such "system" exists.

I don't understand. We definitely can have 64bit dma addresses on x86_32.

Yes but no actual Goldfish environment is built that way

Isn't this a simpler fix?

Sparse compalins about casting void * to u64 on i386. Change the cast to resource_size_t.

Signed-off-by Peter Senna Tschudin <[email protected]>

---

Tested by compilation only. Tested for x86 and x86_64.

 drivers/staging/goldfish/goldfish_nand.c | 3 ++-
 include/linux/goldfish.h                 | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

drivers/staging/goldfish/goldfish_nand.c +0 -0

index d68f216..738fdc4 100644

Quoted 17 lines · Peter Senna Tschudin

Sparse compalins about casting void * to u64 on i386. Change the cast to resource_size_t.

Signed-off-by Peter Senna Tschudin <[email protected]>

---

Tested by compilation only. Tested for x86 and x86_64.

 drivers/staging/goldfish/goldfish_nand.c | 3 ++-
 include/linux/goldfish.h                 | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

drivers/staging/goldfish/goldfish_nand.c +0 -0

index d68f216..738fdc4 100644

drivers/staging/goldfish/goldfish_nand.c static u32 goldfish_nand_cmd(struct mtd_info *mtd, enum nand_cmd cmd, · line 91

Quoted 6 lines

drivers/staging/goldfish/goldfish_nand.c +2 -1

@@ -87,7 +87,8 @@ static u32 goldfish_nand_cmd(struct mtd_info *mtd, enum nand_cmd cmd,
              writel((u32)(addr >> 32), base + NAND_ADDR_HIGH);
              writel((u32)addr, base + NAND_ADDR_LOW);
              writel(len, base + NAND_TRANSFER_SIZE);
-             gf_write64((u64)ptr, base + NAND_DATA, base + NAND_DATA_HIGH);
+             gf_write64((resource_size_t)ptr, base + NAND_DATA,
+                        base + NAND_DATA_HIGH);

write64 is a misleading name because it only writes 32 bits on 32 bit systems.

Are you sure it's a resource_size_t? If that's really true then this code needs a lot more work. The ifdef in gf_write64() should be CONFIG_PHYS_ADDR_T_64BIT and ops->datbuf needs to updated. It would be a lot of changes...

I *think* but I'm not positive that gf_write64 should just take a void pointer like this:

drivers/staging/goldfish/goldfish_nand.c +1 -1

index 213877a..92d6479 100644
@@ -87,7 +87,7 @@ static u32 goldfish_nand_cmd(struct mtd_info *mtd, enum nand_cmd cmd,
                writel((u32)(addr >> 32), base + NAND_ADDR_HIGH);
                writel((u32)addr, base + NAND_ADDR_LOW);
                writel(len, base + NAND_TRANSFER_SIZE);
-               gf_write64((u64)ptr, base + NAND_DATA, base + NAND_DATA_HIGH);
+               gf_write64(ptr, base + NAND_DATA, base + NAND_DATA_HIGH);
                writel(cmd, base + NAND_COMMAND);
                rv = readl(base + NAND_RESULT);
        }

include/linux/goldfish.h +1 -1

index 569236e..4cdb913 100644
@@ -3,12 +3,12 @@
 
 /* Helpers for Goldfish virtual platform */
 
-static inline void gf_write64(unsigned long data,
+static inline void gf_write64(void *data,

Quoted 57 lines · Dan Carpenter

Quoted 17 lines · Peter Senna Tschudin

Sparse compalins about casting void * to u64 on i386. Change the cast to resource_size_t.

Signed-off-by Peter Senna Tschudin <[email protected]>

---

Tested by compilation only. Tested for x86 and x86_64.

 drivers/staging/goldfish/goldfish_nand.c | 3 ++-
 include/linux/goldfish.h                 | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

drivers/staging/goldfish/goldfish_nand.c +0 -0

index d68f216..738fdc4 100644

drivers/staging/goldfish/goldfish_nand.c static u32 goldfish_nand_cmd(struct mtd_info *mtd, enum nand_cmd cmd, · line 91

Quoted 6 lines

drivers/staging/goldfish/goldfish_nand.c +2 -1

@@ -87,7 +87,8 @@ static u32 goldfish_nand_cmd(struct mtd_info *mtd, enum nand_cmd cmd,
              writel((u32)(addr >> 32), base + NAND_ADDR_HIGH);
              writel((u32)addr, base + NAND_ADDR_LOW);
              writel(len, base + NAND_TRANSFER_SIZE);
-             gf_write64((u64)ptr, base + NAND_DATA, base + NAND_DATA_HIGH);
+             gf_write64((resource_size_t)ptr, base + NAND_DATA,
+                        base + NAND_DATA_HIGH);

write64 is a misleading name because it only writes 32 bits on 32 bit systems.

Are you sure it's a resource_size_t? If that's really true then this code needs a lot more work. The ifdef in gf_write64() should be CONFIG_PHYS_ADDR_T_64BIT and ops->datbuf needs to updated. It would be a lot of changes...

I *think* but I'm not positive that gf_write64 should just take a void pointer like this:

drivers/staging/goldfish/goldfish_nand.c +1 -1

index 213877a..92d6479 100644
@@ -87,7 +87,7 @@ static u32 goldfish_nand_cmd(struct mtd_info *mtd, enum nand_cmd cmd,
                writel((u32)(addr >> 32), base + NAND_ADDR_HIGH);
                writel((u32)addr, base + NAND_ADDR_LOW);
                writel(len, base + NAND_TRANSFER_SIZE);
-               gf_write64((u64)ptr, base + NAND_DATA, base + NAND_DATA_HIGH);
+               gf_write64(ptr, base + NAND_DATA, base + NAND_DATA_HIGH);
                writel(cmd, base + NAND_COMMAND);
                rv = readl(base + NAND_RESULT);
        }

include/linux/goldfish.h +0 -0

index 569236e..4cdb913 100644
@@ -3,12 +3,12 @@
 
 /* Helpers for Goldfish virtual platform */
 

include/linux/goldfish.h line 14

Quoted 12 lines

include/linux/goldfish.h +3 -3

-static inline void gf_write64(unsigned long data,
+static inline void gf_write64(void *data,
                void __iomem *portl, void __iomem *porth)
 {
-       writel((u32)data, portl);
+       writel((u32)(unsigned long)data, portl);
 #ifdef CONFIG_64BIT
-       writel(data>>32, porth);
+       writel((unsigned long)data >> 32, porth);
 #endif
 }
 

I like this solution. Should I send a single patch changing gf_write64() and removing all casts when calling the gf_write64(), or if to split, how to split the patch?