arm: align shared memory unconditionally to the SHMLBA boundary

Currently IPC SHM works in a strange way on ARM:
the syscall sys_shmat() requires the argument shmaddr be SHMLBA-aligned (ARM has the macro __ARCH_FORCE_SHMLBA unconditionally defined) but allocates memory that isn't SHMLBA-aligned because the value of memory alignment depends on presense of certain cache aliases.

Such a behavior raises the following problem. Consider the program:

static int p[2];
static pid_t task2_pid;

static int task1()

{
	int task2_status;
	int *n;
	int shmid;

	shmid = shmget(0x94646337, 40960, IPC_CREAT);
	if (shmid < 0) {
		perror("Failed to get a SHM descriptor");
		return 10;
	}

	n = (int*)shmat(shmid, 0, 0);
	if (n == (int*)-1) {
		perror("task1: failed to attach the SHM segment");
		return 11;
	}

	*n = MAGIC;

	shmdt(n);

	if (write(p[1], &n, sizeof(n)) != sizeof(n))
	{
		perror("task1: failed to send the SHM address");
		return 12;
	}

	if (waitpid(task2_pid, &task2_status, 0) != task2_pid)
	{
		perror("task2: failed to wait for task2");
		return 13;
	}

	if (!WIFEXITED(task2_status))
	{
		printf("task1: task2 hasn't exited!\n");
		return 14;
	}

	printf("task1: task2 exited with code %d, "
		"the SHM was attached to %p.\n",
		WEXITSTATUS(task2_status), n);

	return 0;
}

static int task2()

{
	int *n = 0;
	int res;
	void *addr;
	int shmid;

	shmid = shmget(0x94646337, 40960, IPC_CREAT);
	if (shmid < 0) {
		perror("Failed to get a SHM descriptor");
		return 20;
	}

	if (read(p[0], &n, sizeof(n)) != sizeof(n))
	{
		perror("task2: failed to receive the SHM address");
		return 21;
	}

	addr = shmat(shmid, n, 0);
	if (addr == (void*)-1)
	{
		printf("task2: failed to attach the SHM to the address %p: %s\n",
			n, strerror(errno));
		res = 22;
		goto exit_task2;
	}

	if ((void*)n != addr)
	{
		printf("task2: the SHM isn't attached where it's expected:"
				"expected %p, got %p\n", n, addr);
		res = 23;
		goto exit_task2;
	}

	if (*n != MAGIC)
	{
		printf("task2: *n (%d) isn't %d\n", *n, MAGIC);
		res = 24;
		goto exit_task2;
	}

exit_task2:

	shmdt(addr);

	return res;
}

int main()

{
	int res = 0;

	if (pipe(p) < 0)
	{
		perror("Failed to create a pipe");
		return 2;
	}

	task2_pid = fork();
	if (task2_pid)
		res = task1();
	else
		res = task2();

	close(p[0]);
	close(p[1]);

	return res;
}

This program sometimes fails generating the output like that:

root at test:~/shmfail# ./shmfail
task2: failed to attach the SHM to the address 0xb6e3a000: Invalid argument
task1: task2 exited with code 22, the SHM was attached to 0xb6e3a000.

root at test:~/shmfail# ./shmfail
task2: failed to attach the SHM to the address 0xb6e63000: Invalid argument
task1: task2 exited with code 22, the SHM was attached to 0xb6e63000.

and sometimes succeeds:

root at test:~/shmfail# ./shmfail
task1: task2 exited with code 0, the SHM was attached to 0xb6e94000.

The problem is reproducible in the mainstream kernel branch as well as in a stable (3.7.5) kernel release. The program isn't reproducible on x86_64.

As you can see the program fails when the address returned by the function shmat() isn't SHMLBA-aligned.

This patch makes file-mmapped memory be unconditionaly SHMLBA-aligned.

---
 arch/arm/mm/mmap.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

index 10062ce..eca577e3 100644
@@ -65,8 +65,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
 	 * We only need to do colour alignment if either the I or D
 	 * caches alias.
 	 */
-	if (aliasing)
-		do_align = filp || (flags & MAP_SHARED);
+	do_align = filp || (flags & MAP_SHARED);
 
 	/*
 	 * We enforce the MAP_FIXED case.

1.7.9.5

Hi guys, is there some conclusion on this patch? It has been sent almost 5 months back ;)

It's pointless. The alignment is only required for CPUs which have aliasing caches. What the code does in mmap() is correct. However, we can't dynamically select this in the SHM code.

Hi Russell, thanks a lot for reply! I must admit I'm not that familiar with arm hw internals, but if the behaviour is correct than we've a problem with checkpoint/restore procedure on arm platform :( As far as I know Alexander pointed me that he find no other way of fixing the problem except patching the kernel. Alexander, mind to remind where exactly we're failing in c/r procedure, thus maybe arm guys would help us to find a way to pass it by with some user-space trick?

It isn't always possible to pin an IPC SHM region using shmat() at the address returned by shmget() so sometimes it's impossible to restore a process that uses the IPC SHM.

Sincerely yours,
Alexander Kartashov

Intern
Core team

www.parallels.com

Skype: aleksandr.kartashov
Email: alekskartashov at parallels.com

Does it mean that we simply can't checkpoint and restore on same node (ie same arm family) if this patch is not applied, right? Is there any possibility to somehow workaround this problem completely in user-space?

Yes, it does.

No, it's impossible since an IPC SHM region is allocated by the routine shmget() in the dumpee; if the routine returns an incorrectly aligned address it's impossible to reattach the region at this address while restoring the dumpee.

Sincerely yours,
Alexander Kartashov

Intern
Core team

www.parallels.com

Skype: aleksandr.kartashov
Email: alekskartashov at parallels.com

shmget() doesn't allocate space in the process for the SHM region. It merely creates the shm memory and returns an identifier for it which can later be used by shmat() to map it.

Thank you for the correction, I meant shmat() in that comment indeed. I'm sorry for the inconvenience.

Sincerely yours,
Alexander Kartashov

Intern
Core team

www.parallels.com

Skype: aleksandr.kartashov
Email: alekskartashov at parallels.com

Right, so it appears that there's a difference between shmat() with an address and with a NULL address, because the enforcement is done by two completely different bits of code in unrelated parts of the kernel.

I notice Sparc32 seems to have a fix for this along the lines of the (untested) patch below, so let's do the same on ARM. Please test this and let me know if this solves your problem.

Thanks.

 arch/arm/include/asm/shmparam.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

index a5223b3..843db59 100644
@@ -1,16 +1,16 @@
 #ifndef _ASMARM_SHMPARAM_H
 #define _ASMARM_SHMPARAM_H
 
+#include <asm/cachetype.h>
+
 /*
  * This should be the size of the virtually indexed cache/ways,
  * or page size, whichever is greater since the cache aliases
  * every size/ways bytes.
  */
-#define	SHMLBA	(4 * PAGE_SIZE)		 /* attach addr a multiple of this */
+#define	SHMLBA	(cache_is_vipt_aliasing() ? (4 * PAGE_SIZE) : PAGE_SIZE)
 
-/*
- * Enforce SHMLBA in shmat
- */
+/* Enforce SHMLBA in shmat */
 #define __ARCH_FORCE_SHMLBA
 
 #endif /* _ASMARM_SHMPARAM_H */

Dear Russell,

I'm sorry for the delayed response.

Nevertheless, removing these lines fixes both the compilation and shared memory alignment issue. Thank you.

Sincerely yours,
Alexander Kartashov

Intern
Core team

www.parallels.com

Skype: aleksandr.kartashov
Email: alekskartashov at parallels.com