For the implementation (patch 1/3), need fill buffer as full as
possible when buffer space is not enough.
For the caller (patch 2/3, 3/3), need check the return value of
mpol_to_str().
Signed-off-by: Chen Gang <[email protected]>
---
fs/proc/task_mmu.c | 16 ++++++++++++++--
mm/mempolicy.c | 14 ++++++++++----
mm/shmem.c | 15 ++++++++++++++-
3 files changed, 38 insertions(+), 7 deletions(-)
Need still try to fill buffer as full as possible if the buffer space
is not enough, commonly, the caller can bear of it (e.g. print warning
and still continue).
Signed-off-by: Chen Gang <[email protected]>
---
mm/mempolicy.c | 14 ++++++++++----
1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 27022ca..c81b64f 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -2800,6 +2800,8 @@ out:
* Convert a mempolicy into a string.
* Returns the number of characters in buffer (if positive)
* or an error (negative)
+ * If the buffer space is not enough, it will return -ENOSPC,
+ * and try to fill the buffer as full as possible.
*/
int mpol_to_str(char *buffer, int maxlen, struct mempolicy *pol)
{
@@ -2842,11 +2844,10 @@ int mpol_to_str(char *buffer, int maxlen, struct mempolicy *pol)
return -EINVAL;
}
+ strlcpy(p, policy_modes[mode], maxlen);
l = strlen(policy_modes[mode]);
if (buffer + maxlen < p + l + 1)
return -ENOSPC;
-
- strcpy(p, policy_modes[mode]);
p += l;
if (flags & MPOL_MODE_FLAGS) {
@@ -2857,10 +2858,15 @@ int mpol_to_str(char *buffer, int maxlen, struct mempolicy *pol)
/*
* Currently, the only defined flags are mutually exclusive
*/
- if (flags & MPOL_F_STATIC_NODES)
+ if (flags & MPOL_F_STATIC_NODES) {
p += snprintf(p, buffer + maxlen - p, "static");
- else if (flags & MPOL_F_RELATIVE_NODES)
+ if (buffer + maxlen <= p)
+ return -ENOSPC;
+ } else if (flags & MPOL_F_RELATIVE_NODES) {
p += snprintf(p, buffer + maxlen - p, "relative");
+ if (buffer + maxlen <= p)
+ return -ENOSPC;
+ }
}
if (!nodes_empty(nodes)) {
--
1.7.7.6
mpol_to_str() may fail, and not fill the buffer (e.g. -EINVAL), so need
check about it, or buffer may not be zero based, and next seq_printf()
will cause issue.
Also print related warning when the buffer space is not enough.
Signed-off-by: Chen Gang <[email protected]>
---
fs/proc/task_mmu.c | 16 ++++++++++++++--
1 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index a117207..1cb7445 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -1359,7 +1359,7 @@ static int show_numa_map(struct seq_file *m, void *v, int is_pid)
struct mm_struct *mm = vma->vm_mm;
struct mm_walk walk = {};
struct mempolicy *pol;
- int n;
+ int n, ret;
char buffer[50];
if (!mm)
@@ -1376,7 +1376,19 @@ static int show_numa_map(struct seq_file *m, void *v, int is_pid)
walk.mm = mm;
pol = get_vma_policy(task, vma, vma->vm_start);
- mpol_to_str(buffer, sizeof(buffer), pol);
+ ret = mpol_to_str(buffer, sizeof(buffer), pol);
+ if (ret < 0)
+ switch (ret) {
+ case -ENOSPC:
+ pr_warn("in %s: string is truncated in mpol_to_str().\n",
+ __func__);
+ break;
+ default:
+ pr_err("in %s: call mpol_to_str() fail, errcode: %d. buffer: %p, size: %zu, pol: %p\n",
+ __func__, ret, buffer, sizeof(buffer), pol);
+ return ret;
+ }
+
mpol_cond_put(pol);
seq_printf(m, "%08lx %s", vma->vm_start, buffer);
--
1.7.7.6
mpol_to_str() may fail, and not fill the buffer (e.g. -EINVAL), so need
check about it, or buffer may not be zero based, and next seq_printf()
will cause issue.
Also print related warning when the buffer space is not enough.
Signed-off-by: Chen Gang <[email protected]>
---
mm/shmem.c | 15 ++++++++++++++-
1 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/mm/shmem.c b/mm/shmem.c
index 75010ba..eb4eec8 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -886,11 +886,24 @@ redirty:
static void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
{
char buffer[64];
+ int ret;
if (!mpol || mpol->mode == MPOL_DEFAULT)
return; /* show nothing */
- mpol_to_str(buffer, sizeof(buffer), mpol);
+ ret = mpol_to_str(buffer, sizeof(buffer), mpol);
+ if (ret < 0)
+ switch (ret) {
+ case -ENOSPC:
+ printk(KERN_WARNING
+ "in %s: string is truncated in mpol_to_str().\n",
+ __func__);
+ default:
+ printk(KERN_ERR
+ "in %s: call mpol_to_str() fail, errcode: %d. buffer: %p, size: %zu, pol: %p\n",
+ __func__, ret, buffer, sizeof(buffer), mpol);
+ return;
+ }
seq_printf(seq, ",mpol=%s", buffer);
}
--
1.7.7.6
On Tue, Aug 20, 2013 at 11:56:15AM +0800, Chen Gang wrote:
> For the implementation (patch 1/3), need fill buffer as full as
> possible when buffer space is not enough.
>
> For the caller (patch 2/3, 3/3), need check the return value of
> mpol_to_str().
>
> Signed-off-by: Chen Gang <[email protected]>
Won't simple check for mpol_to_str() < 0 be enough? IOW fix all
callers to check that mpol_to_str exit without errors. As far
as I see here are only two users. Something like
show_numa_map
ret = mpol_to_str();
if (ret)
return ret;
shmem_show_mpol
ret = mpol_to_str();
if (ret)
return ret;
sure you'll have to change shmem_show_mpol statement to return int code.
Won't this be more short and convenient?
On 08/20/2013 01:30 PM, Cyrill Gorcunov wrote:
> On Tue, Aug 20, 2013 at 11:56:15AM +0800, Chen Gang wrote:
>> For the implementation (patch 1/3), need fill buffer as full as
>> possible when buffer space is not enough.
>>
>> For the caller (patch 2/3, 3/3), need check the return value of
>> mpol_to_str().
>>
>> Signed-off-by: Chen Gang <[email protected]>
>
> Won't simple check for mpol_to_str() < 0 be enough? IOW fix all
> callers to check that mpol_to_str exit without errors. As far
> as I see here are only two users. Something like
>
> show_numa_map
> ret = mpol_to_str();
> if (ret)
> return ret;
>
> shmem_show_mpol
> ret = mpol_to_str();
> if (ret)
> return ret;
>
need "if (ret < 0)" instead of. ;-)
> sure you'll have to change shmem_show_mpol statement to return int code.
> Won't this be more short and convenient?
>
>
Hmm... if return -ENOSPC, in common processing, it still need continue
(but need let outside know about the string truncation).
So I still suggest to give more check for it.
Thanks.
--
Chen Gang
On Tue, Aug 20, 2013 at 01:41:40PM +0800, Chen Gang wrote:
>
> need "if (ret < 0)" instead of. ;-)
sure, it's details
>
> > sure you'll have to change shmem_show_mpol statement to return int code.
> > Won't this be more short and convenient?
> >
> >
>
> Hmm... if return -ENOSPC, in common processing, it still need continue
> (but need let outside know about the string truncation).
>
> So I still suggest to give more check for it.
I still don't like adding additional code like
+ ret = mpol_to_str(buffer, sizeof(buffer), mpol);
+ if (ret < 0)
+ switch (ret) {
+ case -ENOSPC:
+ printk(KERN_WARNING
+ "in %s: string is truncated in mpol_to_str().\n",
+ __func__);
+ default:
+ printk(KERN_ERR
+ "in %s: call mpol_to_str() fail, errcode: %d. buffer: %p, size: %zu, pol: %p\n",
+ __func__, ret, buffer, sizeof(buffer), mpol);
+ return;
+ }
this code is pretty neat for debugging purpose I think but in most case (if
only I've not missed something obvious) it simply won't be the case.
Won't somthing like below do the same but with smaller code change?
Note I've not even compiled it but it shows the idea.
---
fs/proc/task_mmu.c | 4 +++-
mm/shmem.c | 17 +++++++++--------
2 files changed, 12 insertions(+), 9 deletions(-)
Index: linux-2.6.git/fs/proc/task_mmu.c
===================================================================
--- linux-2.6.git.orig/fs/proc/task_mmu.c
+++ linux-2.6.git/fs/proc/task_mmu.c
@@ -1402,8 +1402,10 @@ static int show_numa_map(struct seq_file
walk.mm = mm;
pol = get_vma_policy(task, vma, vma->vm_start);
- mpol_to_str(buffer, sizeof(buffer), pol);
+ n = mpol_to_str(buffer, sizeof(buffer), pol);
mpol_cond_put(pol);
+ if (n < 0)
+ return n;
seq_printf(m, "%08lx %s", vma->vm_start, buffer);
Index: linux-2.6.git/mm/shmem.c
===================================================================
--- linux-2.6.git.orig/mm/shmem.c
+++ linux-2.6.git/mm/shmem.c
@@ -883,16 +883,20 @@ redirty:
#ifdef CONFIG_NUMA
#ifdef CONFIG_TMPFS
-static void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
+static int shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
{
char buffer[64];
+ int ret;
if (!mpol || mpol->mode == MPOL_DEFAULT)
- return; /* show nothing */
+ return 0; /* show nothing */
- mpol_to_str(buffer, sizeof(buffer), mpol);
+ ret = mpol_to_str(buffer, sizeof(buffer), mpol);
+ if (ret < 0)
+ return ret;
seq_printf(seq, ",mpol=%s", buffer);
+ return 0;
}
static struct mempolicy *shmem_get_sbmpol(struct shmem_sb_info *sbinfo)
@@ -951,9 +955,7 @@ static struct page *shmem_alloc_page(gfp
}
#else /* !CONFIG_NUMA */
#ifdef CONFIG_TMPFS
-static inline void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
-{
-}
+static inline int shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol) { return 0; }
#endif /* CONFIG_TMPFS */
static inline struct page *shmem_swapin(swp_entry_t swap, gfp_t gfp,
@@ -2577,8 +2579,7 @@ static int shmem_show_options(struct seq
if (!gid_eq(sbinfo->gid, GLOBAL_ROOT_GID))
seq_printf(seq, ",gid=%u",
from_kgid_munged(&init_user_ns, sbinfo->gid));
- shmem_show_mpol(seq, sbinfo->mpol);
- return 0;
+ return shmem_show_mpol(seq, sbinfo->mpol);
}
#endif /* CONFIG_TMPFS */
On 08/20/2013 02:47 PM, Cyrill Gorcunov wrote:
> On Tue, Aug 20, 2013 at 01:41:40PM +0800, Chen Gang wrote:
>>
>>> sure you'll have to change shmem_show_mpol statement to return int code.
>>> Won't this be more short and convenient?
>>>
>>>
>>
>> Hmm... if return -ENOSPC, in common processing, it still need continue
>> (but need let outside know about the string truncation).
>>
>> So I still suggest to give more check for it.
>
> I still don't like adding additional code like
>
> + ret = mpol_to_str(buffer, sizeof(buffer), mpol);
> + if (ret < 0)
> + switch (ret) {
> + case -ENOSPC:
> + printk(KERN_WARNING
> + "in %s: string is truncated in mpol_to_str().\n",
> + __func__);
> + default:
> + printk(KERN_ERR
> + "in %s: call mpol_to_str() fail, errcode: %d. buffer: %p, size: %zu, pol: %p\n",
> + __func__, ret, buffer, sizeof(buffer), mpol);
> + return;
> + }
>
> this code is pretty neat for debugging purpose I think but in most case (if
> only I've not missed something obvious) it simply won't be the case.
>
For mpol_to_str(), it is for printing string, I suggest to fill buffer
as full as possible like another printing string functions, -ENOSPC is
not critical error, callers may can bear it, and still want to continue.
For 2 callers, I still suggest to process '-ENOSPC' and continue, it is
really not a critical error, they can continue.
For the 'default' error processing:
I still suggest to 'printk' in shmem_show_mpol(), because when failure occurs, it has no return value to mark the failure to upper caller.
Hmm... but for show_numa_map(), may remove the 'printk', only return the error code is OK. :-)
Thanks.
> Won't somthing like below do the same but with smaller code change?
> Note I've not even compiled it but it shows the idea.
> ---
> fs/proc/task_mmu.c | 4 +++-
> mm/shmem.c | 17 +++++++++--------
> 2 files changed, 12 insertions(+), 9 deletions(-)
>
> Index: linux-2.6.git/fs/proc/task_mmu.c
> ===================================================================
> --- linux-2.6.git.orig/fs/proc/task_mmu.c
> +++ linux-2.6.git/fs/proc/task_mmu.c
> @@ -1402,8 +1402,10 @@ static int show_numa_map(struct seq_file
> walk.mm = mm;
>
> pol = get_vma_policy(task, vma, vma->vm_start);
> - mpol_to_str(buffer, sizeof(buffer), pol);
> + n = mpol_to_str(buffer, sizeof(buffer), pol);
> mpol_cond_put(pol);
> + if (n < 0)
> + return n;
>
> seq_printf(m, "%08lx %s", vma->vm_start, buffer);
>
> Index: linux-2.6.git/mm/shmem.c
> ===================================================================
> --- linux-2.6.git.orig/mm/shmem.c
> +++ linux-2.6.git/mm/shmem.c
> @@ -883,16 +883,20 @@ redirty:
>
> #ifdef CONFIG_NUMA
> #ifdef CONFIG_TMPFS
> -static void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
> +static int shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
> {
> char buffer[64];
> + int ret;
>
> if (!mpol || mpol->mode == MPOL_DEFAULT)
> - return; /* show nothing */
> + return 0; /* show nothing */
>
> - mpol_to_str(buffer, sizeof(buffer), mpol);
> + ret = mpol_to_str(buffer, sizeof(buffer), mpol);
> + if (ret < 0)
> + return ret;
>
> seq_printf(seq, ",mpol=%s", buffer);
> + return 0;
> }
>
> static struct mempolicy *shmem_get_sbmpol(struct shmem_sb_info *sbinfo)
> @@ -951,9 +955,7 @@ static struct page *shmem_alloc_page(gfp
> }
> #else /* !CONFIG_NUMA */
> #ifdef CONFIG_TMPFS
> -static inline void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
> -{
> -}
> +static inline int shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol) { return 0; }
> #endif /* CONFIG_TMPFS */
>
> static inline struct page *shmem_swapin(swp_entry_t swap, gfp_t gfp,
> @@ -2577,8 +2579,7 @@ static int shmem_show_options(struct seq
> if (!gid_eq(sbinfo->gid, GLOBAL_ROOT_GID))
> seq_printf(seq, ",gid=%u",
> from_kgid_munged(&init_user_ns, sbinfo->gid));
> - shmem_show_mpol(seq, sbinfo->mpol);
> - return 0;
> + return shmem_show_mpol(seq, sbinfo->mpol);
> }
> #endif /* CONFIG_TMPFS */
>
>
>
--
Chen Gang
On 08/20/2013 03:48 PM, Chen Gang wrote:
> On 08/20/2013 02:47 PM, Cyrill Gorcunov wrote:
>> On Tue, Aug 20, 2013 at 01:41:40PM +0800, Chen Gang wrote:
>>>
>>>> sure you'll have to change shmem_show_mpol statement to return int code.
>>>> Won't this be more short and convenient?
>>>>
>>>>
>>>
>>> Hmm... if return -ENOSPC, in common processing, it still need continue
>>> (but need let outside know about the string truncation).
>>>
>>> So I still suggest to give more check for it.
>>
>> I still don't like adding additional code like
>>
>> + ret = mpol_to_str(buffer, sizeof(buffer), mpol);
>> + if (ret < 0)
>> + switch (ret) {
>> + case -ENOSPC:
>> + printk(KERN_WARNING
>> + "in %s: string is truncated in mpol_to_str().\n",
>> + __func__);
Oh, that need 'break' in my original patch. :-)
>> + default:
>> + printk(KERN_ERR
>> + "in %s: call mpol_to_str() fail, errcode: %d. buffer: %p, size: %zu, pol: %p\n",
>> + __func__, ret, buffer, sizeof(buffer), mpol);
>> + return;
>> + }
>>
>> this code is pretty neat for debugging purpose I think but in most case (if
>> only I've not missed something obvious) it simply won't be the case.
>>
>
> For mpol_to_str(), it is for printing string, I suggest to fill buffer
> as full as possible like another printing string functions, -ENOSPC is
> not critical error, callers may can bear it, and still want to continue.
>
> For 2 callers, I still suggest to process '-ENOSPC' and continue, it is
> really not a critical error, they can continue.
>
> For the 'default' error processing:
>
> I still suggest to 'printk' in shmem_show_mpol(), because when failure occurs, it has no return value to mark the failure to upper caller.
> Hmm... but for show_numa_map(), may remove the 'printk', only return the error code is OK. :-)
>
>
> Thanks.
>
>> Won't somthing like below do the same but with smaller code change?
>> Note I've not even compiled it but it shows the idea.
>> ---
>> fs/proc/task_mmu.c | 4 +++-
>> mm/shmem.c | 17 +++++++++--------
>> 2 files changed, 12 insertions(+), 9 deletions(-)
>>
>> Index: linux-2.6.git/fs/proc/task_mmu.c
>> ===================================================================
>> --- linux-2.6.git.orig/fs/proc/task_mmu.c
>> +++ linux-2.6.git/fs/proc/task_mmu.c
>> @@ -1402,8 +1402,10 @@ static int show_numa_map(struct seq_file
>> walk.mm = mm;
>>
>> pol = get_vma_policy(task, vma, vma->vm_start);
>> - mpol_to_str(buffer, sizeof(buffer), pol);
>> + n = mpol_to_str(buffer, sizeof(buffer), pol);
>> mpol_cond_put(pol);
>> + if (n < 0)
>> + return n;
>>
>> seq_printf(m, "%08lx %s", vma->vm_start, buffer);
>>
>> Index: linux-2.6.git/mm/shmem.c
>> ===================================================================
>> --- linux-2.6.git.orig/mm/shmem.c
>> +++ linux-2.6.git/mm/shmem.c
>> @@ -883,16 +883,20 @@ redirty:
>>
>> #ifdef CONFIG_NUMA
>> #ifdef CONFIG_TMPFS
>> -static void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
>> +static int shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
>> {
>> char buffer[64];
>> + int ret;
>>
>> if (!mpol || mpol->mode == MPOL_DEFAULT)
>> - return; /* show nothing */
>> + return 0; /* show nothing */
>>
>> - mpol_to_str(buffer, sizeof(buffer), mpol);
>> + ret = mpol_to_str(buffer, sizeof(buffer), mpol);
>> + if (ret < 0)
>> + return ret;
>>
>> seq_printf(seq, ",mpol=%s", buffer);
>> + return 0;
>> }
>>
>> static struct mempolicy *shmem_get_sbmpol(struct shmem_sb_info *sbinfo)
>> @@ -951,9 +955,7 @@ static struct page *shmem_alloc_page(gfp
>> }
>> #else /* !CONFIG_NUMA */
>> #ifdef CONFIG_TMPFS
>> -static inline void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
>> -{
>> -}
>> +static inline int shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol) { return 0; }
>> #endif /* CONFIG_TMPFS */
>>
>> static inline struct page *shmem_swapin(swp_entry_t swap, gfp_t gfp,
>> @@ -2577,8 +2579,7 @@ static int shmem_show_options(struct seq
>> if (!gid_eq(sbinfo->gid, GLOBAL_ROOT_GID))
>> seq_printf(seq, ",gid=%u",
>> from_kgid_munged(&init_user_ns, sbinfo->gid));
>> - shmem_show_mpol(seq, sbinfo->mpol);
>> - return 0;
>> + return shmem_show_mpol(seq, sbinfo->mpol);
>> }
>> #endif /* CONFIG_TMPFS */
>>
>>
>>
>
>
--
Chen Gang
On 08/20/2013 03:51 PM, Chen Gang wrote:
> On 08/20/2013 03:48 PM, Chen Gang wrote:
>> On 08/20/2013 02:47 PM, Cyrill Gorcunov wrote:
>>> On Tue, Aug 20, 2013 at 01:41:40PM +0800, Chen Gang wrote:
>>>>
>>>>> sure you'll have to change shmem_show_mpol statement to return int code.
>>>>> Won't this be more short and convenient?
>>>>>
>>>>>
>>>>
>>>> Hmm... if return -ENOSPC, in common processing, it still need continue
>>>> (but need let outside know about the string truncation).
>>>>
>>>> So I still suggest to give more check for it.
>>>
>>> I still don't like adding additional code like
>>>
>>> + ret = mpol_to_str(buffer, sizeof(buffer), mpol);
>>> + if (ret < 0)
>>> + switch (ret) {
>>> + case -ENOSPC:
>>> + printk(KERN_WARNING
>>> + "in %s: string is truncated in mpol_to_str().\n",
>>> + __func__);
>
> Oh, that need 'break' in my original patch. :-)
>
>>> + default:
>>> + printk(KERN_ERR
>>> + "in %s: call mpol_to_str() fail, errcode: %d. buffer: %p, size: %zu, pol: %p\n",
>>> + __func__, ret, buffer, sizeof(buffer), mpol);
>>> + return;
>>> + }
>>>
>>> this code is pretty neat for debugging purpose I think but in most case (if
>>> only I've not missed something obvious) it simply won't be the case.
>>>
>>
>> For mpol_to_str(), it is for printing string, I suggest to fill buffer
>> as full as possible like another printing string functions, -ENOSPC is
>> not critical error, callers may can bear it, and still want to continue.
>>
>> For 2 callers, I still suggest to process '-ENOSPC' and continue, it is
>> really not a critical error, they can continue.
>>
>> For the 'default' error processing:
>>
>> I still suggest to 'printk' in shmem_show_mpol(), because when failure occurs, it has no return value to mark the failure to upper caller.
>> Hmm... but for show_numa_map(), may remove the 'printk', only return the error code is OK. :-)
>>
>>
>> Thanks.
>>
Oh, for '-ENOSPC', it means critical error, it is my fault.
So, for simplify thinking and implementation, use your patch below is OK
to me (but I suggest to print error information in the none return value
function).
:-)
>>> Won't somthing like below do the same but with smaller code change?
>>> Note I've not even compiled it but it shows the idea.
>>> ---
>>> fs/proc/task_mmu.c | 4 +++-
>>> mm/shmem.c | 17 +++++++++--------
>>> 2 files changed, 12 insertions(+), 9 deletions(-)
>>>
>>> Index: linux-2.6.git/fs/proc/task_mmu.c
>>> ===================================================================
>>> --- linux-2.6.git.orig/fs/proc/task_mmu.c
>>> +++ linux-2.6.git/fs/proc/task_mmu.c
>>> @@ -1402,8 +1402,10 @@ static int show_numa_map(struct seq_file
>>> walk.mm = mm;
>>>
>>> pol = get_vma_policy(task, vma, vma->vm_start);
>>> - mpol_to_str(buffer, sizeof(buffer), pol);
>>> + n = mpol_to_str(buffer, sizeof(buffer), pol);
>>> mpol_cond_put(pol);
>>> + if (n < 0)
>>> + return n;
>>>
>>> seq_printf(m, "%08lx %s", vma->vm_start, buffer);
>>>
>>> Index: linux-2.6.git/mm/shmem.c
>>> ===================================================================
>>> --- linux-2.6.git.orig/mm/shmem.c
>>> +++ linux-2.6.git/mm/shmem.c
>>> @@ -883,16 +883,20 @@ redirty:
>>>
>>> #ifdef CONFIG_NUMA
>>> #ifdef CONFIG_TMPFS
>>> -static void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
>>> +static int shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
>>> {
>>> char buffer[64];
>>> + int ret;
>>>
>>> if (!mpol || mpol->mode == MPOL_DEFAULT)
>>> - return; /* show nothing */
>>> + return 0; /* show nothing */
>>>
>>> - mpol_to_str(buffer, sizeof(buffer), mpol);
>>> + ret = mpol_to_str(buffer, sizeof(buffer), mpol);
>>> + if (ret < 0)
>>> + return ret;
>>>
>>> seq_printf(seq, ",mpol=%s", buffer);
>>> + return 0;
>>> }
>>>
>>> static struct mempolicy *shmem_get_sbmpol(struct shmem_sb_info *sbinfo)
>>> @@ -951,9 +955,7 @@ static struct page *shmem_alloc_page(gfp
>>> }
>>> #else /* !CONFIG_NUMA */
>>> #ifdef CONFIG_TMPFS
>>> -static inline void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
>>> -{
>>> -}
>>> +static inline int shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol) { return 0; }
>>> #endif /* CONFIG_TMPFS */
>>>
>>> static inline struct page *shmem_swapin(swp_entry_t swap, gfp_t gfp,
>>> @@ -2577,8 +2579,7 @@ static int shmem_show_options(struct seq
>>> if (!gid_eq(sbinfo->gid, GLOBAL_ROOT_GID))
>>> seq_printf(seq, ",gid=%u",
>>> from_kgid_munged(&init_user_ns, sbinfo->gid));
>>> - shmem_show_mpol(seq, sbinfo->mpol);
>>> - return 0;
>>> + return shmem_show_mpol(seq, sbinfo->mpol);
>>> }
>>> #endif /* CONFIG_TMPFS */
>>>
>>>
>>>
>>
>>
>
>
--
Chen Gang
On 08/20/2013 04:09 PM, Chen Gang wrote:
> On 08/20/2013 03:51 PM, Chen Gang wrote:
>> On 08/20/2013 03:48 PM, Chen Gang wrote:
>>> On 08/20/2013 02:47 PM, Cyrill Gorcunov wrote:
>>>> On Tue, Aug 20, 2013 at 01:41:40PM +0800, Chen Gang wrote:
>>>>>
>>>>>> sure you'll have to change shmem_show_mpol statement to return int code.
>>>>>> Won't this be more short and convenient?
>>>>>>
>>>>>>
>>>>>
>>>>> Hmm... if return -ENOSPC, in common processing, it still need continue
>>>>> (but need let outside know about the string truncation).
>>>>>
>>>>> So I still suggest to give more check for it.
>>>>
>>>> I still don't like adding additional code like
>>>>
>>>> + ret = mpol_to_str(buffer, sizeof(buffer), mpol);
>>>> + if (ret < 0)
>>>> + switch (ret) {
>>>> + case -ENOSPC:
>>>> + printk(KERN_WARNING
>>>> + "in %s: string is truncated in mpol_to_str().\n",
>>>> + __func__);
>>
>> Oh, that need 'break' in my original patch. :-)
>>
>>>> + default:
>>>> + printk(KERN_ERR
>>>> + "in %s: call mpol_to_str() fail, errcode: %d. buffer: %p, size: %zu, pol: %p\n",
>>>> + __func__, ret, buffer, sizeof(buffer), mpol);
>>>> + return;
>>>> + }
>>>>
>>>> this code is pretty neat for debugging purpose I think but in most case (if
>>>> only I've not missed something obvious) it simply won't be the case.
>>>>
>>>
>>> For mpol_to_str(), it is for printing string, I suggest to fill buffer
>>> as full as possible like another printing string functions, -ENOSPC is
>>> not critical error, callers may can bear it, and still want to continue.
>>>
>>> For 2 callers, I still suggest to process '-ENOSPC' and continue, it is
>>> really not a critical error, they can continue.
>>>
>>> For the 'default' error processing:
>>>
>>> I still suggest to 'printk' in shmem_show_mpol(), because when failure occurs, it has no return value to mark the failure to upper caller.
>>> Hmm... but for show_numa_map(), may remove the 'printk', only return the error code is OK. :-)
>>>
>>>
>>> Thanks.
>>>
>
> Oh, for '-ENOSPC', it means critical error, it is my fault.
>
> So, for simplify thinking and implementation, use your patch below is OK
> to me (but I suggest to print error information in the none return value
> function).
>
> :-)
>
>>>> Won't somthing like below do the same but with smaller code change?
>>>> Note I've not even compiled it but it shows the idea.
>>>> ---
>>>> fs/proc/task_mmu.c | 4 +++-
>>>> mm/shmem.c | 17 +++++++++--------
>>>> 2 files changed, 12 insertions(+), 9 deletions(-)
>>>>
>>>> Index: linux-2.6.git/fs/proc/task_mmu.c
>>>> ===================================================================
>>>> --- linux-2.6.git.orig/fs/proc/task_mmu.c
>>>> +++ linux-2.6.git/fs/proc/task_mmu.c
>>>> @@ -1402,8 +1402,10 @@ static int show_numa_map(struct seq_file
>>>> walk.mm = mm;
>>>>
>>>> pol = get_vma_policy(task, vma, vma->vm_start);
>>>> - mpol_to_str(buffer, sizeof(buffer), pol);
>>>> + n = mpol_to_str(buffer, sizeof(buffer), pol);
>>>> mpol_cond_put(pol);
>>>> + if (n < 0)
>>>> + return n;
>>>>
>>>> seq_printf(m, "%08lx %s", vma->vm_start, buffer);
>>>>
>>>> Index: linux-2.6.git/mm/shmem.c
>>>> ===================================================================
>>>> --- linux-2.6.git.orig/mm/shmem.c
>>>> +++ linux-2.6.git/mm/shmem.c
>>>> @@ -883,16 +883,20 @@ redirty:
>>>>
>>>> #ifdef CONFIG_NUMA
>>>> #ifdef CONFIG_TMPFS
>>>> -static void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
>>>> +static int shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
>>>> {
>>>> char buffer[64];
>>>> + int ret;
>>>>
>>>> if (!mpol || mpol->mode == MPOL_DEFAULT)
>>>> - return; /* show nothing */
>>>> + return 0; /* show nothing */
>>>>
>>>> - mpol_to_str(buffer, sizeof(buffer), mpol);
>>>> + ret = mpol_to_str(buffer, sizeof(buffer), mpol);
>>>> + if (ret < 0)
>>>> + return ret;
>>>>
>>>> seq_printf(seq, ",mpol=%s", buffer);
>>>> + return 0;
>>>> }
>>>>
>>>> static struct mempolicy *shmem_get_sbmpol(struct shmem_sb_info *sbinfo)
>>>> @@ -951,9 +955,7 @@ static struct page *shmem_alloc_page(gfp
>>>> }
>>>> #else /* !CONFIG_NUMA */
>>>> #ifdef CONFIG_TMPFS
>>>> -static inline void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
>>>> -{
>>>> -}
>>>> +static inline int shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol) { return 0; }
>>>> #endif /* CONFIG_TMPFS */
>>>>
>>>> static inline struct page *shmem_swapin(swp_entry_t swap, gfp_t gfp,
>>>> @@ -2577,8 +2579,7 @@ static int shmem_show_options(struct seq
>>>> if (!gid_eq(sbinfo->gid, GLOBAL_ROOT_GID))
>>>> seq_printf(seq, ",gid=%u",
>>>> from_kgid_munged(&init_user_ns, sbinfo->gid));
>>>> - shmem_show_mpol(seq, sbinfo->mpol);
>>>> - return 0;
>>>> + return shmem_show_mpol(seq, sbinfo->mpol);
>>>> }
>>>> #endif /* CONFIG_TMPFS */
>>>>
>>>>
>>>>
Oh, you have done, sorry.
>>>
>>>
>>
>>
>
>
--
Chen Gang
Sorry for reply multiple times to bother many members.
It seems, I am tired, and need have a rest today. :-(
On 08/20/2013 04:13 PM, Chen Gang F T wrote:
> On 08/20/2013 04:09 PM, Chen Gang wrote:
>> On 08/20/2013 03:51 PM, Chen Gang wrote:
>>> On 08/20/2013 03:48 PM, Chen Gang wrote:
>>>> On 08/20/2013 02:47 PM, Cyrill Gorcunov wrote:
>>>>> On Tue, Aug 20, 2013 at 01:41:40PM +0800, Chen Gang wrote:
>>>>>>
>>>>>>> sure you'll have to change shmem_show_mpol statement to return int code.
>>>>>>> Won't this be more short and convenient?
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> Hmm... if return -ENOSPC, in common processing, it still need continue
>>>>>> (but need let outside know about the string truncation).
>>>>>>
>>>>>> So I still suggest to give more check for it.
>>>>>
>>>>> I still don't like adding additional code like
>>>>>
>>>>> + ret = mpol_to_str(buffer, sizeof(buffer), mpol);
>>>>> + if (ret < 0)
>>>>> + switch (ret) {
>>>>> + case -ENOSPC:
>>>>> + printk(KERN_WARNING
>>>>> + "in %s: string is truncated in mpol_to_str().\n",
>>>>> + __func__);
>>>
>>> Oh, that need 'break' in my original patch. :-)
>>>
>>>>> + default:
>>>>> + printk(KERN_ERR
>>>>> + "in %s: call mpol_to_str() fail, errcode: %d. buffer: %p, size: %zu, pol: %p\n",
>>>>> + __func__, ret, buffer, sizeof(buffer), mpol);
>>>>> + return;
>>>>> + }
>>>>>
>>>>> this code is pretty neat for debugging purpose I think but in most case (if
>>>>> only I've not missed something obvious) it simply won't be the case.
>>>>>
>>>>
>>>> For mpol_to_str(), it is for printing string, I suggest to fill buffer
>>>> as full as possible like another printing string functions, -ENOSPC is
>>>> not critical error, callers may can bear it, and still want to continue.
>>>>
>>>> For 2 callers, I still suggest to process '-ENOSPC' and continue, it is
>>>> really not a critical error, they can continue.
>>>>
>>>> For the 'default' error processing:
>>>>
>>>> I still suggest to 'printk' in shmem_show_mpol(), because when failure occurs, it has no return value to mark the failure to upper caller.
>>>> Hmm... but for show_numa_map(), may remove the 'printk', only return the error code is OK. :-)
>>>>
>>>>
>>>> Thanks.
>>>>
>>
>> Oh, for '-ENOSPC', it means critical error, it is my fault.
>>
>> So, for simplify thinking and implementation, use your patch below is OK
>> to me (but I suggest to print error information in the none return value
>> function).
>>
>> :-)
>>
>>>>> Won't somthing like below do the same but with smaller code change?
>>>>> Note I've not even compiled it but it shows the idea.
>>>>> ---
>>>>> fs/proc/task_mmu.c | 4 +++-
>>>>> mm/shmem.c | 17 +++++++++--------
>>>>> 2 files changed, 12 insertions(+), 9 deletions(-)
>>>>>
>>>>> Index: linux-2.6.git/fs/proc/task_mmu.c
>>>>> ===================================================================
>>>>> --- linux-2.6.git.orig/fs/proc/task_mmu.c
>>>>> +++ linux-2.6.git/fs/proc/task_mmu.c
>>>>> @@ -1402,8 +1402,10 @@ static int show_numa_map(struct seq_file
>>>>> walk.mm = mm;
>>>>>
>>>>> pol = get_vma_policy(task, vma, vma->vm_start);
>>>>> - mpol_to_str(buffer, sizeof(buffer), pol);
>>>>> + n = mpol_to_str(buffer, sizeof(buffer), pol);
>>>>> mpol_cond_put(pol);
>>>>> + if (n < 0)
>>>>> + return n;
>>>>>
>>>>> seq_printf(m, "%08lx %s", vma->vm_start, buffer);
>>>>>
>>>>> Index: linux-2.6.git/mm/shmem.c
>>>>> ===================================================================
>>>>> --- linux-2.6.git.orig/mm/shmem.c
>>>>> +++ linux-2.6.git/mm/shmem.c
>>>>> @@ -883,16 +883,20 @@ redirty:
>>>>>
>>>>> #ifdef CONFIG_NUMA
>>>>> #ifdef CONFIG_TMPFS
>>>>> -static void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
>>>>> +static int shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
>>>>> {
>>>>> char buffer[64];
>>>>> + int ret;
>>>>>
>>>>> if (!mpol || mpol->mode == MPOL_DEFAULT)
>>>>> - return; /* show nothing */
>>>>> + return 0; /* show nothing */
>>>>>
>>>>> - mpol_to_str(buffer, sizeof(buffer), mpol);
>>>>> + ret = mpol_to_str(buffer, sizeof(buffer), mpol);
>>>>> + if (ret < 0)
>>>>> + return ret;
>>>>>
>>>>> seq_printf(seq, ",mpol=%s", buffer);
>>>>> + return 0;
>>>>> }
>>>>>
>>>>> static struct mempolicy *shmem_get_sbmpol(struct shmem_sb_info *sbinfo)
>>>>> @@ -951,9 +955,7 @@ static struct page *shmem_alloc_page(gfp
>>>>> }
>>>>> #else /* !CONFIG_NUMA */
>>>>> #ifdef CONFIG_TMPFS
>>>>> -static inline void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
>>>>> -{
>>>>> -}
>>>>> +static inline int shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol) { return 0; }
>>>>> #endif /* CONFIG_TMPFS */
>>>>>
>>>>> static inline struct page *shmem_swapin(swp_entry_t swap, gfp_t gfp,
>>>>> @@ -2577,8 +2579,7 @@ static int shmem_show_options(struct seq
>>>>> if (!gid_eq(sbinfo->gid, GLOBAL_ROOT_GID))
>>>>> seq_printf(seq, ",gid=%u",
>>>>> from_kgid_munged(&init_user_ns, sbinfo->gid));
>>>>> - shmem_show_mpol(seq, sbinfo->mpol);
>>>>> - return 0;
>>>>> + return shmem_show_mpol(seq, sbinfo->mpol);
>>>>> }
>>>>> #endif /* CONFIG_TMPFS */
>>>>>
>>>>>
>>>>>
>
> Oh, you have done, sorry.
>
>>>>
>>>>
>>>
>>>
>>
>>
>
>
--
Chen Gang
On Tue, Aug 20, 2013 at 04:09:22PM +0800, Chen Gang wrote:
> So, for simplify thinking and implementation, use your patch below is OK
> to me (but I suggest to print error information in the none return value
> function).
Chen, I'm not going to dive into this area now, too busy with other stuff
sorry, so if you consider my preliminary draft patch useful -- pick it up,
modify it, test it and send to lkml then (just CC me).
On 08/20/2013 04:25 PM, Cyrill Gorcunov wrote:
> On Tue, Aug 20, 2013 at 04:09:22PM +0800, Chen Gang wrote:
>> So, for simplify thinking and implementation, use your patch below is OK
>> to me (but I suggest to print error information in the none return value
>> function).
>
> Chen, I'm not going to dive into this area now, too busy with other stuff
> sorry, so if you consider my preliminary draft patch useful -- pick it up,
> modify it, test it and send to lkml then (just CC me).
>
>
OK, I will do tomorrow. :-)
Thanks.
--
Chen Gang
mpol_to_str() may fail, and not fill the buffer (e.g. -EINVAL), so need
check about it, or buffer may not be zero based, and next seq_printf()
will cause issue.
Signed-off-by: Chen Gang <[email protected]>
Cc: Cyrill Gorcunov <[email protected]>
---
fs/proc/task_mmu.c | 4 +++-
mm/shmem.c | 16 ++++++++++------
2 files changed, 13 insertions(+), 7 deletions(-)
Let shmem_show_mpol() return value, since it may fail.
Signed-off-by: Chen Gang <[email protected]>
Cc: Cyrill Gorcunov <[email protected]>
---
mm/shmem.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/mm/shmem.c b/mm/shmem.c
index 75010ba..e59d332 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -883,16 +883,17 @@ redirty:
#ifdef CONFIG_NUMA
#ifdef CONFIG_TMPFS
-static void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
+static int shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
{
char buffer[64];
if (!mpol || mpol->mode == MPOL_DEFAULT)
- return; /* show nothing */
+ return 0; /* show nothing */
mpol_to_str(buffer, sizeof(buffer), mpol);
seq_printf(seq, ",mpol=%s", buffer);
+ return 0;
}
static struct mempolicy *shmem_get_sbmpol(struct shmem_sb_info *sbinfo)
@@ -951,8 +952,9 @@ static struct page *shmem_alloc_page(gfp_t gfp,
}
#else /* !CONFIG_NUMA */
#ifdef CONFIG_TMPFS
-static inline void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
+static inline int shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
{
+ return 0;
}
#endif /* CONFIG_TMPFS */
@@ -2578,8 +2580,7 @@ static int shmem_show_options(struct seq_file *seq, struct dentry *root)
if (!gid_eq(sbinfo->gid, GLOBAL_ROOT_GID))
seq_printf(seq, ",gid=%u",
from_kgid_munged(&init_user_ns, sbinfo->gid));
- shmem_show_mpol(seq, sbinfo->mpol);
- return 0;
+ return shmem_show_mpol(seq, sbinfo->mpol);
}
#endif /* CONFIG_TMPFS */
--
1.7.7.6
mpol_to_str() may fail, and not fill the buffer (e.g. -EINVAL), so need
check about it, or buffer may not be zero based, and next seq_printf()
will cause issue.
The failure return need after mpol_cond_put() to match get_vma_policy().
Signed-off-by: Chen Gang <[email protected]>
Cc: Cyrill Gorcunov <[email protected]>
---
fs/proc/task_mmu.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 107d026..d87f5da 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -1376,8 +1376,10 @@ static int show_numa_map(struct seq_file *m, void *v, int is_pid)
walk.mm = mm;
pol = get_vma_policy(task, vma, vma->vm_start);
- mpol_to_str(buffer, sizeof(buffer), pol);
+ n = mpol_to_str(buffer, sizeof(buffer), pol);
mpol_cond_put(pol);
+ if (n < 0)
+ return n;
seq_printf(m, "%08lx %s", vma->vm_start, buffer);
--
1.7.7.6
mpol_to_str() may fail, and not fill the buffer (e.g. -EINVAL), so need
check about it, or buffer may not be zero based, and next seq_printf()
will cause issue.
Signed-off-by: Chen Gang <[email protected]>
Cc: Cyrill Gorcunov <[email protected]>
---
mm/shmem.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/mm/shmem.c b/mm/shmem.c
index e59d332..ae5112f 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -886,11 +886,14 @@ redirty:
static int shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
{
char buffer[64];
+ int ret;
if (!mpol || mpol->mode == MPOL_DEFAULT)
return 0; /* show nothing */
- mpol_to_str(buffer, sizeof(buffer), mpol);
+ ret = mpol_to_str(buffer, sizeof(buffer), mpol);
+ if (ret < 0)
+ return ret;
seq_printf(seq, ",mpol=%s", buffer);
return 0;
--
1.7.7.6
On Wed, Aug 21, 2013 at 10:21:22AM +0800, Chen Gang wrote:
> mpol_to_str() may fail, and not fill the buffer (e.g. -EINVAL), so need
> check about it, or buffer may not be zero based, and next seq_printf()
> will cause issue.
>
> Signed-off-by: Chen Gang <[email protected]>
> Cc: Cyrill Gorcunov <[email protected]>
Looks good to me, thanks!
Reviewed-by: Cyrill Gorcunov <[email protected]>
On 08/21/2013 01:31 PM, Cyrill Gorcunov wrote:
> On Wed, Aug 21, 2013 at 10:21:22AM +0800, Chen Gang wrote:
>> mpol_to_str() may fail, and not fill the buffer (e.g. -EINVAL), so need
>> check about it, or buffer may not be zero based, and next seq_printf()
>> will cause issue.
>>
>> Signed-off-by: Chen Gang <[email protected]>
>> Cc: Cyrill Gorcunov <[email protected]>
>
> Looks good to me, thanks!
>
> Reviewed-by: Cyrill Gorcunov <[email protected]>
>
Thanks.
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to [email protected]. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
>
--
Chen Gang
On Wed, 21 Aug 2013 10:23:58 +0800 Chen Gang <[email protected]> wrote:
> Let shmem_show_mpol() return value, since it may fail.
>
This patch has no effect.
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -883,16 +883,17 @@ redirty:
>
> #ifdef CONFIG_NUMA
> #ifdef CONFIG_TMPFS
> -static void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
> +static int shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
> {
> char buffer[64];
>
> if (!mpol || mpol->mode == MPOL_DEFAULT)
> - return; /* show nothing */
> + return 0; /* show nothing */
>
> mpol_to_str(buffer, sizeof(buffer), mpol);
Perhaps you meant to check the mpol_to_str() return value here.
> seq_printf(seq, ",mpol=%s", buffer);
> + return 0;
> }
>
> static struct mempolicy *shmem_get_sbmpol(struct shmem_sb_info *sbinfo)
On 08/22/2013 06:03 AM, Andrew Morton wrote:
> On Wed, 21 Aug 2013 10:23:58 +0800 Chen Gang <[email protected]> wrote:
>
>> Let shmem_show_mpol() return value, since it may fail.
>>
>
> This patch has no effect.
>
>> --- a/mm/shmem.c
>> +++ b/mm/shmem.c
>> @@ -883,16 +883,17 @@ redirty:
>>
>> #ifdef CONFIG_NUMA
>> #ifdef CONFIG_TMPFS
>> -static void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
>> +static int shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
>> {
>> char buffer[64];
>>
>> if (!mpol || mpol->mode == MPOL_DEFAULT)
>> - return; /* show nothing */
>> + return 0; /* show nothing */
>>
>> mpol_to_str(buffer, sizeof(buffer), mpol);
>
> Perhaps you meant to check the mpol_to_str() return value here.
>
Yes, I will merge them together (merge Patch 2/3 and Patch 3/3).
>> seq_printf(seq, ",mpol=%s", buffer);
>> + return 0;
>> }
>>
>> static struct mempolicy *shmem_get_sbmpol(struct shmem_sb_info *sbinfo)
>
>
>
Thanks.
--
Chen Gang
mpol_to_str() may fail, and not fill the buffer (e.g. -EINVAL), so need
check about it, or buffer may not be zero based, and next seq_printf()
will cause issue.
Also need let shmem_show_mpol() return value, since it may fail.
Signed-off-by: Chen Gang <[email protected]>
Reviewed-by: Cyrill Gorcunov <[email protected]>
---
mm/shmem.c | 16 ++++++++++------
1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/mm/shmem.c b/mm/shmem.c
index f00c1c1..b4d44db 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -883,16 +883,20 @@ redirty:
#ifdef CONFIG_NUMA
#ifdef CONFIG_TMPFS
-static void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
+static int shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
{
char buffer[64];
+ int ret;
if (!mpol || mpol->mode == MPOL_DEFAULT)
- return; /* show nothing */
+ return 0; /* show nothing */
- mpol_to_str(buffer, sizeof(buffer), mpol);
+ ret = mpol_to_str(buffer, sizeof(buffer), mpol);
+ if (ret < 0)
+ return ret;
seq_printf(seq, ",mpol=%s", buffer);
+ return 0;
}
static struct mempolicy *shmem_get_sbmpol(struct shmem_sb_info *sbinfo)
@@ -951,8 +955,9 @@ static struct page *shmem_alloc_page(gfp_t gfp,
}
#else /* !CONFIG_NUMA */
#ifdef CONFIG_TMPFS
-static inline void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
+static inline int shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
{
+ return 0;
}
#endif /* CONFIG_TMPFS */
@@ -2555,8 +2560,7 @@ static int shmem_show_options(struct seq_file *seq, struct dentry *root)
if (!gid_eq(sbinfo->gid, GLOBAL_ROOT_GID))
seq_printf(seq, ",gid=%u",
from_kgid_munged(&init_user_ns, sbinfo->gid));
- shmem_show_mpol(seq, sbinfo->mpol);
- return 0;
+ return shmem_show_mpol(seq, sbinfo->mpol);
}
#endif /* CONFIG_TMPFS */
--
1.7.7.6
Hello Maintainers:
Please help check this patch, when you have time.
If it need additional test, please let me know, I should try (better to
provide some suggestions for test).
Thanks.
On 08/22/2013 09:04 AM, Chen Gang wrote:
> mpol_to_str() may fail, and not fill the buffer (e.g. -EINVAL), so need
> check about it, or buffer may not be zero based, and next seq_printf()
> will cause issue.
>
> Also need let shmem_show_mpol() return value, since it may fail.
>
> Signed-off-by: Chen Gang <[email protected]>
> Reviewed-by: Cyrill Gorcunov <[email protected]>
> ---
> mm/shmem.c | 16 ++++++++++------
> 1 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/mm/shmem.c b/mm/shmem.c
> index f00c1c1..b4d44db 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -883,16 +883,20 @@ redirty:
>
> #ifdef CONFIG_NUMA
> #ifdef CONFIG_TMPFS
> -static void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
> +static int shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
> {
> char buffer[64];
> + int ret;
>
> if (!mpol || mpol->mode == MPOL_DEFAULT)
> - return; /* show nothing */
> + return 0; /* show nothing */
>
> - mpol_to_str(buffer, sizeof(buffer), mpol);
> + ret = mpol_to_str(buffer, sizeof(buffer), mpol);
> + if (ret < 0)
> + return ret;
>
> seq_printf(seq, ",mpol=%s", buffer);
> + return 0;
> }
>
> static struct mempolicy *shmem_get_sbmpol(struct shmem_sb_info *sbinfo)
> @@ -951,8 +955,9 @@ static struct page *shmem_alloc_page(gfp_t gfp,
> }
> #else /* !CONFIG_NUMA */
> #ifdef CONFIG_TMPFS
> -static inline void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
> +static inline int shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol)
> {
> + return 0;
> }
> #endif /* CONFIG_TMPFS */
>
> @@ -2555,8 +2560,7 @@ static int shmem_show_options(struct seq_file *seq, struct dentry *root)
> if (!gid_eq(sbinfo->gid, GLOBAL_ROOT_GID))
> seq_printf(seq, ",gid=%u",
> from_kgid_munged(&init_user_ns, sbinfo->gid));
> - shmem_show_mpol(seq, sbinfo->mpol);
> - return 0;
> + return shmem_show_mpol(seq, sbinfo->mpol);
> }
> #endif /* CONFIG_TMPFS */
>
>
--
Chen Gang