[PATCH 1/1] mm: msync: fix the behaviour msync on tmpfs

[PATCH 1/1] mm: msync: fix the behaviour msync on tmpfs

[Zumeng Chen]

For tmpfs, no matter which flag(ASYNC or SYNC) gets from msync,
there is not so much thing to do for CPUs without cache alias,
But for some CPUs with cache alias, msync has to flush cache
explicitly, which makes sure the data coherency between memory
file and cache.

Signed-off-by: Zumeng Chen <zumeng.chen@windriver.com>
---
 include/linux/mm.h |    1 +
 mm/msync.c         |   20 +++++++++++++++++++-
 mm/shmem.c         |    5 +++++
 3 files changed, 25 insertions(+), 1 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index f59179b..858db90 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -868,6 +868,7 @@ extern void pagefault_out_of_memory(void);
 extern void show_free_areas(unsigned int flags);
 extern bool skip_free_areas_node(unsigned int flags, int nid);
 
+int is_shmem_file(struct file *file);
 int shmem_lock(struct file *file, int lock, struct user_struct *user);
 struct file *shmem_file_setup(const char *name, loff_t size, unsigned long flags);
 void shmem_set_file(struct vm_area_struct *vma, struct file *file);
diff --git a/mm/msync.c b/mm/msync.c
index 632df45..2f0d8fa 100644
--- a/mm/msync.c
+++ b/mm/msync.c
@@ -13,6 +13,7 @@
 #include <linux/file.h>
 #include <linux/syscalls.h>
 #include <linux/sched.h>
+#include <asm/cacheflush.h>
 
 /*
  * MS_SYNC syncs the entire file - including mappings.
@@ -33,6 +34,7 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
 	unsigned long end;
 	struct mm_struct *mm = current->mm;
 	struct vm_area_struct *vma;
+	struct file *file;
 	int unmapped_error = 0;
 	int error = -EINVAL;
 
@@ -56,8 +58,24 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
 	 */
 	down_read(&mm->mmap_sem);
 	vma = find_vma(mm, start);
+
+#ifdef CONFIG_TMPFS
+	/*
+	 * For tmpfs, no matter which flag(ASYNC or SYNC) gets from msync,
+	 * there is not so much thing to do for CPUs without cache alias,
+	 * But for some CPUs with cache alias, msync has to flush cache
+	 * explicitly, which makes sure the data coherency between memory
+	 * file and cache.
+	 */
+	file =  vma->vm_file;
+	if (is_shmem_file(file)) {
+		flush_cache_range(vma, start, start+len);
+		error = 0;
+		goto out_unlock;
+	}
+#endif
+
 	for (;;) {
-		struct file *file;
 
 		/* Still start < end. */
 		error = -ENOMEM;
diff --git a/mm/shmem.c b/mm/shmem.c
index 92e5c15..4fabfac 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2793,6 +2793,11 @@ static struct file_system_type tmpfs_fs_type = {
 	.kill_sb	= kill_litter_super,
 };
 
+int is_shmem_file(struct file *file)
+{
+	return (file->f_op == &shmem_file_operations)? 1 : 0;
+} 
+
 int __init init_tmpfs(void)
 {
 	int error;
-- 
1.7.5.4

Re: [PATCH 1/1] mm: msync: fix the behaviour msync on tmpfs

[zumeng.chen]

This patch has been validated by the following commands
on both CPUs with or without cache alias, which is for

http://bugzilla.pokylinux.org/show_bug.cgi?id=1429

root@routerstationpro:~# dmesg|grep alias
Primary data cache 32kB, 4-way, VIPT, cache aliases, linesize 32 bytes
root@routerstationpro:~# for i in `seq 1 1000`; do ./msync01 ;done


root@localhost:/tmp> dmesg|grep alias
Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 bytes
root@localhost:/tmp> zcat /proc/config.gz |grep WINPATH
CONFIG_WINTEGRA_WINPATH=y
CONFIG_WINTEGRA_WINPATH3=y
CONFIG_SERIAL_8250_WINPATH=y
root@localhost:/tmp> for i in `seq 1 1000`; do ./msync01 ;done

Passed.

On 2011年12月21日 23:17, Zumeng Chen wrote:
> For tmpfs, no matter which flag(ASYNC or SYNC) gets from msync,
> there is not so much thing to do for CPUs without cache alias,
> But for some CPUs with cache alias, msync has to flush cache
> explicitly, which makes sure the data coherency between memory
> file and cache.
>
> Signed-off-by: Zumeng Chen<zumeng.chen@windriver.com>
> ---
>   include/linux/mm.h |    1 +
>   mm/msync.c         |   20 +++++++++++++++++++-
>   mm/shmem.c         |    5 +++++
>   3 files changed, 25 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index f59179b..858db90 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -868,6 +868,7 @@ extern void pagefault_out_of_memory(void);
>   extern void show_free_areas(unsigned int flags);
>   extern bool skip_free_areas_node(unsigned int flags, int nid);
>
> +int is_shmem_file(struct file *file);
>   int shmem_lock(struct file *file, int lock, struct user_struct *user);
>   struct file *shmem_file_setup(const char *name, loff_t size, unsigned long flags);
>   void shmem_set_file(struct vm_area_struct *vma, struct file *file);
> diff --git a/mm/msync.c b/mm/msync.c
> index 632df45..2f0d8fa 100644
> --- a/mm/msync.c
> +++ b/mm/msync.c
> @@ -13,6 +13,7 @@
>   #include<linux/file.h>
>   #include<linux/syscalls.h>
>   #include<linux/sched.h>
> +#include<asm/cacheflush.h>
>
>   /*
>    * MS_SYNC syncs the entire file - including mappings.
> @@ -33,6 +34,7 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
>   	unsigned long end;
>   	struct mm_struct *mm = current->mm;
>   	struct vm_area_struct *vma;
> +	struct file *file;
>   	int unmapped_error = 0;
>   	int error = -EINVAL;
>
> @@ -56,8 +58,24 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
>   	 */
>   	down_read(&mm->mmap_sem);
>   	vma = find_vma(mm, start);
> +
> +#ifdef CONFIG_TMPFS
> +	/*
> +	 * For tmpfs, no matter which flag(ASYNC or SYNC) gets from msync,
> +	 * there is not so much thing to do for CPUs without cache alias,
> +	 * But for some CPUs with cache alias, msync has to flush cache
> +	 * explicitly, which makes sure the data coherency between memory
> +	 * file and cache.
> +	 */
> +	file =  vma->vm_file;
> +	if (is_shmem_file(file)) {
> +		flush_cache_range(vma, start, start+len);
> +		error = 0;
> +		goto out_unlock;
> +	}
> +#endif
> +
>   	for (;;) {
> -		struct file *file;
>
>   		/* Still start<  end. */
>   		error = -ENOMEM;
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 92e5c15..4fabfac 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -2793,6 +2793,11 @@ static struct file_system_type tmpfs_fs_type = {
>   	.kill_sb	= kill_litter_super,
>   };
>
> +int is_shmem_file(struct file *file)
> +{
> +	return (file->f_op ==&shmem_file_operations)? 1 : 0;
> +}
> +
>   int __init init_tmpfs(void)
>   {
>   	int error;

Re: [PATCH 1/1] mm: msync: fix the behaviour msync on tmpfs

[Bruce Ashfield]

On 11-12-21 10:24 AM, zumeng.chen wrote:
> This patch has been validated by the following commands
> on both CPUs with or without cache alias, which is for
>
> http://bugzilla.pokylinux.org/show_bug.cgi?id=1429

Glad to see this. I was composing an email that had just this
question. Which bug, and which kernel version is the issue.

>
> root@routerstationpro:~# dmesg|grep alias
> Primary data cache 32kB, 4-way, VIPT, cache aliases, linesize 32 bytes
> root@routerstationpro:~# for i in `seq 1 1000`; do ./msync01 ;done
>
>
> root@localhost:/tmp> dmesg|grep alias
> Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 bytes
> root@localhost:/tmp> zcat /proc/config.gz |grep WINPATH
> CONFIG_WINTEGRA_WINPATH=y
> CONFIG_WINTEGRA_WINPATH3=y
> CONFIG_SERIAL_8250_WINPATH=y
> root@localhost:/tmp> for i in `seq 1 1000`; do ./msync01 ;done
>
> Passed.
>
> On 2011年12月21日 23:17, Zumeng Chen wrote:
>> For tmpfs, no matter which flag(ASYNC or SYNC) gets from msync,
>> there is not so much thing to do for CPUs without cache alias,
>> But for some CPUs with cache alias, msync has to flush cache
>> explicitly, which makes sure the data coherency between memory
>> file and cache.

Is it just coherency, or are there corruption concerns as well ?

In the commit message, it would be useful to put an example
interaction that can trigger this issue, since it isn't entirely
obvious from the message. More information is always better.

Is this for the 3.0 and 2.6.37 kernels .. or just one, or the
other ?


>>
>> Signed-off-by: Zumeng Chen<zumeng.chen@windriver.com>
>> ---
>> include/linux/mm.h | 1 +
>> mm/msync.c | 20 +++++++++++++++++++-
>> mm/shmem.c | 5 +++++
>> 3 files changed, 25 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index f59179b..858db90 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -868,6 +868,7 @@ extern void pagefault_out_of_memory(void);
>> extern void show_free_areas(unsigned int flags);
>> extern bool skip_free_areas_node(unsigned int flags, int nid);
>>
>> +int is_shmem_file(struct file *file);

If there aren't any more users of this other than your
msync.c below, we shouldn't need this exposed in mm.h. I
realized that it is implemented in shmem.c, but isn't there
a less global place to add this export (consider mm/internal.h)?
Also shouldn't it be extern, like the other defines in the file ?

>> int shmem_lock(struct file *file, int lock, struct user_struct *user);
>> struct file *shmem_file_setup(const char *name, loff_t size, unsigned
>> long flags);
>> void shmem_set_file(struct vm_area_struct *vma, struct file *file);
>> diff --git a/mm/msync.c b/mm/msync.c
>> index 632df45..2f0d8fa 100644
>> --- a/mm/msync.c
>> +++ b/mm/msync.c
>> @@ -13,6 +13,7 @@
>> #include<linux/file.h>
>> #include<linux/syscalls.h>
>> #include<linux/sched.h>
>> +#include<asm/cacheflush.h>
>>
>> /*
>> * MS_SYNC syncs the entire file - including mappings.
>> @@ -33,6 +34,7 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t,
>> len, int, flags)
>> unsigned long end;
>> struct mm_struct *mm = current->mm;
>> struct vm_area_struct *vma;
>> + struct file *file;
>> int unmapped_error = 0;
>> int error = -EINVAL;
>>
>> @@ -56,8 +58,24 @@ SYSCALL_DEFINE3(msync, unsigned long, start,
>> size_t, len, int, flags)
>> */
>> down_read(&mm->mmap_sem);
>> vma = find_vma(mm, start);
>> +
>> +#ifdef CONFIG_TMPFS
>> + /*
>> + * For tmpfs, no matter which flag(ASYNC or SYNC) gets from msync,
>> + * there is not so much thing to do for CPUs without cache alias,
>> + * But for some CPUs with cache alias, msync has to flush cache
>> + * explicitly, which makes sure the data coherency between memory
>> + * file and cache.
>> + */
>> + file = vma->vm_file;
>> + if (is_shmem_file(file)) {
>> + flush_cache_range(vma, start, start+len);
>> + error = 0;

Hasn't error already been set to 0 ? .. I see it in the
original function being set right before this chunk of code
will run.

>> + goto out_unlock;

Do we really want this for all architectures/all boards ? This
is common/global code, so we need to tread carefully.

Is there a way that we could do this by arch hooks .. or is
flush_cache_range() sufficiently benign that it is little overhead
and safe everywhere ?

In the end, I'm not a mm expert, so we can fix this up, think
about it, and then run it past the people that really know :)

>> + }
>> +#endif
>> +
>> for (;;) {
>> - struct file *file;

There's an assignment:

   file = vma->vm_file;

Later in this routine, you've already done that above, so can't
it be dropped ?

Bruce

>>
>> /* Still start< end. */
>> error = -ENOMEM;
>> diff --git a/mm/shmem.c b/mm/shmem.c
>> index 92e5c15..4fabfac 100644
>> --- a/mm/shmem.c
>> +++ b/mm/shmem.c
>> @@ -2793,6 +2793,11 @@ static struct file_system_type tmpfs_fs_type = {
>> .kill_sb = kill_litter_super,
>> };
>>
>> +int is_shmem_file(struct file *file)
>> +{
>> + return (file->f_op ==&shmem_file_operations)? 1 : 0;
>> +}
>> +
>> int __init init_tmpfs(void)
>> {
>> int error;
>

Re: [PATCH 1/1] mm: msync: fix the behaviour msync on tmpfs

[Zumeng Chen]

于 2011年12月21日 23:38, Bruce Ashfield 写道:
> On 11-12-21 10:24 AM, zumeng.chen wrote:
>> This patch has been validated by the following commands
>> on both CPUs with or without cache alias, which is for
>>
>> http://bugzilla.pokylinux.org/show_bug.cgi?id=1429
>
> Glad to see this. I was composing an email that had just this
> question. Which bug, and which kernel version is the issue.
For all versions, including the latest mainline kernel

>>
>> root@routerstationpro:~# dmesg|grep alias
>> Primary data cache 32kB, 4-way, VIPT, cache aliases, linesize 32 bytes
>> root@routerstationpro:~# for i in `seq 1 1000`; do ./msync01 ;done
>>
>>
>> root@localhost:/tmp> dmesg|grep alias
>> Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 bytes
>> root@localhost:/tmp> zcat /proc/config.gz |grep WINPATH
>> CONFIG_WINTEGRA_WINPATH=y
>> CONFIG_WINTEGRA_WINPATH3=y
>> CONFIG_SERIAL_8250_WINPATH=y
>> root@localhost:/tmp> for i in `seq 1 1000`; do ./msync01 ;done
>>
>> Passed.
>>
>> On 2011年12月21日 23:17, Zumeng Chen wrote:
>>> For tmpfs, no matter which flag(ASYNC or SYNC) gets from msync,
>>> there is not so much thing to do for CPUs without cache alias,
>>> But for some CPUs with cache alias, msync has to flush cache
>>> explicitly, which makes sure the data coherency between memory
>>> file and cache.
>
> Is it just coherency, or are there corruption concerns as well ?
There are two issues:
1 ) for TMPFS, nothing need to be done in sys_msync,
      we just return for all arches.

2 ) But for MIPS CPUs with cache alias(dmesg|grep alias),
      it maybe has the issue which reported by 1429 while
      the memory of memset used by msycn01 has other
      alias, then failure will be report.
      So, in this situation, we need to flush the related vma
      to make sure read correctly.
>
> In the commit message, it would be useful to put an example
> interaction that can trigger this issue, since it isn't entirely
> obvious from the message. More information is always better.
>
> Is this for the 3.0 and 2.6.37 kernels .. or just one, or the
> other ?
For both and the latest mainline.
>
>
>>>
>>> Signed-off-by: Zumeng Chen<zumeng.chen@windriver.com>
>>> ---
>>> include/linux/mm.h | 1 +
>>> mm/msync.c | 20 +++++++++++++++++++-
>>> mm/shmem.c | 5 +++++
>>> 3 files changed, 25 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>>> index f59179b..858db90 100644
>>> --- a/include/linux/mm.h
>>> +++ b/include/linux/mm.h
>>> @@ -868,6 +868,7 @@ extern void pagefault_out_of_memory(void);
>>> extern void show_free_areas(unsigned int flags);
>>> extern bool skip_free_areas_node(unsigned int flags, int nid);
>>>
>>> +int is_shmem_file(struct file *file);
>
> If there aren't any more users of this other than your
> msync.c below, we shouldn't need this exposed in mm.h. I
> realized that it is implemented in shmem.c, but isn't there
> a less global place to add this export (consider mm/internal.h)?
> Also shouldn't it be extern, like the other defines in the file ?
Yes, I'll export it in include/linux/shmem_fs.h V2.
>
>>> int shmem_lock(struct file *file, int lock, struct user_struct *user);
>>> struct file *shmem_file_setup(const char *name, loff_t size, unsigned
>>> long flags);
>>> void shmem_set_file(struct vm_area_struct *vma, struct file *file);
>>> diff --git a/mm/msync.c b/mm/msync.c
>>> index 632df45..2f0d8fa 100644
>>> --- a/mm/msync.c
>>> +++ b/mm/msync.c
>>> @@ -13,6 +13,7 @@
>>> #include<linux/file.h>
>>> #include<linux/syscalls.h>
>>> #include<linux/sched.h>
>>> +#include<asm/cacheflush.h>
>>>
>>> /*
>>> * MS_SYNC syncs the entire file - including mappings.
>>> @@ -33,6 +34,7 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t,
>>> len, int, flags)
>>> unsigned long end;
>>> struct mm_struct *mm = current->mm;
>>> struct vm_area_struct *vma;
>>> + struct file *file;
>>> int unmapped_error = 0;
>>> int error = -EINVAL;
>>>
>>> @@ -56,8 +58,24 @@ SYSCALL_DEFINE3(msync, unsigned long, start,
>>> size_t, len, int, flags)
>>> */
>>> down_read(&mm->mmap_sem);
>>> vma = find_vma(mm, start);
>>> +
>>> +#ifdef CONFIG_TMPFS
>>> + /*
>>> + * For tmpfs, no matter which flag(ASYNC or SYNC) gets from msync,
>>> + * there is not so much thing to do for CPUs without cache alias,
>>> + * But for some CPUs with cache alias, msync has to flush cache
>>> + * explicitly, which makes sure the data coherency between memory
>>> + * file and cache.
>>> + */
>>> + file = vma->vm_file;
>>> + if (is_shmem_file(file)) {
>>> + flush_cache_range(vma, start, start+len);
>>> + error = 0;
>
> Hasn't error already been set to 0 ? .. I see it in the
> original function being set right before this chunk of code
> will run.
Yes, see above item1
>
>>> + goto out_unlock;
>
> Do we really want this for all architectures/all boards ? This
> is common/global code, so we need to tread carefully.
Yes, done on powerpc 8513, and PMC without cache alias.
>
> Is there a way that we could do this by arch hooks .. or is
> flush_cache_range() sufficiently benign that it is little overhead
> and safe everywhere ?
Yeah, I have already taken this into accounts, and it
seems should be OK since only msync trigger this.
And only for TMPFS, for others, we'll go through and
skip flush_cache_range.
I'll add CPU_has_alias check in V2 too.
>
> In the end, I'm not a mm expert, so we can fix this up, think
> about it, and then run it past the people that really know :)
>
>>> + }
>>> +#endif
>>> +
>>> for (;;) {
>>> - struct file *file;
>
> There's an assignment:
>
>   file = vma->vm_file;
>
> Later in this routine, you've already done that above, so can't
> it be dropped ?
No, we'll goto the end if TMPFS.

Regards,
Zumeng
>
> Bruce
>
>>>
>>> /* Still start< end. */
>>> error = -ENOMEM;
>>> diff --git a/mm/shmem.c b/mm/shmem.c
>>> index 92e5c15..4fabfac 100644
>>> --- a/mm/shmem.c
>>> +++ b/mm/shmem.c
>>> @@ -2793,6 +2793,11 @@ static struct file_system_type tmpfs_fs_type = {
>>> .kill_sb = kill_litter_super,
>>> };
>>>
>>> +int is_shmem_file(struct file *file)
>>> +{
>>> + return (file->f_op ==&shmem_file_operations)? 1 : 0;
>>> +}
>>> +
>>> int __init init_tmpfs(void)
>>> {
>>> int error;
>>
>

[V2 PATCH 1/1] mm: msync: fix issues of sys_msync on tmpfs

[Zumeng Chen]

There are two problems as follows shown:
1 ) for TMPFS, nothing need to be done in sys_msync,
    sys_msync just return 0 for all arches.

2 ) But for MIPS CPUs with cache alias(dmesg|grep alias),
    it maybe has the issue, which reported by msync test
    suites in ltp-full, when the memory of memset used by
    msycn01 has cache alias.
    So, in this situation, we have to flush the related
    vma to make sure correct read.

Signed-off-by: Zumeng Chen <zumeng.chen@windriver.com>
---
 include/linux/shmem_fs.h |   10 ++++++++++
 mm/msync.c               |   22 +++++++++++++++++++++-
 mm/shmem.c               |    5 +++++
 3 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h
index aa08fa8..62a2d57 100644
--- a/include/linux/shmem_fs.h
+++ b/include/linux/shmem_fs.h
@@ -12,6 +12,15 @@
 
 #define SHMEM_SYMLINK_INLINE_LEN (SHMEM_NR_DIRECT * sizeof(swp_entry_t))
 
+/*
+
+*/
+#ifndef cpu_has_dc_aliases
+#define CPU_HAS_CACHE_ALIAS 0
+#else
+#define CPU_HAS_CACHE_ALIAS cpu_has_dc_aliases
+#endif
+
 struct shmem_inode_info {
 	spinlock_t		lock;
 	unsigned long		flags;
@@ -49,6 +58,7 @@ static inline struct shmem_inode_info *SHMEM_I(struct inode *inode)
 /*
  * Functions in mm/shmem.c called directly from elsewhere:
  */
+extern int is_shmem_file(struct file *file);
 extern int init_tmpfs(void);
 extern int shmem_fill_super(struct super_block *sb, void *data, int silent);
 extern struct file *shmem_file_setup(const char *name,
diff --git a/mm/msync.c b/mm/msync.c
index 632df45..84cb068 100644
--- a/mm/msync.c
+++ b/mm/msync.c
@@ -13,6 +13,8 @@
 #include <linux/file.h>
 #include <linux/syscalls.h>
 #include <linux/sched.h>
+#include <linux/shmem_fs.h>
+#include <asm/cacheflush.h>
 
 /*
  * MS_SYNC syncs the entire file - including mappings.
@@ -33,6 +35,7 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
 	unsigned long end;
 	struct mm_struct *mm = current->mm;
 	struct vm_area_struct *vma;
+	struct file *file;
 	int unmapped_error = 0;
 	int error = -EINVAL;
 
@@ -56,8 +59,25 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
 	 */
 	down_read(&mm->mmap_sem);
 	vma = find_vma(mm, start);
+
+#ifdef CONFIG_TMPFS
+	/*
+	 * For tmpfs, no matter which flag(ASYNC or SYNC) gets from msync,
+	 * there is not so much thing to do for CPUs without cache alias,
+	 * But for some CPUs with cache alias, msync has to flush cache
+	 * explicitly, which makes sure the data coherency between memory
+	 * file and cache.
+	 */
+	file =  vma->vm_file;
+	if (is_shmem_file(file)) {
+		if(CPU_HAS_CACHE_ALIAS)
+			flush_cache_range(vma, start, start+len);
+		error = 0;
+		goto out_unlock;
+	}
+#endif
+
 	for (;;) {
-		struct file *file;
 
 		/* Still start < end. */
 		error = -ENOMEM;
diff --git a/mm/shmem.c b/mm/shmem.c
index 92e5c15..4fabfac 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2793,6 +2793,11 @@ static struct file_system_type tmpfs_fs_type = {
 	.kill_sb	= kill_litter_super,
 };
 
+int is_shmem_file(struct file *file)
+{
+	return (file->f_op == &shmem_file_operations)? 1 : 0;
+} 
+
 int __init init_tmpfs(void)
 {
 	int error;
-- 
1.7.5.4

Re: [V2 PATCH 1/1] mm: msync: fix issues of sys_msync on tmpfs

[Bruce Ashfield]

On 11-12-21 10:07 PM, Zumeng Chen wrote:
> There are two problems as follows shown:
> 1 ) for TMPFS, nothing need to be done in sys_msync,
>      sys_msync just return 0 for all arches.
>
> 2 ) But for MIPS CPUs with cache alias(dmesg|grep alias),
>      it maybe has the issue, which reported by msync test
>      suites in ltp-full, when the memory of memset used by
>      msycn01 has cache alias.
>      So, in this situation, we have to flush the related
>      vma to make sure correct read.

Looks good. I'll queue this for the next kernel update.

Cheers,

Bruce

>
> Signed-off-by: Zumeng Chen<zumeng.chen@windriver.com>
> ---
>   include/linux/shmem_fs.h |   10 ++++++++++
>   mm/msync.c               |   22 +++++++++++++++++++++-
>   mm/shmem.c               |    5 +++++
>   3 files changed, 36 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h
> index aa08fa8..62a2d57 100644
> --- a/include/linux/shmem_fs.h
> +++ b/include/linux/shmem_fs.h
> @@ -12,6 +12,15 @@
>
>   #define SHMEM_SYMLINK_INLINE_LEN (SHMEM_NR_DIRECT * sizeof(swp_entry_t))
>
> +/*
> +
> +*/
> +#ifndef cpu_has_dc_aliases
> +#define CPU_HAS_CACHE_ALIAS 0
> +#else
> +#define CPU_HAS_CACHE_ALIAS cpu_has_dc_aliases
> +#endif
> +
>   struct shmem_inode_info {
>   	spinlock_t		lock;
>   	unsigned long		flags;
> @@ -49,6 +58,7 @@ static inline struct shmem_inode_info *SHMEM_I(struct inode *inode)
>   /*
>    * Functions in mm/shmem.c called directly from elsewhere:
>    */
> +extern int is_shmem_file(struct file *file);
>   extern int init_tmpfs(void);
>   extern int shmem_fill_super(struct super_block *sb, void *data, int silent);
>   extern struct file *shmem_file_setup(const char *name,
> diff --git a/mm/msync.c b/mm/msync.c
> index 632df45..84cb068 100644
> --- a/mm/msync.c
> +++ b/mm/msync.c
> @@ -13,6 +13,8 @@
>   #include<linux/file.h>
>   #include<linux/syscalls.h>
>   #include<linux/sched.h>
> +#include<linux/shmem_fs.h>
> +#include<asm/cacheflush.h>
>
>   /*
>    * MS_SYNC syncs the entire file - including mappings.
> @@ -33,6 +35,7 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
>   	unsigned long end;
>   	struct mm_struct *mm = current->mm;
>   	struct vm_area_struct *vma;
> +	struct file *file;
>   	int unmapped_error = 0;
>   	int error = -EINVAL;
>
> @@ -56,8 +59,25 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
>   	 */
>   	down_read(&mm->mmap_sem);
>   	vma = find_vma(mm, start);
> +
> +#ifdef CONFIG_TMPFS
> +	/*
> +	 * For tmpfs, no matter which flag(ASYNC or SYNC) gets from msync,
> +	 * there is not so much thing to do for CPUs without cache alias,
> +	 * But for some CPUs with cache alias, msync has to flush cache
> +	 * explicitly, which makes sure the data coherency between memory
> +	 * file and cache.
> +	 */
> +	file =  vma->vm_file;
> +	if (is_shmem_file(file)) {
> +		if(CPU_HAS_CACHE_ALIAS)
> +			flush_cache_range(vma, start, start+len);
> +		error = 0;
> +		goto out_unlock;
> +	}
> +#endif
> +
>   	for (;;) {
> -		struct file *file;
>
>   		/* Still start<  end. */
>   		error = -ENOMEM;
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 92e5c15..4fabfac 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -2793,6 +2793,11 @@ static struct file_system_type tmpfs_fs_type = {
>   	.kill_sb	= kill_litter_super,
>   };
>
> +int is_shmem_file(struct file *file)
> +{
> +	return (file->f_op ==&shmem_file_operations)? 1 : 0;
> +}
> +
>   int __init init_tmpfs(void)
>   {
>   	int error;

Re: [V2 PATCH 1/1] mm: msync: fix issues of sys_msync on tmpfs

[Khem Raj]

On (22/12/11 09:24), Bruce Ashfield wrote:
> On 11-12-21 10:07 PM, Zumeng Chen wrote:
> >There are two problems as follows shown:
> >1 ) for TMPFS, nothing need to be done in sys_msync,
> >     sys_msync just return 0 for all arches.
> >
> >2 ) But for MIPS CPUs with cache alias(dmesg|grep alias),
> >     it maybe has the issue, which reported by msync test
> >     suites in ltp-full, when the memory of memset used by
> >     msycn01 has cache alias.
> >     So, in this situation, we have to flush the related
> >     vma to make sure correct read.
> 
> Looks good. I'll queue this for the next kernel update.
> 


has this been proposed to lkml ?

> Cheers,
> 
> Bruce
> 
> >
> >Signed-off-by: Zumeng Chen<zumeng.chen@windriver.com>
> >---
> >  include/linux/shmem_fs.h |   10 ++++++++++
> >  mm/msync.c               |   22 +++++++++++++++++++++-
> >  mm/shmem.c               |    5 +++++
> >  3 files changed, 36 insertions(+), 1 deletions(-)
> >
> >diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h
> >index aa08fa8..62a2d57 100644
> >--- a/include/linux/shmem_fs.h
> >+++ b/include/linux/shmem_fs.h
> >@@ -12,6 +12,15 @@
> >
> >  #define SHMEM_SYMLINK_INLINE_LEN (SHMEM_NR_DIRECT * sizeof(swp_entry_t))
> >
> >+/*
> >+
> >+*/
> >+#ifndef cpu_has_dc_aliases
> >+#define CPU_HAS_CACHE_ALIAS 0
> >+#else
> >+#define CPU_HAS_CACHE_ALIAS cpu_has_dc_aliases
> >+#endif
> >+
> >  struct shmem_inode_info {
> >  	spinlock_t		lock;
> >  	unsigned long		flags;
> >@@ -49,6 +58,7 @@ static inline struct shmem_inode_info *SHMEM_I(struct inode *inode)
> >  /*
> >   * Functions in mm/shmem.c called directly from elsewhere:
> >   */
> >+extern int is_shmem_file(struct file *file);
> >  extern int init_tmpfs(void);
> >  extern int shmem_fill_super(struct super_block *sb, void *data, int silent);
> >  extern struct file *shmem_file_setup(const char *name,
> >diff --git a/mm/msync.c b/mm/msync.c
> >index 632df45..84cb068 100644
> >--- a/mm/msync.c
> >+++ b/mm/msync.c
> >@@ -13,6 +13,8 @@
> >  #include<linux/file.h>
> >  #include<linux/syscalls.h>
> >  #include<linux/sched.h>
> >+#include<linux/shmem_fs.h>
> >+#include<asm/cacheflush.h>
> >
> >  /*
> >   * MS_SYNC syncs the entire file - including mappings.
> >@@ -33,6 +35,7 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
> >  	unsigned long end;
> >  	struct mm_struct *mm = current->mm;
> >  	struct vm_area_struct *vma;
> >+	struct file *file;
> >  	int unmapped_error = 0;
> >  	int error = -EINVAL;
> >
> >@@ -56,8 +59,25 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
> >  	 */
> >  	down_read(&mm->mmap_sem);
> >  	vma = find_vma(mm, start);
> >+
> >+#ifdef CONFIG_TMPFS
> >+	/*
> >+	 * For tmpfs, no matter which flag(ASYNC or SYNC) gets from msync,
> >+	 * there is not so much thing to do for CPUs without cache alias,
> >+	 * But for some CPUs with cache alias, msync has to flush cache
> >+	 * explicitly, which makes sure the data coherency between memory
> >+	 * file and cache.
> >+	 */
> >+	file =  vma->vm_file;
> >+	if (is_shmem_file(file)) {
> >+		if(CPU_HAS_CACHE_ALIAS)
> >+			flush_cache_range(vma, start, start+len);
> >+		error = 0;
> >+		goto out_unlock;
> >+	}
> >+#endif
> >+
> >  	for (;;) {
> >-		struct file *file;
> >
> >  		/* Still start<  end. */
> >  		error = -ENOMEM;
> >diff --git a/mm/shmem.c b/mm/shmem.c
> >index 92e5c15..4fabfac 100644
> >--- a/mm/shmem.c
> >+++ b/mm/shmem.c
> >@@ -2793,6 +2793,11 @@ static struct file_system_type tmpfs_fs_type = {
> >  	.kill_sb	= kill_litter_super,
> >  };
> >
> >+int is_shmem_file(struct file *file)
> >+{
> >+	return (file->f_op ==&shmem_file_operations)? 1 : 0;
> >+}
> >+
> >  int __init init_tmpfs(void)
> >  {
> >  	int error;
> 
> _______________________________________________
> yocto mailing list
> yocto@yoctoproject.org
> https://lists.yoctoproject.org/listinfo/yocto

-- 
-Khem

Re: [V2 PATCH 1/1] mm: msync: fix issues of sys_msync on tmpfs

[Bruce Ashfield]

On 12-01-03 11:53 PM, Khem Raj wrote:
> On (22/12/11 09:24), Bruce Ashfield wrote:
>> On 11-12-21 10:07 PM, Zumeng Chen wrote:
>>> There are two problems as follows shown:
>>> 1 ) for TMPFS, nothing need to be done in sys_msync,
>>>      sys_msync just return 0 for all arches.
>>>
>>> 2 ) But for MIPS CPUs with cache alias(dmesg|grep alias),
>>>      it maybe has the issue, which reported by msync test
>>>      suites in ltp-full, when the memory of memset used by
>>>      msycn01 has cache alias.
>>>      So, in this situation, we have to flush the related
>>>      vma to make sure correct read.
>>
>> Looks good. I'll queue this for the next kernel update.
>>
>
>
> has this been proposed to lkml ?

Not yet. We are still reviewing it @ Wind River via our process, and
since we picked it up on MIPS, we are running it past Ralf as well.
Still time to get it in place for 3.3, if it is legit.

Cheers,

Bruce

>
>> Cheers,
>>
>> Bruce
>>
>>>
>>> Signed-off-by: Zumeng Chen<zumeng.chen@windriver.com>
>>> ---
>>>   include/linux/shmem_fs.h |   10 ++++++++++
>>>   mm/msync.c               |   22 +++++++++++++++++++++-
>>>   mm/shmem.c               |    5 +++++
>>>   3 files changed, 36 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h
>>> index aa08fa8..62a2d57 100644
>>> --- a/include/linux/shmem_fs.h
>>> +++ b/include/linux/shmem_fs.h
>>> @@ -12,6 +12,15 @@
>>>
>>>   #define SHMEM_SYMLINK_INLINE_LEN (SHMEM_NR_DIRECT * sizeof(swp_entry_t))
>>>
>>> +/*
>>> +
>>> +*/
>>> +#ifndef cpu_has_dc_aliases
>>> +#define CPU_HAS_CACHE_ALIAS 0
>>> +#else
>>> +#define CPU_HAS_CACHE_ALIAS cpu_has_dc_aliases
>>> +#endif
>>> +
>>>   struct shmem_inode_info {
>>>   	spinlock_t		lock;
>>>   	unsigned long		flags;
>>> @@ -49,6 +58,7 @@ static inline struct shmem_inode_info *SHMEM_I(struct inode *inode)
>>>   /*
>>>    * Functions in mm/shmem.c called directly from elsewhere:
>>>    */
>>> +extern int is_shmem_file(struct file *file);
>>>   extern int init_tmpfs(void);
>>>   extern int shmem_fill_super(struct super_block *sb, void *data, int silent);
>>>   extern struct file *shmem_file_setup(const char *name,
>>> diff --git a/mm/msync.c b/mm/msync.c
>>> index 632df45..84cb068 100644
>>> --- a/mm/msync.c
>>> +++ b/mm/msync.c
>>> @@ -13,6 +13,8 @@
>>>   #include<linux/file.h>
>>>   #include<linux/syscalls.h>
>>>   #include<linux/sched.h>
>>> +#include<linux/shmem_fs.h>
>>> +#include<asm/cacheflush.h>
>>>
>>>   /*
>>>    * MS_SYNC syncs the entire file - including mappings.
>>> @@ -33,6 +35,7 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
>>>   	unsigned long end;
>>>   	struct mm_struct *mm = current->mm;
>>>   	struct vm_area_struct *vma;
>>> +	struct file *file;
>>>   	int unmapped_error = 0;
>>>   	int error = -EINVAL;
>>>
>>> @@ -56,8 +59,25 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
>>>   	 */
>>>   	down_read(&mm->mmap_sem);
>>>   	vma = find_vma(mm, start);
>>> +
>>> +#ifdef CONFIG_TMPFS
>>> +	/*
>>> +	 * For tmpfs, no matter which flag(ASYNC or SYNC) gets from msync,
>>> +	 * there is not so much thing to do for CPUs without cache alias,
>>> +	 * But for some CPUs with cache alias, msync has to flush cache
>>> +	 * explicitly, which makes sure the data coherency between memory
>>> +	 * file and cache.
>>> +	 */
>>> +	file =  vma->vm_file;
>>> +	if (is_shmem_file(file)) {
>>> +		if(CPU_HAS_CACHE_ALIAS)
>>> +			flush_cache_range(vma, start, start+len);
>>> +		error = 0;
>>> +		goto out_unlock;
>>> +	}
>>> +#endif
>>> +
>>>   	for (;;) {
>>> -		struct file *file;
>>>
>>>   		/* Still start<   end. */
>>>   		error = -ENOMEM;
>>> diff --git a/mm/shmem.c b/mm/shmem.c
>>> index 92e5c15..4fabfac 100644
>>> --- a/mm/shmem.c
>>> +++ b/mm/shmem.c
>>> @@ -2793,6 +2793,11 @@ static struct file_system_type tmpfs_fs_type = {
>>>   	.kill_sb	= kill_litter_super,
>>>   };
>>>
>>> +int is_shmem_file(struct file *file)
>>> +{
>>> +	return (file->f_op ==&shmem_file_operations)? 1 : 0;
>>> +}
>>> +
>>>   int __init init_tmpfs(void)
>>>   {
>>>   	int error;
>>
>> _______________________________________________
>> yocto mailing list
>> yocto@yoctoproject.org
>> https://lists.yoctoproject.org/listinfo/yocto
>