We've changed the discard command handling into parallel manner.
But, in this change, I forgot decreasing the usage count of the bio
which was used to send discard request. I'm sorry about that.
Signed-off-by: Daeho Jeong <[email protected]>
Fixes: a015434480dc ("ext4: send parallel discards on commit
completions")
---
fs/ext4/mballoc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index ab70b69e644c..88317b0cf7b8 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2892,8 +2892,10 @@ void ext4_process_freed_data(struct super_block *sb, tid_t commit_tid)
break;
}
- if (discard_bio)
+ if (discard_bio) {
submit_bio_wait(discard_bio);
+ bio_put(discard_bio);
+ }
}
list_for_each_entry_safe(entry, tmp, &freed_data_list, efd_list)
--
2.13.0
On Tue 01-08-17 11:29:28, Daeho Jeong wrote:
> We've changed the discard command handling into parallel manner.
> But, in this change, I forgot decreasing the usage count of the bio
> which was used to send discard request. I'm sorry about that.
>
> Signed-off-by: Daeho Jeong <[email protected]>
> Fixes: a015434480dc ("ext4: send parallel discards on commit
> completions")
Why do you think this is needed? submit_bio_wait() consumes the reference
that you've got from __blkdev_issue_discard()...
Honza
> ---
> fs/ext4/mballoc.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index ab70b69e644c..88317b0cf7b8 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -2892,8 +2892,10 @@ void ext4_process_freed_data(struct super_block *sb, tid_t commit_tid)
> break;
> }
>
> - if (discard_bio)
> + if (discard_bio) {
> submit_bio_wait(discard_bio);
> + bio_put(discard_bio);
> + }
> }
>
> list_for_each_entry_safe(entry, tmp, &freed_data_list, efd_list)
> --
> 2.13.0
>
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
> > We've changed the discard command handling into parallel manner.
> > But, in this change, I forgot decreasing the usage count of the bio
> > which was used to send discard request. I'm sorry about that.
> >
> > Signed-off-by: Daeho Jeong <[email protected]>
> > Fixes: a015434480dc ("ext4: send parallel discards on commit
> > completions")
> Why do you think this is needed? submit_bio_wait() consumes the reference
> that you've got from __blkdev_issue_discard()...
>
> Honza
Hi Jan,
I thought like you, but submit_bio_wait() doesn't consume the reference
of the bio and the bio cannot be released after the I/O has been completed.
The caller of submit_bio_wait() should invoke bio_put() in person.
You can see what we have to do after calling submit_bio_wait() in
fs/crypto/bio.c.
Actually, in our device, I can see that the slab memory grows gradually
because of the unreleased discard bios.
On Tue 01-08-17 23:31:38, Daeho Jeong wrote:
>
> > >?We've?changed?the?discard?command?handling?into?parallel?manner.
> > >?But,?in?this?change,?I?forgot?decreasing?the?usage?count?of?the?bio
> > >?which?was?used?to?send?discard?request.?I'm?sorry?about?that.
> > >?
> > >?Signed-off-by:?Daeho?Jeong?<[email protected]>
> > >?Fixes:?a015434480dc?("ext4:?send?parallel?discards?on?commit
> > >?completions")
> ?
> > Why?do?you?think?this?is?needed??submit_bio_wait()?consumes?the?reference
> > that?you've?got?from?__blkdev_issue_discard()...
> > ?
> > ??????????????????????????????????????????????????????????????Honza
>
> Hi Jan,
>
> I thought like you, but submit_bio_wait() doesn't consume the reference
> of the bio and the bio cannot be released after the I/O has been completed.
> The caller of submit_bio_wait() should invoke bio_put() in person.
> You can see what we have to do after calling submit_bio_wait() in
> fs/crypto/bio.c.
>
> Actually, in our device, I can see that the slab memory grows gradually
> because of the unreleased discard bios.
Ah, good point. I had a deeper look now and indeed submit_bio_wait() uses
it's own end_io function which does not drop the bio reference. So I
retract my objection and feel free to add:
Reviewed-by: Jan Kara <[email protected]>
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR