* [PATCH] Btrfs: don't delete ordered roots from list during cleanup
@ 2013-09-27 20:37 Josef Bacik
2013-09-27 23:18 ` Zach Brown
0 siblings, 1 reply; 4+ messages in thread
From: Josef Bacik @ 2013-09-27 20:37 UTC (permalink / raw
To: linux-btrfs
During transaction cleanup after an abort we are just removing roots from the
ordered roots list which is incorrect. We have a BUG_ON() to make sure that the
root is still part of the ordered roots list when we put our ordered extent
which we were tripping in this case. So do like we do everywhere else and just
move it to the tail of the ordered roots list and allow the normal cleanup to
take care of stuff. Thanks,
Signed-off-by: Josef Bacik <jbacik@fusionio.com>
---
fs/btrfs/disk-io.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index f38211f..872b4ce 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3835,7 +3835,8 @@ static void btrfs_destroy_all_ordered_extents(struct btrfs_fs_info *fs_info)
while (!list_empty(&splice)) {
root = list_first_entry(&splice, struct btrfs_root,
ordered_root);
- list_del_init(&root->ordered_root);
+ list_move_tail(&root->ordered_root,
+ &fs_info->ordered_roots);
btrfs_destroy_ordered_extents(root);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Btrfs: don't delete ordered roots from list during cleanup
2013-09-27 20:37 [PATCH] Btrfs: don't delete ordered roots from list during cleanup Josef Bacik
@ 2013-09-27 23:18 ` Zach Brown
2013-09-30 15:36 ` Josef Bacik
0 siblings, 1 reply; 4+ messages in thread
From: Zach Brown @ 2013-09-27 23:18 UTC (permalink / raw
To: Josef Bacik; +Cc: linux-btrfs
On Fri, Sep 27, 2013 at 04:37:46PM -0400, Josef Bacik wrote:
> During transaction cleanup after an abort we are just removing roots from the
> ordered roots list which is incorrect. We have a BUG_ON() to make sure that the
> root is still part of the ordered roots list when we put our ordered extent
> which we were tripping in this case. So do like we do everywhere else and just
> move it to the tail of the ordered roots list and allow the normal cleanup to
> take care of stuff. Thanks,
>
> Signed-off-by: Josef Bacik <jbacik@fusionio.com>
> ---
> fs/btrfs/disk-io.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index f38211f..872b4ce 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3835,7 +3835,8 @@ static void btrfs_destroy_all_ordered_extents(struct btrfs_fs_info *fs_info)
> while (!list_empty(&splice)) {
> root = list_first_entry(&splice, struct btrfs_root,
> ordered_root);
> - list_del_init(&root->ordered_root);
> + list_move_tail(&root->ordered_root,
> + &fs_info->ordered_roots);
This function basically only does:
lock
list_for_each
lock
list_for_each
set_bit
Could we instead add a bit to the root or trans or fs_info or anything
else that could be trivialy set in _destroy_all_ordered_extents and
tested in _finish_ordered_io()? It'd remove a bunch of tedious locking
and iteration here.
The similar metaphor in the core page cache is (address_space->flags |
AS_EIO).
Would that be too coarse or racey?
- z
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Btrfs: don't delete ordered roots from list during cleanup
2013-09-27 23:18 ` Zach Brown
@ 2013-09-30 15:36 ` Josef Bacik
2013-09-30 17:34 ` Zach Brown
0 siblings, 1 reply; 4+ messages in thread
From: Josef Bacik @ 2013-09-30 15:36 UTC (permalink / raw
To: Zach Brown; +Cc: Josef Bacik, linux-btrfs
On Fri, Sep 27, 2013 at 04:18:14PM -0700, Zach Brown wrote:
> On Fri, Sep 27, 2013 at 04:37:46PM -0400, Josef Bacik wrote:
> > During transaction cleanup after an abort we are just removing roots from the
> > ordered roots list which is incorrect. We have a BUG_ON() to make sure that the
> > root is still part of the ordered roots list when we put our ordered extent
> > which we were tripping in this case. So do like we do everywhere else and just
> > move it to the tail of the ordered roots list and allow the normal cleanup to
> > take care of stuff. Thanks,
> >
> > Signed-off-by: Josef Bacik <jbacik@fusionio.com>
> > ---
> > fs/btrfs/disk-io.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> > index f38211f..872b4ce 100644
> > --- a/fs/btrfs/disk-io.c
> > +++ b/fs/btrfs/disk-io.c
> > @@ -3835,7 +3835,8 @@ static void btrfs_destroy_all_ordered_extents(struct btrfs_fs_info *fs_info)
> > while (!list_empty(&splice)) {
> > root = list_first_entry(&splice, struct btrfs_root,
> > ordered_root);
> > - list_del_init(&root->ordered_root);
> > + list_move_tail(&root->ordered_root,
> > + &fs_info->ordered_roots);
>
> This function basically only does:
>
> lock
> list_for_each
> lock
> list_for_each
> set_bit
>
> Could we instead add a bit to the root or trans or fs_info or anything
> else that could be trivialy set in _destroy_all_ordered_extents and
> tested in _finish_ordered_io()? It'd remove a bunch of tedious locking
> and iteration here.
>
> The similar metaphor in the core page cache is (address_space->flags |
> AS_EIO).
>
> Would that be too coarse or racey?
So I _think_ we may need to truncate the ordered range in the inode as well, but
I haven't had a consistent reproducer for this case. I want to leave it like
this for now until I'm sure we don't need the truncate and then we could
probably just replace this with a test for FS_ERROR in finish_ordered_io.
Thanks,
Josef
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Btrfs: don't delete ordered roots from list during cleanup
2013-09-30 15:36 ` Josef Bacik
@ 2013-09-30 17:34 ` Zach Brown
0 siblings, 0 replies; 4+ messages in thread
From: Zach Brown @ 2013-09-30 17:34 UTC (permalink / raw
To: Josef Bacik; +Cc: linux-btrfs
> So I _think_ we may need to truncate the ordered range in the inode as well, but
> I haven't had a consistent reproducer for this case. I want to leave it like
> this for now until I'm sure we don't need the truncate and then we could
> probably just replace this with a test for FS_ERROR in finish_ordered_io.
*nod*, added to the list :)
- z
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-09-30 17:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-27 20:37 [PATCH] Btrfs: don't delete ordered roots from list during cleanup Josef Bacik
2013-09-27 23:18 ` Zach Brown
2013-09-30 15:36 ` Josef Bacik
2013-09-30 17:34 ` Zach Brown
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.