* [Cluster-devel] [GFS2 PATCH 0/2] Fix regression in "Ignore journal log writes for jdata holes"
@ 2020-11-12 14:54 Bob Peterson
2020-11-12 14:54 ` [Cluster-devel] [GFS2 PATCH 1/1] gfs2: Make special version of gfs2_get_block_noalloc for jdata Bob Peterson
0 siblings, 1 reply; 4+ messages in thread
From: Bob Peterson @ 2020-11-12 14:54 UTC (permalink / raw)
To: cluster-devel.redhat.com
Patch b2a846dbef4e ("gfs2: Ignore journal log writes for jdata holes"
caused a regression. It fixed one specific problem while breaking others.
The problem was that it changed the behavior of function gfs2_block_map
which is used by multiple callers so it had unintended consequences for
other callers.
This patch set reverts the patch and replaces it with a more targeted
solution that fixes just the one case it needs to.
Bob Peterson (2):
Revert "gfs2: Ignore journal log writes for jdata holes"
gfs2: Make special version of gfs2_get_block_noalloc for jdata
fs/gfs2/aops.c | 30 ++++++++++++++++++++++++++++--
fs/gfs2/bmap.c | 8 ++------
fs/gfs2/log.c | 2 ++
3 files changed, 32 insertions(+), 8 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Cluster-devel] [GFS2 PATCH 1/1] gfs2: Make special version of gfs2_get_block_noalloc for jdata
2020-11-12 14:54 [Cluster-devel] [GFS2 PATCH 0/2] Fix regression in "Ignore journal log writes for jdata holes" Bob Peterson
@ 2020-11-12 14:54 ` Bob Peterson
0 siblings, 0 replies; 4+ messages in thread
From: Bob Peterson @ 2020-11-12 14:54 UTC (permalink / raw)
To: cluster-devel.redhat.com
Patch b2a846dbef4ef54ef032f0f5ee188c609a0278a7 fixed cases in which
writes were attempted to blocks that had been freed by punch_hole in a jdata
file, but the blocks were still represented on the ail1 list.
It had an unfortunate side-effect of returning -ENODATA in cases that
were not jdata because it affected function gfs2_block_map which is used by
many other callers. This resulted in several xfstests test failures. For
example, generic/029 would fail with this error:
+hexdump: /mnt/scratch/testfile: Input/output error
00000000 58 58 58 58 58 58 58 58 58 58 58 58 58 58 58 58 |XXXXXXXXXXXXXXXX|
This patch creates a special version of gfs2_get_block_noalloc for
jdata which returns -ENODATA instead of -EIO for unmapped blocks.
This is a more targeted approach that doesn't break non-jdata cases.
Fixes: b2a846dbef4e ("gfs2: Ignore journal log writes for jdata holes")
Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
fs/gfs2/aops.c | 30 ++++++++++++++++++++++++++++--
fs/gfs2/log.c | 2 ++
2 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 9cd2ecad07db..835c50e6a871 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -80,6 +80,32 @@ static int gfs2_get_block_noalloc(struct inode *inode, sector_t lblock,
return -EIO;
return 0;
}
+/**
+ * get_jdata_block_noalloc - jdata version of gfs2_get_block_noalloc
+ * @inode: The inode
+ * @lblock: The block number to look up
+ * @bh_result: The buffer head to return the result in
+ * @create: Non-zero if we may add block to the file
+ *
+ * The reason we have a separate function for jdata is that jdata go through
+ * the journal, and we don't want jdata holes (left by punch_hole) to appear
+ * as IO errors. We can safely ignore them.
+ *
+ * Returns: errno
+ */
+static int get_jdata_block_noalloc(struct inode *inode, sector_t lblock,
+ struct buffer_head *bh_result,
+ int create)
+{
+ int error;
+
+ error = gfs2_block_map(inode, lblock, bh_result, 0);
+ if (error)
+ return error;
+ if (!buffer_mapped(bh_result))
+ return -ENODATA;
+ return 0;
+}
/**
* gfs2_writepage - Write page for writeback mappings
@@ -133,8 +159,8 @@ static int gfs2_write_jdata_page(struct page *page,
if (page->index == end_index && offset)
zero_user_segment(page, offset, PAGE_SIZE);
- return __block_write_full_page(inode, page, gfs2_get_block_noalloc, wbc,
- end_buffer_async_write);
+ return __block_write_full_page(inode, page, get_jdata_block_noalloc,
+ wbc, end_buffer_async_write);
}
/**
diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index 9133b3178677..2e9314091c81 100644
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -132,6 +132,8 @@ __acquires(&sdp->sd_ail_lock)
spin_unlock(&sdp->sd_ail_lock);
ret = generic_writepages(mapping, wbc);
spin_lock(&sdp->sd_ail_lock);
+ if (ret == -ENODATA) /* if a jdata write into a new hole */
+ ret = 0; /* ignore it */
if (ret || wbc->nr_to_write <= 0)
break;
return -EBUSY;
--
2.26.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Cluster-devel] [GFS2 PATCH 0/2] Fix regression in "Ignore journal log writes for jdata holes"
2020-11-12 14:57 [Cluster-devel] [GFS2 PATCH 0/2] Fix regression in "Ignore journal log writes for jdata holes" Bob Peterson
@ 2020-11-12 15:44 ` Bob Peterson
0 siblings, 0 replies; 4+ messages in thread
From: Bob Peterson @ 2020-11-12 15:44 UTC (permalink / raw)
To: cluster-devel.redhat.com
----- Original Message -----
> Patch b2a846dbef4e ("gfs2: Ignore journal log writes for jdata holes"
> caused a regression. It fixed one specific problem while breaking others.
> The problem was that it changed the behavior of function gfs2_block_map
> which is used by multiple callers so it had unintended consequences for
> other callers.
>
> This patch set reverts the patch and replaces it with a more targeted
> solution that fixes just the one case it needs to.
>
> Bob Peterson (2):
> Revert "gfs2: Ignore journal log writes for jdata holes"
> gfs2: Make special version of gfs2_get_block_noalloc for jdata
>
> fs/gfs2/aops.c | 30 ++++++++++++++++++++++++++++--
> fs/gfs2/bmap.c | 8 ++------
> fs/gfs2/log.c | 2 ++
> 3 files changed, 32 insertions(+), 8 deletions(-)
>
> --
> 2.26.2
NACK. Ignore this for now. Something's wrong and I'm investigating.
Regards,
Bob Peterson
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Cluster-devel] [GFS2 PATCH 0/2] Fix regression in "Ignore journal log writes for jdata holes"
@ 2020-11-12 14:57 Bob Peterson
2020-11-12 15:44 ` Bob Peterson
0 siblings, 1 reply; 4+ messages in thread
From: Bob Peterson @ 2020-11-12 14:57 UTC (permalink / raw)
To: cluster-devel.redhat.com
Patch b2a846dbef4e ("gfs2: Ignore journal log writes for jdata holes"
caused a regression. It fixed one specific problem while breaking others.
The problem was that it changed the behavior of function gfs2_block_map
which is used by multiple callers so it had unintended consequences for
other callers.
This patch set reverts the patch and replaces it with a more targeted
solution that fixes just the one case it needs to.
Bob Peterson (2):
Revert "gfs2: Ignore journal log writes for jdata holes"
gfs2: Make special version of gfs2_get_block_noalloc for jdata
fs/gfs2/aops.c | 30 ++++++++++++++++++++++++++++--
fs/gfs2/bmap.c | 8 ++------
fs/gfs2/log.c | 2 ++
3 files changed, 32 insertions(+), 8 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-11-12 15:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-12 14:54 [Cluster-devel] [GFS2 PATCH 0/2] Fix regression in "Ignore journal log writes for jdata holes" Bob Peterson
2020-11-12 14:54 ` [Cluster-devel] [GFS2 PATCH 1/1] gfs2: Make special version of gfs2_get_block_noalloc for jdata Bob Peterson
2020-11-12 14:57 [Cluster-devel] [GFS2 PATCH 0/2] Fix regression in "Ignore journal log writes for jdata holes" Bob Peterson
2020-11-12 15:44 ` Bob Peterson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.