2013-10-15 13:32:36

by Jan Kara

[permalink] [raw]
Subject: [PATCH 1/2] ext4: Fix assertion in ext4_add_complete_io()

It doesn't make sense to require io_end->handle when we are in nojournal
mode. So update the assertion accordingly to avoid false warnings from
ext4_add_complete_io().

Reported-by: Eric Whitney <[email protected]>
Signed-off-by: Jan Kara <[email protected]>
---
fs/ext4/page-io.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
index d7d0c7b46ed4..d488f80ee32d 100644
--- a/fs/ext4/page-io.c
+++ b/fs/ext4/page-io.c
@@ -197,14 +197,15 @@ static void dump_completed_IO(struct inode *inode, struct list_head *head)
static void ext4_add_complete_io(ext4_io_end_t *io_end)
{
struct ext4_inode_info *ei = EXT4_I(io_end->inode);
+ struct ext4_sb_info *sbi = EXT4_SB(io_end->inode->i_sb);
struct workqueue_struct *wq;
unsigned long flags;

/* Only reserved conversions from writeback should enter here */
WARN_ON(!(io_end->flag & EXT4_IO_END_UNWRITTEN));
- WARN_ON(!io_end->handle);
+ WARN_ON(!io_end->handle && sbi->s_journal);
spin_lock_irqsave(&ei->i_completed_io_lock, flags);
- wq = EXT4_SB(io_end->inode->i_sb)->rsv_conversion_wq;
+ wq = sbi->rsv_conversion_wq;
if (list_empty(&ei->i_rsv_conversion_list))
queue_work(wq, &ei->i_rsv_conversion_work);
list_add_tail(&io_end->list, &ei->i_rsv_conversion_list);
--
1.8.1.4



2013-10-15 13:32:41

by Jan Kara

[permalink] [raw]
Subject: [PATCH 2/2] ext4: Fixup kerndoc annotation of mpage_map_and_submit_extent()

Document give_up_on_write argument of mpage_map_and_submit_extent().

Signed-off-by: Jan Kara <[email protected]>
---
fs/ext4/inode.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index e274e9c1171f..e7e5b3d8f002 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2178,6 +2178,9 @@ static int mpage_map_one_extent(handle_t *handle, struct mpage_da_data *mpd)
*
* @handle - handle for journal operations
* @mpd - extent to map
+ * @give_up_on_write - we set this to true iff there is a fatal error and there
+ * is no hope of writing the data. The caller should discard
+ * dirty pages to avoid infinite loops.
*
* The function maps extent starting at mpd->lblk of length mpd->len. If it is
* delayed, blocks are allocated, if it is unwritten, we may need to convert
--
1.8.1.4


2013-10-15 15:38:00

by Eric Whitney

[permalink] [raw]
Subject: Re: [PATCH 1/2] ext4: Fix assertion in ext4_add_complete_io()

* Jan Kara <[email protected]>:
> It doesn't make sense to require io_end->handle when we are in nojournal
> mode. So update the assertion accordingly to avoid false warnings from
> ext4_add_complete_io().
>
> Reported-by: Eric Whitney <[email protected]>
> Signed-off-by: Jan Kara <[email protected]>
> ---
> fs/ext4/page-io.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
> index d7d0c7b46ed4..d488f80ee32d 100644
> --- a/fs/ext4/page-io.c
> +++ b/fs/ext4/page-io.c
> @@ -197,14 +197,15 @@ static void dump_completed_IO(struct inode *inode, struct list_head *head)
> static void ext4_add_complete_io(ext4_io_end_t *io_end)
> {
> struct ext4_inode_info *ei = EXT4_I(io_end->inode);
> + struct ext4_sb_info *sbi = EXT4_SB(io_end->inode->i_sb);
> struct workqueue_struct *wq;
> unsigned long flags;
>
> /* Only reserved conversions from writeback should enter here */
> WARN_ON(!(io_end->flag & EXT4_IO_END_UNWRITTEN));
> - WARN_ON(!io_end->handle);
> + WARN_ON(!io_end->handle && sbi->s_journal);
> spin_lock_irqsave(&ei->i_completed_io_lock, flags);
> - wq = EXT4_SB(io_end->inode->i_sb)->rsv_conversion_wq;
> + wq = sbi->rsv_conversion_wq;
> if (list_empty(&ei->i_rsv_conversion_list))
> queue_work(wq, &ei->i_rsv_conversion_work);
> list_add_tail(&io_end->list, &ei->i_rsv_conversion_list);
> --
> 1.8.1.4
>
> --

I've successfully tested this patch against 3.12-rc5 - it silences the
warning triggered by ext4/271 in xfstests-bld's dioread_nolock test config.

Thanks very much!
Eric


2013-10-15 17:20:18

by Lukas Czerner

[permalink] [raw]
Subject: Re: [PATCH 1/2] ext4: Fix assertion in ext4_add_complete_io()

On Tue, 15 Oct 2013, Jan Kara wrote:

> Date: Tue, 15 Oct 2013 15:32:28 +0200
> From: Jan Kara <[email protected]>
> To: Ted Tso <[email protected]>
> Cc: [email protected], Jan Kara <[email protected]>
> Subject: [PATCH 1/2] ext4: Fix assertion in ext4_add_complete_io()
>
> It doesn't make sense to require io_end->handle when we are in nojournal
> mode. So update the assertion accordingly to avoid false warnings from
> ext4_add_complete_io().

Looks good.

Reviewed-by: Lukas Czerner <[email protected]>

>
> Reported-by: Eric Whitney <[email protected]>
> Signed-off-by: Jan Kara <[email protected]>
> ---
> fs/ext4/page-io.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
> index d7d0c7b46ed4..d488f80ee32d 100644
> --- a/fs/ext4/page-io.c
> +++ b/fs/ext4/page-io.c
> @@ -197,14 +197,15 @@ static void dump_completed_IO(struct inode *inode, struct list_head *head)
> static void ext4_add_complete_io(ext4_io_end_t *io_end)
> {
> struct ext4_inode_info *ei = EXT4_I(io_end->inode);
> + struct ext4_sb_info *sbi = EXT4_SB(io_end->inode->i_sb);
> struct workqueue_struct *wq;
> unsigned long flags;
>
> /* Only reserved conversions from writeback should enter here */
> WARN_ON(!(io_end->flag & EXT4_IO_END_UNWRITTEN));
> - WARN_ON(!io_end->handle);
> + WARN_ON(!io_end->handle && sbi->s_journal);
> spin_lock_irqsave(&ei->i_completed_io_lock, flags);
> - wq = EXT4_SB(io_end->inode->i_sb)->rsv_conversion_wq;
> + wq = sbi->rsv_conversion_wq;
> if (list_empty(&ei->i_rsv_conversion_list))
> queue_work(wq, &ei->i_rsv_conversion_work);
> list_add_tail(&io_end->list, &ei->i_rsv_conversion_list);
>

2013-10-15 19:54:19

by jon ernst

[permalink] [raw]
Subject: Re: [PATCH 1/2] ext4: Fix assertion in ext4_add_complete_io()

On Tue, Oct 15, 2013 at 1:20 PM, Lukáš Czerner <[email protected]> wrote:
> On Tue, 15 Oct 2013, Jan Kara wrote:
>
>> Date: Tue, 15 Oct 2013 15:32:28 +0200
>> From: Jan Kara <[email protected]>
>> To: Ted Tso <[email protected]>
>> Cc: [email protected], Jan Kara <[email protected]>
>> Subject: [PATCH 1/2] ext4: Fix assertion in ext4_add_complete_io()
>>
>> It doesn't make sense to require io_end->handle when we are in nojournal
>> mode. So update the assertion accordingly to avoid false warnings from
>> ext4_add_complete_io().
>
> Looks good.
>
> Reviewed-by: Lukas Czerner <[email protected]>
>
>>
>> Reported-by: Eric Whitney <[email protected]>
>> Signed-off-by: Jan Kara <[email protected]>
>> ---
>> fs/ext4/page-io.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
>> index d7d0c7b46ed4..d488f80ee32d 100644
>> --- a/fs/ext4/page-io.c
>> +++ b/fs/ext4/page-io.c
>> @@ -197,14 +197,15 @@ static void dump_completed_IO(struct inode *inode, struct list_head *head)
>> static void ext4_add_complete_io(ext4_io_end_t *io_end)
>> {
>> struct ext4_inode_info *ei = EXT4_I(io_end->inode);
>> + struct ext4_sb_info *sbi = EXT4_SB(io_end->inode->i_sb);
>> struct workqueue_struct *wq;
>> unsigned long flags;
>>
>> /* Only reserved conversions from writeback should enter here */
>> WARN_ON(!(io_end->flag & EXT4_IO_END_UNWRITTEN));
>> - WARN_ON(!io_end->handle);
>> + WARN_ON(!io_end->handle && sbi->s_journal);

swap 2 conditions would be better in my opinion. (for no-journal
case, it will be short-circuited. For journal case, no harm, no
difference.)
i know this is too picky. :)

>> spin_lock_irqsave(&ei->i_completed_io_lock, flags);
>> - wq = EXT4_SB(io_end->inode->i_sb)->rsv_conversion_wq;
>> + wq = sbi->rsv_conversion_wq;
>> if (list_empty(&ei->i_rsv_conversion_list))
>> queue_work(wq, &ei->i_rsv_conversion_work);
>> list_add_tail(&io_end->list, &ei->i_rsv_conversion_list);
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2013-10-16 12:01:41

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH 1/2] ext4: Fix assertion in ext4_add_complete_io()

On Tue 15-10-13 15:54:18, jon ernst wrote:
> On Tue, Oct 15, 2013 at 1:20 PM, Lukáš Czerner <[email protected]> wrote:
> > On Tue, 15 Oct 2013, Jan Kara wrote:
> >
> >> Date: Tue, 15 Oct 2013 15:32:28 +0200
> >> From: Jan Kara <[email protected]>
> >> To: Ted Tso <[email protected]>
> >> Cc: [email protected], Jan Kara <[email protected]>
> >> Subject: [PATCH 1/2] ext4: Fix assertion in ext4_add_complete_io()
> >>
> >> It doesn't make sense to require io_end->handle when we are in nojournal
> >> mode. So update the assertion accordingly to avoid false warnings from
> >> ext4_add_complete_io().
> >
> > Looks good.
> >
> > Reviewed-by: Lukas Czerner <[email protected]>
> >
> >>
> >> Reported-by: Eric Whitney <[email protected]>
> >> Signed-off-by: Jan Kara <[email protected]>
> >> ---
> >> fs/ext4/page-io.c | 5 +++--
> >> 1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
> >> index d7d0c7b46ed4..d488f80ee32d 100644
> >> --- a/fs/ext4/page-io.c
> >> +++ b/fs/ext4/page-io.c
> >> @@ -197,14 +197,15 @@ static void dump_completed_IO(struct inode *inode, struct list_head *head)
> >> static void ext4_add_complete_io(ext4_io_end_t *io_end)
> >> {
> >> struct ext4_inode_info *ei = EXT4_I(io_end->inode);
> >> + struct ext4_sb_info *sbi = EXT4_SB(io_end->inode->i_sb);
> >> struct workqueue_struct *wq;
> >> unsigned long flags;
> >>
> >> /* Only reserved conversions from writeback should enter here */
> >> WARN_ON(!(io_end->flag & EXT4_IO_END_UNWRITTEN));
> >> - WARN_ON(!io_end->handle);
> >> + WARN_ON(!io_end->handle && sbi->s_journal);
>
> swap 2 conditions would be better in my opinion. (for no-journal
> case, it will be short-circuited. For journal case, no harm, no
> difference.)
> i know this is too picky. :)
OTOH you will check sbi->s_journal unnecessarily for the more common
case when the journal is enabled. But I don't care tham much really.

Honza

--
Jan Kara <[email protected]>
SUSE Labs, CR

2013-10-17 03:31:49

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH 1/2] ext4: Fix assertion in ext4_add_complete_io()

On Tue, Oct 15, 2013 at 03:32:28PM +0200, Jan Kara wrote:
> It doesn't make sense to require io_end->handle when we are in nojournal
> mode. So update the assertion accordingly to avoid false warnings from
> ext4_add_complete_io().
>
> Reported-by: Eric Whitney <[email protected]>
> Signed-off-by: Jan Kara <[email protected]>

Thanks, applied.

- Ted

2013-10-17 03:32:00

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH 2/2] ext4: Fixup kerndoc annotation of mpage_map_and_submit_extent()

On Tue, Oct 15, 2013 at 03:32:29PM +0200, Jan Kara wrote:
> Document give_up_on_write argument of mpage_map_and_submit_extent().
>
> Signed-off-by: Jan Kara <[email protected]>

Thanks, applied.

- Ted