From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9392 invoked by alias); 2 Oct 2008 18:13:25 -0000 Received: (qmail 9386 invoked by alias); 2 Oct 2008 18:13:25 -0000 X-Spam-Status: No, hits=-0.8 required=5.0 tests=AWL,BAYES_00,KAM_MX,SPF_HELO_PASS X-Spam-Check-By: sourceware.org X-Spam-Checker-Version: SpamAssassin 3.2.4 (2008-01-01) on bastion.fedora.phx.redhat.com X-Spam-Level: Subject: STABLE2 - gfs-kernel: GFS: madvise system call causes assertion To: cluster-cvs-relay@redhat.com X-Project: Cluster Project X-Git-Module: cluster.git X-Git-Refname: refs/heads/STABLE2 X-Git-Reftype: branch X-Git-Oldrev: 39fab5d033b57b3b8804d88bae22be3cb04fac17 X-Git-Newrev: 7eb24e29894c827072a82907c81a4fbcbd0d0883 From: Abhijith Das Message-Id: <20081002181217.7DF28C07B7@lists.fedorahosted.org> Date: Thu, 02 Oct 2008 18:13:00 -0000 X-Scanned-By: MIMEDefang 2.58 on 172.16.52.254 Mailing-List: contact cluster-cvs-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: cluster-cvs-owner@sourceware.org X-SW-Source: 2008-q4/txt/msg00004.txt.bz2 Gitweb: http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=7eb24e29894c827072a82907c81a4fbcbd0d0883 Commit: 7eb24e29894c827072a82907c81a4fbcbd0d0883 Parent: 39fab5d033b57b3b8804d88bae22be3cb04fac17 Author: Abhijith Das AuthorDate: Thu Oct 2 13:07:44 2008 -0500 Committer: Abhijith Das CommitterDate: Thu Oct 2 13:10:04 2008 -0500 gfs-kernel: GFS: madvise system call causes assertion Since the madvise system call was enabled by the patch to bug 429343, it's possible for a inode glock holder to never get dequeued through gfs_readpage. This causes an assertion (bug 464837) GFS: fsid=cl102a:gfs1.1: warning: assertion "(gh->gh_flags & LM_FLAG_ANY) || (tmp_gh->gh_flags & LM_FLAG_ANY)" failed GFS: fsid=cl102a:gfs1.1: function = add_to_queue GFS: fsid=cl102a:gfs1.1: file = /builddir/build/BUILD/gfs-kmod-0.1.23/_kmod_build_/src/gfs/glock.c, line = 1418 GFS: fsid=cl102a:gfs1.1: time = 1222739610 This patch reverts the patch to bz 429343. Don't log any warnings/errors and simply return ENOSYS when you arrive at gfs_readpage without the inode glock held. (madvise syscall case) --- gfs-kernel/src/gfs/glock.h | 15 ++++++++------- gfs-kernel/src/gfs/ops_address.c | 29 ++--------------------------- 2 files changed, 10 insertions(+), 34 deletions(-) diff --git a/gfs-kernel/src/gfs/glock.h b/gfs-kernel/src/gfs/glock.h index 198dc39..a0342b1 100644 --- a/gfs-kernel/src/gfs/glock.h +++ b/gfs-kernel/src/gfs/glock.h @@ -20,17 +20,17 @@ #define GL_NOCACHE (0x00000400) /* Release glock when done, don't cache */ #define GL_SYNC (0x00000800) /* Sync to disk when no more holders */ #define GL_NOCANCEL (0x00001000) /* Don't ever cancel this request */ -#define GL_READPAGE (0x00002000) /* gfs_readpage() issued this lock request */ #define GL_NOCANCEL_OTHER (0x00004000) /* Don't cancel other locks for this */ #define GLR_TRYFAILED (13) #define GLR_CANCELED (14) -static __inline__ struct gfs_holder* +static __inline__ int gfs_glock_is_locked_by_me(struct gfs_glock *gl) { struct list_head *tmp, *head; struct gfs_holder *gh; + int locked = FALSE; /* Look in glock's list of holders for one with current task as owner */ spin_lock(&gl->gl_spin); @@ -38,13 +38,14 @@ gfs_glock_is_locked_by_me(struct gfs_glock *gl) tmp != head; tmp = tmp->next) { gh = list_entry(tmp, struct gfs_holder, gh_list); - if (gh->gh_owner == current) - goto out; + if (gh->gh_owner == current) { + locked = TRUE; + break; + } } - gh = NULL; -out: spin_unlock(&gl->gl_spin); - return gh; + + return locked; } static __inline__ int gfs_glock_is_held_excl(struct gfs_glock *gl) diff --git a/gfs-kernel/src/gfs/ops_address.c b/gfs-kernel/src/gfs/ops_address.c index 63854ef..ff08c66 100644 --- a/gfs-kernel/src/gfs/ops_address.c +++ b/gfs-kernel/src/gfs/ops_address.c @@ -259,33 +259,13 @@ gfs_readpage(struct file *file, struct page *page) { struct gfs_inode *ip = get_v2ip(page->mapping->host); struct gfs_sbd *sdp = ip->i_sbd; - struct gfs_holder *gh; int error; atomic_inc(&sdp->sd_ops_address); - /* When gfs_readpage is called from the sys_madvise code through the - * readahead code, the inode glock is not held. In this case, we hold - * the inode glock, unlock the page and return AOP_TRUNCATED_PAGE. The - * caller will then reload the page and call gfs_readpage again. We - * also add the flag GL_READPAGE to denote that the glock was held in - * this function and if so, we unlock it before leaving this function - */ - gh = gfs_glock_is_locked_by_me(ip->i_gl); - if (!gh) { - gh = kmalloc(sizeof(struct gfs_holder), GFP_NOFS); - if (!gh) - return -ENOBUFS; - gfs_holder_init(ip->i_gl, LM_ST_SHARED, - GL_READPAGE | LM_FLAG_ANY, gh); + if (!gfs_glock_is_locked_by_me(ip->i_gl)) { unlock_page(page); - error = gfs_glock_nq(gh); - if (error) { - gfs_holder_uninit(gh); - kfree(gh); - goto out; - } - return AOP_TRUNCATED_PAGE; + return -ENOSYS; } if (!gfs_is_jdata(ip)) { @@ -300,11 +280,6 @@ gfs_readpage(struct file *file, struct page *page) if (unlikely(test_bit(SDF_SHUTDOWN, &sdp->sd_flags))) error = -EIO; - if (gh->gh_flags & GL_READPAGE) { /* If we grabbed the glock here */ - gfs_glock_dq_uninit(gh); - kfree(gh); - } -out: return error; }