* [Cluster-devel] [PATCH v2 1/6] gfs2: gfs2_stuffed_write_end cleanup
2018-01-29 22:18 [Cluster-devel] [PATCH v2 0/6] gfs2 iomap buffered write support Andreas Gruenbacher
@ 2018-01-29 22:18 ` Andreas Gruenbacher
2018-01-29 22:18 ` [Cluster-devel] [PATCH v2 2/6] gfs2: Remove ordered write mode handling from gfs2_trans_add_data Andreas Gruenbacher
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Andreas Gruenbacher @ 2018-01-29 22:18 UTC (permalink / raw)
To: cluster-devel.redhat.com
First, change the sanity check in gfs2_stuffed_write_end to check for
the actual write size instead of the requested write size.
Second, use the existing teardown code in gfs2_write_end instead of
duplicating it in gfs2_stuffed_write_end.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/gfs2/aops.c | 49 ++++++++++++++++++-------------------------------
1 file changed, 18 insertions(+), 31 deletions(-)
diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 2f725b4a386b..8ed34aa9997c 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -814,7 +814,6 @@ static void adjust_fs_space(struct inode *inode)
* @inode: The inode
* @dibh: The buffer_head containing the on-disk inode
* @pos: The file position
- * @len: The length of the write
* @copied: How much was actually copied by the VFS
* @page: The page
*
@@ -824,17 +823,15 @@ static void adjust_fs_space(struct inode *inode)
* Returns: errno
*/
static int gfs2_stuffed_write_end(struct inode *inode, struct buffer_head *dibh,
- loff_t pos, unsigned len, unsigned copied,
+ loff_t pos, unsigned copied,
struct page *page)
{
struct gfs2_inode *ip = GFS2_I(inode);
- struct gfs2_sbd *sdp = GFS2_SB(inode);
- struct gfs2_inode *m_ip = GFS2_I(sdp->sd_statfs_inode);
u64 to = pos + copied;
void *kaddr;
unsigned char *buf = dibh->b_data + sizeof(struct gfs2_dinode);
- BUG_ON(pos + len > gfs2_max_stuffed_size(ip));
+ BUG_ON(pos + copied > gfs2_max_stuffed_size(ip));
kaddr = kmap_atomic(page);
memcpy(buf + pos, kaddr + pos, copied);
@@ -850,20 +847,6 @@ static int gfs2_stuffed_write_end(struct inode *inode, struct buffer_head *dibh,
i_size_write(inode, to);
mark_inode_dirty(inode);
}
-
- if (inode == sdp->sd_rindex) {
- adjust_fs_space(inode);
- sdp->sd_rindex_uptodate = 0;
- }
-
- brelse(dibh);
- gfs2_trans_end(sdp);
- if (inode == sdp->sd_rindex) {
- gfs2_glock_dq(&m_ip->i_gh);
- gfs2_holder_uninit(&m_ip->i_gh);
- }
- gfs2_glock_dq(&ip->i_gh);
- gfs2_holder_uninit(&ip->i_gh);
return copied;
}
@@ -877,9 +860,8 @@ static int gfs2_stuffed_write_end(struct inode *inode, struct buffer_head *dibh,
* @page: The page that has been written
* @fsdata: The fsdata (unused in GFS2)
*
- * The main write_end function for GFS2. We have a separate one for
- * stuffed files as they are slightly different, otherwise we just
- * put our locking around the VFS provided functions.
+ * The main write_end function for GFS2. We just put our locking around the VFS
+ * provided functions.
*
* Returns: errno
*/
@@ -900,32 +882,37 @@ static int gfs2_write_end(struct file *file, struct address_space *mapping,
BUG_ON(gfs2_glock_is_locked_by_me(ip->i_gl) == NULL);
ret = gfs2_meta_inode_buffer(ip, &dibh);
- if (unlikely(ret)) {
- unlock_page(page);
- put_page(page);
- goto failed;
- }
+ if (unlikely(ret))
+ goto out;
- if (gfs2_is_stuffed(ip))
- return gfs2_stuffed_write_end(inode, dibh, pos, len, copied, page);
+ if (gfs2_is_stuffed(ip)) {
+ ret = gfs2_stuffed_write_end(inode, dibh, pos, copied, page);
+ page = NULL;
+ goto out2;
+ }
if (!gfs2_is_writeback(ip))
gfs2_page_add_databufs(ip, page, pos & ~PAGE_MASK, len);
ret = generic_write_end(file, mapping, pos, len, copied, page, fsdata);
+ page = NULL;
if (tr->tr_num_buf_new)
__mark_inode_dirty(inode, I_DIRTY_DATASYNC);
else
gfs2_trans_add_meta(ip->i_gl, dibh);
-
+out2:
if (inode == sdp->sd_rindex) {
adjust_fs_space(inode);
sdp->sd_rindex_uptodate = 0;
}
brelse(dibh);
-failed:
+out:
+ if (page) {
+ unlock_page(page);
+ put_page(page);
+ }
gfs2_trans_end(sdp);
gfs2_inplace_release(ip);
if (ip->i_qadata && ip->i_qadata->qa_qd_num)
--
2.14.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Cluster-devel] [PATCH v2 2/6] gfs2: Remove ordered write mode handling from gfs2_trans_add_data
2018-01-29 22:18 [Cluster-devel] [PATCH v2 0/6] gfs2 iomap buffered write support Andreas Gruenbacher
2018-01-29 22:18 ` [Cluster-devel] [PATCH v2 1/6] gfs2: gfs2_stuffed_write_end cleanup Andreas Gruenbacher
@ 2018-01-29 22:18 ` Andreas Gruenbacher
2018-01-30 9:39 ` Steven Whitehouse
2018-01-29 22:18 ` [Cluster-devel] [PATCH v2 3/6] gfs2: Iomap cleanups and improvements Andreas Gruenbacher
` (3 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Andreas Gruenbacher @ 2018-01-29 22:18 UTC (permalink / raw)
To: cluster-devel.redhat.com
In journaled data mode, we need to add each buffer head to the current
transaction. In ordered write mode, we only need to add the inode to
the ordered inode list. So far, both cases are handled in
gfs2_trans_add_data. This makes the code look misleading and is
inefficient for small block sizes as well. Handle both cases separately
instead.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/gfs2/aops.c | 7 ++++---
fs/gfs2/bmap.c | 12 ++++++++----
fs/gfs2/log.h | 7 ++++++-
fs/gfs2/quota.c | 5 ++++-
fs/gfs2/trans.c | 27 ++++++++-------------------
5 files changed, 30 insertions(+), 28 deletions(-)
diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 8ed34aa9997c..84db5759fe3d 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -54,8 +54,7 @@ static void gfs2_page_add_databufs(struct gfs2_inode *ip, struct page *page,
continue;
if (start >= to)
break;
- if (gfs2_is_jdata(ip))
- set_buffer_uptodate(bh);
+ set_buffer_uptodate(bh);
gfs2_trans_add_data(ip->i_gl, bh);
}
}
@@ -891,8 +890,10 @@ static int gfs2_write_end(struct file *file, struct address_space *mapping,
goto out2;
}
- if (!gfs2_is_writeback(ip))
+ if (gfs2_is_jdata(ip))
gfs2_page_add_databufs(ip, page, pos & ~PAGE_MASK, len);
+ else
+ gfs2_ordered_add_inode(ip);
ret = generic_write_end(file, mapping, pos, len, copied, page, fsdata);
page = NULL;
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 86863792f36a..72e22b99ade7 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -89,10 +89,12 @@ static int gfs2_unstuffer_page(struct gfs2_inode *ip, struct buffer_head *dibh,
map_bh(bh, inode->i_sb, block);
set_buffer_uptodate(bh);
- if (!gfs2_is_jdata(ip))
- mark_buffer_dirty(bh);
- if (!gfs2_is_writeback(ip))
+ if (gfs2_is_jdata(ip))
gfs2_trans_add_data(ip->i_gl, bh);
+ else {
+ mark_buffer_dirty(bh);
+ gfs2_ordered_add_inode(ip);
+ }
if (release) {
unlock_page(page);
@@ -951,8 +953,10 @@ static int gfs2_block_zero_range(struct inode *inode, loff_t from,
err = 0;
}
- if (!gfs2_is_writeback(ip))
+ if (gfs2_is_jdata(ip))
gfs2_trans_add_data(ip->i_gl, bh);
+ else
+ gfs2_ordered_add_inode(ip);
zero_user(page, offset, length);
mark_buffer_dirty(bh);
diff --git a/fs/gfs2/log.h b/fs/gfs2/log.h
index 93b52ac1ca1f..1634c67bd660 100644
--- a/fs/gfs2/log.h
+++ b/fs/gfs2/log.h
@@ -14,6 +14,7 @@
#include <linux/spinlock.h>
#include <linux/writeback.h>
#include "incore.h"
+#include "inode.h"
/**
* gfs2_log_lock - acquire the right to mess with the log manager
@@ -50,8 +51,12 @@ static inline void gfs2_log_pointers_init(struct gfs2_sbd *sdp,
static inline void gfs2_ordered_add_inode(struct gfs2_inode *ip)
{
- struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
+ struct gfs2_sbd *sdp;
+ if (!gfs2_is_ordered(ip))
+ return;
+
+ sdp = GFS2_SB(&ip->i_inode);
if (!test_bit(GIF_ORDERED, &ip->i_flags)) {
spin_lock(&sdp->sd_ordered_lock);
if (!test_and_set_bit(GIF_ORDERED, &ip->i_flags))
diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 7a98abd340ee..e8585dfd209f 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -735,7 +735,10 @@ static int gfs2_write_buf_to_page(struct gfs2_inode *ip, unsigned long index,
if (!buffer_uptodate(bh))
goto unlock_out;
}
- gfs2_trans_add_data(ip->i_gl, bh);
+ if (gfs2_is_jdata(ip))
+ gfs2_trans_add_data(ip->i_gl, bh);
+ else
+ gfs2_ordered_add_inode(ip);
/* If we need to write to the next block as well */
if (to_write > (bsize - boff)) {
diff --git a/fs/gfs2/trans.c b/fs/gfs2/trans.c
index c75cacaa349b..064c9a0ef046 100644
--- a/fs/gfs2/trans.c
+++ b/fs/gfs2/trans.c
@@ -143,32 +143,21 @@ static struct gfs2_bufdata *gfs2_alloc_bufdata(struct gfs2_glock *gl,
* @gl: The inode glock associated with the buffer
* @bh: The buffer to add
*
- * This is used in two distinct cases:
- * i) In ordered write mode
- * We put the data buffer on a list so that we can ensure that it's
- * synced to disk at the right time
- * ii) In journaled data mode
- * We need to journal the data block in the same way as metadata in
- * the functions above. The difference is that here we have a tag
- * which is two __be64's being the block number (as per meta data)
- * and a flag which says whether the data block needs escaping or
- * not. This means we need a new log entry for each 251 or so data
- * blocks, which isn't an enormous overhead but twice as much as
- * for normal metadata blocks.
+ * This is used in journaled data mode.
+ * We need to journal the data block in the same way as metadata in
+ * the functions above. The difference is that here we have a tag
+ * which is two __be64's being the block number (as per meta data)
+ * and a flag which says whether the data block needs escaping or
+ * not. This means we need a new log entry for each 251 or so data
+ * blocks, which isn't an enormous overhead but twice as much as
+ * for normal metadata blocks.
*/
void gfs2_trans_add_data(struct gfs2_glock *gl, struct buffer_head *bh)
{
struct gfs2_trans *tr = current->journal_info;
struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
- struct address_space *mapping = bh->b_page->mapping;
- struct gfs2_inode *ip = GFS2_I(mapping->host);
struct gfs2_bufdata *bd;
- if (!gfs2_is_jdata(ip)) {
- gfs2_ordered_add_inode(ip);
- return;
- }
-
lock_buffer(bh);
if (buffer_pinned(bh)) {
set_bit(TR_TOUCHED, &tr->tr_flags);
--
2.14.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Cluster-devel] [PATCH v2 2/6] gfs2: Remove ordered write mode handling from gfs2_trans_add_data
2018-01-29 22:18 ` [Cluster-devel] [PATCH v2 2/6] gfs2: Remove ordered write mode handling from gfs2_trans_add_data Andreas Gruenbacher
@ 2018-01-30 9:39 ` Steven Whitehouse
2018-01-30 13:22 ` Andreas Gruenbacher
0 siblings, 1 reply; 9+ messages in thread
From: Steven Whitehouse @ 2018-01-30 9:39 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
On 29/01/18 22:18, Andreas Gruenbacher wrote:
> In journaled data mode, we need to add each buffer head to the current
> transaction. In ordered write mode, we only need to add the inode to
> the ordered inode list. So far, both cases are handled in
> gfs2_trans_add_data. This makes the code look misleading and is
> inefficient for small block sizes as well. Handle both cases separately
> instead.
>
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
> fs/gfs2/aops.c | 7 ++++---
> fs/gfs2/bmap.c | 12 ++++++++----
> fs/gfs2/log.h | 7 ++++++-
> fs/gfs2/quota.c | 5 ++++-
> fs/gfs2/trans.c | 27 ++++++++-------------------
> 5 files changed, 30 insertions(+), 28 deletions(-)
>
> diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
> index 8ed34aa9997c..84db5759fe3d 100644
> --- a/fs/gfs2/aops.c
> +++ b/fs/gfs2/aops.c
> @@ -54,8 +54,7 @@ static void gfs2_page_add_databufs(struct gfs2_inode *ip, struct page *page,
> continue;
> if (start >= to)
> break;
> - if (gfs2_is_jdata(ip))
> - set_buffer_uptodate(bh);
> + set_buffer_uptodate(bh);
> gfs2_trans_add_data(ip->i_gl, bh);
> }
> }
> @@ -891,8 +890,10 @@ static int gfs2_write_end(struct file *file, struct address_space *mapping,
> goto out2;
> }
>
> - if (!gfs2_is_writeback(ip))
> + if (gfs2_is_jdata(ip))
> gfs2_page_add_databufs(ip, page, pos & ~PAGE_MASK, len);
> + else
> + gfs2_ordered_add_inode(ip);
>
> ret = generic_write_end(file, mapping, pos, len, copied, page, fsdata);
> page = NULL;
Does this still work for data=writeback? I know we don't support that
mode, but it can be useful for testing, so we should probably keep it,
Steve.
> diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
> index 86863792f36a..72e22b99ade7 100644
> --- a/fs/gfs2/bmap.c
> +++ b/fs/gfs2/bmap.c
> @@ -89,10 +89,12 @@ static int gfs2_unstuffer_page(struct gfs2_inode *ip, struct buffer_head *dibh,
> map_bh(bh, inode->i_sb, block);
>
> set_buffer_uptodate(bh);
> - if (!gfs2_is_jdata(ip))
> - mark_buffer_dirty(bh);
> - if (!gfs2_is_writeback(ip))
> + if (gfs2_is_jdata(ip))
> gfs2_trans_add_data(ip->i_gl, bh);
> + else {
> + mark_buffer_dirty(bh);
> + gfs2_ordered_add_inode(ip);
> + }
>
> if (release) {
> unlock_page(page);
> @@ -951,8 +953,10 @@ static int gfs2_block_zero_range(struct inode *inode, loff_t from,
> err = 0;
> }
>
> - if (!gfs2_is_writeback(ip))
> + if (gfs2_is_jdata(ip))
> gfs2_trans_add_data(ip->i_gl, bh);
> + else
> + gfs2_ordered_add_inode(ip);
>
> zero_user(page, offset, length);
> mark_buffer_dirty(bh);
> diff --git a/fs/gfs2/log.h b/fs/gfs2/log.h
> index 93b52ac1ca1f..1634c67bd660 100644
> --- a/fs/gfs2/log.h
> +++ b/fs/gfs2/log.h
> @@ -14,6 +14,7 @@
> #include <linux/spinlock.h>
> #include <linux/writeback.h>
> #include "incore.h"
> +#include "inode.h"
>
> /**
> * gfs2_log_lock - acquire the right to mess with the log manager
> @@ -50,8 +51,12 @@ static inline void gfs2_log_pointers_init(struct gfs2_sbd *sdp,
>
> static inline void gfs2_ordered_add_inode(struct gfs2_inode *ip)
> {
> - struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
> + struct gfs2_sbd *sdp;
>
> + if (!gfs2_is_ordered(ip))
> + return;
> +
> + sdp = GFS2_SB(&ip->i_inode);
> if (!test_bit(GIF_ORDERED, &ip->i_flags)) {
> spin_lock(&sdp->sd_ordered_lock);
> if (!test_and_set_bit(GIF_ORDERED, &ip->i_flags))
> diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> index 7a98abd340ee..e8585dfd209f 100644
> --- a/fs/gfs2/quota.c
> +++ b/fs/gfs2/quota.c
> @@ -735,7 +735,10 @@ static int gfs2_write_buf_to_page(struct gfs2_inode *ip, unsigned long index,
> if (!buffer_uptodate(bh))
> goto unlock_out;
> }
> - gfs2_trans_add_data(ip->i_gl, bh);
> + if (gfs2_is_jdata(ip))
> + gfs2_trans_add_data(ip->i_gl, bh);
> + else
> + gfs2_ordered_add_inode(ip);
>
> /* If we need to write to the next block as well */
> if (to_write > (bsize - boff)) {
> diff --git a/fs/gfs2/trans.c b/fs/gfs2/trans.c
> index c75cacaa349b..064c9a0ef046 100644
> --- a/fs/gfs2/trans.c
> +++ b/fs/gfs2/trans.c
> @@ -143,32 +143,21 @@ static struct gfs2_bufdata *gfs2_alloc_bufdata(struct gfs2_glock *gl,
> * @gl: The inode glock associated with the buffer
> * @bh: The buffer to add
> *
> - * This is used in two distinct cases:
> - * i) In ordered write mode
> - * We put the data buffer on a list so that we can ensure that it's
> - * synced to disk at the right time
> - * ii) In journaled data mode
> - * We need to journal the data block in the same way as metadata in
> - * the functions above. The difference is that here we have a tag
> - * which is two __be64's being the block number (as per meta data)
> - * and a flag which says whether the data block needs escaping or
> - * not. This means we need a new log entry for each 251 or so data
> - * blocks, which isn't an enormous overhead but twice as much as
> - * for normal metadata blocks.
> + * This is used in journaled data mode.
> + * We need to journal the data block in the same way as metadata in
> + * the functions above. The difference is that here we have a tag
> + * which is two __be64's being the block number (as per meta data)
> + * and a flag which says whether the data block needs escaping or
> + * not. This means we need a new log entry for each 251 or so data
> + * blocks, which isn't an enormous overhead but twice as much as
> + * for normal metadata blocks.
> */
> void gfs2_trans_add_data(struct gfs2_glock *gl, struct buffer_head *bh)
> {
> struct gfs2_trans *tr = current->journal_info;
> struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
> - struct address_space *mapping = bh->b_page->mapping;
> - struct gfs2_inode *ip = GFS2_I(mapping->host);
> struct gfs2_bufdata *bd;
>
> - if (!gfs2_is_jdata(ip)) {
> - gfs2_ordered_add_inode(ip);
> - return;
> - }
> -
> lock_buffer(bh);
> if (buffer_pinned(bh)) {
> set_bit(TR_TOUCHED, &tr->tr_flags);
^ permalink raw reply [flat|nested] 9+ messages in thread* [Cluster-devel] [PATCH v2 2/6] gfs2: Remove ordered write mode handling from gfs2_trans_add_data
2018-01-30 9:39 ` Steven Whitehouse
@ 2018-01-30 13:22 ` Andreas Gruenbacher
0 siblings, 0 replies; 9+ messages in thread
From: Andreas Gruenbacher @ 2018-01-30 13:22 UTC (permalink / raw)
To: cluster-devel.redhat.com
On 30 January 2018 at 10:39, Steven Whitehouse <swhiteho@redhat.com> wrote:
> On 29/01/18 22:18, Andreas Gruenbacher wrote:
>> In journaled data mode, we need to add each buffer head to the current
>> transaction. In ordered write mode, we only need to add the inode to
>> the ordered inode list. So far, both cases are handled in
>> gfs2_trans_add_data. This makes the code look misleading and is
>> inefficient for small block sizes as well. Handle both cases separately
>> instead.
>>
>> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
>> ---
>> fs/gfs2/aops.c | 7 ++++---
>> fs/gfs2/bmap.c | 12 ++++++++----
>> fs/gfs2/log.h | 7 ++++++-
>> fs/gfs2/quota.c | 5 ++++-
>> fs/gfs2/trans.c | 27 ++++++++-------------------
>> 5 files changed, 30 insertions(+), 28 deletions(-)
>>
>> diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
>> index 8ed34aa9997c..84db5759fe3d 100644
>> --- a/fs/gfs2/aops.c
>> +++ b/fs/gfs2/aops.c
>> @@ -54,8 +54,7 @@ static void gfs2_page_add_databufs(struct gfs2_inode
>> *ip, struct page *page,
>> continue;
>> if (start >= to)
>> break;
>> - if (gfs2_is_jdata(ip))
>> - set_buffer_uptodate(bh);
>> + set_buffer_uptodate(bh);
>> gfs2_trans_add_data(ip->i_gl, bh);
>> }
>> }
>> @@ -891,8 +890,10 @@ static int gfs2_write_end(struct file *file, struct
>> address_space *mapping,
>> goto out2;
>> }
>> - if (!gfs2_is_writeback(ip))
>> + if (gfs2_is_jdata(ip))
>> gfs2_page_add_databufs(ip, page, pos & ~PAGE_MASK, len);
>> + else
>> + gfs2_ordered_add_inode(ip);
>> ret = generic_write_end(file, mapping, pos, len, copied, page,
>> fsdata);
>> page = NULL;
>
> Does this still work for data=writeback? I know we don't support that mode,
> but it can be useful for testing, so we should probably keep it,
It certainly should; this patch is supposed to be an invariant
transformation. In the original version, gfs2_page_add_databufs did
handle the ordered and jdata cases; with this patch,
gfs2_ordered_add_inode deals with the ordered and writeback cases
where necessary. I did open code the ordered vs. writeback distinction
in an earlier version as well, but that didn't improve the code.
>> diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
>> index 86863792f36a..72e22b99ade7 100644
>> --- a/fs/gfs2/bmap.c
>> +++ b/fs/gfs2/bmap.c
>> @@ -89,10 +89,12 @@ static int gfs2_unstuffer_page(struct gfs2_inode *ip,
>> struct buffer_head *dibh,
>> map_bh(bh, inode->i_sb, block);
>> set_buffer_uptodate(bh);
>> - if (!gfs2_is_jdata(ip))
>> - mark_buffer_dirty(bh);
>> - if (!gfs2_is_writeback(ip))
>> + if (gfs2_is_jdata(ip))
>> gfs2_trans_add_data(ip->i_gl, bh);
>> + else {
>> + mark_buffer_dirty(bh);
>> + gfs2_ordered_add_inode(ip);
>> + }
>> if (release) {
>> unlock_page(page);
>> @@ -951,8 +953,10 @@ static int gfs2_block_zero_range(struct inode *inode,
>> loff_t from,
>> err = 0;
>> }
>> - if (!gfs2_is_writeback(ip))
>> + if (gfs2_is_jdata(ip))
>> gfs2_trans_add_data(ip->i_gl, bh);
>> + else
>> + gfs2_ordered_add_inode(ip);
>> zero_user(page, offset, length);
>> mark_buffer_dirty(bh);
>> diff --git a/fs/gfs2/log.h b/fs/gfs2/log.h
>> index 93b52ac1ca1f..1634c67bd660 100644
>> --- a/fs/gfs2/log.h
>> +++ b/fs/gfs2/log.h
>> @@ -14,6 +14,7 @@
>> #include <linux/spinlock.h>
>> #include <linux/writeback.h>
>> #include "incore.h"
>> +#include "inode.h"
>> /**
>> * gfs2_log_lock - acquire the right to mess with the log manager
>> @@ -50,8 +51,12 @@ static inline void gfs2_log_pointers_init(struct
>> gfs2_sbd *sdp,
>> static inline void gfs2_ordered_add_inode(struct gfs2_inode *ip)
>> {
>> - struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
>> + struct gfs2_sbd *sdp;
>> + if (!gfs2_is_ordered(ip))
>> + return;
>> +
>> + sdp = GFS2_SB(&ip->i_inode);
>> if (!test_bit(GIF_ORDERED, &ip->i_flags)) {
>> spin_lock(&sdp->sd_ordered_lock);
>> if (!test_and_set_bit(GIF_ORDERED, &ip->i_flags))
>> diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
>> index 7a98abd340ee..e8585dfd209f 100644
>> --- a/fs/gfs2/quota.c
>> +++ b/fs/gfs2/quota.c
>> @@ -735,7 +735,10 @@ static int gfs2_write_buf_to_page(struct gfs2_inode
>> *ip, unsigned long index,
>> if (!buffer_uptodate(bh))
>> goto unlock_out;
>> }
>> - gfs2_trans_add_data(ip->i_gl, bh);
>> + if (gfs2_is_jdata(ip))
>> + gfs2_trans_add_data(ip->i_gl, bh);
>> + else
>> + gfs2_ordered_add_inode(ip);
>> /* If we need to write to the next block as well */
>> if (to_write > (bsize - boff)) {
>> diff --git a/fs/gfs2/trans.c b/fs/gfs2/trans.c
>> index c75cacaa349b..064c9a0ef046 100644
>> --- a/fs/gfs2/trans.c
>> +++ b/fs/gfs2/trans.c
>> @@ -143,32 +143,21 @@ static struct gfs2_bufdata
>> *gfs2_alloc_bufdata(struct gfs2_glock *gl,
>> * @gl: The inode glock associated with the buffer
>> * @bh: The buffer to add
>> *
>> - * This is used in two distinct cases:
>> - * i) In ordered write mode
>> - * We put the data buffer on a list so that we can ensure that it's
>> - * synced to disk at the right time
>> - * ii) In journaled data mode
>> - * We need to journal the data block in the same way as metadata in
>> - * the functions above. The difference is that here we have a tag
>> - * which is two __be64's being the block number (as per meta data)
>> - * and a flag which says whether the data block needs escaping or
>> - * not. This means we need a new log entry for each 251 or so data
>> - * blocks, which isn't an enormous overhead but twice as much as
>> - * for normal metadata blocks.
>> + * This is used in journaled data mode.
>> + * We need to journal the data block in the same way as metadata in
>> + * the functions above. The difference is that here we have a tag
>> + * which is two __be64's being the block number (as per meta data)
>> + * and a flag which says whether the data block needs escaping or
>> + * not. This means we need a new log entry for each 251 or so data
>> + * blocks, which isn't an enormous overhead but twice as much as
>> + * for normal metadata blocks.
>> */
>> void gfs2_trans_add_data(struct gfs2_glock *gl, struct buffer_head *bh)
>> {
>> struct gfs2_trans *tr = current->journal_info;
>> struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
>> - struct address_space *mapping = bh->b_page->mapping;
>> - struct gfs2_inode *ip = GFS2_I(mapping->host);
>> struct gfs2_bufdata *bd;
>> - if (!gfs2_is_jdata(ip)) {
>> - gfs2_ordered_add_inode(ip);
>> - return;
>> - }
>> -
>> lock_buffer(bh);
>> if (buffer_pinned(bh)) {
>> set_bit(TR_TOUCHED, &tr->tr_flags);
Andreas
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Cluster-devel] [PATCH v2 3/6] gfs2: Iomap cleanups and improvements
2018-01-29 22:18 [Cluster-devel] [PATCH v2 0/6] gfs2 iomap buffered write support Andreas Gruenbacher
2018-01-29 22:18 ` [Cluster-devel] [PATCH v2 1/6] gfs2: gfs2_stuffed_write_end cleanup Andreas Gruenbacher
2018-01-29 22:18 ` [Cluster-devel] [PATCH v2 2/6] gfs2: Remove ordered write mode handling from gfs2_trans_add_data Andreas Gruenbacher
@ 2018-01-29 22:18 ` Andreas Gruenbacher
2018-01-29 22:18 ` [Cluster-devel] [PATCH v2 4/6] iomap: Add write_{begin, end} iomap operations Andreas Gruenbacher
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Andreas Gruenbacher @ 2018-01-29 22:18 UTC (permalink / raw)
To: cluster-devel.redhat.com
Clean up gfs2_iomap_alloc and gfs2_iomap_get. Document how
gfs2_iomap_alloc works. gfs2_iomap_alloc now needs to be called
separately after gfs2_iomap_get where necessary; this will be used later
by iomap write. Move gfs2_iomap_ops into bmap.c.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/gfs2/bmap.c | 197 ++++++++++++++++++++++++++++++--------------------------
fs/gfs2/bmap.h | 4 +-
fs/gfs2/inode.c | 4 --
3 files changed, 109 insertions(+), 96 deletions(-)
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 72e22b99ade7..c9edf48f3fe8 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -422,22 +422,6 @@ static inline unsigned int gfs2_extent_length(void *start, unsigned int len, __b
return (ptr - first);
}
-static inline void bmap_lock(struct gfs2_inode *ip, int create)
-{
- if (create)
- down_write(&ip->i_rw_mutex);
- else
- down_read(&ip->i_rw_mutex);
-}
-
-static inline void bmap_unlock(struct gfs2_inode *ip, int create)
-{
- if (create)
- up_write(&ip->i_rw_mutex);
- else
- up_read(&ip->i_rw_mutex);
-}
-
static inline __be64 *gfs2_indirect_init(struct metapath *mp,
struct gfs2_glock *gl, unsigned int i,
unsigned offset, u64 bn)
@@ -464,15 +448,11 @@ enum alloc_state {
};
/**
- * gfs2_bmap_alloc - Build a metadata tree of the requested height
+ * gfs2_iomap_alloc - Build a metadata tree of the requested height
* @inode: The GFS2 inode
- * @lblock: The logical starting block of the extent
- * @bh_map: This is used to return the mapping details
- * @zero_new: True if newly allocated blocks should be zeroed
+ * @iomap: The iomap structure
+ * @flags: iomap flags
* @mp: The metapath, with proper height information calculated
- * @maxlen: The max number of data blocks to alloc
- * @dblock: Pointer to return the resulting new block
- * @dblks: Pointer to return the number of blocks allocated
*
* In this routine we may have to alloc:
* i) Indirect blocks to grow the metadata tree height
@@ -485,6 +465,13 @@ enum alloc_state {
* blocks are available, there will only be one request per bmap call)
* and uses the state machine to initialise the blocks in order.
*
+ * Right now, this function will allocate at most one indirect block
+ * worth of data -- with a default block size of 4K, that's slightly
+ * less than 2M. If this limitation is ever removed to allow huge
+ * allocations, we would probably still want to limit the iomap size we
+ * return to avoid stalling other tasks during huge writes; the next
+ * iomap iteration would then find the blocks already allocated.
+ *
* Returns: errno on error
*/
@@ -511,6 +498,8 @@ static int gfs2_iomap_alloc(struct inode *inode, struct iomap *iomap,
gfs2_trans_add_meta(ip->i_gl, dibh);
+ down_write(&ip->i_rw_mutex);
+
if (mp->mp_fheight == mp->mp_aheight) {
struct buffer_head *bh;
int eob;
@@ -546,11 +535,10 @@ static int gfs2_iomap_alloc(struct inode *inode, struct iomap *iomap,
blks = dblks + iblks;
i = mp->mp_aheight;
do {
- int error;
n = blks - alloced;
- error = gfs2_alloc_blocks(ip, &bn, &n, 0, NULL);
- if (error)
- return error;
+ ret = gfs2_alloc_blocks(ip, &bn, &n, 0, NULL);
+ if (ret)
+ goto out;
alloced += n;
if (state != ALLOC_DATA || gfs2_is_jdata(ip))
gfs2_trans_add_unrevoke(sdp, bn, n);
@@ -606,7 +594,7 @@ static int gfs2_iomap_alloc(struct inode *inode, struct iomap *iomap,
dblks = n;
ptr = metapointer(end_of_metadata, mp);
iomap->addr = bn << inode->i_blkbits;
- iomap->flags |= IOMAP_F_NEW;
+ iomap->flags |= IOMAP_F_MERGED | IOMAP_F_NEW;
while (n-- > 0)
*ptr++ = cpu_to_be64(bn++);
if (flags & IOMAP_ZERO) {
@@ -625,8 +613,10 @@ static int gfs2_iomap_alloc(struct inode *inode, struct iomap *iomap,
iomap->length = (u64)dblks << inode->i_blkbits;
ip->i_height = mp->mp_fheight;
gfs2_add_inode_blocks(&ip->i_inode, alloced);
- gfs2_dinode_out(ip, mp->mp_bh[0]->b_data);
- return 0;
+ gfs2_dinode_out(ip, dibh->b_data);
+out:
+ up_write(&ip->i_rw_mutex);
+ return ret;
}
/**
@@ -635,7 +625,7 @@ static int gfs2_iomap_alloc(struct inode *inode, struct iomap *iomap,
* @lblock: The logical starting block number
* @mp: The metapath
*
- * Returns: The hole size in bytes
+ * Returns: The hole size in blocks
*
*/
static u64 hole_size(struct inode *inode, sector_t lblock, struct metapath *mp)
@@ -682,7 +672,7 @@ static u64 hole_size(struct inode *inode, sector_t lblock, struct metapath *mp)
if (hgt && (mp->mp_list[hgt - 1] < mp_eof.mp_list[hgt - 1]))
(mp->mp_list[hgt - 1])++;
}
- return holesz << inode->i_blkbits;
+ return holesz;
}
static void gfs2_stuffed_iomap(struct inode *inode, struct iomap *iomap)
@@ -698,55 +688,46 @@ static void gfs2_stuffed_iomap(struct inode *inode, struct iomap *iomap)
}
/**
- * gfs2_iomap_begin - Map blocks from an inode to disk blocks
+ * gfs2_iomap_get - Map blocks from an inode to disk blocks
* @inode: The inode
* @pos: Starting position in bytes
* @length: Length to map, in bytes
* @flags: iomap flags
* @iomap: The iomap structure
+ * @mp: The metapath
*
* Returns: errno
*/
-int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
- unsigned flags, struct iomap *iomap)
+static int gfs2_iomap_get(struct inode *inode, loff_t pos, loff_t length,
+ unsigned flags, struct iomap *iomap,
+ struct metapath *mp)
{
struct gfs2_inode *ip = GFS2_I(inode);
struct gfs2_sbd *sdp = GFS2_SB(inode);
- struct metapath mp = { .mp_aheight = 1, };
unsigned int factor = sdp->sd_sb.sb_bsize;
const u64 *arr = sdp->sd_heightsize;
__be64 *ptr;
sector_t lblock;
- sector_t lend;
+ sector_t lblock_stop;
int ret;
int eob;
- unsigned int len;
+ u64 len;
struct buffer_head *bh;
u8 height;
- trace_gfs2_iomap_start(ip, pos, length, flags);
- if (!length) {
- ret = -EINVAL;
- goto out;
- }
+ if (!length)
+ return -EINVAL;
if ((flags & IOMAP_REPORT) && gfs2_is_stuffed(ip)) {
- gfs2_stuffed_iomap(inode, iomap);
- if (pos >= iomap->length)
+ if (pos >= i_size_read(inode))
return -ENOENT;
- ret = 0;
- goto out;
+ gfs2_stuffed_iomap(inode, iomap);
+ return 0;
}
lblock = pos >> inode->i_blkbits;
- lend = (pos + length + sdp->sd_sb.sb_bsize - 1) >> inode->i_blkbits;
-
- iomap->offset = lblock << inode->i_blkbits;
- iomap->addr = IOMAP_NULL_ADDR;
- iomap->type = IOMAP_HOLE;
- iomap->length = (u64)(lend - lblock) << inode->i_blkbits;
- iomap->flags = IOMAP_F_MERGED;
- bmap_lock(ip, 0);
+ lblock_stop = (pos + length - 1) >> inode->i_blkbits;
+ len = lblock_stop - lblock + 1;
/*
* Directory data blocks have a struct gfs2_meta_header header, so the
@@ -758,61 +739,87 @@ int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
arr = sdp->sd_jheightsize;
}
- ret = gfs2_meta_inode_buffer(ip, &mp.mp_bh[0]);
+ down_read(&ip->i_rw_mutex);
+
+ ret = gfs2_meta_inode_buffer(ip, &mp->mp_bh[0]);
if (ret)
- goto out_release;
+ goto unlock;
height = ip->i_height;
while ((lblock + 1) * factor > arr[height])
height++;
- find_metapath(sdp, lblock, &mp, height);
+ find_metapath(sdp, lblock, mp, height);
if (height > ip->i_height || gfs2_is_stuffed(ip))
goto do_alloc;
- ret = lookup_metapath(ip, &mp);
- if (ret)
- goto out_release;
+ ret = lookup_metapath(ip, mp);
+ if (ret < 0)
+ goto unlock;
+ ret = 0;
- if (mp.mp_aheight != ip->i_height)
+ if (mp->mp_aheight != ip->i_height)
goto do_alloc;
- ptr = metapointer(ip->i_height - 1, &mp);
+ ptr = metapointer(ip->i_height - 1, mp);
if (*ptr == 0)
goto do_alloc;
- iomap->type = IOMAP_MAPPED;
iomap->addr = be64_to_cpu(*ptr) << inode->i_blkbits;
+ iomap->type = IOMAP_MAPPED;
- bh = mp.mp_bh[ip->i_height - 1];
- len = gfs2_extent_length(bh->b_data, bh->b_size, ptr, lend - lblock, &eob);
+ bh = mp->mp_bh[ip->i_height - 1];
+ len = gfs2_extent_length(bh->b_data, bh->b_size, ptr, len, &eob);
+ iomap->flags = IOMAP_F_MERGED;
if (eob)
iomap->flags |= IOMAP_F_BOUNDARY;
- iomap->length = (u64)len << inode->i_blkbits;
- ret = 0;
-
-out_release:
- release_metapath(&mp);
- bmap_unlock(ip, 0);
out:
- trace_gfs2_iomap_end(ip, iomap, ret);
+ iomap->offset = lblock << inode->i_blkbits;
+ iomap->length = len << inode->i_blkbits;
+ iomap->bdev = inode->i_sb->s_bdev;
+unlock:
+ up_read(&ip->i_rw_mutex);
return ret;
do_alloc:
if (!(flags & IOMAP_WRITE)) {
if (pos >= i_size_read(inode)) {
ret = -ENOENT;
- goto out_release;
+ goto unlock;
}
- ret = 0;
- iomap->length = hole_size(inode, lblock, &mp);
- goto out_release;
+ len = hole_size(inode, lblock, mp);
}
+ iomap->addr = IOMAP_NULL_ADDR;
+ iomap->type = IOMAP_HOLE;
+ iomap->flags = 0;
+ goto out;
+}
- ret = gfs2_iomap_alloc(inode, iomap, flags, &mp);
- goto out_release;
+static int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
+ unsigned flags, struct iomap *iomap)
+{
+ struct gfs2_inode *ip = GFS2_I(inode);
+ struct metapath mp = { .mp_aheight = 1, };
+ int ret;
+
+ trace_gfs2_iomap_start(ip, pos, length, flags);
+ if (flags & IOMAP_WRITE) {
+ ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp);
+ if (!ret && iomap->type == IOMAP_HOLE)
+ ret = gfs2_iomap_alloc(inode, iomap, flags, &mp);
+ release_metapath(&mp);
+ } else {
+ ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp);
+ release_metapath(&mp);
+ }
+ trace_gfs2_iomap_end(ip, iomap, ret);
+ return ret;
}
+const struct iomap_ops gfs2_iomap_ops = {
+ .iomap_begin = gfs2_iomap_begin,
+};
+
/**
* gfs2_block_map - Map a block from an inode to a disk block
* @inode: The inode
@@ -831,27 +838,37 @@ int gfs2_block_map(struct inode *inode, sector_t lblock,
struct buffer_head *bh_map, int create)
{
struct gfs2_inode *ip = GFS2_I(inode);
+ loff_t pos = (loff_t)lblock << inode->i_blkbits;
+ loff_t length = bh_map->b_size;
+ struct metapath mp = { .mp_aheight = 1, };
struct iomap iomap;
- int ret, flags = 0;
+ int ret;
clear_buffer_mapped(bh_map);
clear_buffer_new(bh_map);
clear_buffer_boundary(bh_map);
trace_gfs2_bmap(ip, bh_map, lblock, create, 1);
- if (create)
- flags |= IOMAP_WRITE;
- if (buffer_zeronew(bh_map))
- flags |= IOMAP_ZERO;
- ret = gfs2_iomap_begin(inode, (loff_t)lblock << inode->i_blkbits,
- bh_map->b_size, flags, &iomap);
- if (ret) {
- if (!create && ret == -ENOENT) {
- /* Return unmapped buffer beyond the end of file. */
+ if (create) {
+ int flags = IOMAP_WRITE;
+ if (buffer_zeronew(bh_map))
+ flags |= IOMAP_ZERO;
+ ret = gfs2_iomap_get(inode, pos, length, flags, &iomap, &mp);
+ if (!ret && iomap.type == IOMAP_HOLE)
+ ret = gfs2_iomap_alloc(inode, &iomap, flags, &mp);
+ release_metapath(&mp);
+ } else {
+ ret = gfs2_iomap_get(inode, pos, length, 0, &iomap, &mp);
+ release_metapath(&mp);
+
+ /* Return unmapped buffer beyond the end of file. */
+ if (ret == -ENOENT) {
ret = 0;
+ goto out;
}
- goto out;
}
+ if (ret)
+ goto out;
if (iomap.length > bh_map->b_size) {
iomap.length = bh_map->b_size;
diff --git a/fs/gfs2/bmap.h b/fs/gfs2/bmap.h
index c3402fe00653..5d563c29cb0a 100644
--- a/fs/gfs2/bmap.h
+++ b/fs/gfs2/bmap.h
@@ -46,11 +46,11 @@ static inline void gfs2_write_calc_reserv(const struct gfs2_inode *ip,
}
}
+extern const struct iomap_ops gfs2_iomap_ops;
+
extern int gfs2_unstuff_dinode(struct gfs2_inode *ip, struct page *page);
extern int gfs2_block_map(struct inode *inode, sector_t lblock,
struct buffer_head *bh, int create);
-extern int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
- unsigned flags, struct iomap *iomap);
extern int gfs2_extent_map(struct inode *inode, u64 lblock, int *new,
u64 *dblock, unsigned *extlen);
extern int gfs2_setattr_size(struct inode *inode, u64 size);
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 20281992d456..5cc193e5737b 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -2015,10 +2015,6 @@ static int gfs2_getattr(const struct path *path, struct kstat *stat,
return 0;
}
-const struct iomap_ops gfs2_iomap_ops = {
- .iomap_begin = gfs2_iomap_begin,
-};
-
static int gfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
u64 start, u64 len)
{
--
2.14.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Cluster-devel] [PATCH v2 4/6] iomap: Add write_{begin, end} iomap operations
2018-01-29 22:18 [Cluster-devel] [PATCH v2 0/6] gfs2 iomap buffered write support Andreas Gruenbacher
` (2 preceding siblings ...)
2018-01-29 22:18 ` [Cluster-devel] [PATCH v2 3/6] gfs2: Iomap cleanups and improvements Andreas Gruenbacher
@ 2018-01-29 22:18 ` Andreas Gruenbacher
2018-01-29 22:18 ` [Cluster-devel] [PATCH v2 5/6] gfs2: Implement iomap buffered write support (1) Andreas Gruenbacher
2018-01-29 22:18 ` [Cluster-devel] [PATCH v2 6/6] gfs2: Implement iomap buffered write support (2) Andreas Gruenbacher
5 siblings, 0 replies; 9+ messages in thread
From: Andreas Gruenbacher @ 2018-01-29 22:18 UTC (permalink / raw)
To: cluster-devel.redhat.com
Add write_begin and write_end operations to struct iomap_ops to provide
a way of overriding the default behavior of iomap_write_begin and
iomap_write_end. This is needed for implementing data journaling: in
the data journaling case, pages are written into the journal before
being written back to their proper on-disk locations.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Cc: Dave Chinner <dchinner@redhat.com>
---
fs/ext2/inode.c | 2 ++
fs/ext4/inode.c | 2 ++
fs/iomap.c | 62 +++++++++++++++++++++++++++++++++++----------------
fs/xfs/xfs_iomap.c | 2 ++
include/linux/iomap.h | 22 ++++++++++++++++++
5 files changed, 71 insertions(+), 19 deletions(-)
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 9b2ac55ac34f..9b8ea6108b30 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -848,6 +848,8 @@ ext2_iomap_end(struct inode *inode, loff_t offset, loff_t length,
const struct iomap_ops ext2_iomap_ops = {
.iomap_begin = ext2_iomap_begin,
+ .write_begin = iomap_write_begin,
+ .write_end = iomap_write_end,
.iomap_end = ext2_iomap_end,
};
#else
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 534a9130f625..76f4ad697853 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3601,6 +3601,8 @@ static int ext4_iomap_end(struct inode *inode, loff_t offset, loff_t length,
const struct iomap_ops ext4_iomap_ops = {
.iomap_begin = ext4_iomap_begin,
+ .write_begin = iomap_write_begin,
+ .write_end = iomap_write_end,
.iomap_end = ext4_iomap_end,
};
diff --git a/fs/iomap.c b/fs/iomap.c
index 47d29ccffaef..2e5866ea4522 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -106,7 +106,7 @@ iomap_write_failed(struct inode *inode, loff_t pos, unsigned len)
truncate_pagecache_range(inode, max(pos, i_size), pos + len);
}
-static int
+int
iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, unsigned flags,
struct page **pagep, struct iomap *iomap)
{
@@ -135,8 +135,9 @@ iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, unsigned flags,
*pagep = page;
return status;
}
+EXPORT_SYMBOL_GPL(iomap_write_begin);
-static int
+int
iomap_write_end(struct inode *inode, loff_t pos, unsigned len,
unsigned copied, struct page *page)
{
@@ -148,12 +149,19 @@ iomap_write_end(struct inode *inode, loff_t pos, unsigned len,
iomap_write_failed(inode, pos, len);
return ret;
}
+EXPORT_SYMBOL_GPL(iomap_write_end);
+
+struct iomap_write_args {
+ struct iov_iter *iter;
+ const struct iomap_ops *ops;
+};
static loff_t
iomap_write_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
struct iomap *iomap)
{
- struct iov_iter *i = data;
+ struct iomap_write_args *args = data;
+ struct iov_iter *i = args->iter;
long status = 0;
ssize_t written = 0;
unsigned int flags = AOP_FLAG_NOFS;
@@ -186,7 +194,7 @@ iomap_write_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
break;
}
- status = iomap_write_begin(inode, pos, bytes, flags, &page,
+ status = args->ops->write_begin(inode, pos, bytes, flags, &page,
iomap);
if (unlikely(status))
break;
@@ -198,7 +206,7 @@ iomap_write_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
flush_dcache_page(page);
- status = iomap_write_end(inode, pos, bytes, copied, page);
+ status =args->ops->write_end(inode, pos, bytes, copied, page);
if (unlikely(status < 0))
break;
copied = status;
@@ -235,10 +243,14 @@ iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *iter,
{
struct inode *inode = iocb->ki_filp->f_mapping->host;
loff_t pos = iocb->ki_pos, ret = 0, written = 0;
+ struct iomap_write_args args = {
+ .iter = iter,
+ .ops = ops,
+ };
while (iov_iter_count(iter)) {
ret = iomap_apply(inode, pos, iov_iter_count(iter),
- IOMAP_WRITE, ops, iter, iomap_write_actor);
+ IOMAP_WRITE, ops, &args, iomap_write_actor);
if (ret <= 0)
break;
pos += ret;
@@ -269,6 +281,7 @@ static loff_t
iomap_dirty_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
struct iomap *iomap)
{
+ const struct iomap_ops *ops = data;
long status = 0;
ssize_t written = 0;
@@ -284,15 +297,15 @@ iomap_dirty_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
if (IS_ERR(rpage))
return PTR_ERR(rpage);
- status = iomap_write_begin(inode, pos, bytes,
- AOP_FLAG_NOFS, &page, iomap);
+ status = ops->write_begin(inode, pos, bytes,
+ AOP_FLAG_NOFS, &page, iomap);
put_page(rpage);
if (unlikely(status))
return status;
WARN_ON_ONCE(!PageUptodate(page));
- status = iomap_write_end(inode, pos, bytes, bytes, page);
+ status = ops->write_end(inode, pos, bytes, bytes, page);
if (unlikely(status <= 0)) {
if (WARN_ON_ONCE(status == 0))
return -EIO;
@@ -318,7 +331,7 @@ iomap_file_dirty(struct inode *inode, loff_t pos, loff_t len,
loff_t ret;
while (len) {
- ret = iomap_apply(inode, pos, len, IOMAP_WRITE, ops, NULL,
+ ret = iomap_apply(inode, pos, len, IOMAP_WRITE, ops, (void *)ops,
iomap_dirty_actor);
if (ret <= 0)
return ret;
@@ -331,20 +344,21 @@ iomap_file_dirty(struct inode *inode, loff_t pos, loff_t len,
EXPORT_SYMBOL_GPL(iomap_file_dirty);
static int iomap_zero(struct inode *inode, loff_t pos, unsigned offset,
- unsigned bytes, struct iomap *iomap)
+ unsigned bytes, const struct iomap_ops *ops,
+ struct iomap *iomap)
{
struct page *page;
int status;
- status = iomap_write_begin(inode, pos, bytes, AOP_FLAG_NOFS, &page,
- iomap);
+ status = ops->write_begin(inode, pos, bytes, AOP_FLAG_NOFS, &page,
+ iomap);
if (status)
return status;
zero_user(page, offset, bytes);
mark_page_accessed(page);
- return iomap_write_end(inode, pos, bytes, bytes, page);
+ return ops->write_end(inode, pos, bytes, bytes, page);
}
static int iomap_dax_zero(loff_t pos, unsigned offset, unsigned bytes,
@@ -357,11 +371,16 @@ static int iomap_dax_zero(loff_t pos, unsigned offset, unsigned bytes,
offset, bytes);
}
+struct iomap_zero_range_args {
+ const struct iomap_ops *ops;
+ bool *did_zero;
+};
+
static loff_t
iomap_zero_range_actor(struct inode *inode, loff_t pos, loff_t count,
void *data, struct iomap *iomap)
{
- bool *did_zero = data;
+ struct iomap_zero_range_args *args = data;
loff_t written = 0;
int status;
@@ -378,15 +397,16 @@ iomap_zero_range_actor(struct inode *inode, loff_t pos, loff_t count,
if (IS_DAX(inode))
status = iomap_dax_zero(pos, offset, bytes, iomap);
else
- status = iomap_zero(inode, pos, offset, bytes, iomap);
+ status = iomap_zero(inode, pos, offset, bytes,
+ (void *)args->ops, iomap);
if (status < 0)
return status;
pos += bytes;
count -= bytes;
written += bytes;
- if (did_zero)
- *did_zero = true;
+ if (args->did_zero)
+ *args->did_zero = true;
} while (count > 0);
return written;
@@ -396,11 +416,15 @@ int
iomap_zero_range(struct inode *inode, loff_t pos, loff_t len, bool *did_zero,
const struct iomap_ops *ops)
{
+ struct iomap_zero_range_args args = {
+ .ops = ops,
+ .did_zero = did_zero,
+ };
loff_t ret;
while (len > 0) {
ret = iomap_apply(inode, pos, len, IOMAP_ZERO,
- ops, did_zero, iomap_zero_range_actor);
+ ops, &args, iomap_zero_range_actor);
if (ret <= 0)
return ret;
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 7ab52a8bc0a9..a53e3ab229df 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -1181,6 +1181,8 @@ xfs_file_iomap_end(
const struct iomap_ops xfs_iomap_ops = {
.iomap_begin = xfs_file_iomap_begin,
+ .write_begin = iomap_write_begin,
+ .write_end = iomap_write_end,
.iomap_end = xfs_file_iomap_end,
};
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 19a07de28212..423f7ecc1231 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -61,6 +61,8 @@ struct iomap {
#define IOMAP_DIRECT (1 << 4) /* direct I/O */
#define IOMAP_NOWAIT (1 << 5) /* Don't wait for writeback */
+struct page;
+
struct iomap_ops {
/*
* Return the existing mapping@pos, or reserve space starting at
@@ -70,6 +72,21 @@ struct iomap_ops {
int (*iomap_begin)(struct inode *inode, loff_t pos, loff_t length,
unsigned flags, struct iomap *iomap);
+ /*
+ * Begin writing to a page, similar to block_write_begin but in a
+ * mapping returned by iomap_begin. Usually initialized to
+ * iomap_write_begin.
+ */
+ int (*write_begin)(struct inode *inode, loff_t pos, unsigned len,
+ unsigned flags, struct page **pagep,
+ struct iomap *iomap);
+
+ /*
+ * End writing to a page. Usually initialized to iomap_write_end.
+ */
+ int (*write_end)(struct inode *inode, loff_t pos, unsigned len,
+ unsigned copied, struct page *page);
+
/*
* Commit and/or unreserve space previous allocated using iomap_begin.
* Written indicates the length of the successful write operation which
@@ -80,6 +97,11 @@ struct iomap_ops {
ssize_t written, unsigned flags, struct iomap *iomap);
};
+int iomap_write_begin(struct inode *inode, loff_t pos, unsigned len,
+ unsigned flags, struct page **pagep, struct iomap *iomap);
+int iomap_write_end(struct inode *inode, loff_t pos, unsigned len,
+ unsigned copied, struct page *page);
+
ssize_t iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *from,
const struct iomap_ops *ops);
int iomap_file_dirty(struct inode *inode, loff_t pos, loff_t len,
--
2.14.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Cluster-devel] [PATCH v2 5/6] gfs2: Implement iomap buffered write support (1)
2018-01-29 22:18 [Cluster-devel] [PATCH v2 0/6] gfs2 iomap buffered write support Andreas Gruenbacher
` (3 preceding siblings ...)
2018-01-29 22:18 ` [Cluster-devel] [PATCH v2 4/6] iomap: Add write_{begin, end} iomap operations Andreas Gruenbacher
@ 2018-01-29 22:18 ` Andreas Gruenbacher
2018-01-29 22:18 ` [Cluster-devel] [PATCH v2 6/6] gfs2: Implement iomap buffered write support (2) Andreas Gruenbacher
5 siblings, 0 replies; 9+ messages in thread
From: Andreas Gruenbacher @ 2018-01-29 22:18 UTC (permalink / raw)
To: cluster-devel.redhat.com
With the traditional page-based writes, blocks are allocated separately
for each page written to. With iomap writes, we can allocate a lot more
blocks at once, with a fraction of the allocation overhead for each
page.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/gfs2/aops.c | 7 ++-
fs/gfs2/aops.h | 18 ++++++
fs/gfs2/bmap.c | 194 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
fs/gfs2/file.c | 45 ++++++++++++-
4 files changed, 254 insertions(+), 10 deletions(-)
create mode 100644 fs/gfs2/aops.h
diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 84db5759fe3d..5d843c9f9e05 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -36,10 +36,11 @@
#include "super.h"
#include "util.h"
#include "glops.h"
+#include "aops.h"
-static void gfs2_page_add_databufs(struct gfs2_inode *ip, struct page *page,
- unsigned int from, unsigned int len)
+void gfs2_page_add_databufs(struct gfs2_inode *ip, struct page *page,
+ unsigned int from, unsigned int len)
{
struct buffer_head *head = page_buffers(page);
unsigned int bsize = head->b_size;
@@ -773,7 +774,7 @@ static int gfs2_write_begin(struct file *file, struct address_space *mapping,
* adjust_fs_space - Adjusts the free space available due to gfs2_grow
* @inode: the rindex inode
*/
-static void adjust_fs_space(struct inode *inode)
+void adjust_fs_space(struct inode *inode)
{
struct gfs2_sbd *sdp = inode->i_sb->s_fs_info;
struct gfs2_inode *m_ip = GFS2_I(sdp->sd_statfs_inode);
diff --git a/fs/gfs2/aops.h b/fs/gfs2/aops.h
new file mode 100644
index 000000000000..9a2fa61d8ca4
--- /dev/null
+++ b/fs/gfs2/aops.h
@@ -0,0 +1,18 @@
+/*
+ * Copyright (C) 2017 Red Hat, Inc. All rights reserved.
+ *
+ * This copyrighted material is made available to anyone wishing to use,
+ * modify, copy, or redistribute it subject to the terms and conditions
+ * of the GNU General Public License version 2.
+ */
+
+#ifndef __AOPS_DOT_H__
+#define __AOPS_DOT_H__
+
+#include "incore.h"
+
+extern void adjust_fs_space(struct inode *inode);
+extern void gfs2_page_add_databufs(struct gfs2_inode *ip, struct page *page,
+ unsigned int from, unsigned int len);
+
+#endif /* __AOPS_DOT_H__ */
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index c9edf48f3fe8..0b26982dab3a 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -28,6 +28,7 @@
#include "trans.h"
#include "dir.h"
#include "util.h"
+#include "aops.h"
#include "trace_gfs2.h"
/* This doesn't need to be that large as max 64 bit pointers in a 4k
@@ -41,6 +42,8 @@ struct metapath {
int mp_aheight; /* actual height (lookup height) */
};
+static int punch_hole(struct gfs2_inode *ip, u64 offset, u64 length);
+
/**
* gfs2_unstuffer_page - unstuff a stuffed inode into a block cached by a page
* @ip: the inode
@@ -375,7 +378,7 @@ static int fillup_metapath(struct gfs2_inode *ip, struct metapath *mp, int h)
return mp->mp_aheight - x - 1;
}
-static inline void release_metapath(struct metapath *mp)
+static void release_metapath(struct metapath *mp)
{
int i;
@@ -383,6 +386,7 @@ static inline void release_metapath(struct metapath *mp)
if (mp->mp_bh[i] == NULL)
break;
brelse(mp->mp_bh[i]);
+ mp->mp_bh[i] = NULL;
}
}
@@ -795,6 +799,139 @@ static int gfs2_iomap_get(struct inode *inode, loff_t pos, loff_t length,
goto out;
}
+static int gfs2_write_lock(struct inode *inode)
+{
+ struct gfs2_inode *ip = GFS2_I(inode);
+ struct gfs2_sbd *sdp = GFS2_SB(inode);
+ int error;
+
+ gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &ip->i_gh);
+ error = gfs2_glock_nq(&ip->i_gh);
+ if (error)
+ goto out_uninit;
+ if (&ip->i_inode == sdp->sd_rindex) {
+ struct gfs2_inode *m_ip = GFS2_I(sdp->sd_statfs_inode);
+
+ error = gfs2_glock_nq_init(m_ip->i_gl, LM_ST_EXCLUSIVE,
+ GL_NOCACHE, &m_ip->i_gh);
+ if (error)
+ goto out_unlock;
+ }
+ return 0;
+
+out_unlock:
+ gfs2_glock_dq(&ip->i_gh);
+out_uninit:
+ gfs2_holder_uninit(&ip->i_gh);
+ return error;
+}
+
+static void gfs2_write_unlock(struct inode *inode)
+{
+ struct gfs2_inode *ip = GFS2_I(inode);
+ struct gfs2_sbd *sdp = GFS2_SB(inode);
+
+ if (&ip->i_inode == sdp->sd_rindex) {
+ struct gfs2_inode *m_ip = GFS2_I(sdp->sd_statfs_inode);
+
+ gfs2_glock_dq_uninit(&m_ip->i_gh);
+ }
+ gfs2_glock_dq_uninit(&ip->i_gh);
+}
+
+static int gfs2_iomap_begin_write(struct inode *inode, loff_t pos, loff_t length,
+ unsigned flags, struct iomap *iomap)
+{
+ struct metapath mp = { .mp_aheight = 1, };
+ struct gfs2_inode *ip = GFS2_I(inode);
+ struct gfs2_sbd *sdp = GFS2_SB(inode);
+ unsigned int data_blocks = 0, ind_blocks = 0, rblocks;
+ int alloc_required;
+ int ret;
+
+ ret = gfs2_write_lock(inode);
+ if (ret)
+ return ret;
+
+ if (gfs2_is_stuffed(ip)) {
+ if (pos + length <= gfs2_max_stuffed_size(ip)) {
+ ret = -ENOTBLK;
+ goto out_unlock;
+ }
+ }
+
+ ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp);
+ if (ret)
+ goto out_release;
+ alloc_required = iomap->type != IOMAP_MAPPED;
+
+ if (alloc_required || gfs2_is_jdata(ip))
+ gfs2_write_calc_reserv(ip, iomap->length, &data_blocks, &ind_blocks);
+
+ if (alloc_required) {
+ struct gfs2_alloc_parms ap = { .target = data_blocks + ind_blocks };
+ ret = gfs2_quota_lock_check(ip, &ap);
+ if (ret)
+ goto out_release;
+
+ ret = gfs2_inplace_reserve(ip, &ap);
+ if (ret)
+ goto out_qunlock;
+ }
+
+ rblocks = RES_DINODE + ind_blocks;
+ if (gfs2_is_jdata(ip))
+ rblocks += data_blocks;
+ if (ind_blocks || data_blocks)
+ rblocks += RES_STATFS + RES_QUOTA;
+ if (inode == sdp->sd_rindex)
+ rblocks += 2 * RES_STATFS;
+ if (alloc_required)
+ rblocks += gfs2_rg_blocks(ip, data_blocks + ind_blocks);
+
+ ret = gfs2_trans_begin(sdp, rblocks, iomap->length >> inode->i_blkbits);
+ if (ret)
+ goto out_trans_fail;
+
+ if (gfs2_is_stuffed(ip)) {
+ ret = gfs2_unstuff_dinode(ip, NULL);
+ if (ret)
+ goto out_trans_end;
+ release_metapath(&mp);
+ ret = gfs2_iomap_get(inode, iomap->offset, iomap->length,
+ flags, iomap, &mp);
+ if (ret)
+ goto out_trans_end;
+ }
+
+ if (iomap->type != IOMAP_MAPPED) {
+ ret = gfs2_iomap_alloc(inode, iomap, flags, &mp);
+ if (ret) {
+ gfs2_trans_end(sdp);
+ if (alloc_required)
+ gfs2_inplace_release(ip);
+ punch_hole(ip, iomap->offset, iomap->length);
+ goto out_qunlock;
+ }
+ }
+ release_metapath(&mp);
+ return 0;
+
+out_trans_end:
+ gfs2_trans_end(sdp);
+out_trans_fail:
+ if (alloc_required)
+ gfs2_inplace_release(ip);
+out_qunlock:
+ if (alloc_required)
+ gfs2_quota_unlock(ip);
+out_release:
+ release_metapath(&mp);
+out_unlock:
+ gfs2_write_unlock(inode);
+ return ret;
+}
+
static int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
unsigned flags, struct iomap *iomap)
{
@@ -804,10 +941,7 @@ static int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
trace_gfs2_iomap_start(ip, pos, length, flags);
if (flags & IOMAP_WRITE) {
- ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp);
- if (!ret && iomap->type == IOMAP_HOLE)
- ret = gfs2_iomap_alloc(inode, iomap, flags, &mp);
- release_metapath(&mp);
+ ret = gfs2_iomap_begin_write(inode, pos, length, flags, iomap);
} else {
ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp);
release_metapath(&mp);
@@ -816,8 +950,58 @@ static int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
return ret;
}
+static int gfs2_iomap_write_end(struct inode *inode, loff_t pos, unsigned len,
+ unsigned copied, struct page *page)
+{
+ struct gfs2_inode *ip = GFS2_I(inode);
+
+ if (gfs2_is_jdata(ip))
+ gfs2_page_add_databufs(ip, page, offset_in_page(pos), len);
+ return iomap_write_end(inode, pos, len, copied, page);
+}
+
+static int gfs2_iomap_end(struct inode *inode, loff_t pos, loff_t length,
+ ssize_t written, unsigned flags, struct iomap *iomap)
+{
+ struct gfs2_inode *ip = GFS2_I(inode);
+ struct gfs2_sbd *sdp = GFS2_SB(inode);
+ struct gfs2_trans *tr = current->journal_info;
+
+ if (!(flags & IOMAP_WRITE))
+ return 0;
+
+ gfs2_ordered_add_inode(ip);
+
+ if (tr->tr_num_buf_new)
+ __mark_inode_dirty(inode, I_DIRTY_DATASYNC);
+
+ 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)) {
+ /* Deallocate blocks that were just allocated. */
+ pos += written;
+ length -= written;
+ truncate_pagecache_range(inode, pos, pos + length - 1);
+ punch_hole(ip, pos, length);
+ }
+
+ if (ip->i_qadata && ip->i_qadata->qa_qd_num)
+ gfs2_quota_unlock(ip);
+ gfs2_write_unlock(inode);
+ return 0;
+}
+
const struct iomap_ops gfs2_iomap_ops = {
.iomap_begin = gfs2_iomap_begin,
+ .write_begin = iomap_write_begin,
+ .write_end = gfs2_iomap_write_end,
+ .iomap_end = gfs2_iomap_end,
};
/**
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index 4f88e201b3f0..69096c3c3bfd 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -26,6 +26,7 @@
#include <linux/dlm.h>
#include <linux/dlm_plock.h>
#include <linux/delay.h>
+#include <linux/backing-dev.h>
#include "gfs2.h"
#include "incore.h"
@@ -688,6 +689,41 @@ static int gfs2_fsync(struct file *file, loff_t start, loff_t end,
return ret ? ret : ret1;
}
+static ssize_t gfs2_file_buffered_write(struct kiocb *iocb, struct iov_iter *from)
+{
+ struct file *file = iocb->ki_filp;
+ struct inode *inode = file->f_mapping->host;
+ ssize_t ret;
+
+ inode_lock(inode);
+ ret = generic_write_checks(iocb, from);
+ if (ret <= 0)
+ goto out;
+
+ ret = file_remove_privs(file);
+ if (ret)
+ goto out;
+
+ ret = file_update_time(file);
+ if (ret)
+ goto out;
+
+ /* We can write back this queue in page reclaim */
+ current->backing_dev_info = inode_to_bdi(inode);
+
+ ret = iomap_file_buffered_write(iocb, from, &gfs2_iomap_ops);
+
+ current->backing_dev_info = NULL;
+
+out:
+ inode_unlock(inode);
+ if (likely(ret > 0)) {
+ /* Handle various SYNC-type writes */
+ ret = generic_write_sync(iocb, ret);
+ }
+ return ret;
+}
+
/**
* gfs2_file_write_iter - Perform a write to a file
* @iocb: The io context
@@ -706,7 +742,7 @@ static ssize_t gfs2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
{
struct file *file = iocb->ki_filp;
struct gfs2_inode *ip = GFS2_I(file_inode(file));
- int ret;
+ ssize_t ret;
ret = gfs2_rsqa_alloc(ip);
if (ret)
@@ -723,7 +759,12 @@ static ssize_t gfs2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
gfs2_glock_dq_uninit(&gh);
}
- return generic_file_write_iter(iocb, from);
+ if (iocb->ki_flags & IOCB_DIRECT)
+ return generic_file_write_iter(iocb, from);
+ ret = gfs2_file_buffered_write(iocb, from);
+ if (ret == -ENOTBLK)
+ ret = generic_file_write_iter(iocb, from);
+ return ret;
}
static int fallocate_chunk(struct inode *inode, loff_t offset, loff_t len,
--
2.14.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Cluster-devel] [PATCH v2 6/6] gfs2: Implement iomap buffered write support (2)
2018-01-29 22:18 [Cluster-devel] [PATCH v2 0/6] gfs2 iomap buffered write support Andreas Gruenbacher
` (4 preceding siblings ...)
2018-01-29 22:18 ` [Cluster-devel] [PATCH v2 5/6] gfs2: Implement iomap buffered write support (1) Andreas Gruenbacher
@ 2018-01-29 22:18 ` Andreas Gruenbacher
5 siblings, 0 replies; 9+ messages in thread
From: Andreas Gruenbacher @ 2018-01-29 22:18 UTC (permalink / raw)
To: cluster-devel.redhat.com
Instead of falling back to generic_file_write_iter when writing to a
stuffed file that stays stuffed, implement that case separately. We
eventually want to get rid of the remaining users of gfs2_write_begin +
gfs2_write_end so that those functions can eventually be removed, and
generic_file_write_iter uses that interface.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/gfs2/aops.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
fs/gfs2/aops.h | 1 +
fs/gfs2/bmap.c | 7 ++---
fs/gfs2/bmap.h | 1 +
fs/gfs2/file.c | 10 ++++---
5 files changed, 104 insertions(+), 10 deletions(-)
diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 5d843c9f9e05..01f8657905af 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -22,6 +22,7 @@
#include <linux/backing-dev.h>
#include <linux/uio.h>
#include <trace/events/writeback.h>
+#include <linux/sched/signal.h>
#include "gfs2.h"
#include "incore.h"
@@ -820,7 +821,7 @@ void adjust_fs_space(struct inode *inode)
* This copies the data from the page into the inode block after
* the inode data structure itself.
*
- * Returns: errno
+ * Returns: copied bytes or errno
*/
static int gfs2_stuffed_write_end(struct inode *inode, struct buffer_head *dibh,
loff_t pos, unsigned copied,
@@ -850,6 +851,96 @@ static int gfs2_stuffed_write_end(struct inode *inode, struct buffer_head *dibh,
return copied;
}
+ssize_t gfs2_stuffed_write(struct kiocb *iocb, struct iov_iter *from)
+{
+ struct inode *inode = iocb->ki_filp->f_mapping->host;
+ struct gfs2_inode *ip = GFS2_I(inode);
+ struct gfs2_sbd *sdp = GFS2_SB(inode);
+ loff_t pos = iocb->ki_pos;
+ struct page *page = NULL;
+ ssize_t written = 0, ret;
+
+ BUG_ON(!gfs2_glock_is_locked_by_me(ip->i_gl));
+ BUG_ON(!gfs2_is_stuffed(ip));
+
+ ret = gfs2_trans_begin(sdp, RES_DINODE, 0);
+ if (ret)
+ return ret;
+
+ do {
+ struct buffer_head *dibh;
+ unsigned long offset;
+ unsigned long bytes;
+ size_t copied;
+
+ offset = pos & (PAGE_SIZE - 1);
+ bytes = min_t(unsigned long, PAGE_SIZE - offset,
+ iov_iter_count(from));
+again:
+ /*
+ * Bring in the user page that we will copy from _first_.
+ * Otherwise there's a nasty deadlock on copying from the
+ * same page as we're writing to, without it being marked
+ * up-to-date.
+ *
+ * Not only is this an optimisation, but it is also required
+ * to check that the address is actually valid, when atomic
+ * usercopies are used, below.
+ */
+ if (unlikely(iov_iter_fault_in_readable(from, bytes))) {
+ ret = -EFAULT;
+ goto out;
+ }
+
+ if (fatal_signal_pending(current)) {
+ ret = -EINTR;
+ goto out;
+ }
+
+ page = grab_cache_page_write_begin(inode->i_mapping, pos >> PAGE_SHIFT, AOP_FLAG_NOFS);
+ if (!page)
+ return -ENOMEM;
+
+ if (!PageUptodate(page)) {
+ ret = stuffed_readpage(ip, page);
+ if (ret)
+ goto out;
+ }
+
+ if (mapping_writably_mapped(inode->i_mapping))
+ flush_dcache_page(page);
+
+ copied = iov_iter_copy_from_user_atomic(page, from, pos, bytes);
+
+ flush_dcache_page(page);
+
+ ret = gfs2_meta_inode_buffer(ip, &dibh);
+ if (ret)
+ goto out;
+ ret = gfs2_stuffed_write_end(inode, dibh, pos, copied, page);
+ page = NULL;
+ brelse(dibh);
+ if (ret < 0)
+ goto out;
+
+ iov_iter_advance(from, copied);
+ if (unlikely(copied == 0)) {
+ bytes = iov_iter_single_seg_count(from);
+ goto again;
+ }
+ pos += copied;
+ written += copied;
+ } while (iov_iter_count(from));
+
+out:
+ if (page) {
+ unlock_page(page);
+ put_page(page);
+ }
+ gfs2_trans_end(sdp);
+ return written ? written : ret;
+}
+
/**
* gfs2_write_end
* @file: The file to write to
@@ -863,7 +954,7 @@ static int gfs2_stuffed_write_end(struct inode *inode, struct buffer_head *dibh,
* The main write_end function for GFS2. We just put our locking around the VFS
* provided functions.
*
- * Returns: errno
+ * Returns: copied bytes or errno
*/
static int gfs2_write_end(struct file *file, struct address_space *mapping,
diff --git a/fs/gfs2/aops.h b/fs/gfs2/aops.h
index 9a2fa61d8ca4..600dbe14c4b7 100644
--- a/fs/gfs2/aops.h
+++ b/fs/gfs2/aops.h
@@ -11,6 +11,7 @@
#include "incore.h"
+extern ssize_t gfs2_stuffed_write(struct kiocb *iocb, struct iov_iter *from);
extern void adjust_fs_space(struct inode *inode);
extern void gfs2_page_add_databufs(struct gfs2_inode *ip, struct page *page,
unsigned int from, unsigned int len);
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 0b26982dab3a..05b1a6198d35 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -826,7 +826,7 @@ static int gfs2_write_lock(struct inode *inode)
return error;
}
-static void gfs2_write_unlock(struct inode *inode)
+void gfs2_write_unlock(struct inode *inode)
{
struct gfs2_inode *ip = GFS2_I(inode);
struct gfs2_sbd *sdp = GFS2_SB(inode);
@@ -855,8 +855,8 @@ static int gfs2_iomap_begin_write(struct inode *inode, loff_t pos, loff_t length
if (gfs2_is_stuffed(ip)) {
if (pos + length <= gfs2_max_stuffed_size(ip)) {
- ret = -ENOTBLK;
- goto out_unlock;
+ /* Keep the inode locked! */
+ return -ENOTBLK;
}
}
@@ -927,7 +927,6 @@ static int gfs2_iomap_begin_write(struct inode *inode, loff_t pos, loff_t length
gfs2_quota_unlock(ip);
out_release:
release_metapath(&mp);
-out_unlock:
gfs2_write_unlock(inode);
return ret;
}
diff --git a/fs/gfs2/bmap.h b/fs/gfs2/bmap.h
index 5d563c29cb0a..7aa4cf56a6b2 100644
--- a/fs/gfs2/bmap.h
+++ b/fs/gfs2/bmap.h
@@ -62,5 +62,6 @@ extern int gfs2_write_alloc_required(struct gfs2_inode *ip, u64 offset,
extern int gfs2_map_journal_extents(struct gfs2_sbd *sdp, struct gfs2_jdesc *jd);
extern void gfs2_free_journal_extents(struct gfs2_jdesc *jd);
extern int __gfs2_punch_hole(struct file *file, loff_t offset, loff_t length);
+extern void gfs2_write_unlock(struct inode *inode);
#endif /* __BMAP_DOT_H__ */
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index 69096c3c3bfd..1674ed004cb0 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -31,6 +31,7 @@
#include "gfs2.h"
#include "incore.h"
#include "bmap.h"
+#include "aops.h"
#include "dir.h"
#include "glock.h"
#include "glops.h"
@@ -712,6 +713,10 @@ static ssize_t gfs2_file_buffered_write(struct kiocb *iocb, struct iov_iter *fro
current->backing_dev_info = inode_to_bdi(inode);
ret = iomap_file_buffered_write(iocb, from, &gfs2_iomap_ops);
+ if (ret == -ENOTBLK) {
+ ret = gfs2_stuffed_write(iocb, from);
+ gfs2_write_unlock(inode);
+ }
current->backing_dev_info = NULL;
@@ -761,10 +766,7 @@ static ssize_t gfs2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
if (iocb->ki_flags & IOCB_DIRECT)
return generic_file_write_iter(iocb, from);
- ret = gfs2_file_buffered_write(iocb, from);
- if (ret == -ENOTBLK)
- ret = generic_file_write_iter(iocb, from);
- return ret;
+ return gfs2_file_buffered_write(iocb, from);
}
static int fallocate_chunk(struct inode *inode, loff_t offset, loff_t len,
--
2.14.3
^ permalink raw reply related [flat|nested] 9+ messages in thread