[Cluster-devel] [GFS2/DLM] Some small bug fixes

[Cluster-devel] [GFS2/DLM] Some small bug fixes

[Steven Whitehouse]

Hi,

The following patches are the bug fix patches in the current GFS2 -nmw
git tree which I've extracted into the -fixes tree since they are relatively
small and self contained. They are relative to 2.6.22-rc5,

Steve.

[GFS2/DLM] Some small bug fixes

[Steven Whitehouse]

Hi,

The following patches are the bug fix patches in the current GFS2 -nmw
git tree which I've extracted into the -fixes tree since they are relatively
small and self contained. They are relative to 2.6.22-rc5,

Steve.

[Cluster-devel] [PATCH 1/4] [GFS2] flush the glock completely in inode_go_sync

[Steven Whitehouse]

Fix for bz #231910
When filemap_fdatawrite() is called on the inode mapping in data=ordered mode,
it will add the glock to the log. In inode_go_sync(), if you do the
gfs2_log_flush() before this, after the filemap_fdatawrite() call, the glock
and its associated data buffers will be on the log again. This means you can
demote a lock from exclusive, without having it flushed from the log. The
attached patch simply moves the gfs2_log_flush up to after the
filemap_fdatawrite() call.

Originally, I tried moving the gfs2_log_flush to after gfs2_meta_sync(), but
that caused me to trip the following assert.

GFS2: fsid=cypher-36:test.0: fatal: assertion "!buffer_busy(bh)" failed
GFS2: fsid=cypher-36:test.0:   function = gfs2_ail_empty_gl, file = fs/gfs2/glops.c, line = 61

It appears that gfs2_log_flush() puts some of the glocks buffers in the busy
state and the filemap_fdatawrite() call is necessary to flush them. This makes
me worry slightly that a related problem could happen because of moving the
gfs2_log_flush() after the initial filemap_fdatawrite(), but I assume that
gfs2_ail_empty_gl() would catch that case as well.

Signed-off-by: Benjamin E. Marzinski <bmarzins@redhat.com>
Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
---
 fs/gfs2/glops.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: e92666ae590962682cf23fcae2c3bdcb6e5e2d27.diff
Type: text/x-patch
Size: 473 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20070618/65bbac21/attachment.bin>

[PATCH 1/4] [GFS2] flush the glock completely in inode_go_sync

[Steven Whitehouse]

[-- Attachment #1: Type: text/plain, Size: 44 bytes --]

This is a multi-part message in MIME format.

[-- Attachment #2: Type: text/plain, Size: 1330 bytes --]


Fix for bz #231910
When filemap_fdatawrite() is called on the inode mapping in data=ordered mode,
it will add the glock to the log. In inode_go_sync(), if you do the
gfs2_log_flush() before this, after the filemap_fdatawrite() call, the glock
and its associated data buffers will be on the log again. This means you can
demote a lock from exclusive, without having it flushed from the log. The
attached patch simply moves the gfs2_log_flush up to after the
filemap_fdatawrite() call.

Originally, I tried moving the gfs2_log_flush to after gfs2_meta_sync(), but
that caused me to trip the following assert.

GFS2: fsid=cypher-36:test.0: fatal: assertion "!buffer_busy(bh)" failed
GFS2: fsid=cypher-36:test.0:   function = gfs2_ail_empty_gl, file = fs/gfs2/glops.c, line = 61

It appears that gfs2_log_flush() puts some of the glocks buffers in the busy
state and the filemap_fdatawrite() call is necessary to flush them. This makes
me worry slightly that a related problem could happen because of moving the
gfs2_log_flush() after the initial filemap_fdatawrite(), but I assume that
gfs2_ail_empty_gl() would catch that case as well.

Signed-off-by: Benjamin E. Marzinski <bmarzins@redhat.com>
Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
---
 fs/gfs2/glops.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: e92666ae590962682cf23fcae2c3bdcb6e5e2d27.diff --]
[-- Type: text/x-patch; name="e92666ae590962682cf23fcae2c3bdcb6e5e2d27.diff", Size: 473 bytes --]

diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index 7b82657..777ca46 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -156,9 +156,9 @@ static void inode_go_sync(struct gfs2_glock *gl)
 		ip = NULL;
 
 	if (test_bit(GLF_DIRTY, &gl->gl_flags)) {
-		gfs2_log_flush(gl->gl_sbd, gl);
 		if (ip)
 			filemap_fdatawrite(ip->i_inode.i_mapping);
+		gfs2_log_flush(gl->gl_sbd, gl);
 		gfs2_meta_sync(gl);
 		if (ip) {
 			struct address_space *mapping = ip->i_inode.i_mapping;

[Cluster-devel] [PATCH 2/4] [DLM] fix a couple of races

[Steven Whitehouse]

Fix two races in fs/dlm/config.c:

(1) Grab the configfs subsystem semaphore before calling
config_group_find_obj() in get_space(). This solves a potential race
between get_space() and concurrent mkdir(2) or rmdir(2).

(2) Grab a reference on the found config_item _while_ holding the configfs
subsystem semaphore in get_comm(), and not after it. This solves a
potential race between get_comm() and concurrent rmdir(2).

Signed-off-by: Satyam Sharma <ssatyam@cse.iitk.ac.in>
Signed-off-by: David Teigland <teigland@redhat.com>
Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
---
 fs/dlm/config.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 1f692e48dd1f3fa00629c69b2e34ab35c9929b45.diff
Type: text/x-patch
Size: 1066 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20070618/badca0be/attachment.bin>

[PATCH 2/4] [DLM] fix a couple of races

[Steven Whitehouse]

[-- Attachment #1: Type: text/plain, Size: 44 bytes --]

This is a multi-part message in MIME format.

[-- Attachment #2: Type: text/plain, Size: 677 bytes --]


Fix two races in fs/dlm/config.c:

(1) Grab the configfs subsystem semaphore before calling
config_group_find_obj() in get_space(). This solves a potential race
between get_space() and concurrent mkdir(2) or rmdir(2).

(2) Grab a reference on the found config_item _while_ holding the configfs
subsystem semaphore in get_comm(), and not after it. This solves a
potential race between get_comm() and concurrent rmdir(2).

Signed-off-by: Satyam Sharma <ssatyam@cse.iitk.ac.in>
Signed-off-by: David Teigland <teigland@redhat.com>
Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
---
 fs/dlm/config.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 1f692e48dd1f3fa00629c69b2e34ab35c9929b45.diff --]
[-- Type: text/x-patch; name="1f692e48dd1f3fa00629c69b2e34ab35c9929b45.diff", Size: 1066 bytes --]

diff --git a/fs/dlm/config.c b/fs/dlm/config.c
index 822abdc..5a3d390 100644
--- a/fs/dlm/config.c
+++ b/fs/dlm/config.c
@@ -748,9 +748,16 @@ static ssize_t node_weight_write(struct node *nd, const char *buf, size_t len)
 
 static struct space *get_space(char *name)
 {
+	struct config_item *i;
+
 	if (!space_list)
 		return NULL;
-	return to_space(config_group_find_obj(space_list, name));
+
+	down(&space_list->cg_subsys->su_sem);
+	i = config_group_find_obj(space_list, name);
+	up(&space_list->cg_subsys->su_sem);
+
+	return to_space(i);
 }
 
 static void put_space(struct space *sp)
@@ -776,20 +783,20 @@ static struct comm *get_comm(int nodeid, struct sockaddr_storage *addr)
 			if (cm->nodeid != nodeid)
 				continue;
 			found = 1;
+			config_item_get(i);
 			break;
 		} else {
 			if (!cm->addr_count ||
 			    memcmp(cm->addr[0], addr, sizeof(*addr)))
 				continue;
 			found = 1;
+			config_item_get(i);
 			break;
 		}
 	}
 	up(&clusters_root.subsys.su_sem);
 
-	if (found)
-		config_item_get(i);
-	else
+	if (!found)
 		cm = NULL;
 	return cm;
 }

[Cluster-devel] [PATCH 3/4] [GFS2] use zero_user_page

[Steven Whitehouse]

Use zero_user_page() instead of open-coding it.

Signed-off-by: Nate Diller <nate.diller@gmail.com>
Cc: Steven Whitehouse <swhiteho@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
 fs/gfs2/bmap.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 5f8edce5584b74bbe25c240ffe7a55934e8cb9de.diff
Type: text/x-patch
Size: 782 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20070618/f018165b/attachment.bin>

[PATCH 3/4] [GFS2] use zero_user_page

[Steven Whitehouse]

[-- Attachment #1: Type: text/plain, Size: 44 bytes --]

This is a multi-part message in MIME format.

[-- Attachment #2: Type: text/plain, Size: 285 bytes --]


Use zero_user_page() instead of open-coding it.

Signed-off-by: Nate Diller <nate.diller@gmail.com>
Cc: Steven Whitehouse <swhiteho@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
 fs/gfs2/bmap.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 5f8edce5584b74bbe25c240ffe7a55934e8cb9de.diff --]
[-- Type: text/x-patch; name="5f8edce5584b74bbe25c240ffe7a55934e8cb9de.diff", Size: 782 bytes --]

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index c53a5d2..1c40c4b 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -885,7 +885,6 @@ static int gfs2_block_truncate_page(struct address_space *mapping)
 	unsigned blocksize, iblock, length, pos;
 	struct buffer_head *bh;
 	struct page *page;
-	void *kaddr;
 	int err;
 
 	page = grab_cache_page(mapping, index);
@@ -933,10 +932,7 @@ static int gfs2_block_truncate_page(struct address_space *mapping)
 	if (sdp->sd_args.ar_data == GFS2_DATA_ORDERED || gfs2_is_jdata(ip))
 		gfs2_trans_add_bh(ip->i_gl, bh, 0);
 
-	kaddr = kmap_atomic(page, KM_USER0);
-	memset(kaddr + offset, 0, length);
-	flush_dcache_page(page);
-	kunmap_atomic(kaddr, KM_USER0);
+	zero_user_page(page, offset, length, KM_USER0);
 
 unlock:
 	unlock_page(page);

[Cluster-devel] [PATCH 4/4] [DLM] keep dlm from panicing when traversing rsb list in debugfs

[Steven Whitehouse]

This problem was originally reported against GFS6.1, but the same issue exists
in upstream DLM.  This patch keeps the rsb iterator assigning under the rsbtbl
list lock.  Each time we process an rsb we grab a reference to it to make sure
it is not freed out from underneath us, and then put it when we get the next rsb
in the list or move onto another list.

Signed-off-by: Josef Bacik <jwhiter@redhat.com>
Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
---
 fs/dlm/debug_fs.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: b366510ec8e4fcf5a918a0c9fc1ba967d543838b.diff
Type: text/x-patch
Size: 1326 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20070618/f89d0ea7/attachment.bin>

[PATCH 4/4] [DLM] keep dlm from panicing when traversing rsb list in debugfs

[Steven Whitehouse]

[-- Attachment #1: Type: text/plain, Size: 44 bytes --]

This is a multi-part message in MIME format.

[-- Attachment #2: Type: text/plain, Size: 551 bytes --]


This problem was originally reported against GFS6.1, but the same issue exists
in upstream DLM.  This patch keeps the rsb iterator assigning under the rsbtbl
list lock.  Each time we process an rsb we grab a reference to it to make sure
it is not freed out from underneath us, and then put it when we get the next rsb
in the list or move onto another list.

Signed-off-by: Josef Bacik <jwhiter@redhat.com>
Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
---
 fs/dlm/debug_fs.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: b366510ec8e4fcf5a918a0c9fc1ba967d543838b.diff --]
[-- Type: text/x-patch; name="b366510ec8e4fcf5a918a0c9fc1ba967d543838b.diff", Size: 1326 bytes --]

diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c
index 61ba670..9e27a16 100644
--- a/fs/dlm/debug_fs.c
+++ b/fs/dlm/debug_fs.c
@@ -17,6 +17,7 @@
 #include <linux/debugfs.h>
 
 #include "dlm_internal.h"
+#include "lock.h"
 
 #define DLM_DEBUG_BUF_LEN 4096
 static char debug_buf[DLM_DEBUG_BUF_LEN];
@@ -166,6 +167,9 @@ static int rsb_iter_next(struct rsb_iter *ri)
 			read_lock(&ls->ls_rsbtbl[i].lock);
 			if (!list_empty(&ls->ls_rsbtbl[i].list)) {
 				ri->next = ls->ls_rsbtbl[i].list.next;
+				ri->rsb = list_entry(ri->next, struct dlm_rsb,
+							res_hashchain);
+				dlm_hold_rsb(ri->rsb);
 				read_unlock(&ls->ls_rsbtbl[i].lock);
 				break;
 			}
@@ -176,6 +180,7 @@ static int rsb_iter_next(struct rsb_iter *ri)
 		if (ri->entry >= ls->ls_rsbtbl_size)
 			return 1;
 	} else {
+		struct dlm_rsb *old = ri->rsb;
 		i = ri->entry;
 		read_lock(&ls->ls_rsbtbl[i].lock);
 		ri->next = ri->next->next;
@@ -184,11 +189,13 @@ static int rsb_iter_next(struct rsb_iter *ri)
 			ri->next = NULL;
 			ri->entry++;
 			read_unlock(&ls->ls_rsbtbl[i].lock);
+			dlm_put_rsb(old);
 			goto top;
                 }
+		ri->rsb = list_entry(ri->next, struct dlm_rsb, res_hashchain);
 		read_unlock(&ls->ls_rsbtbl[i].lock);
+		dlm_put_rsb(old);
 	}
-	ri->rsb = list_entry(ri->next, struct dlm_rsb, res_hashchain);
 
 	return 0;
 }

[Cluster-devel] [GFS2/DLM] Pull request

[Steven Whitehouse]

Hi,

Please consider pulling the following patches from the GFS2 fixes git tree,

Steve.

------------------------------------------------------------------------

The following changes since commit 188e1f81ba31af1b65a2f3611df4c670b092bbac:
  Linus Torvalds (1):
        Linux 2.6.22-rc5

are found in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/steve/gfs2-2.6-fixes.git

Benjamin Marzinski (1):
      [GFS2] flush the glock completely in inode_go_sync

Josef Bacik (1):
      [DLM] keep dlm from panicing when traversing rsb list in debugfs

Nate Diller (1):
      [GFS2] use zero_user_page

Satyam Sharma (1):
      [DLM] fix a couple of races

 fs/dlm/config.c   |   15 +++++++++++----
 fs/dlm/debug_fs.c |    9 ++++++++-
 fs/gfs2/bmap.c    |    6 +-----
 fs/gfs2/glops.c   |    2 +-
 4 files changed, 21 insertions(+), 11 deletions(-)

[GFS2/DLM] Pull request

[Steven Whitehouse]

Hi,

Please consider pulling the following patches from the GFS2 fixes git tree,

Steve.

------------------------------------------------------------------------

The following changes since commit 188e1f81ba31af1b65a2f3611df4c670b092bbac:
  Linus Torvalds (1):
        Linux 2.6.22-rc5

are found in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/steve/gfs2-2.6-fixes.git

Benjamin Marzinski (1):
      [GFS2] flush the glock completely in inode_go_sync

Josef Bacik (1):
      [DLM] keep dlm from panicing when traversing rsb list in debugfs

Nate Diller (1):
      [GFS2] use zero_user_page

Satyam Sharma (1):
      [DLM] fix a couple of races

 fs/dlm/config.c   |   15 +++++++++++----
 fs/dlm/debug_fs.c |    9 ++++++++-
 fs/gfs2/bmap.c    |    6 +-----
 fs/gfs2/glops.c   |    2 +-
 4 files changed, 21 insertions(+), 11 deletions(-)

[Cluster-devel] [GFS2/DLM] Pull request

[Steven Whitehouse]

Hi,

On Mon, 2007-06-18 at 16:13 +0100, Steven Whitehouse wrote:
> Hi,
> 
> Please consider pulling the following patches from the GFS2 fixes git tree,
> 
> Steve.
> 
Dave Teigland has just asked me not to push the fourth patch in the
series until later, so there are now only three patches in the set.
Sorry about that. An updated change log is below:

Steve.

--------------------------------------------------
The following changes since commit 188e1f81ba31af1b65a2f3611df4c670b092bbac:
  Linus Torvalds (1):
        Linux 2.6.22-rc5

are found in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/steve/gfs2-2.6-fixes.git

Benjamin Marzinski (1):
      [GFS2] flush the glock completely in inode_go_sync

Nate Diller (1):
      [GFS2] use zero_user_page

Satyam Sharma (1):
      [DLM] fix a couple of races

 fs/dlm/config.c |   15 +++++++++++----
 fs/gfs2/bmap.c  |    6 +-----
 fs/gfs2/glops.c |    2 +-
 3 files changed, 13 insertions(+), 10 deletions(-)

Re: [Cluster-devel] [GFS2/DLM] Pull request

[Steven Whitehouse]

Hi,

On Mon, 2007-06-18 at 16:13 +0100, Steven Whitehouse wrote:
> Hi,
> 
> Please consider pulling the following patches from the GFS2 fixes git tree,
> 
> Steve.
> 
Dave Teigland has just asked me not to push the fourth patch in the
series until later, so there are now only three patches in the set.
Sorry about that. An updated change log is below:

Steve.

--------------------------------------------------
The following changes since commit 188e1f81ba31af1b65a2f3611df4c670b092bbac:
  Linus Torvalds (1):
        Linux 2.6.22-rc5

are found in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/steve/gfs2-2.6-fixes.git

Benjamin Marzinski (1):
      [GFS2] flush the glock completely in inode_go_sync

Nate Diller (1):
      [GFS2] use zero_user_page

Satyam Sharma (1):
      [DLM] fix a couple of races

 fs/dlm/config.c |   15 +++++++++++----
 fs/gfs2/bmap.c  |    6 +-----
 fs/gfs2/glops.c |    2 +-
 3 files changed, 13 insertions(+), 10 deletions(-)