[GFS2] Fix lock ordering bug in page fault path
Mmapped files were able to trigger a lock ordering bug. Private maps do not need to take the glock so early on. Shared maps do unfortunately, however we can get around that by adding a flag into the flags for the struct gfs2_file. This only works because we are taking an exclusive lock at this point, so we know that nobody else can be racing with us. Fixes Red Hat bugzilla: #201196 Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
This commit is contained in:
parent
899bb26450
commit
59a1cc6bda
5 changed files with 32 additions and 25 deletions
|
@ -279,6 +279,7 @@ static inline struct gfs2_sbd *GFS2_SB(struct inode *inode)
|
|||
|
||||
enum {
|
||||
GFF_DID_DIRECT_ALLOC = 0,
|
||||
GFF_EXLOCK = 1,
|
||||
};
|
||||
|
||||
struct gfs2_file {
|
||||
|
|
|
@ -256,8 +256,8 @@ static unsigned int current_tail(struct gfs2_sbd *sdp)
|
|||
if (list_empty(&sdp->sd_ail1_list))
|
||||
tail = sdp->sd_log_head;
|
||||
else {
|
||||
ai = list_entry(sdp->sd_ail1_list.prev,
|
||||
struct gfs2_ail, ai_list);
|
||||
ai = list_entry(sdp->sd_ail1_list.prev, struct gfs2_ail,
|
||||
ai_list);
|
||||
tail = ai->ai_first;
|
||||
}
|
||||
|
||||
|
|
|
@ -237,14 +237,22 @@ static int gfs2_readpage(struct file *file, struct page *page)
|
|||
struct gfs2_sbd *sdp = GFS2_SB(page->mapping->host);
|
||||
struct gfs2_holder gh;
|
||||
int error;
|
||||
int do_unlock = 0;
|
||||
|
||||
if (likely(file != &gfs2_internal_file_sentinal)) {
|
||||
if (file) {
|
||||
struct gfs2_file *gf = file->private_data;
|
||||
if (test_bit(GFF_EXLOCK, &gf->f_flags))
|
||||
goto skip_lock;
|
||||
}
|
||||
gfs2_holder_init(ip->i_gl, LM_ST_SHARED, GL_ATIME|GL_AOP, &gh);
|
||||
do_unlock = 1;
|
||||
error = gfs2_glock_nq_m_atime(1, &gh);
|
||||
if (unlikely(error))
|
||||
goto out_unlock;
|
||||
}
|
||||
|
||||
skip_lock:
|
||||
if (gfs2_is_stuffed(ip)) {
|
||||
error = stuffed_readpage(ip, page);
|
||||
unlock_page(page);
|
||||
|
@ -262,7 +270,7 @@ static int gfs2_readpage(struct file *file, struct page *page)
|
|||
return error;
|
||||
out_unlock:
|
||||
unlock_page(page);
|
||||
if (file != &gfs2_internal_file_sentinal)
|
||||
if (do_unlock)
|
||||
gfs2_holder_uninit(&gh);
|
||||
goto out;
|
||||
}
|
||||
|
@ -291,17 +299,24 @@ static int gfs2_readpages(struct file *file, struct address_space *mapping,
|
|||
struct gfs2_holder gh;
|
||||
unsigned page_idx;
|
||||
int ret;
|
||||
int do_unlock = 0;
|
||||
|
||||
if (likely(file != &gfs2_internal_file_sentinal)) {
|
||||
if (file) {
|
||||
struct gfs2_file *gf = file->private_data;
|
||||
if (test_bit(GFF_EXLOCK, &gf->f_flags))
|
||||
goto skip_lock;
|
||||
}
|
||||
gfs2_holder_init(ip->i_gl, LM_ST_SHARED,
|
||||
LM_FLAG_TRY_1CB|GL_ATIME|GL_AOP, &gh);
|
||||
do_unlock = 1;
|
||||
ret = gfs2_glock_nq_m_atime(1, &gh);
|
||||
if (ret == GLR_TRYFAILED)
|
||||
goto out_noerror;
|
||||
if (unlikely(ret))
|
||||
goto out_unlock;
|
||||
}
|
||||
|
||||
skip_lock:
|
||||
if (gfs2_is_stuffed(ip)) {
|
||||
struct pagevec lru_pvec;
|
||||
pagevec_init(&lru_pvec, 0);
|
||||
|
@ -326,7 +341,7 @@ static int gfs2_readpages(struct file *file, struct address_space *mapping,
|
|||
ret = mpage_readpages(mapping, pages, nr_pages, gfs2_get_block);
|
||||
}
|
||||
|
||||
if (likely(file != &gfs2_internal_file_sentinal)) {
|
||||
if (do_unlock) {
|
||||
gfs2_glock_dq_m(1, &gh);
|
||||
gfs2_holder_uninit(&gh);
|
||||
}
|
||||
|
@ -344,7 +359,7 @@ static int gfs2_readpages(struct file *file, struct address_space *mapping,
|
|||
unlock_page(page);
|
||||
page_cache_release(page);
|
||||
}
|
||||
if (likely(file != &gfs2_internal_file_sentinal))
|
||||
if (do_unlock)
|
||||
gfs2_holder_uninit(&gh);
|
||||
goto out;
|
||||
}
|
||||
|
|
|
@ -46,13 +46,7 @@ static struct page *gfs2_private_nopage(struct vm_area_struct *area,
|
|||
unsigned long address, int *type)
|
||||
{
|
||||
struct gfs2_inode *ip = GFS2_I(area->vm_file->f_mapping->host);
|
||||
struct gfs2_holder i_gh;
|
||||
struct page *result;
|
||||
int error;
|
||||
|
||||
error = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED, 0, &i_gh);
|
||||
if (error)
|
||||
return NULL;
|
||||
|
||||
set_bit(GIF_PAGED, &ip->i_flags);
|
||||
|
||||
|
@ -61,8 +55,6 @@ static struct page *gfs2_private_nopage(struct vm_area_struct *area,
|
|||
if (result && result != NOPAGE_OOM)
|
||||
pfault_be_greedy(ip);
|
||||
|
||||
gfs2_glock_dq_uninit(&i_gh);
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
|
@ -141,7 +133,9 @@ static int alloc_page_backing(struct gfs2_inode *ip, struct page *page)
|
|||
static struct page *gfs2_sharewrite_nopage(struct vm_area_struct *area,
|
||||
unsigned long address, int *type)
|
||||
{
|
||||
struct gfs2_inode *ip = GFS2_I(area->vm_file->f_mapping->host);
|
||||
struct file *file = area->vm_file;
|
||||
struct gfs2_file *gf = file->private_data;
|
||||
struct gfs2_inode *ip = GFS2_I(file->f_mapping->host);
|
||||
struct gfs2_holder i_gh;
|
||||
struct page *result = NULL;
|
||||
unsigned long index = ((address - area->vm_start) >> PAGE_CACHE_SHIFT) +
|
||||
|
@ -156,13 +150,14 @@ static struct page *gfs2_sharewrite_nopage(struct vm_area_struct *area,
|
|||
set_bit(GIF_PAGED, &ip->i_flags);
|
||||
set_bit(GIF_SW_PAGED, &ip->i_flags);
|
||||
|
||||
error = gfs2_write_alloc_required(ip,
|
||||
(uint64_t)index << PAGE_CACHE_SHIFT,
|
||||
error = gfs2_write_alloc_required(ip, (u64)index << PAGE_CACHE_SHIFT,
|
||||
PAGE_CACHE_SIZE, &alloc_required);
|
||||
if (error)
|
||||
goto out;
|
||||
|
||||
set_bit(GFF_EXLOCK, &gf->f_flags);
|
||||
result = filemap_nopage(area, address, type);
|
||||
clear_bit(GFF_EXLOCK, &gf->f_flags);
|
||||
if (!result || result == NOPAGE_OOM)
|
||||
goto out;
|
||||
|
||||
|
@ -177,8 +172,7 @@ static struct page *gfs2_sharewrite_nopage(struct vm_area_struct *area,
|
|||
}
|
||||
|
||||
pfault_be_greedy(ip);
|
||||
|
||||
out:
|
||||
out:
|
||||
gfs2_glock_dq_uninit(&i_gh);
|
||||
|
||||
return result;
|
||||
|
|
|
@ -153,8 +153,7 @@ static int get_log_header(struct gfs2_jdesc *jd, unsigned int blk,
|
|||
|
||||
if (lh.lh_header.mh_magic != GFS2_MAGIC ||
|
||||
lh.lh_header.mh_type != GFS2_METATYPE_LH ||
|
||||
lh.lh_blkno != blk ||
|
||||
lh.lh_hash != hash)
|
||||
lh.lh_blkno != blk || lh.lh_hash != hash)
|
||||
return 1;
|
||||
|
||||
*head = lh;
|
||||
|
@ -482,11 +481,9 @@ int gfs2_recover_journal(struct gfs2_jdesc *jd)
|
|||
|
||||
/* Acquire a shared hold on the transaction lock */
|
||||
|
||||
error = gfs2_glock_nq_init(sdp->sd_trans_gl,
|
||||
LM_ST_SHARED,
|
||||
error = gfs2_glock_nq_init(sdp->sd_trans_gl, LM_ST_SHARED,
|
||||
LM_FLAG_NOEXP | LM_FLAG_PRIORITY |
|
||||
GL_NOCANCEL | GL_NOCACHE,
|
||||
&t_gh);
|
||||
GL_NOCANCEL | GL_NOCACHE, &t_gh);
|
||||
if (error)
|
||||
goto fail_gunlock_ji;
|
||||
|
||||
|
|
Loading…
Reference in a new issue