MPC5200b jffs2 memcpy alignment problem

Hello!

First of all, please apologize my cross posting.

I have a problem running jffs2 on an MPC5200b board. I run kernel 3.4, but older kernels like 3.1.5 are also affected. Every time I mount jffs2, previously written content gets garbled.

The problem was nailed down to memcpy(&fd->name, rd->name, checkedlen); in jffs2_scan_dirent_node in fs/jffs2/scan.c.

This is a copy directly from the mapped flash.

memcpy from copy_32.S only ensures that the 32 bit stores to the destination ptr are aligned. And that's the problem with the MPC5200b because the user manual (Chapter 9.2, LocalPlus bus features) clearly states that unaligned access is not supported. But the code above results in unaligned loads from the LocalPlus bus.

I think there are two possible solutions:

1. Rewrite memcpy this way: Only if both src and dst are 32 bit aligned, use 32 bit load/store to copy, otherwise use byte load/store instructions. (btw. is there a reason for not to use the load multiple and store multiple instructions in memcpy?) This might be the best way to do because in my opinion the semantics of memcpy put no restrictions on the alignment of src and dst.

2. use memcpy_fromio in the jffs2 code. memcpy_fromio behaves exactly in the way I described above. This could be also a good solution because flash access via LocalPlus bus is clearly IO.

What is your opinion where to fix this problem?

Regards,

Stephan

Hi Stephan:

Am 30.06.12 21:16 schrieb(en) Stephan Gatzka:

I have a problem running jffs2 on an MPC5200b board. I run kernel 3.4, but older kernels like 3.1.5 are also affected. Every time I mount jffs2, previously written content gets garbled.

The problem was nailed down to memcpy(&fd->name, rd->name, checkedlen); in jffs2_scan_dirent_node in fs/jffs2/scan.c.

[snip]

2. use memcpy_fromio in the jffs2 code. memcpy_fromio behaves exactly in the way I described above. This could be also a good solution because flash access via LocalPlus bus is clearly IO.

I don't recall who proposed this patch, but exactly this solution is around for a longer time (mayby you search archives...).  On my board, I have a flash chip attached to the LocalBus in 16-bit mode.  Based on 3.2.16, the patch is:

---8<----------------------------------------------------------------------------

--- linux-3.2.16-orig/fs/jffs2/scan.c   2012-04-23 00:31:32.000000000 +0200
+++ linux-3.2.16/fs/jffs2/scan.c        2012-04-27 13:23:06.000000000 +0200

@@ -509,7 +509,11 @@
                                        sumptr = kmalloc(sumlen, GFP_KERNEL);
                                        if (!sumptr)
                                                return -ENOMEM;
+#ifdef CONFIG_PPC_MPC52xx
+                                       memcpy_fromio(sumptr + sumlen - buf_len, buf + buf_size - buf_len, buf_len);
+#else
                                        memcpy(sumptr + sumlen - buf_len, buf + buf_size - buf_len, buf_len);
+#endif
                                }
                                if (buf_len < sumlen) {
                                        /* Need to read more so that the entire summary node is present */

@@ -1039,7 +1043,11 @@
        if (!fd) {
                return -ENOMEM;
        }
+#ifdef CONFIG_PPC_MPC52xx
+       memcpy_fromio(&fd->name, rd->name, checkedlen);
+#else
        memcpy(&fd->name, rd->name, checkedlen);
+#endif
        fd->name[checkedlen] = 0;

        crc = crc32(0, fd->name, rd->nsize);
---8<----------------------------------------------------------------------------

Works perfectly with it...

Hope this helps,
Albrecht.

Hi Albrecht,

I don't recall who proposed this patch, but exactly this solution is
around for a longer time (mayby you search archives...).  On my board, I
have a flash chip attached to the LocalBus in 16-bit mode.  Based on
3.2.16, the patch is:

Thanks for your answer and yes, this patch will definitely work. But I want to have a solution in the mainline kernel, that's why I'm asking how to deal best with this problem.

Regards,

Stephan

(SPAM)


Em 01-07-2012 03:47, Stephan Gatzka escreveu:

Hi Albrecht,

quoted

I don't recall who proposed this patch, but exactly this solution is
around for a longer time (mayby you search archives...).  On my board, I
have a flash chip attached to the LocalBus in 16-bit mode.  Based on
3.2.16, the patch is:

Thanks for your answer and yes, this patch will definitely work. But I want to have a solution in the mainline kernel, that's why I'm asking how to deal best with this problem.

Regards,

Stephan



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Hi Stephan,

On Sun, 01 Jul 2012 08:47:01 +0200
Stephan Gatzka [off-list ref] wrote:

Hi Albrecht,

 > I don't recall who proposed this patch, but exactly this solution is
 > around for a longer time (mayby you search archives...).  On my board, I
 > have a flash chip attached to the LocalBus in 16-bit mode.  Based on
 > 3.2.16, the patch is:

Thanks for your answer and yes, this patch will definitely work. But I 
want to have a solution in the mainline kernel, that's why I'm asking 
how to deal best with this problem.

This problem has been discussed several times [1], [2], but wasn't
resolved yet. The clean solution suggested was to implement a custom
mapping driver [3].

Thanks,
Anatolij

[1] http://thread.gmane.org/gmane.linux.drivers.mtd/21521
[2] http://thread.gmane.org/gmane.linux.ports.ppc.embedded/36324
[3] http://thread.gmane.org/gmane.linux.ports.ppc.embedded/36324/focus=36608

Hi!

This problem has been discussed several times [1], [2], but wasn't
resolved yet. The clean solution suggested was to implement a custom
mapping driver [3].

Then I'll do that. It doesn't look terribly complicated.

It would be helpful if someone can test the first version if it's available.

Regards,

Stephan