[PATCH 0/2] mm: do not call frontswap_init() during swapoff

[PATCH 0/2] mm: do not call frontswap_init() during swapoff

[Cesar Eduardo Barros]

The call to frontswap_init() was added in a place where it is called not
only from sys_swapon, but also from sys_swapoff. This pair of patches
fixes that.

The first patch moves the acquisition of swap_lock from enable_swap_info
to two separate helpers, one for sys_swapon and one for sys_swapoff. As
a bonus, it also makes the code for sys_swapoff less subtle.

The second patch moves the call to frontswap_init() from the common code
to the helper used only by sys_swapon.

Compile-tested only, but should be safe.

Cesar Eduardo Barros (2):
  mm: refactor reinsert of swap_info in sys_swapoff
  mm: do not call frontswap_init() during swapoff

 mm/swapfile.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

-- 
1.7.11.7

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/2] mm: refactor reinsert of swap_info in sys_swapoff

[Cesar Eduardo Barros]

The block within sys_swapoff which re-inserts the swap_info into the
swap_list in case of failure of try_to_unuse() reads a few values outside
the swap_lock. While this is safe at that point, it is subtle code.

Simplify the code by moving the reading of these values to a separate
function, refactoring it a bit so they are read from within the
swap_lock. This is easier to understand, and matches better the way it
worked before I unified the insertion of the swap_info from both
sys_swapon and sys_swapoff.

This change should make no functional difference. The only real change
is moving the read of two or three structure fields to within the lock
(frontswap_map_get() is nothing more than a read of p->frontswap_map).

Signed-off-by: Cesar Eduardo Barros <cesarb@cesarb.net>
---
 mm/swapfile.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 71cd288..886db96 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1443,13 +1443,12 @@ static int setup_swap_extents(struct swap_info_struct *sis, sector_t *span)
 	return generic_swapfile_activate(sis, swap_file, span);
 }
 
-static void enable_swap_info(struct swap_info_struct *p, int prio,
+static void _enable_swap_info(struct swap_info_struct *p, int prio,
 				unsigned char *swap_map,
 				unsigned long *frontswap_map)
 {
 	int i, prev;
 
-	spin_lock(&swap_lock);
 	if (prio >= 0)
 		p->prio = prio;
 	else
@@ -1473,6 +1472,21 @@ static void enable_swap_info(struct swap_info_struct *p, int prio,
 	else
 		swap_info[prev]->next = p->type;
 	frontswap_init(p->type);
+}
+
+static void enable_swap_info(struct swap_info_struct *p, int prio,
+				unsigned char *swap_map,
+				unsigned long *frontswap_map)
+{
+	spin_lock(&swap_lock);
+	_enable_swap_info(p, prio, swap_map, frontswap_map);
+	spin_unlock(&swap_lock);
+}
+
+static void reinsert_swap_info(struct swap_info_struct *p)
+{
+	spin_lock(&swap_lock);
+	_enable_swap_info(p, p->prio, p->swap_map, frontswap_map_get(p));
 	spin_unlock(&swap_lock);
 }
 
@@ -1549,14 +1563,8 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
 	compare_swap_oom_score_adj(OOM_SCORE_ADJ_MAX, oom_score_adj);
 
 	if (err) {
-		/*
-		 * reading p->prio and p->swap_map outside the lock is
-		 * safe here because only sys_swapon and sys_swapoff
-		 * change them, and there can be no other sys_swapon or
-		 * sys_swapoff for this swap_info_struct at this point.
-		 */
 		/* re-insert swap space back into swap_list */
-		enable_swap_info(p, p->prio, p->swap_map, frontswap_map_get(p));
+		reinsert_swap_info(p);
 		goto out_dput;
 	}
 
-- 
1.7.11.7

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/2] mm: do not call frontswap_init() during swapoff

[Cesar Eduardo Barros]

The call to frontswap_init() was added within enable_swap_info(), which
was called not only during sys_swapon, but also to reinsert the
swap_info into the swap_list in case of failure of try_to_unuse() within
sys_swapoff. This means that frontswap_init() might be called more than
once for the same swap area.

While as far as I could see no frontswap implementation has any problem
with it (and in fact, all the ones I found ignore the parameter passed
to frontswap_init), this could change in the future.

To prevent future problems, move the call to frontswap_init() to outside
the code shared between sys_swapon and sys_swapoff.

Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Dan Magenheimer <dan.magenheimer@oracle.com>
Signed-off-by: Cesar Eduardo Barros <cesarb@cesarb.net>
---
 mm/swapfile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 886db96..088daf4 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1471,7 +1471,6 @@ static void _enable_swap_info(struct swap_info_struct *p, int prio,
 		swap_list.head = swap_list.next = p->type;
 	else
 		swap_info[prev]->next = p->type;
-	frontswap_init(p->type);
 }
 
 static void enable_swap_info(struct swap_info_struct *p, int prio,
@@ -1480,6 +1479,7 @@ static void enable_swap_info(struct swap_info_struct *p, int prio,
 {
 	spin_lock(&swap_lock);
 	_enable_swap_info(p, prio, swap_map, frontswap_map);
+	frontswap_init(p->type);
 	spin_unlock(&swap_lock);
 }
 
-- 
1.7.11.7

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] mm: refactor reinsert of swap_info in sys_swapoff

[Andrew Morton]

On Sat, 27 Oct 2012 19:20:46 -0200
Cesar Eduardo Barros <cesarb@cesarb.net> wrote:

> The block within sys_swapoff which re-inserts the swap_info into the
> swap_list in case of failure of try_to_unuse() reads a few values outside
> the swap_lock. While this is safe at that point, it is subtle code.
> 
> Simplify the code by moving the reading of these values to a separate
> function, refactoring it a bit so they are read from within the
> swap_lock. This is easier to understand, and matches better the way it
> worked before I unified the insertion of the swap_info from both
> sys_swapon and sys_swapoff.
> 
> This change should make no functional difference. The only real change
> is moving the read of two or three structure fields to within the lock
> (frontswap_map_get() is nothing more than a read of p->frontswap_map).

Your patch doesn't change this, but...  it is very unusual for any
subsystem's ->init method to be called under a spinlock.  Because it is
highly likely that such a method will wish to do things such as memory
allocation.

It is rare and unlikely for an ->init() method to *need* such external
locking, because all the objects it is dealing with cannot be looked up
by other threads because nothing has been registered anywhere yet.

So either frontswap is doing something wrong here or there's some
subtlety which escapes me.  If the former then we should try to get
that ->init call to happen outside swap_lock.

And if we can do that, perhaps we can fix the regrettable GFP_ATOMIC
in zcache_new_pool().

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] mm: refactor reinsert of swap_info in sys_swapoff

[Cesar Eduardo Barros]

Em 30-10-2012 19:04, Andrew Morton escreveu:
> Your patch doesn't change this, but...  it is very unusual for any
> subsystem's ->init method to be called under a spinlock.  Because it is
> highly likely that such a method will wish to do things such as memory
> allocation.
>
> It is rare and unlikely for an ->init() method to *need* such external
> locking, because all the objects it is dealing with cannot be looked up
> by other threads because nothing has been registered anywhere yet.

The frontswap_init() method is being passed the swap_info_struct's 
->type, which it uses to get back the swap_info_struct itself, which it 
then uses to check if the frontswap_map allocation succeeded. As noted 
by the commit message for commit 38b5faf (mm: frontswap: core swap 
subsystem hooks and headers), that allocation can fail, which will do 
nothing more than not enable frontswap for that swap area.

The same parameter is then passed down to the ->init() method, which 
proceeds to sumarily ignore it on the three in-tree implementations I 
looked at.

Yeah, it looks like a violation of YAGNI to me, and doing things in a 
more roundabount way than is justified. Why pass the ->type and then get 
the pointer from it instead of just passing the pointer in the first 
place? Or better yet, why not pass the frontswap_map pointer? Even 
better, why not a boolean saying whether it worked? Even better, *why 
not just put the conditional inside enable_swap_info* and pass no 
parameter at all?

> So either frontswap is doing something wrong here or there's some
> subtlety which escapes me.  If the former then we should try to get
> that ->init call to happen outside swap_lock.

I believe the swap_lock is protecting the poolid. It is possible that 
other things in the frontswap code are being called before the first 
swapon with a successful frontswap_map allocation (which is when the 
poolid would get allocated).

A quick look suggests the poolid only gets set on an initcall or in the 
->init() method; perhaps a local mutex (to prevent double allocation) 
and an atomic update of the poolid would be enough to move it outside 
the lock (and also outside the swapon_mutex).

But that would work only if no out-of-tree frontswap module needs it 
within the swap_lock.

> And if we can do that, perhaps we can fix the regrettable GFP_ATOMIC
> in zcache_new_pool().


-- 
Cesar Eduardo Barros
cesarb@cesarb.net
cesar.barros@gmail.com

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH RFC] mm: simplify frontswap_init()

[Cesar Eduardo Barros]

The function frontswap_init() uses the passed parameter only to check
for the presence of the frontswap_map. It is also passed down to
frontswap_ops.init(), but all implementations of it in the kernel ignore
the parameter.

Do the check for frontswap_map in the caller instead and remove the
parameter from frontswap_init() and frontswap_ops.init().

Also, __frontswap_init() was exported, but its only caller (via an
inline function) is mm/swapfile.c, which cannot be built as a module.
Remove the unnecessary export.

Signed-off-by: Cesar Eduardo Barros <cesarb@cesarb.net>
---

Not even compile tested, just a quick patch to show what I was thinking
of, but feel free to apply if you think it is good.

I might write another patch to move it outside the lock later, but I
would have to read the frontswap code more carefully first.

 drivers/staging/ramster/zcache-main.c |  2 +-
 drivers/staging/zcache/zcache-main.c  |  2 +-
 drivers/xen/tmem.c                    |  2 +-
 include/linux/frontswap.h             |  8 ++++----
 mm/frontswap.c                        | 10 ++--------
 mm/swapfile.c                         |  3 ++-
 6 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/ramster/zcache-main.c b/drivers/staging/ramster/zcache-main.c
index a09dd5c..b3f01c9 100644
--- a/drivers/staging/ramster/zcache-main.c
+++ b/drivers/staging/ramster/zcache-main.c
@@ -1610,7 +1610,7 @@ static void zcache_frontswap_flush_area(unsigned type)
 	}
 }
 
-static void zcache_frontswap_init(unsigned ignored)
+static void zcache_frontswap_init(void)
 {
 	/* a single tmem poolid is used for all frontswap "types" (swapfiles) */
 	if (zcache_frontswap_poolid < 0)
diff --git a/drivers/staging/zcache/zcache-main.c b/drivers/staging/zcache/zcache-main.c
index 52b43b7..cb67635 100644
--- a/drivers/staging/zcache/zcache-main.c
+++ b/drivers/staging/zcache/zcache-main.c
@@ -1903,7 +1903,7 @@ static void zcache_frontswap_flush_area(unsigned type)
 	}
 }
 
-static void zcache_frontswap_init(unsigned ignored)
+static void zcache_frontswap_init(void)
 {
 	/* a single tmem poolid is used for all frontswap "types" (swapfiles) */
 	if (zcache_frontswap_poolid < 0)
diff --git a/drivers/xen/tmem.c b/drivers/xen/tmem.c
index 144564e..7156ff0 100644
--- a/drivers/xen/tmem.c
+++ b/drivers/xen/tmem.c
@@ -343,7 +343,7 @@ static void tmem_frontswap_flush_area(unsigned type)
 		(void)xen_tmem_flush_object(pool, oswiz(type, ind));
 }
 
-static void tmem_frontswap_init(unsigned ignored)
+static void tmem_frontswap_init(void)
 {
 	struct tmem_pool_uuid private = TMEM_POOL_PRIVATE_UUID;
 
diff --git a/include/linux/frontswap.h b/include/linux/frontswap.h
index 3044254..6374c80 100644
--- a/include/linux/frontswap.h
+++ b/include/linux/frontswap.h
@@ -6,7 +6,7 @@
 #include <linux/bitops.h>
 
 struct frontswap_ops {
-	void (*init)(unsigned);
+	void (*init)(void);
 	int (*store)(unsigned, pgoff_t, struct page *);
 	int (*load)(unsigned, pgoff_t, struct page *);
 	void (*invalidate_page)(unsigned, pgoff_t);
@@ -22,7 +22,7 @@ extern void frontswap_writethrough(bool);
 #define FRONTSWAP_HAS_EXCLUSIVE_GETS
 extern void frontswap_tmem_exclusive_gets(bool);
 
-extern void __frontswap_init(unsigned type);
+extern void __frontswap_init(void);
 extern int __frontswap_store(struct page *page);
 extern int __frontswap_load(struct page *page);
 extern void __frontswap_invalidate_page(unsigned, pgoff_t);
@@ -120,10 +120,10 @@ static inline void frontswap_invalidate_area(unsigned type)
 		__frontswap_invalidate_area(type);
 }
 
-static inline void frontswap_init(unsigned type)
+static inline void frontswap_init(void)
 {
 	if (frontswap_enabled)
-		__frontswap_init(type);
+		__frontswap_init();
 }
 
 #endif /* _LINUX_FRONTSWAP_H */
diff --git a/mm/frontswap.c b/mm/frontswap.c
index 2890e67..d13661b 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -115,16 +115,10 @@ EXPORT_SYMBOL(frontswap_tmem_exclusive_gets);
 /*
  * Called when a swap device is swapon'd.
  */
-void __frontswap_init(unsigned type)
+void __frontswap_init(void)
 {
-	struct swap_info_struct *sis = swap_info[type];
-
-	BUG_ON(sis == NULL);
-	if (sis->frontswap_map == NULL)
-		return;
-	frontswap_ops.init(type);
+	frontswap_ops.init();
 }
-EXPORT_SYMBOL(__frontswap_init);
 
 static inline void __frontswap_clear(struct swap_info_struct *sis, pgoff_t offset)
 {
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 088daf4..28c26bd 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1479,7 +1479,8 @@ static void enable_swap_info(struct swap_info_struct *p, int prio,
 {
 	spin_lock(&swap_lock);
 	_enable_swap_info(p, prio, swap_map, frontswap_map);
-	frontswap_init(p->type);
+	if (frontswap_map)
+		frontswap_init();
 	spin_unlock(&swap_lock);
 }
 
-- 
1.7.11.7

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH RFC] mm: simplify frontswap_init()

[Konrad Rzeszutek Wilk]

On Tue, Oct 30, 2012 at 09:12:53PM -0200, Cesar Eduardo Barros wrote:
> The function frontswap_init() uses the passed parameter only to check
> for the presence of the frontswap_map. It is also passed down to
> frontswap_ops.init(), but all implementations of it in the kernel ignore
> the parameter.
> 
> Do the check for frontswap_map in the caller instead and remove the
> parameter from frontswap_init() and frontswap_ops.init().
> 
> Also, __frontswap_init() was exported, but its only caller (via an
> inline function) is mm/swapfile.c, which cannot be built as a module.
> Remove the unnecessary export.
> 
> Signed-off-by: Cesar Eduardo Barros <cesarb@cesarb.net>
> ---
> 
> Not even compile tested, just a quick patch to show what I was thinking
> of, but feel free to apply if you think it is good.

That looks good.
> 
> I might write another patch to move it outside the lock later, but I
> would have to read the frontswap code more carefully first.
> 
>  drivers/staging/ramster/zcache-main.c |  2 +-
>  drivers/staging/zcache/zcache-main.c  |  2 +-
>  drivers/xen/tmem.c                    |  2 +-
>  include/linux/frontswap.h             |  8 ++++----
>  mm/frontswap.c                        | 10 ++--------
>  mm/swapfile.c                         |  3 ++-
>  6 files changed, 11 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/staging/ramster/zcache-main.c b/drivers/staging/ramster/zcache-main.c
> index a09dd5c..b3f01c9 100644
> --- a/drivers/staging/ramster/zcache-main.c
> +++ b/drivers/staging/ramster/zcache-main.c
> @@ -1610,7 +1610,7 @@ static void zcache_frontswap_flush_area(unsigned type)
>  	}
>  }
>  
> -static void zcache_frontswap_init(unsigned ignored)
> +static void zcache_frontswap_init(void)
>  {
>  	/* a single tmem poolid is used for all frontswap "types" (swapfiles) */
>  	if (zcache_frontswap_poolid < 0)
> diff --git a/drivers/staging/zcache/zcache-main.c b/drivers/staging/zcache/zcache-main.c
> index 52b43b7..cb67635 100644
> --- a/drivers/staging/zcache/zcache-main.c
> +++ b/drivers/staging/zcache/zcache-main.c
> @@ -1903,7 +1903,7 @@ static void zcache_frontswap_flush_area(unsigned type)
>  	}
>  }
>  
> -static void zcache_frontswap_init(unsigned ignored)
> +static void zcache_frontswap_init(void)
>  {
>  	/* a single tmem poolid is used for all frontswap "types" (swapfiles) */
>  	if (zcache_frontswap_poolid < 0)
> diff --git a/drivers/xen/tmem.c b/drivers/xen/tmem.c
> index 144564e..7156ff0 100644
> --- a/drivers/xen/tmem.c
> +++ b/drivers/xen/tmem.c
> @@ -343,7 +343,7 @@ static void tmem_frontswap_flush_area(unsigned type)
>  		(void)xen_tmem_flush_object(pool, oswiz(type, ind));
>  }
>  
> -static void tmem_frontswap_init(unsigned ignored)
> +static void tmem_frontswap_init(void)
>  {
>  	struct tmem_pool_uuid private = TMEM_POOL_PRIVATE_UUID;
>  
> diff --git a/include/linux/frontswap.h b/include/linux/frontswap.h
> index 3044254..6374c80 100644
> --- a/include/linux/frontswap.h
> +++ b/include/linux/frontswap.h
> @@ -6,7 +6,7 @@
>  #include <linux/bitops.h>
>  
>  struct frontswap_ops {
> -	void (*init)(unsigned);
> +	void (*init)(void);
>  	int (*store)(unsigned, pgoff_t, struct page *);
>  	int (*load)(unsigned, pgoff_t, struct page *);
>  	void (*invalidate_page)(unsigned, pgoff_t);
> @@ -22,7 +22,7 @@ extern void frontswap_writethrough(bool);
>  #define FRONTSWAP_HAS_EXCLUSIVE_GETS
>  extern void frontswap_tmem_exclusive_gets(bool);
>  
> -extern void __frontswap_init(unsigned type);
> +extern void __frontswap_init(void);
>  extern int __frontswap_store(struct page *page);
>  extern int __frontswap_load(struct page *page);
>  extern void __frontswap_invalidate_page(unsigned, pgoff_t);
> @@ -120,10 +120,10 @@ static inline void frontswap_invalidate_area(unsigned type)
>  		__frontswap_invalidate_area(type);
>  }
>  
> -static inline void frontswap_init(unsigned type)
> +static inline void frontswap_init(void)
>  {
>  	if (frontswap_enabled)
> -		__frontswap_init(type);
> +		__frontswap_init();
>  }
>  
>  #endif /* _LINUX_FRONTSWAP_H */
> diff --git a/mm/frontswap.c b/mm/frontswap.c
> index 2890e67..d13661b 100644
> --- a/mm/frontswap.c
> +++ b/mm/frontswap.c
> @@ -115,16 +115,10 @@ EXPORT_SYMBOL(frontswap_tmem_exclusive_gets);
>  /*
>   * Called when a swap device is swapon'd.
>   */
> -void __frontswap_init(unsigned type)
> +void __frontswap_init(void)
>  {
> -	struct swap_info_struct *sis = swap_info[type];
> -
> -	BUG_ON(sis == NULL);
> -	if (sis->frontswap_map == NULL)
> -		return;
> -	frontswap_ops.init(type);
> +	frontswap_ops.init();
>  }
> -EXPORT_SYMBOL(__frontswap_init);
>  
>  static inline void __frontswap_clear(struct swap_info_struct *sis, pgoff_t offset)
>  {
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 088daf4..28c26bd 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -1479,7 +1479,8 @@ static void enable_swap_info(struct swap_info_struct *p, int prio,
>  {
>  	spin_lock(&swap_lock);
>  	_enable_swap_info(p, prio, swap_map, frontswap_map);
> -	frontswap_init(p->type);
> +	if (frontswap_map)
> +		frontswap_init();
>  	spin_unlock(&swap_lock);
>  }
>  
> -- 
> 1.7.11.7

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] mm: refactor reinsert of swap_info in sys_swapoff

[Konrad Rzeszutek Wilk]

On Tue, Oct 30, 2012 at 02:04:17PM -0700, Andrew Morton wrote:
> On Sat, 27 Oct 2012 19:20:46 -0200
> Cesar Eduardo Barros <cesarb@cesarb.net> wrote:
> 
> > The block within sys_swapoff which re-inserts the swap_info into the
> > swap_list in case of failure of try_to_unuse() reads a few values outside
> > the swap_lock. While this is safe at that point, it is subtle code.
> > 
> > Simplify the code by moving the reading of these values to a separate
> > function, refactoring it a bit so they are read from within the
> > swap_lock. This is easier to understand, and matches better the way it
> > worked before I unified the insertion of the swap_info from both
> > sys_swapon and sys_swapoff.
> > 
> > This change should make no functional difference. The only real change
> > is moving the read of two or three structure fields to within the lock
> > (frontswap_map_get() is nothing more than a read of p->frontswap_map).
> 
> Your patch doesn't change this, but...  it is very unusual for any
> subsystem's ->init method to be called under a spinlock.  Because it is
> highly likely that such a method will wish to do things such as memory
> allocation.
> 
> It is rare and unlikely for an ->init() method to *need* such external
> locking, because all the objects it is dealing with cannot be looked up
> by other threads because nothing has been registered anywhere yet.

I don't believe it actually needs that locking. Dan, do you recall
the details of this?
> 
> So either frontswap is doing something wrong here or there's some
> subtlety which escapes me.  If the former then we should try to get
> that ->init call to happen outside swap_lock.

Agreed.
> 
> And if we can do that, perhaps we can fix the regrettable GFP_ATOMIC
> in zcache_new_pool().

Ouch. Yes.


FYI, thanks for pulling those two patches - they looked good to me
but I hadn't had a chance to test them so did not want to comment on them
until that happen. Dan beat me to it and he did test them.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>