The follow_page() function can potentially return -EFAULT so I added
checks for this.
Also I silenced an uninitialized variable warning on my version of gcc
(version 4.3.2).
Signed-off-by: Dan Carpenter <[email protected]>
---
I'm not very familiar with this code, so handle with care.
diff --git a/mm/ksm.c b/mm/ksm.c
index 8cdfc2a..956880f 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -365,7 +365,7 @@ static int break_ksm(struct vm_area_struct *vma, unsigned long addr)
do {
cond_resched();
page = follow_page(vma, addr, FOLL_GET);
- if (!page)
+ if (IS_ERR_OR_NULL(page))
break;
if (PageKsm(page))
ret = handle_mm_fault(vma->vm_mm, vma, addr,
@@ -447,7 +447,7 @@ static struct page *get_mergeable_page(struct rmap_item *rmap_item)
goto out;
page = follow_page(vma, addr, FOLL_GET);
- if (!page)
+ if (IS_ERR_OR_NULL(page))
goto out;
if (PageAnon(page)) {
flush_anon_page(vma, page, addr);
@@ -1086,7 +1086,7 @@ struct rmap_item *unstable_tree_search_insert(struct rmap_item *rmap_item,
cond_resched();
tree_rmap_item = rb_entry(*new, struct rmap_item, node);
tree_page = get_mergeable_page(tree_rmap_item);
- if (!tree_page)
+ if (IS_ERR_OR_NULL(tree_page))
return NULL;
/*
@@ -1294,7 +1294,7 @@ next_mm:
if (ksm_test_exit(mm))
break;
*page = follow_page(vma, ksm_scan.address, FOLL_GET);
- if (*page && PageAnon(*page)) {
+ if (!IS_ERR_OR_NULL(*page) && PageAnon(*page)) {
flush_anon_page(vma, *page, ksm_scan.address);
flush_dcache_page(*page);
rmap_item = get_next_rmap_item(slot,
@@ -1308,7 +1308,7 @@ next_mm:
up_read(&mm->mmap_sem);
return rmap_item;
}
- if (*page)
+ if (!IS_ERR_OR_NULL(*page))
put_page(*page);
ksm_scan.address += PAGE_SIZE;
cond_resched();
@@ -1367,7 +1367,7 @@ next_mm:
static void ksm_do_scan(unsigned int scan_npages)
{
struct rmap_item *rmap_item;
- struct page *page;
+ struct page *uninitialized_var(page);
while (scan_npages--) {
cond_resched();
On 04/21/2010 06:27 AM, Dan Carpenter wrote:
> The follow_page() function can potentially return -EFAULT so I added
> checks for this.
>
> Also I silenced an uninitialized variable warning on my version of gcc
> (version 4.3.2).
>
> Signed-off-by: Dan Carpenter<[email protected]>
Acked-by: Rik van Riel <[email protected]>
On Wed, Apr 21, 2010 at 11:24:24AM -0400, Rik van Riel wrote:
> On 04/21/2010 06:27 AM, Dan Carpenter wrote:
> > The follow_page() function can potentially return -EFAULT so I added
> > checks for this.
> >
> > Also I silenced an uninitialized variable warning on my version of gcc
> > (version 4.3.2).
> >
> > Signed-off-by: Dan Carpenter<[email protected]>
>
> Acked-by: Rik van Riel <[email protected]>
while (!(page = follow_page(vma, start, foll_flags)))
{
gup only checks for null, so when exactly is follow_page going to
return -EFAULT? It's not immediately clear.
On Wed, Apr 21, 2010 at 07:46:15PM +0200, Andrea Arcangeli wrote:
> On Wed, Apr 21, 2010 at 11:24:24AM -0400, Rik van Riel wrote:
> > On 04/21/2010 06:27 AM, Dan Carpenter wrote:
> > > The follow_page() function can potentially return -EFAULT so I added
> > > checks for this.
> > >
> > > Also I silenced an uninitialized variable warning on my version of gcc
> > > (version 4.3.2).
> > >
> > > Signed-off-by: Dan Carpenter<[email protected]>
> >
> > Acked-by: Rik van Riel <[email protected]>
>
> while (!(page = follow_page(vma, start, foll_flags)))
> {
>
> gup only checks for null, so when exactly is follow_page going to
> return -EFAULT? It's not immediately clear.
Check below that loop. If it returns non-null, the first check is
whether it IS_ERR().
How about the below?
Hannes
---
From: Johannes Weiner <[email protected]>
Subject: mm: document follow_page()
Signed-off-by: Johannes Weiner <[email protected]>
---
mm/memory.c | 13 +++++++++++--
1 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/mm/memory.c b/mm/memory.c
index 833952d..119b7cc 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1227,8 +1227,17 @@ int zap_vma_ptes(struct vm_area_struct *vma, unsigned long address,
}
EXPORT_SYMBOL_GPL(zap_vma_ptes);
-/*
- * Do a quick page-table lookup for a single page.
+/**
+ * follow_page - look up a page descriptor from a user-virtual address
+ * @vma: vm_area_struct mapping @address
+ * @address: virtual address to look up
+ * @flags: flags modifying lookup behaviour
+ *
+ * @flags can have FOLL_ flags set, defined in <linux/mm.h>
+ *
+ * Returns the mapped (struct page *), %NULL if no mapping exists, or
+ * an error pointer if there is a mapping to something not represented
+ * by a page descriptor (see also vm_normal_page()).
*/
struct page *follow_page(struct vm_area_struct *vma, unsigned long address,
unsigned int flags)
--
1.7.0.2
On Wed, Apr 21, 2010 at 10:53:05PM +0200, Johannes Weiner wrote:
> Check below that loop. If it returns non-null, the first check is
> whether it IS_ERR().
Indeed.
> + * Returns the mapped (struct page *), %NULL if no mapping exists, or
> + * an error pointer if there is a mapping to something not represented
> + * by a page descriptor (see also vm_normal_page()).
where exactly in vm_normal_page? Note I already checked vm_normal_page
before sending the prev email and I didn't immediately see. I search
return and they all return NULL except the return pfn_to_page(pfn), so
is pfn_to_page that returns -EFAULT (the implementations I checked
don't but there are plenty that I didn't check...).
On Wed, Apr 21, 2010 at 10:58:07PM +0200, Andrea Arcangeli wrote:
> On Wed, Apr 21, 2010 at 10:53:05PM +0200, Johannes Weiner wrote:
> > + * Returns the mapped (struct page *), %NULL if no mapping exists, or
> > + * an error pointer if there is a mapping to something not represented
> > + * by a page descriptor (see also vm_normal_page()).
>
> where exactly in vm_normal_page? Note I already checked vm_normal_page
> before sending the prev email and I didn't immediately see. I search
> return and they all return NULL except the return pfn_to_page(pfn), so
> is pfn_to_page that returns -EFAULT (the implementations I checked
> don't but there are plenty that I didn't check...).
It's not vm_normal_page() that returns -EFAULT. It is follow_page()
that translates NULL from vm_normal_page() into -EFAULT.
On Wed, 21 Apr 2010 12:27:59 +0200
Dan Carpenter <[email protected]> wrote:
Hello
> The follow_page() function can potentially return -EFAULT so I added
> checks for this.
>
> Also I silenced an uninitialized variable warning on my version of gcc
> (version 4.3.2).
>
> Signed-off-by: Dan Carpenter <[email protected]>
> ---
> I'm not very familiar with this code, so handle with care.
Acked-by: Izik Eidus <[email protected]>
Thanks.