>From 2ca99501fa5422e84f18333918a503433449e2b5 Mon Sep 17 00:00:00 2001
From: Steven Whitehouse <[email protected]>
Date: Wed, 8 Nov 2006 10:26:54 -0500
Subject: [PATCH] [GFS2] Fix page lock/glock deadlock
This fixes a race between the glock and the page lock encountered
during truncate in gfs2_readpage and gfs2_prepare_write. The gfs2_readpages
function doesn't need the same fix since it only uses a try lock anyway, so
it will fail back to gfs2_readpage in the case of a potential deadlock.
This bug was spotted by Russell Cattelan.
Cc: Russell Cattelan <[email protected]>
Signed-off-by: Steven Whitehouse <[email protected]>
---
fs/gfs2/glock.h | 1 -
fs/gfs2/ops_address.c | 13 +++++++++----
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/fs/gfs2/glock.h b/fs/gfs2/glock.h
index b985627..a331bf8 100644
--- a/fs/gfs2/glock.h
+++ b/fs/gfs2/glock.h
@@ -27,7 +27,6 @@ #define GL_SKIP 0x00000100
#define GL_ATIME 0x00000200
#define GL_NOCACHE 0x00000400
#define GL_NOCANCEL 0x00001000
-#define GL_AOP 0x00004000
#define GLR_TRYFAILED 13
#define GLR_CANCELED 14
diff --git a/fs/gfs2/ops_address.c b/fs/gfs2/ops_address.c
index 5c3962c..3822189 100644
--- a/fs/gfs2/ops_address.c
+++ b/fs/gfs2/ops_address.c
@@ -230,7 +230,7 @@ static int gfs2_readpage(struct file *fi
/* gfs2_sharewrite_nopage has grabbed the ip->i_gl already */
goto skip_lock;
}
- gfs2_holder_init(ip->i_gl, LM_ST_SHARED, GL_ATIME|GL_AOP, &gh);
+ gfs2_holder_init(ip->i_gl, LM_ST_SHARED, GL_ATIME|LM_FLAG_TRY_1CB, &gh);
do_unlock = 1;
error = gfs2_glock_nq_m_atime(1, &gh);
if (unlikely(error))
@@ -254,6 +254,8 @@ skip_lock:
out:
return error;
out_unlock:
+ if (error == GLR_TRYFAILED)
+ error = AOP_TRUNCATED_PAGE;
unlock_page(page);
if (do_unlock)
gfs2_holder_uninit(&gh);
@@ -293,7 +295,7 @@ static int gfs2_readpages(struct file *f
goto skip_lock;
}
gfs2_holder_init(ip->i_gl, LM_ST_SHARED,
- LM_FLAG_TRY_1CB|GL_ATIME|GL_AOP, &gh);
+ LM_FLAG_TRY_1CB|GL_ATIME, &gh);
do_unlock = 1;
ret = gfs2_glock_nq_m_atime(1, &gh);
if (ret == GLR_TRYFAILED)
@@ -366,10 +368,13 @@ static int gfs2_prepare_write(struct fil
unsigned int write_len = to - from;
- gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, GL_ATIME|GL_AOP, &ip->i_gh);
+ gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, GL_ATIME|LM_FLAG_TRY_1CB, &ip->i_gh);
error = gfs2_glock_nq_m_atime(1, &ip->i_gh);
- if (error)
+ if (unlikely(error)) {
+ if (error == GLR_TRYFAILED)
+ error = AOP_TRUNCATED_PAGE;
goto out_uninit;
+ }
gfs2_write_calc_reserv(ip, write_len, &data_blocks, &ind_blocks);
--
1.4.1
On Thu, 2006-11-30 at 12:17 +0000, Steven Whitehouse wrote:
> >From 2ca99501fa5422e84f18333918a503433449e2b5 Mon Sep 17 00:00:00 2001
> From: Steven Whitehouse <[email protected]>
> Date: Wed, 8 Nov 2006 10:26:54 -0500
> Subject: [PATCH] [GFS2] Fix page lock/glock deadlock
>
> This fixes a race between the glock and the page lock encountered
> during truncate in gfs2_readpage and gfs2_prepare_write. The gfs2_readpages
> function doesn't need the same fix since it only uses a try lock anyway, so
> it will fail back to gfs2_readpage in the case of a potential deadlock.
>
> This bug was spotted by Russell Cattelan.\
This bug was fixed correctly by Russell Cattelan
and this is an incomplete version of my patch.
As I keep trying to point out readpages is still wrong in that
the stuffed case is not correct and results in a panic
if a file is being truncated down.
If nr_pages is calculated prior a truncate the stuffed inode
case will try to operate on pages that are no longer there.
The correct fix is to not try and handled the stuffed inode
case at all in readpages and simply return AOP_TRUNCATED_PAGE
and let things things fall back to readpage.
>
> Cc: Russell Cattelan <[email protected]>
> Signed-off-by: Steven Whitehouse <[email protected]>
> ---
> fs/gfs2/glock.h | 1 -
> fs/gfs2/ops_address.c | 13 +++++++++----
> 2 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/fs/gfs2/glock.h b/fs/gfs2/glock.h
> index b985627..a331bf8 100644
> --- a/fs/gfs2/glock.h
> +++ b/fs/gfs2/glock.h
> @@ -27,7 +27,6 @@ #define GL_SKIP 0x00000100
> #define GL_ATIME 0x00000200
> #define GL_NOCACHE 0x00000400
> #define GL_NOCANCEL 0x00001000
> -#define GL_AOP 0x00004000
>
> #define GLR_TRYFAILED 13
> #define GLR_CANCELED 14
> diff --git a/fs/gfs2/ops_address.c b/fs/gfs2/ops_address.c
> index 5c3962c..3822189 100644
> --- a/fs/gfs2/ops_address.c
> +++ b/fs/gfs2/ops_address.c
> @@ -230,7 +230,7 @@ static int gfs2_readpage(struct file *fi
> /* gfs2_sharewrite_nopage has grabbed the ip->i_gl already */
> goto skip_lock;
> }
> - gfs2_holder_init(ip->i_gl, LM_ST_SHARED, GL_ATIME|GL_AOP, &gh);
> + gfs2_holder_init(ip->i_gl, LM_ST_SHARED, GL_ATIME|LM_FLAG_TRY_1CB, &gh);
> do_unlock = 1;
> error = gfs2_glock_nq_m_atime(1, &gh);
> if (unlikely(error))
> @@ -254,6 +254,8 @@ skip_lock:
> out:
> return error;
> out_unlock:
> + if (error == GLR_TRYFAILED)
> + error = AOP_TRUNCATED_PAGE;
> unlock_page(page);
> if (do_unlock)
> gfs2_holder_uninit(&gh);
> @@ -293,7 +295,7 @@ static int gfs2_readpages(struct file *f
> goto skip_lock;
> }
> gfs2_holder_init(ip->i_gl, LM_ST_SHARED,
> - LM_FLAG_TRY_1CB|GL_ATIME|GL_AOP, &gh);
> + LM_FLAG_TRY_1CB|GL_ATIME, &gh);
> do_unlock = 1;
> ret = gfs2_glock_nq_m_atime(1, &gh);
> if (ret == GLR_TRYFAILED)
> @@ -366,10 +368,13 @@ static int gfs2_prepare_write(struct fil
> unsigned int write_len = to - from;
>
>
> - gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, GL_ATIME|GL_AOP, &ip->i_gh);
> + gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, GL_ATIME|LM_FLAG_TRY_1CB, &ip->i_gh);
> error = gfs2_glock_nq_m_atime(1, &ip->i_gh);
> - if (error)
> + if (unlikely(error)) {
> + if (error == GLR_TRYFAILED)
> + error = AOP_TRUNCATED_PAGE;
> goto out_uninit;
> + }
>
> gfs2_write_calc_reserv(ip, write_len, &data_blocks, &ind_blocks);
>
--
Russell Cattelan <[email protected]>