android ashmem patch [review request]

android ashmem patch [review request]

[Akers, Jason B]

I wrapped the ashmem calls with the existing shm* function definitions. It's a little ugly, but makes for an easy patch touching only os/os-android.h.

Tested on both arm and x86 android phones.

Jason Akers


--- a/os/os-android.h	2013-04-30 14:30:05.014461848 -0700
+++ b/os/os-android.h	2013-04-30 14:24:49.442213734 -0700
@@ -57,24 +57,65 @@
 #include <linux/shm.h>
 #define SHM_HUGETLB    04000
 
+#include <stdio.h>
+#include <linux/ashmem.h>
+#include <sys/mman.h>
+
+#define ASHMEM_DEVICE	"/dev/ashmem"
+
 static inline int shmctl (int __shmid, int __cmd, struct shmid_ds *__buf)
 {
-	return syscall(__NR_shmctl, __shmid, __cmd, __buf);
+	int ret=0;
+	if (__cmd == IPC_RMID)
+	{
+		int length = ioctl(__shmid, ASHMEM_GET_SIZE, NULL);
+		struct ashmem_pin pin = {0 , length};
+		ret = ioctl(__shmid, ASHMEM_UNPIN, &pin);
+		close(__shmid);
+	}
+	return ret;
 }
 
 static inline int shmget (key_t __key, size_t __size, int __shmflg)
 {
-	return syscall(__NR_shmget, __key, __size, __shmflg);
+	int fd,ret;
+	char key[11];
+	
+	fd = open(ASHMEM_DEVICE, O_RDWR);
+	if (fd < 0)
+		return fd;
+
+	sprintf(key,"%d",__key);
+	ret = ioctl(fd, ASHMEM_SET_NAME, key);
+	if (ret < 0)
+		goto error;
+
+	ret = ioctl(fd, ASHMEM_SET_SIZE, __size);
+	if (ret < 0)
+		goto error;
+
+	return fd;
+	
+error:
+	close(fd);
+	return ret;
 }
 
 static inline void *shmat (int __shmid, const void *__shmaddr, int __shmflg)
 {
-	return (void *)syscall(__NR_shmat, __shmid, __shmaddr, __shmflg);
+	size_t *ptr, size = ioctl(__shmid, ASHMEM_GET_SIZE, NULL);
+	ptr = mmap(NULL, size + sizeof(size_t), PROT_READ | PROT_WRITE, MAP_SHARED, __shmid, 0);
+	*ptr = size;    //save size at beginning of buffer, for use with munmap
+	return &ptr[1];
 }
 
 static inline int shmdt (const void *__shmaddr)
 {
-	return syscall(__NR_shmctl, __shmaddr);
+	size_t *ptr, size;
+	ptr = (size_t *)__shmaddr;
+	ptr--;
+	size = *ptr;    //find mmap size which we stored at the beginning of the buffer
+	return munmap((void *)ptr, size + sizeof(size_t));
 }

Re: android ashmem patch [review request]

[Aaron Carroll]

On 1 May 2013 08:04, Akers, Jason B <jason.b.akers@intel.com> wrote:
> I wrapped the ashmem calls with the existing shm* function definitions. It's a little ugly, but makes for an easy patch touching only os/os-android.h.
>
> Tested on both arm and x86 android phones.
>
> [..]
>
>  static inline int shmctl (int __shmid, int __cmd, struct shmid_ds *__buf)
>  {
> -       return syscall(__NR_shmctl, __shmid, __cmd, __buf);
> +       int ret=0;
> +       if (__cmd == IPC_RMID)
> +       {
> +               int length = ioctl(__shmid, ASHMEM_GET_SIZE, NULL);
> +               struct ashmem_pin pin = {0 , length};
> +               ret = ioctl(__shmid, ASHMEM_UNPIN, &pin);

I'm not sure that you want to make the region unpinned. How can we
recover if the region is reclaimed?

Other than that, my only comment would be that these functions
shouldn't be static inline since they're quite big, but since there
are only a few callsites it's probably not a big deal.


    -- Aaron

Re: android ashmem patch [review request]

[Aaron Carroll]

On 1 May 2013 13:34, Aaron Carroll <xaaronc@gmail.com> wrote:
> On 1 May 2013 08:04, Akers, Jason B <jason.b.akers@intel.com> wrote:
>> I wrapped the ashmem calls with the existing shm* function definitions. It's a little ugly, but makes for an easy patch touching only os/os-android.h.
>>
>> Tested on both arm and x86 android phones.
>>
>> [..]
>>
>>  static inline int shmctl (int __shmid, int __cmd, struct shmid_ds *__buf)
>>  {
>> -       return syscall(__NR_shmctl, __shmid, __cmd, __buf);
>> +       int ret=0;
>> +       if (__cmd == IPC_RMID)
>> +       {
>> +               int length = ioctl(__shmid, ASHMEM_GET_SIZE, NULL);
>> +               struct ashmem_pin pin = {0 , length};
>> +               ret = ioctl(__shmid, ASHMEM_UNPIN, &pin);
>
> I'm not sure that you want to make the region unpinned. How can we
> recover if the region is reclaimed?

Oops, didn't notice that was on the RMID path, ignore that.

  -- Aaron

Re: android ashmem patch [review request]

[Jens Axboe]

On Tue, Apr 30 2013, Akers, Jason B wrote:
> I wrapped the ashmem calls with the existing shm* function definitions. It's a little ugly, but makes for an easy patch touching only os/os-android.h.
> 
> Tested on both arm and x86 android phones.

Thanks, applied. Would be a smidgen better to add os/os-android.c and
put the functions in there. It is a little much code to put in a header.

-- 
Jens Axboe