From: Steven Whitehouse <swhiteho@redhat.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [PATCH] Fix freeze of cluster-2.03.11
Date: Wed, 22 Apr 2009 12:59:38 +0100 [thread overview]
Message-ID: <1240401578.29604.79.camel@localhost.localdomain> (raw)
In-Reply-To: <alpine.DEB.2.00.0904221342520.30938@lxserv1.kfki.hu>
Hi,
On Wed, 2009-04-22 at 13:55 +0200, Kadlecsik Jozsef wrote:
> On Tue, 21 Apr 2009, Steven Whitehouse wrote:
>
> > Yes, it doesn't surprise me that you'd see lockups without your patch.
> > To be on the safe side, try moving the call to gfs_sync_page_i into
> > ->delete_inode so that you can do it after the inode lock has already
> > been dropped (and after the state has been set correctly too). It won't
> > harm anything to have that around, but it might slow things down a bit,
>
> What do you think about this patch?
>
> --- gfs-orig/ops_super.c 2009-01-22 13:33:51.000000000 +0100
> +++ gfs/ops_super.c 2009-04-22 13:51:06.000000000 +0200
> @@ -49,7 +49,7 @@
> }
>
> /**
> - * gfs_drop_inode - drop an inode
> + * gfs_delete_inode - delete an inode
> * @inode: The inode
> *
> * If i_nlink is zero, any dirty data for the inode is thrown away.
> @@ -58,19 +58,19 @@
> */
>
> static void
> -gfs_drop_inode(struct inode *inode)
> +gfs_delete_inode(struct inode *inode)
> {
> struct gfs_sbd *sdp = get_v2sdp(inode->i_sb);
> struct gfs_inode *ip = get_v2ip(inode);
>
> - atomic_inc(&sdp->sd_ops_super);
> -
> if (ip &&
> !inode->i_nlink &&
That looks much better, but you don't need to test for !inode->i_nlink
as this function only gets called if that is the case.
> S_ISREG(inode->i_mode) &&
> !sdp->sd_args.ar_localcaching)
> gfs_sync_page_i(inode, DIO_START | DIO_WAIT);
I still wonder whether gfs2_sync_page_i is really needed here at all,
but its probably safer to keep it for now I guess.
> - generic_drop_inode(inode);
> +
> + truncate_inode_pages(&inode->i_data, 0);
> + clear_inode(inode);
> }
>
> /**
> @@ -443,7 +443,7 @@
>
> struct super_operations gfs_super_ops = {
> .write_inode = gfs_write_inode,
> - .drop_inode = gfs_drop_inode,
> + .delete_inode = gfs_delete_inode,
> .put_super = gfs_put_super,
> .write_super = gfs_write_super,
> .write_super_lockfs = gfs_write_super_lockfs,
>
> I'll be able to test it (or it's successor) around the next week.
>
> Best regards,
> Jozsef
Ok, if we here some positive test results, then we can put this in.
Thanks,
Steve.
prev parent reply other threads:[~2009-04-22 11:59 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-04-20 12:27 [Cluster-devel] [PATCH] Fix freeze of cluster-2.03.11 Kadlecsik Jozsef
2009-04-20 13:14 ` Fabio M. Di Nitto
2009-04-20 13:33 ` Steven Whitehouse
2009-04-20 16:36 ` Kadlecsik Jozsef
2009-04-21 8:39 ` Steven Whitehouse
2009-04-22 11:55 ` Kadlecsik Jozsef
2009-04-22 11:59 ` Steven Whitehouse [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1240401578.29604.79.camel@localhost.localdomain \
--to=swhiteho@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox