Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d0a22a4b authored by Andreas Gruenbacher's avatar Andreas Gruenbacher
Browse files

gfs2: Fix iomap write page reclaim deadlock 



Since commit 64bc06bb ("gfs2: iomap buffered write support"), gfs2 is doing
buffered writes by starting a transaction in iomap_begin, writing a range of
pages, and ending that transaction in iomap_end.  This approach suffers from
two problems:

  (1) Any allocations necessary for the write are done in iomap_begin, so when
  the data aren't journaled, there is no need for keeping the transaction open
  until iomap_end.

  (2) Transactions keep the gfs2 log flush lock held.  When
  iomap_file_buffered_write calls balance_dirty_pages, this can end up calling
  gfs2_write_inode, which will try to flush the log.  This requires taking the
  log flush lock which is already held, resulting in a deadlock.

Fix both of these issues by not keeping transactions open from iomap_begin to
iomap_end.  Instead, start a small transaction in page_prepare and end it in
page_done when necessary.

Reported-by: default avatarEdwin Török <edvin.torok@citrix.com>
Fixes: 64bc06bb ("gfs2: iomap buffered write support")
Signed-off-by: default avatarAndreas Gruenbacher <agruenba@redhat.com>
Signed-off-by: default avatarBob Peterson <rpeterso@redhat.com>
parent 8f918219

fs/gfs2/aops.c

+10 −4
Original line number Diff line number Diff line
@@ -649,7 +649,7 @@ static int gfs2_readpages(struct file *file, struct address_space *mapping, 
 */

void adjust_fs_space(struct inode *inode)

{

	struct gfs2_sbd *sdp = inode->i_sb->s_fs_info;

	struct gfs2_sbd *sdp = GFS2_SB(inode);

	struct gfs2_inode *m_ip = GFS2_I(sdp->sd_statfs_inode);

	struct gfs2_inode *l_ip = GFS2_I(sdp->sd_sc_inode);

	struct gfs2_statfs_change_host *m_sc = &sdp->sd_statfs_master;
@@ -657,10 +657,13 @@ void adjust_fs_space(struct inode *inode) 
	struct buffer_head *m_bh, *l_bh;

	u64 fs_total, new_free;



	if (gfs2_trans_begin(sdp, 2 * RES_STATFS, 0) != 0)

		return;



	/* Total up the file system space, according to the latest rindex. */

	fs_total = gfs2_ri_total(sdp);

	if (gfs2_meta_inode_buffer(m_ip, &m_bh) != 0)

		return;

		goto out;



	spin_lock(&sdp->sd_statfs_spin);

	gfs2_statfs_change_in(m_sc, m_bh->b_data +
@@ -675,11 +678,14 @@ void adjust_fs_space(struct inode *inode) 
	gfs2_statfs_change(sdp, new_free, new_free, 0);



	if (gfs2_meta_inode_buffer(l_ip, &l_bh) != 0)

		goto out;

		goto out2;

	update_statfs(sdp, m_bh, l_bh);

	brelse(l_bh);

out:

out2:

	brelse(m_bh);

out:

	sdp->sd_rindex_uptodate = 0;

	gfs2_trans_end(sdp);

}



/**

fs/gfs2/bmap.c

+48 −40
Original line number Diff line number Diff line
@@ -991,17 +991,28 @@ static void gfs2_write_unlock(struct inode *inode) 
	gfs2_glock_dq_uninit(&ip->i_gh);

}



static int gfs2_iomap_page_prepare(struct inode *inode, loff_t pos,

				   unsigned len, struct iomap *iomap)

{

	struct gfs2_sbd *sdp = GFS2_SB(inode);



	return gfs2_trans_begin(sdp, RES_DINODE + (len >> inode->i_blkbits), 0);

}



static void gfs2_iomap_page_done(struct inode *inode, loff_t pos,

				 unsigned copied, struct page *page,

				 struct iomap *iomap)

{

	struct gfs2_inode *ip = GFS2_I(inode);

	struct gfs2_sbd *sdp = GFS2_SB(inode);



	if (page)

	if (page && !gfs2_is_stuffed(ip))

		gfs2_page_add_databufs(ip, page, offset_in_page(pos), copied);

	gfs2_trans_end(sdp);

}



static const struct iomap_page_ops gfs2_iomap_page_ops = {

	.page_prepare = gfs2_iomap_page_prepare,

	.page_done = gfs2_iomap_page_done,

};
@@ -1057,7 +1068,11 @@ static int gfs2_iomap_begin_write(struct inode *inode, loff_t pos, 
	if (alloc_required)

		rblocks += gfs2_rg_blocks(ip, data_blocks + ind_blocks);



	ret = gfs2_trans_begin(sdp, rblocks, iomap->length >> inode->i_blkbits);

	if (unstuff || iomap->type == IOMAP_HOLE) {

		struct gfs2_trans *tr;



		ret = gfs2_trans_begin(sdp, rblocks,

				       iomap->length >> inode->i_blkbits);

		if (ret)

			goto out_trans_fail;
@@ -1066,8 +1081,8 @@ static int gfs2_iomap_begin_write(struct inode *inode, loff_t pos, 
			if (ret)

				goto out_trans_end;

			release_metapath(mp);

		ret = gfs2_iomap_get(inode, iomap->offset, iomap->length,

				     flags, iomap, mp);

			ret = gfs2_iomap_get(inode, iomap->offset,

					     iomap->length, flags, iomap, mp);

			if (ret)

				goto out_trans_end;

		}
@@ -1081,7 +1096,17 @@ static int gfs2_iomap_begin_write(struct inode *inode, loff_t pos, 
				goto out_qunlock;

			}

		}

	if (!gfs2_is_stuffed(ip) && gfs2_is_jdata(ip))



		tr = current->journal_info;

		if (tr->tr_num_buf_new)

			__mark_inode_dirty(inode, I_DIRTY_DATASYNC);

		else

			gfs2_trans_add_meta(ip->i_gl, mp->mp_bh[0]);



		gfs2_trans_end(sdp);

	}



	if (gfs2_is_stuffed(ip) || gfs2_is_jdata(ip))

		iomap->page_ops = &gfs2_iomap_page_ops;

	return 0;
@@ -1121,10 +1146,6 @@ static int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length, 
		    iomap->type != IOMAP_MAPPED)

			ret = -ENOTBLK;

	}

	if (!ret) {

		get_bh(mp.mp_bh[0]);

		iomap->private = mp.mp_bh[0];

	}

	release_metapath(&mp);

	trace_gfs2_iomap_end(ip, iomap, ret);

	return ret;
@@ -1135,27 +1156,16 @@ static int gfs2_iomap_end(struct inode *inode, loff_t pos, loff_t length, 
{

	struct gfs2_inode *ip = GFS2_I(inode);

	struct gfs2_sbd *sdp = GFS2_SB(inode);

	struct gfs2_trans *tr = current->journal_info;

	struct buffer_head *dibh = iomap->private;



	if ((flags & (IOMAP_WRITE | IOMAP_DIRECT)) != IOMAP_WRITE)

		goto out;



	if (iomap->type != IOMAP_INLINE) {

	if (!gfs2_is_stuffed(ip))

		gfs2_ordered_add_inode(ip);



		if (tr->tr_num_buf_new)

			__mark_inode_dirty(inode, I_DIRTY_DATASYNC);

		else

			gfs2_trans_add_meta(ip->i_gl, dibh);

	}



	if (inode == sdp->sd_rindex) {

	if (inode == sdp->sd_rindex)

		adjust_fs_space(inode);

		sdp->sd_rindex_uptodate = 0;

	}



	gfs2_trans_end(sdp);

	gfs2_inplace_release(ip);



	if (length != written && (iomap->flags & IOMAP_F_NEW)) {
@@ -1175,8 +1185,6 @@ static int gfs2_iomap_end(struct inode *inode, loff_t pos, loff_t length, 
	gfs2_write_unlock(inode);



out:

	if (dibh)

		brelse(dibh);

	return 0;

}