* [PATCH 1/2] ext4: remove a wrong BUG_ON in ext4_ext_convert_to_initialized
@ 2011-12-07 5:21 Yongqiang Yang
2011-12-07 5:21 ` [PATCH 2/2] ext4: let ext4_bio_write_page handle EOF correctly Yongqiang Yang
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Yongqiang Yang @ 2011-12-07 5:21 UTC (permalink / raw)
To: linux-ext4; +Cc: achender, hughd, tytso, Yongqiang Yang
If a file is fallocated on a hole, map->m_lblk + map->m_len may be greater
than ee_block + ee_len.
Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
---
fs/ext4/extents.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 6f0300e..29bb629 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2943,7 +2943,6 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
/* Pre-conditions */
BUG_ON(!ext4_ext_is_uninitialized(ex));
BUG_ON(!in_range(map->m_lblk, ee_block, ee_len));
- BUG_ON(map->m_lblk + map->m_len > ee_block + ee_len);
/*
* Attempt to transfer newly initialized blocks from the currently
--
1.7.5.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] ext4: let ext4_bio_write_page handle EOF correctly 2011-12-07 5:21 [PATCH 1/2] ext4: remove a wrong BUG_ON in ext4_ext_convert_to_initialized Yongqiang Yang @ 2011-12-07 5:21 ` Yongqiang Yang 2011-12-14 3:33 ` Ted Ts'o 2011-12-09 22:41 ` [PATCH 1/2] ext4: remove a wrong BUG_ON in ext4_ext_convert_to_initialized Eric Gouriou 2011-12-14 3:33 ` Ted Ts'o 2 siblings, 1 reply; 6+ messages in thread From: Yongqiang Yang @ 2011-12-07 5:21 UTC (permalink / raw) To: linux-ext4; +Cc: achender, hughd, tytso, Yongqiang Yang We need to zero out part of a page which beyond EOF before setting uptodate, otherwise, mapread or write will see non-zero data beyond EOF. Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com> --- fs/ext4/page-io.c | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c index 235b79d..9e145b8 100644 --- a/fs/ext4/page-io.c +++ b/fs/ext4/page-io.c @@ -385,6 +385,18 @@ int ext4_bio_write_page(struct ext4_io_submit *io, block_end = block_start + blocksize; if (block_start >= len) { + /* + * Comments copied from block_write_full_page_endio: + * + * The page straddles i_size. It must be zeroed out on + * each and every writepage invocation because it may + * be mmapped. "A file is mapped in multiples of the + * page size. For a file that is not a multiple of + * the page size, the remaining memory is zeroed when + * mapped, and writes to that region are not written + * out to the file." + */ + zero_user_segment(page, block_start, block_end); clear_buffer_dirty(bh); set_buffer_uptodate(bh); continue; -- 1.7.5.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] ext4: let ext4_bio_write_page handle EOF correctly 2011-12-07 5:21 ` [PATCH 2/2] ext4: let ext4_bio_write_page handle EOF correctly Yongqiang Yang @ 2011-12-14 3:33 ` Ted Ts'o 0 siblings, 0 replies; 6+ messages in thread From: Ted Ts'o @ 2011-12-14 3:33 UTC (permalink / raw) To: Yongqiang Yang; +Cc: linux-ext4, achender, hughd On Wed, Dec 07, 2011 at 01:21:27PM +0800, Yongqiang Yang wrote: > We need to zero out part of a page which beyond EOF before setting uptodate, > otherwise, mapread or write will see non-zero data beyond EOF. > > Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com> Thanks, applied. - Ted ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] ext4: remove a wrong BUG_ON in ext4_ext_convert_to_initialized 2011-12-07 5:21 [PATCH 1/2] ext4: remove a wrong BUG_ON in ext4_ext_convert_to_initialized Yongqiang Yang 2011-12-07 5:21 ` [PATCH 2/2] ext4: let ext4_bio_write_page handle EOF correctly Yongqiang Yang @ 2011-12-09 22:41 ` Eric Gouriou [not found] ` <CAGBYx2boc0OtAOq7BNurQkCZG0Oeyf2LLd+F-9EvwL31CNccmw@mail.gmail.com> 2011-12-14 3:33 ` Ted Ts'o 2 siblings, 1 reply; 6+ messages in thread From: Eric Gouriou @ 2011-12-09 22:41 UTC (permalink / raw) To: Yongqiang Yang; +Cc: linux-ext4, achender, hughd, tytso On Tue, Dec 6, 2011 at 21:21, Yongqiang Yang <xiaoqiangnk@gmail.com> wrote: > If a file is fallocated on a hole, map->m_lblk + map->m_len may be greater > than ee_block + ee_len. Could you please detail a scenario that leads to this check being invalid? As I'm to blame for the faulty BUG_ON I'd like to use this as an opportunity to get properly edified. > Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com> > --- > fs/ext4/extents.c | 1 - > 1 files changed, 0 insertions(+), 1 deletions(-) > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index 6f0300e..29bb629 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -2943,7 +2943,6 @@ static int ext4_ext_convert_to_initialized(handle_t *handle, > /* Pre-conditions */ > BUG_ON(!ext4_ext_is_uninitialized(ex)); > BUG_ON(!in_range(map->m_lblk, ee_block, ee_len)); > - BUG_ON(map->m_lblk + map->m_len > ee_block + ee_len); For a bit I thought this would break the fast path logic, however it gets protected by the checks marked /*L1*/ and /*L2*/ since m_lblk == ee_block (L1) and m_len < ee_len (L2). Regards - Eric > > /* > * Attempt to transfer newly initialized blocks from the currently > -- > 1.7.5.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <CAGBYx2boc0OtAOq7BNurQkCZG0Oeyf2LLd+F-9EvwL31CNccmw@mail.gmail.com>]
* Re: [PATCH 1/2] ext4: remove a wrong BUG_ON in ext4_ext_convert_to_initialized [not found] ` <CAGBYx2boc0OtAOq7BNurQkCZG0Oeyf2LLd+F-9EvwL31CNccmw@mail.gmail.com> @ 2011-12-13 0:01 ` Eric Gouriou 0 siblings, 0 replies; 6+ messages in thread From: Eric Gouriou @ 2011-12-13 0:01 UTC (permalink / raw) To: Yongqiang Yang Cc: linux-ext4@vger.kernel.org, achender@linux.vnet.ibm.com, hughd@google.com, tytso@mit.edu [Yet another resend, without HTML and _with_ the list CC'ed. Apologies for the spam] On Fri, Dec 9, 2011 at 19:36, Yongqiang Yang <xiaoqiangnk@gmail.com> wrote: > > > On Saturday, December 10, 2011, Eric Gouriou <egouriou@google.com> wrote: >> On Tue, Dec 6, 2011 at 21:21, Yongqiang Yang <xiaoqiangnk@gmail.com> >> wrote: >>> If a file is fallocated on a hole, map->m_lblk + map->m_len may be >>> greater >>> than ee_block + ee_len. >> >> Could you please detail a scenario that leads to this check being invalid? >> As I'm to blame for the faulty BUG_ON I'd like to use this as an >> opportunity >> to get properly edified. > It's easy to reproduce, I think. Just need to write beyond fallocated > blocks, write back would submit a request to ext4_map_blocks with blocks > part of which are fallocated while other part of which are not, then the > bug-on would happen. Thanks, this makes sense. > > I met the bug_on during fsx and after the patch applied it works normal. > > I think Allison also tested with this patch. > > Yongqiang.. > >> >>> Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com> Reviewed-by: Eric Gouriou <egouriou@google.com> Thanks - Eric >>> --- >>> fs/ext4/extents.c | 1 - >>> 1 files changed, 0 insertions(+), 1 deletions(-) >>> >>> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c >>> index 6f0300e..29bb629 100644 >>> --- a/fs/ext4/extents.c >>> +++ b/fs/ext4/extents.c >>> @@ -2943,7 +2943,6 @@ static int ext4_ext_convert_to_initialized(handle_t >>> *handle, >>> /* Pre-conditions */ >>> BUG_ON(!ext4_ext_is_uninitialized(ex)); >>> BUG_ON(!in_range(map->m_lblk, ee_block, ee_len)); >>> - BUG_ON(map->m_lblk + map->m_len > ee_block + ee_len); >> >> For a bit I thought this would break the fast path logic, however it gets >> protected by the checks marked /*L1*/ and /*L2*/ since m_lblk == ee_block >> (L1) and m_len < ee_len (L2). >> >> Regards - Eric >> >>> >>> /* >>> * Attempt to transfer newly initialized blocks from the currently >>> -- >>> 1.7.5.1 >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > > -- > Best Wishes > Yongqiang Yang > -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] ext4: remove a wrong BUG_ON in ext4_ext_convert_to_initialized 2011-12-07 5:21 [PATCH 1/2] ext4: remove a wrong BUG_ON in ext4_ext_convert_to_initialized Yongqiang Yang 2011-12-07 5:21 ` [PATCH 2/2] ext4: let ext4_bio_write_page handle EOF correctly Yongqiang Yang 2011-12-09 22:41 ` [PATCH 1/2] ext4: remove a wrong BUG_ON in ext4_ext_convert_to_initialized Eric Gouriou @ 2011-12-14 3:33 ` Ted Ts'o 2 siblings, 0 replies; 6+ messages in thread From: Ted Ts'o @ 2011-12-14 3:33 UTC (permalink / raw) To: Yongqiang Yang; +Cc: linux-ext4, achender, hughd On Wed, Dec 07, 2011 at 01:21:26PM +0800, Yongqiang Yang wrote: > If a file is fallocated on a hole, map->m_lblk + map->m_len may be greater > than ee_block + ee_len. > > Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com> Thanks, applied. - Ted ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-12-14 3:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-07 5:21 [PATCH 1/2] ext4: remove a wrong BUG_ON in ext4_ext_convert_to_initialized Yongqiang Yang
2011-12-07 5:21 ` [PATCH 2/2] ext4: let ext4_bio_write_page handle EOF correctly Yongqiang Yang
2011-12-14 3:33 ` Ted Ts'o
2011-12-09 22:41 ` [PATCH 1/2] ext4: remove a wrong BUG_ON in ext4_ext_convert_to_initialized Eric Gouriou
[not found] ` <CAGBYx2boc0OtAOq7BNurQkCZG0Oeyf2LLd+F-9EvwL31CNccmw@mail.gmail.com>
2011-12-13 0:01 ` Eric Gouriou
2011-12-14 3:33 ` Ted Ts'o
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).