[dpdk-dev,v2,2/5] mem: add API to obtain memory-backed file info

Message ID 1454671228-33284-3-git-send-email-jianfeng.tan@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Commit Message

Jianfeng Tan Feb. 5, 2016, 11:20 a.m. UTC
  A new API named rte_eal_get_backfile_info() and a new data
struct back_file is added to obstain information of memory-
backed file info.

Signed-off-by: Huawei Xie <huawei.xie@intel.com>
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 lib/librte_eal/common/include/rte_memory.h | 16 ++++++++++++
 lib/librte_eal/linuxapp/eal/eal_memory.c   | 40 +++++++++++++++++++++++++++++-
 2 files changed, 55 insertions(+), 1 deletion(-)
  

Comments

Yuanhan Liu March 7, 2016, 1:22 p.m. UTC | #1
On Fri, Feb 05, 2016 at 07:20:25PM +0800, Jianfeng Tan wrote:
> A new API named rte_eal_get_backfile_info() and a new data
> struct back_file is added to obstain information of memory-
> backed file info.

I would normally suggest to try hard to find some solution else, instead
of introducing yet another new API, espeically when you just came up with
one user only.

> 
> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---
>  lib/librte_eal/common/include/rte_memory.h | 16 ++++++++++++
>  lib/librte_eal/linuxapp/eal/eal_memory.c   | 40 +++++++++++++++++++++++++++++-
>  2 files changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_eal/common/include/rte_memory.h b/lib/librte_eal/common/include/rte_memory.h
> index 587a25d..b09397e 100644
> --- a/lib/librte_eal/common/include/rte_memory.h
> +++ b/lib/librte_eal/common/include/rte_memory.h
> @@ -109,6 +109,22 @@ struct rte_memseg {
>  } __rte_packed;
>  
>  /**
> + * This struct is used to store information about memory-backed file that
> + * we mapped in memory initialization.
> + */
> +struct back_file {
> +	void *addr;         /**< virtual addr */
> +	size_t size;        /**< the page size */
> +	char filepath[PATH_MAX]; /**< path to backing file on filesystem */
> +};

So, that's all the info you'd like to get. I'm thinking you may don't
need another new API to retrieve them at all:

Say, you can get the filepath and fd from /proc/self/fd (by filtering it
with "rtemap_"):

    $ ls /proc/3487/fd -l
    total 0
    lrwx------ 1 root root 64 Mar  7 20:37 0 -> /dev/pts/2
    lrwx------ 1 root root 64 Mar  7 20:37 1 -> /dev/pts/2
    lrwx------ 1 root root 64 Mar  7 20:37 2 -> /dev/pts/2
    lrwx------ 1 root root 64 Mar  7 20:37 3 -> /run/.rte_config
    lr-x------ 1 root root 64 Mar  7 20:37 4 -> /dev/hugepages
    lr-x------ 1 root root 64 Mar  7 20:37 5 -> /mnt
==> lrwx------ 1 root root 64 Mar  7 20:37 6 -> /dev/hugepages/rtemap_0


Which could also save you an extra "open" at caller side for that
file as well.

And you can get the virtual addr and size from /proc/self/maps:

    $ grep rtemap_ /proc/3487/maps
    7fff40000000-7fffc0000000 rw-s 00000000 00:22 21082 /dev/hugepages/rtemap_0


Will that work for you?

	--yliu
  
Jianfeng Tan March 8, 2016, 2:31 a.m. UTC | #2
On 3/7/2016 9:22 PM, Yuanhan Liu wrote:
> On Fri, Feb 05, 2016 at 07:20:25PM +0800, Jianfeng Tan wrote:
>> A new API named rte_eal_get_backfile_info() and a new data
>> struct back_file is added to obstain information of memory-
>> backed file info.
> I would normally suggest to try hard to find some solution else, instead
> of introducing yet another new API, espeically when you just came up with
> one user only.

Actually, Tetsuya's qtest patchset will make it two.

>
>> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
>> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
>> ---
>>   lib/librte_eal/common/include/rte_memory.h | 16 ++++++++++++
>>   lib/librte_eal/linuxapp/eal/eal_memory.c   | 40 +++++++++++++++++++++++++++++-
>>   2 files changed, 55 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/librte_eal/common/include/rte_memory.h b/lib/librte_eal/common/include/rte_memory.h
>> index 587a25d..b09397e 100644
>> --- a/lib/librte_eal/common/include/rte_memory.h
>> +++ b/lib/librte_eal/common/include/rte_memory.h
>> @@ -109,6 +109,22 @@ struct rte_memseg {
>>   } __rte_packed;
>>   
>>   /**
>> + * This struct is used to store information about memory-backed file that
>> + * we mapped in memory initialization.
>> + */
>> +struct back_file {
>> +	void *addr;         /**< virtual addr */
>> +	size_t size;        /**< the page size */
>> +	char filepath[PATH_MAX]; /**< path to backing file on filesystem */
>> +};
> So, that's all the info you'd like to get. I'm thinking you may don't
> need another new API to retrieve them at all:
>
> Say, you can get the filepath and fd from /proc/self/fd (by filtering it
> with "rtemap_"):
>
>      $ ls /proc/3487/fd -l
>      total 0
>      lrwx------ 1 root root 64 Mar  7 20:37 0 -> /dev/pts/2
>      lrwx------ 1 root root 64 Mar  7 20:37 1 -> /dev/pts/2
>      lrwx------ 1 root root 64 Mar  7 20:37 2 -> /dev/pts/2
>      lrwx------ 1 root root 64 Mar  7 20:37 3 -> /run/.rte_config
>      lr-x------ 1 root root 64 Mar  7 20:37 4 -> /dev/hugepages
>      lr-x------ 1 root root 64 Mar  7 20:37 5 -> /mnt
> ==> lrwx------ 1 root root 64 Mar  7 20:37 6 -> /dev/hugepages/rtemap_0

I guess this rtemap_xxx has been closed after memory initialization and 
cannot be obtained from /proc/xxx/fd. I believe /proc/xxx/maps is what 
you want to say.

>
>
> Which could also save you an extra "open" at caller side for that
> file as well.

Same reason, we cannot save extra "open".

>
> And you can get the virtual addr and size from /proc/self/maps:
>
>      $ grep rtemap_ /proc/3487/maps
>      7fff40000000-7fffc0000000 rw-s 00000000 00:22 21082 /dev/hugepages/rtemap_0
>
>
> Will that work for you?

Yes, from function's side, it works for me. But it needs some string 
processing. Another way is to just exposed an global variable pointing 
to the address of /run/.rte_config, so that callers extract needed 
information by themselves using "struct hugepage_file". How do you think?

Thanks,
Jianfeng

>
> 	--yliu
  
Yuanhan Liu March 8, 2016, 2:53 a.m. UTC | #3
On Tue, Mar 08, 2016 at 10:31:10AM +0800, Tan, Jianfeng wrote:
> 
> 
> On 3/7/2016 9:22 PM, Yuanhan Liu wrote:
> >On Fri, Feb 05, 2016 at 07:20:25PM +0800, Jianfeng Tan wrote:
> >>A new API named rte_eal_get_backfile_info() and a new data
> >>struct back_file is added to obstain information of memory-
> >>backed file info.
> >I would normally suggest to try hard to find some solution else, instead
> >of introducing yet another new API, espeically when you just came up with
> >one user only.
> 
> Actually, Tetsuya's qtest patchset will make it two.

Well, it's actually a same story. So, still one user to me.

> >
> >>Signed-off-by: Huawei Xie <huawei.xie@intel.com>
> >>Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> >>---
> >>  lib/librte_eal/common/include/rte_memory.h | 16 ++++++++++++
> >>  lib/librte_eal/linuxapp/eal/eal_memory.c   | 40 +++++++++++++++++++++++++++++-
> >>  2 files changed, 55 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/lib/librte_eal/common/include/rte_memory.h b/lib/librte_eal/common/include/rte_memory.h
> >>index 587a25d..b09397e 100644
> >>--- a/lib/librte_eal/common/include/rte_memory.h
> >>+++ b/lib/librte_eal/common/include/rte_memory.h
> >>@@ -109,6 +109,22 @@ struct rte_memseg {
> >>  } __rte_packed;
> >>  /**
> >>+ * This struct is used to store information about memory-backed file that
> >>+ * we mapped in memory initialization.
> >>+ */
> >>+struct back_file {
> >>+	void *addr;         /**< virtual addr */
> >>+	size_t size;        /**< the page size */
> >>+	char filepath[PATH_MAX]; /**< path to backing file on filesystem */
> >>+};
> >So, that's all the info you'd like to get. I'm thinking you may don't
> >need another new API to retrieve them at all:
> >
> >Say, you can get the filepath and fd from /proc/self/fd (by filtering it
> >with "rtemap_"):
> >
> >     $ ls /proc/3487/fd -l
> >     total 0
> >     lrwx------ 1 root root 64 Mar  7 20:37 0 -> /dev/pts/2
> >     lrwx------ 1 root root 64 Mar  7 20:37 1 -> /dev/pts/2
> >     lrwx------ 1 root root 64 Mar  7 20:37 2 -> /dev/pts/2
> >     lrwx------ 1 root root 64 Mar  7 20:37 3 -> /run/.rte_config
> >     lr-x------ 1 root root 64 Mar  7 20:37 4 -> /dev/hugepages
> >     lr-x------ 1 root root 64 Mar  7 20:37 5 -> /mnt
> >==> lrwx------ 1 root root 64 Mar  7 20:37 6 -> /dev/hugepages/rtemap_0
> 
> I guess this rtemap_xxx has been closed after memory initialization and
> cannot be obtained from /proc/xxx/fd. I believe /proc/xxx/maps is what you
> want to say.

Yes, I forgot to mention that you need keep that file open.
So, you just need one line or two to not close that file
in this case.

> 
> >
> >
> >Which could also save you an extra "open" at caller side for that
> >file as well.
> 
> Same reason, we cannot save extra "open".

We could, if we keep the file open.

> >
> >And you can get the virtual addr and size from /proc/self/maps:
> >
> >     $ grep rtemap_ /proc/3487/maps
> >     7fff40000000-7fffc0000000 rw-s 00000000 00:22 21082 /dev/hugepages/rtemap_0
> >
> >
> >Will that work for you?
> 
> Yes, from function's side, it works for me. But it needs some string
> processing.

What's wrong of the string processing? I have seen many string
processings in DPDK code, even in rte_memory.c.

> Another way is to just exposed an global variable pointing to
> the address of /run/.rte_config, so that callers extract needed information
> by themselves using "struct hugepage_file". How do you think?

That doens't seem elegant to me.

	--yliu
  

Patch

diff --git a/lib/librte_eal/common/include/rte_memory.h b/lib/librte_eal/common/include/rte_memory.h
index 587a25d..b09397e 100644
--- a/lib/librte_eal/common/include/rte_memory.h
+++ b/lib/librte_eal/common/include/rte_memory.h
@@ -109,6 +109,22 @@  struct rte_memseg {
 } __rte_packed;
 
 /**
+ * This struct is used to store information about memory-backed file that
+ * we mapped in memory initialization.
+ */
+struct back_file {
+	void *addr;         /**< virtual addr */
+	size_t size;        /**< the page size */
+	char filepath[PATH_MAX]; /**< path to backing file on filesystem */
+};
+
+/**
+  * Get the hugepage file information. Caller to free.
+  * Return number of hugepage files used.
+  */
+int rte_eal_get_backfile_info(struct back_file **);
+
+/**
  * Lock page in physical memory and prevent from swapping.
  *
  * @param virt
diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
index 68ef49a..a6b3616 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memory.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
@@ -743,6 +743,9 @@  sort_by_physaddr(struct hugepage_file *hugepg_tbl, struct hugepage_info *hpi)
 	return 0;
 }
 
+static struct hugepage_file *hugepage_files;
+static int num_hugepage_files;
+
 /*
  * Uses mmap to create a shared memory area for storage of data
  * Used in this file to store the hugepage file map on disk
@@ -760,9 +763,30 @@  create_shared_memory(const char *filename, const size_t mem_size)
 	}
 	retval = mmap(NULL, mem_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
 	close(fd);
+
+	hugepage_files = retval;
+	num_hugepage_files = mem_size / (sizeof(struct hugepage_file));
+
 	return retval;
 }
 
+int
+rte_eal_get_backfile_info(struct back_file **p)
+{
+	struct back_file *backfiles;
+	int i, num_backfiles = num_hugepage_files;
+
+	backfiles = malloc(sizeof(struct back_file) * num_backfiles);
+	for (i = 0; i < num_backfiles; ++i) {
+		backfiles[i].addr = hugepage_files[i].final_va;
+		backfiles[i].size = hugepage_files[i].size;
+		strcpy(backfiles[i].filepath, hugepage_files[i].filepath);
+	}
+
+	*p = backfiles;
+	return num_backfiles;
+}
+
 /*
  * this copies *active* hugepages from one hugepage table to another.
  * destination is typically the shared memory.
@@ -1148,8 +1172,22 @@  rte_eal_hugepage_init(void)
 		mcfg->memseg[0].len = internal_config.memory;
 		mcfg->memseg[0].socket_id = socket_id;
 
-		close(fd);
+		hugepage = create_shared_memory(eal_hugepage_info_path(),
+						sizeof(struct hugepage_file));
+		hugepage->orig_va = addr;
+		hugepage->final_va = addr;
+		hugepage->physaddr = rte_mem_virt2phy(addr);
+		/* Suppose we have a very huge hugefile here */
+		hugepage->size = internal_config.memory;
+		hugepage->socket_id = socket_id;
+		hugepage->file_id = 0;
+		hugepage->memseg_id = 0;
+#ifdef RTE_EAL_SINGLE_FILE_SEGMENTS
+		hugepage->repeated = internal_config.memory / pagesize;
+#endif
+		strncpy(hugepage->filepath, filepath, MAX_HUGEPAGE_PATH);
 
+		close(fd);
 		return 0;
 	}