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

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

[Jan Kara]

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 <enwlinux@gmail.com>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 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

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

[Jan Kara]

Document give_up_on_write argument of mpage_map_and_submit_extent().

Signed-off-by: Jan Kara <jack@suse.cz>
---
 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

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

[Eric Whitney]

* Jan Kara <jack@suse.cz>:
> 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 <enwlinux@gmail.com>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  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

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

[Lukáš Czerner]

On Tue, 15 Oct 2013, Jan Kara wrote:

> Date: Tue, 15 Oct 2013 15:32:28 +0200
> From: Jan Kara <jack@suse.cz>
> To: Ted Tso <tytso@mit.edu>
> Cc: linux-ext4@vger.kernel.org, Jan Kara <jack@suse.cz>
> 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 <lczerner@redhat.com>

> 
> Reported-by: Eric Whitney <enwlinux@gmail.com>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  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);
> 

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

[jon ernst]

On Tue, Oct 15, 2013 at 1:20 PM, Lukáš Czerner <lczerner@redhat.com> wrote:
> On Tue, 15 Oct 2013, Jan Kara wrote:
>
>> Date: Tue, 15 Oct 2013 15:32:28 +0200
>> From: Jan Kara <jack@suse.cz>
>> To: Ted Tso <tytso@mit.edu>
>> Cc: linux-ext4@vger.kernel.org, Jan Kara <jack@suse.cz>
>> 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 <lczerner@redhat.com>
>
>>
>> Reported-by: Eric Whitney <enwlinux@gmail.com>
>> Signed-off-by: Jan Kara <jack@suse.cz>
>> ---
>>  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 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

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

[Jan Kara]

On Tue 15-10-13 15:54:18, jon ernst wrote:
> On Tue, Oct 15, 2013 at 1:20 PM, Lukáš Czerner <lczerner@redhat.com> wrote:
> > On Tue, 15 Oct 2013, Jan Kara wrote:
> >
> >> Date: Tue, 15 Oct 2013 15:32:28 +0200
> >> From: Jan Kara <jack@suse.cz>
> >> To: Ted Tso <tytso@mit.edu>
> >> Cc: linux-ext4@vger.kernel.org, Jan Kara <jack@suse.cz>
> >> 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 <lczerner@redhat.com>
> >
> >>
> >> Reported-by: Eric Whitney <enwlinux@gmail.com>
> >> Signed-off-by: Jan Kara <jack@suse.cz>
> >> ---
> >>  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 <jack@suse.cz>
SUSE Labs, CR
--
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

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

[Theodore Ts'o]

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 <enwlinux@gmail.com>
> Signed-off-by: Jan Kara <jack@suse.cz>

Thanks, applied.

					- Ted

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

[Theodore Ts'o]

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 <jack@suse.cz>

Thanks, applied.

					- Ted