Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1601064 > unrolled thread

[PATCH v2 10/10] mm: remove SWAP_[SUCCESS|AGAIN|FAIL]

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 10/10] mm: remove SWAP_[SUCCESS|AGAIN|FAIL] Minchan Kim <minchan@kernel.org> - 2017-03-15 06:30 +0100
    Re: [PATCH v2 10/10] mm: remove SWAP_[SUCCESS|AGAIN|FAIL] Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-03-16 05:50 +0100
      Re: [PATCH v2 10/10] mm: remove SWAP_[SUCCESS|AGAIN|FAIL] Minchan Kim <minchan@kernel.org> - 2017-03-16 06:40 +0100
        Re: [PATCH v2 10/10] mm: remove SWAP_[SUCCESS|AGAIN|FAIL] Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-03-16 06:50 +0100
          Re: [PATCH v2 10/10] mm: remove SWAP_[SUCCESS|AGAIN|FAIL] Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-03-16 07:00 +0100
          Re: [PATCH v2 10/10] mm: remove SWAP_[SUCCESS|AGAIN|FAIL] Minchan Kim <minchan@kernel.org> - 2017-03-16 07:30 +0100
    Re: [PATCH v2 10/10] mm: remove SWAP_[SUCCESS|AGAIN|FAIL] kbuild test robot <lkp@intel.com> - 2017-03-16 20:00 +0100

#1601064 — [PATCH v2 10/10] mm: remove SWAP_[SUCCESS|AGAIN|FAIL]

There is no user for it. Remove it.

Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 include/linux/rmap.h | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/include/linux/rmap.h b/include/linux/rmap.h
index 13ed232..43ef2c3 100644
--- a/include/linux/rmap.h
+++ b/include/linux/rmap.h
@@ -295,11 +295,4 @@ static inline int page_mkclean(struct page *page)
 
 #endif	/* CONFIG_MMU */
 
-/*
- * Return values of try_to_unmap
- */
-#define SWAP_SUCCESS	0
-#define SWAP_AGAIN	1
-#define SWAP_FAIL	2
-
 #endif	/* _LINUX_RMAP_H */
-- 
2.7.4

[toc] | [next] | [standalone]

#1601928

Hello,


On (03/15/17 14:24), Minchan Kim wrote:
> There is no user for it. Remove it.
> 

there is one.

mm/rmap.c

try_to_unmap_one()
...
	if (unlikely(PageSwapBacked(page) != PageSwapCache(page))) {
		WARN_ON_ONCE(1);
		ret = SWAP_FAIL;
		page_vma_mapped_walk_done(&pvmw);
		break;
	}

	-ss

[toc] | [prev] | [next] | [standalone]

#1601935

Hey, Sergey,

On Thu, Mar 16, 2017 at 01:40:23PM +0900, Sergey Senozhatsky wrote:
> Hello,
> 
> 
> On (03/15/17 14:24), Minchan Kim wrote:
> > There is no user for it. Remove it.
> > 
> 
> there is one.
> 
> mm/rmap.c
> 
> try_to_unmap_one()
> ...
> 	if (unlikely(PageSwapBacked(page) != PageSwapCache(page))) {
> 		WARN_ON_ONCE(1);
> 		ret = SWAP_FAIL;
> 		page_vma_mapped_walk_done(&pvmw);
> 		break;
> 	}

"There is no user for it"

I was liar so need to be a honest guy.
Thanks, Sergey!

Andrew, Please make me honest. Sorry about that.

From 286eae763d145338dc6c437258a741f293fab609 Mon Sep 17 00:00:00 2001
From: Minchan Kim <minchan@kernel.org>
Date: Thu, 16 Mar 2017 14:20:42 +0900
Subject: [PATCH] mm: use false instead of SWAP_FAIL

There was mistake with git-rebase.
SWAP_FAIL was removed. Use false.
It could be folded into mm-make-ttus-return-boolean.patch.

Reported-by: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 mm/rmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/rmap.c b/mm/rmap.c
index aa25fde..2422758 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1414,7 +1414,7 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 			 */
 			if (unlikely(PageSwapBacked(page) != PageSwapCache(page))) {
 				WARN_ON_ONCE(1);
-				ret = SWAP_FAIL;
+				ret = false;
 				page_vma_mapped_walk_done(&pvmw);
 				break;
 			}
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1601960

On (03/16/17 14:33), Minchan Kim wrote:
[..]
> "There is no user for it"
> 
> I was liar so need to be a honest guy.

ha-ha-ha. I didn't say that :)

[..]
> @@ -1414,7 +1414,7 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
>  			 */
>  			if (unlikely(PageSwapBacked(page) != PageSwapCache(page))) {
>  				WARN_ON_ONCE(1);
> -				ret = SWAP_FAIL;
> +				ret = false;
>  				page_vma_mapped_walk_done(&pvmw);
>  				break;
>  			}


one thing to notice here is that 'ret = false' and 'ret = SWAP_FAIL'
are not the same and must produce different results. `ret' is bool
and SWAP_FAIL was 2. it's return 1 vs return 0, isn't it? so was
there a bug before?

	-ss

[toc] | [prev] | [next] | [standalone]

#1601961

On (03/16/17 14:51), Minchan Kim wrote:
[..]
> > > @@ -1414,7 +1414,7 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
> > >  			 */
> > >  			if (unlikely(PageSwapBacked(page) != PageSwapCache(page))) {
> > >  				WARN_ON_ONCE(1);
> > > -				ret = SWAP_FAIL;
> > > +				ret = false;
> > >  				page_vma_mapped_walk_done(&pvmw);
> > >  				break;
> > >  			}
> > 
> > 
> > one thing to notice here is that 'ret = false' and 'ret = SWAP_FAIL'
> > are not the same and must produce different results. `ret' is bool
> > and SWAP_FAIL was 2. it's return 1 vs return 0, isn't it? so was
> > there a bug before?
> 
> No, it was not a bug. Just my patchset changed return value meaning.
> Look at this.
> https://marc.info/?l=linux-mm&m=148955552314806&w=2
> 
> So, false means SWAP_FAIL(ie., stop rmap scanning and bail out) now.

ah, indeed. sorry, didn't notice that.

thanks.

	-ss

[toc] | [prev] | [next] | [standalone]

#1601981

On Thu, Mar 16, 2017 at 02:44:30PM +0900, Sergey Senozhatsky wrote:
> On (03/16/17 14:33), Minchan Kim wrote:
> [..]
> > "There is no user for it"
> > 
> > I was liar so need to be a honest guy.
> 
> ha-ha-ha. I didn't say that :)
> 
> [..]
> > @@ -1414,7 +1414,7 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
> >  			 */
> >  			if (unlikely(PageSwapBacked(page) != PageSwapCache(page))) {
> >  				WARN_ON_ONCE(1);
> > -				ret = SWAP_FAIL;
> > +				ret = false;
> >  				page_vma_mapped_walk_done(&pvmw);
> >  				break;
> >  			}
> 
> 
> one thing to notice here is that 'ret = false' and 'ret = SWAP_FAIL'
> are not the same and must produce different results. `ret' is bool
> and SWAP_FAIL was 2. it's return 1 vs return 0, isn't it? so was
> there a bug before?

No, it was not a bug. Just my patchset changed return value meaning.
Look at this.
https://marc.info/?l=linux-mm&m=148955552314806&w=2

So, false means SWAP_FAIL(ie., stop rmap scanning and bail out) now.

Thanks.

[toc] | [prev] | [next] | [standalone]

#1602778

[Multipart message — attachments visible in raw view] — view raw

Hi Minchan,

[auto build test ERROR on mmotm/master]
[also build test ERROR on next-20170310]
[cannot apply to v4.11-rc2]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Minchan-Kim/make-try_to_unmap-simple/20170317-020635
base:   git://git.cmpxchg.org/linux-mmotm.git master
config: i386-tinyconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   mm/rmap.c: In function 'try_to_unmap_one':
>> mm/rmap.c:1417:11: error: 'SWAP_FAIL' undeclared (first use in this function)
        ret = SWAP_FAIL;
              ^~~~~~~~~
   mm/rmap.c:1417:11: note: each undeclared identifier is reported only once for each function it appears in

vim +/SWAP_FAIL +1417 mm/rmap.c

^1da177e Linus Torvalds 2005-04-16  1411  			/*
^1da177e Linus Torvalds 2005-04-16  1412  			 * Store the swap location in the pte.
^1da177e Linus Torvalds 2005-04-16  1413  			 * See handle_pte_fault() ...
^1da177e Linus Torvalds 2005-04-16  1414  			 */
efeba3bd Minchan Kim    2017-03-10  1415  			if (unlikely(PageSwapBacked(page) != PageSwapCache(page))) {
efeba3bd Minchan Kim    2017-03-10  1416  				WARN_ON_ONCE(1);
3154f021 Minchan Kim    2017-03-10 @1417  				ret = SWAP_FAIL;
3154f021 Minchan Kim    2017-03-10  1418  				page_vma_mapped_walk_done(&pvmw);
3154f021 Minchan Kim    2017-03-10  1419  				break;
3154f021 Minchan Kim    2017-03-10  1420  			}

:::::: The code at line 1417 was first introduced by commit
:::::: 3154f021001fba264cc2cba4c4ff4bfb5a3e2f92 mm: fix lazyfree BUG_ON check in try_to_unmap_one()

:::::: TO: Minchan Kim <minchan@kernel.org>
:::::: CC: Johannes Weiner <hannes@cmpxchg.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web