forked from Minki/linux
[GFS2] Fix a page lock / glock deadlock
We've previously been using a "try lock" in readpage on the basis that it would prevent deadlocks due to the inverted lock ordering (our normal lock ordering is glock first and then page lock). Unfortunately tests have shown that this isn't enough. If the glock has a demote request queued such that run_queue() in the glock code tries to do a demote when its called under readpage then it will try and write out all the dirty pages which requires locking them. This then deadlocks with the page locked by readpage. The solution is to always require two calls into readpage. The first unlocks the page, gets the glock and returns AOP_TRUNCATED_PAGE, the second does the actual readpage and unlocks the glock & page as required. Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
This commit is contained in:
parent
60b779cfc1
commit
7afd88d916
@ -32,24 +32,23 @@
|
|||||||
#define GLR_TRYFAILED 13
|
#define GLR_TRYFAILED 13
|
||||||
#define GLR_CANCELED 14
|
#define GLR_CANCELED 14
|
||||||
|
|
||||||
static inline int gfs2_glock_is_locked_by_me(struct gfs2_glock *gl)
|
static inline struct gfs2_holder *gfs2_glock_is_locked_by_me(struct gfs2_glock *gl)
|
||||||
{
|
{
|
||||||
struct gfs2_holder *gh;
|
struct gfs2_holder *gh;
|
||||||
int locked = 0;
|
|
||||||
struct pid *pid;
|
struct pid *pid;
|
||||||
|
|
||||||
/* Look in glock's list of holders for one with current task as owner */
|
/* Look in glock's list of holders for one with current task as owner */
|
||||||
spin_lock(&gl->gl_spin);
|
spin_lock(&gl->gl_spin);
|
||||||
pid = task_pid(current);
|
pid = task_pid(current);
|
||||||
list_for_each_entry(gh, &gl->gl_holders, gh_list) {
|
list_for_each_entry(gh, &gl->gl_holders, gh_list) {
|
||||||
if (gh->gh_owner_pid == pid) {
|
if (gh->gh_owner_pid == pid)
|
||||||
locked = 1;
|
goto out;
|
||||||
break;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
gh = NULL;
|
||||||
|
out:
|
||||||
spin_unlock(&gl->gl_spin);
|
spin_unlock(&gl->gl_spin);
|
||||||
|
|
||||||
return locked;
|
return gh;
|
||||||
}
|
}
|
||||||
|
|
||||||
static inline int gfs2_glock_is_held_excl(struct gfs2_glock *gl)
|
static inline int gfs2_glock_is_held_excl(struct gfs2_glock *gl)
|
||||||
|
@ -493,7 +493,7 @@ struct inode *gfs2_lookupi(struct inode *dir, const struct qstr *name,
|
|||||||
return dir;
|
return dir;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (gfs2_glock_is_locked_by_me(dip->i_gl) == 0) {
|
if (gfs2_glock_is_locked_by_me(dip->i_gl) == NULL) {
|
||||||
error = gfs2_glock_nq_init(dip->i_gl, LM_ST_SHARED, 0, &d_gh);
|
error = gfs2_glock_nq_init(dip->i_gl, LM_ST_SHARED, 0, &d_gh);
|
||||||
if (error)
|
if (error)
|
||||||
return ERR_PTR(error);
|
return ERR_PTR(error);
|
||||||
|
@ -508,23 +508,26 @@ static int __gfs2_readpage(void *file, struct page *page)
|
|||||||
static int gfs2_readpage(struct file *file, struct page *page)
|
static int gfs2_readpage(struct file *file, struct page *page)
|
||||||
{
|
{
|
||||||
struct gfs2_inode *ip = GFS2_I(page->mapping->host);
|
struct gfs2_inode *ip = GFS2_I(page->mapping->host);
|
||||||
struct gfs2_holder gh;
|
struct gfs2_holder *gh;
|
||||||
int error;
|
int error;
|
||||||
|
|
||||||
gfs2_holder_init(ip->i_gl, LM_ST_SHARED, GL_ATIME|LM_FLAG_TRY_1CB, &gh);
|
gh = gfs2_glock_is_locked_by_me(ip->i_gl);
|
||||||
error = gfs2_glock_nq_atime(&gh);
|
if (!gh) {
|
||||||
if (unlikely(error)) {
|
gh = kmalloc(sizeof(struct gfs2_holder), GFP_NOFS);
|
||||||
|
if (!gh)
|
||||||
|
return -ENOBUFS;
|
||||||
|
gfs2_holder_init(ip->i_gl, LM_ST_SHARED, GL_ATIME, gh);
|
||||||
unlock_page(page);
|
unlock_page(page);
|
||||||
goto out;
|
error = gfs2_glock_nq_atime(gh);
|
||||||
}
|
if (likely(error != 0))
|
||||||
error = __gfs2_readpage(file, page);
|
goto out;
|
||||||
gfs2_glock_dq(&gh);
|
|
||||||
out:
|
|
||||||
gfs2_holder_uninit(&gh);
|
|
||||||
if (error == GLR_TRYFAILED) {
|
|
||||||
yield();
|
|
||||||
return AOP_TRUNCATED_PAGE;
|
return AOP_TRUNCATED_PAGE;
|
||||||
}
|
}
|
||||||
|
error = __gfs2_readpage(file, page);
|
||||||
|
gfs2_glock_dq(gh);
|
||||||
|
out:
|
||||||
|
gfs2_holder_uninit(gh);
|
||||||
|
kfree(gh);
|
||||||
return error;
|
return error;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -826,7 +829,7 @@ static int gfs2_write_end(struct file *file, struct address_space *mapping,
|
|||||||
unsigned int to = from + len;
|
unsigned int to = from + len;
|
||||||
int ret;
|
int ret;
|
||||||
|
|
||||||
BUG_ON(gfs2_glock_is_locked_by_me(ip->i_gl) == 0);
|
BUG_ON(gfs2_glock_is_locked_by_me(ip->i_gl) == NULL);
|
||||||
|
|
||||||
ret = gfs2_meta_inode_buffer(ip, &dibh);
|
ret = gfs2_meta_inode_buffer(ip, &dibh);
|
||||||
if (unlikely(ret)) {
|
if (unlikely(ret)) {
|
||||||
|
@ -43,7 +43,7 @@ static int gfs2_drevalidate(struct dentry *dentry, struct nameidata *nd)
|
|||||||
struct gfs2_holder d_gh;
|
struct gfs2_holder d_gh;
|
||||||
struct gfs2_inode *ip = NULL;
|
struct gfs2_inode *ip = NULL;
|
||||||
int error;
|
int error;
|
||||||
int had_lock=0;
|
int had_lock = 0;
|
||||||
|
|
||||||
if (inode) {
|
if (inode) {
|
||||||
if (is_bad_inode(inode))
|
if (is_bad_inode(inode))
|
||||||
@ -54,7 +54,7 @@ static int gfs2_drevalidate(struct dentry *dentry, struct nameidata *nd)
|
|||||||
if (sdp->sd_args.ar_localcaching)
|
if (sdp->sd_args.ar_localcaching)
|
||||||
goto valid;
|
goto valid;
|
||||||
|
|
||||||
had_lock = gfs2_glock_is_locked_by_me(dip->i_gl);
|
had_lock = (gfs2_glock_is_locked_by_me(dip->i_gl) != NULL);
|
||||||
if (!had_lock) {
|
if (!had_lock) {
|
||||||
error = gfs2_glock_nq_init(dip->i_gl, LM_ST_SHARED, 0, &d_gh);
|
error = gfs2_glock_nq_init(dip->i_gl, LM_ST_SHARED, 0, &d_gh);
|
||||||
if (error)
|
if (error)
|
||||||
|
@ -898,7 +898,7 @@ static int gfs2_permission(struct inode *inode, int mask, struct nameidata *nd)
|
|||||||
int error;
|
int error;
|
||||||
int unlock = 0;
|
int unlock = 0;
|
||||||
|
|
||||||
if (gfs2_glock_is_locked_by_me(ip->i_gl) == 0) {
|
if (gfs2_glock_is_locked_by_me(ip->i_gl) == NULL) {
|
||||||
error = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED, LM_FLAG_ANY, &i_gh);
|
error = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED, LM_FLAG_ANY, &i_gh);
|
||||||
if (error)
|
if (error)
|
||||||
return error;
|
return error;
|
||||||
@ -1065,7 +1065,7 @@ static int gfs2_getattr(struct vfsmount *mnt, struct dentry *dentry,
|
|||||||
int error;
|
int error;
|
||||||
int unlock = 0;
|
int unlock = 0;
|
||||||
|
|
||||||
if (gfs2_glock_is_locked_by_me(ip->i_gl) == 0) {
|
if (gfs2_glock_is_locked_by_me(ip->i_gl) == NULL) {
|
||||||
error = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED, LM_FLAG_ANY, &gh);
|
error = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED, LM_FLAG_ANY, &gh);
|
||||||
if (error)
|
if (error)
|
||||||
return error;
|
return error;
|
||||||
|
Loading…
Reference in New Issue
Block a user