linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Btrfs: fix fsync race leading to ordered extent memory leaks
@ 2015-02-06 12:34 Filipe Manana
  2015-02-06 22:09 ` [PATCH v2] " Filipe Manana
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Filipe Manana @ 2015-02-06 12:34 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Filipe Manana

We can have two concurrent fsync operations against the same file
that target different ranges of the file, in which case both fsyncs
will process ordered extents that overlap both ranges. This allows
for a time window where a race can lead to those ordered extents
never getting their reference count decremented to 0, leading to
memory leaks, getting referenced by multiple lists and resulting in
inode leaks too (an iput for the ordered extent's inode is scheduled
only when the ordered extent's refcount drops to 0).
The following sequence diagram explains this race:

         CPU 1                                        CPU 2

btrfs_sync_file() range [0, 8K]              btrfs_sync_file() range [8K, 16K]

  mutex_lock(inode->i_mutex)
  btrfs_log_inode()
    btrfs_get_logged_extents()
      --> collected ordered extent X that
          covers range [4K, 12K]
      --> incremented ordered extent X's
          refcount
    btrfs_submit_logged_extents()
  mutex_unlock(inode->i_mutex)

                                               mutex_lock(inode->i_mutex)
                                               btrfs_log_inode()
                                                  btrfs_get_logged_extents()
                                                    --> collected ordered extent X
                                                        as well
                                                    --> incremented ordered extent
                                                        X's refcount
                                                    --> ordered extent X is now
                                                        referenced by 2 different
                                                        lists, due to use of
                                                        list_add() and not list_move()
                                                  btrfs_submit_logged_extents()
                                               mutex_unlock(inode->i_mutex)

  btrfs_sync_log()
     btrfs_wait_logged_extents()
       --> set flag BTRFS_ORDERED_LOGGED
           on ordered extent X and adds it
           to trans->ordered
  btrfs_sync_log() finishes

                                               btrfs_sync_log()
                                                 btrfs_wait_logged_extents()
                                                   --> Sees flag BTRFS_ORDERED_LOGGED set
                                                       in ordered extent and therefore
                                                       won't do anything with it
                                                   --> The increment on ordered extent
                                                       X's refcount done before by the
                                                       task at CPU 2 will never have a
                                                       matching decrement

Fix this by using the flag BTRFS_ORDERED_LOGGED differently. Use it to
mean that an ordered extent is already being processed by an fsync call,
which prevents it from ever being collected by multiple concurrent
fsync operations against the same inode.

This race was introduced with the following change (added in 3.19 and
backported to stable 3.18 and 3.17):

  Btrfs: make sure logged extents complete in the current transaction V3
  commit 50d9aa99bd35c77200e0e3dd7a72274f8304701f

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/ordered-data.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index 534544e..87c8976 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -454,7 +454,7 @@ void btrfs_get_logged_extents(struct inode *inode,
 			break;
 		if (!list_empty(&ordered->log_list))
 			continue;
-		if (test_bit(BTRFS_ORDERED_LOGGED, &ordered->flags))
+		if (test_and_set_bit(BTRFS_ORDERED_LOGGED, &ordered->flags))
 			continue;
 		list_add(&ordered->log_list, logged_list);
 		atomic_inc(&ordered->refs);
@@ -470,6 +470,7 @@ void btrfs_put_logged_extents(struct list_head *logged_list)
 		ordered = list_first_entry(logged_list,
 					   struct btrfs_ordered_extent,
 					   log_list);
+		clear_bit(BTRFS_ORDERED_LOGGED, &ordered->flags);
 		list_del_init(&ordered->log_list);
 		btrfs_put_ordered_extent(ordered);
 	}
@@ -511,8 +512,7 @@ void btrfs_wait_logged_extents(struct btrfs_trans_handle *trans,
 		wait_event(ordered->wait, test_bit(BTRFS_ORDERED_IO_DONE,
 						   &ordered->flags));
 
-		if (!test_and_set_bit(BTRFS_ORDERED_LOGGED, &ordered->flags))
-			list_add_tail(&ordered->trans_list, &trans->ordered);
+		list_add_tail(&ordered->trans_list, &trans->ordered);
 		spin_lock_irq(&log->log_extents_lock[index]);
 	}
 	spin_unlock_irq(&log->log_extents_lock[index]);
-- 
2.1.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2] Btrfs: fix fsync race leading to ordered extent memory leaks
  2015-02-06 12:34 [PATCH] Btrfs: fix fsync race leading to ordered extent memory leaks Filipe Manana
@ 2015-02-06 22:09 ` Filipe Manana
  2015-02-07 18:57 ` [PATCH v3] " Filipe Manana
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Filipe Manana @ 2015-02-06 22:09 UTC (permalink / raw)
  To: linux-btrfs; +Cc: stable, Filipe Manana

We can have two concurrent fsync operations against the same file,
in which case both fsyncs can collect and process the same ordered
This allows for a time window where a race can lead to those ordered
extents never getting their reference count decremented to 0, leading
to memory leaks, getting referenced by multiple lists and resulting
in inode leaks too (an iput for the ordered extent's inode is scheduled
only when the ordered extent's refcount drops to 0). The following
sequence diagram explains this race:

         CPU 1                                        CPU 2

btrfs_sync_file()                              btrfs_sync_file()

  mutex_lock(inode->i_mutex)
  btrfs_log_inode()
    btrfs_get_logged_extents()
      --> collected ordered extent X
      --> incremented ordered extent X's
          refcount
    btrfs_submit_logged_extents()
  mutex_unlock(inode->i_mutex)

  btrfs_sync_log()
    --> blocks on
        mutex_lock(root->log_mutex)

                                               mutex_lock(inode->i_mutex)
                                               btrfs_log_inode()
                                                  btrfs_get_logged_extents()
                                                    --> collected ordered extent X
                                                        as well
                                                    --> incremented ordered extent
                                                        X's refcount
                                                    --> ordered extent X is now
                                                        referenced by 2 different
                                                        lists, due to use of
                                                        list_add() and not list_move()
                                                  btrfs_submit_logged_extents()
                                               mutex_unlock(inode->i_mutex)

   --> mutex root->log_mutex became
       free and task unblocked

     btrfs_wait_logged_extents()
       --> set flag BTRFS_ORDERED_LOGGED
           on ordered extent X and adds it
           to trans->ordered
  btrfs_sync_log() finishes

                                               btrfs_sync_log()
                                                 btrfs_wait_logged_extents()
                                                   --> Sees flag BTRFS_ORDERED_LOGGED set
                                                       in ordered extent and therefore
                                                       won't do anything with it
                                                   --> The increment on ordered extent
                                                       X's refcount done before by the
                                                       task at CPU 2 will never have a
                                                       matching decrement

Fix this by using the flag BTRFS_ORDERED_LOGGED differently. Use it to
mean that an ordered extent is already being processed by an fsync call,
which prevents it from ever being collected by multiple concurrent
fsync operations against the same inode.

This race was introduced with the following change (added in 3.19 and
backported to stable 3.18 and 3.17):

  Btrfs: make sure logged extents complete in the current transaction V3
  commit 50d9aa99bd35c77200e0e3dd7a72274f8304701f

CC: <stable@vger.kernel.org> # 3.19, 3.18 and 3.17
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---

V2: No code changes, re-worded commit message and diagram.
    CC'ed stable too.

 fs/btrfs/ordered-data.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index 534544e..87c8976 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -454,7 +454,7 @@ void btrfs_get_logged_extents(struct inode *inode,
 			break;
 		if (!list_empty(&ordered->log_list))
 			continue;
-		if (test_bit(BTRFS_ORDERED_LOGGED, &ordered->flags))
+		if (test_and_set_bit(BTRFS_ORDERED_LOGGED, &ordered->flags))
 			continue;
 		list_add(&ordered->log_list, logged_list);
 		atomic_inc(&ordered->refs);
@@ -470,6 +470,7 @@ void btrfs_put_logged_extents(struct list_head *logged_list)
 		ordered = list_first_entry(logged_list,
 					   struct btrfs_ordered_extent,
 					   log_list);
+		clear_bit(BTRFS_ORDERED_LOGGED, &ordered->flags);
 		list_del_init(&ordered->log_list);
 		btrfs_put_ordered_extent(ordered);
 	}
@@ -511,8 +512,7 @@ void btrfs_wait_logged_extents(struct btrfs_trans_handle *trans,
 		wait_event(ordered->wait, test_bit(BTRFS_ORDERED_IO_DONE,
 						   &ordered->flags));
 
-		if (!test_and_set_bit(BTRFS_ORDERED_LOGGED, &ordered->flags))
-			list_add_tail(&ordered->trans_list, &trans->ordered);
+		list_add_tail(&ordered->trans_list, &trans->ordered);
 		spin_lock_irq(&log->log_extents_lock[index]);
 	}
 	spin_unlock_irq(&log->log_extents_lock[index]);
-- 
2.1.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v3] Btrfs: fix fsync race leading to ordered extent memory leaks
  2015-02-06 12:34 [PATCH] Btrfs: fix fsync race leading to ordered extent memory leaks Filipe Manana
  2015-02-06 22:09 ` [PATCH v2] " Filipe Manana
@ 2015-02-07 18:57 ` Filipe Manana
  2015-02-08  0:45 ` [PATCH v4] " Filipe Manana
  2015-02-09 17:17 ` [PATCH v5] " Filipe Manana
  3 siblings, 0 replies; 5+ messages in thread
From: Filipe Manana @ 2015-02-07 18:57 UTC (permalink / raw)
  To: linux-btrfs; +Cc: stable, Filipe Manana

We can have two concurrent fsync operations against the same file,
in which case both fsyncs can collect the same ordered extents.
This allows for a time window where a race can lead to those ordered
extents never getting their reference count decremented to 0, leading
to memory leaks, getting referenced by multiple lists and resulting in
inode leaks too (an iput for the ordered extent's inode is scheduled
only when the ordered extent's refcount drops to 0). The following
sequence diagram explains this race:

         CPU 1                                    CPU 2

btrfs_sync_file()                          btrfs_sync_file()

  mutex_lock(inode->i_mutex)
  btrfs_log_inode()
    btrfs_get_logged_extents()
      --> collects ordered extent X
      --> increments ordered
          extent X's refcount
    btrfs_submit_logged_extents()
  mutex_unlock(inode->i_mutex)

  btrfs_sync_log()
    --> blocks on
        mutex_lock(root->log_mutex)

                                           mutex_lock(inode->i_mutex)
                                             btrfs_log_inode()
                                               btrfs_get_logged_extents()
                                                 --> collects ordered extent X
                                                     as well
                                                 --> increments ordered extent
                                                     X's refcount
                                                 --> ordered extent X is now
                                                     referenced by 2 different
                                                     lists, due to use of
                                                     list_add() and not list_move()
                                               btrfs_submit_logged_extents()
                                           mutex_unlock(inode->i_mutex)

   --> mutex root->log_mutex became
       free and task is unblocked

     btrfs_wait_logged_extents()
       --> sets bit BTRFS_ORDERED_LOGGED
           on ordered extent X's flags
           and adds it to trans->ordered
  btrfs_sync_log() finishes

                                           btrfs_sync_log()
                                             --> Same log_transid as task at CPU 1
                                             --> root->log_commit[log_transid % 2] > 0
                                             wait_log_commit()
                                             return ctx->log_ret
                                             --> The increment on ordered extent
                                                 X's refcount done before by the
                                                 task at CPU 2 will never have a
                                                 matching decrement

Fix this by using the flag BTRFS_ORDERED_LOGGED differently. Use it to
mean that an ordered extent is already being processed by an fsync call,
which prevents it from ever being collected by multiple concurrent
fsync operations against the same inode.

This race was introduced with the following change (added in 3.19 and
backported to stable 3.18 and 3.17):

  Btrfs: make sure logged extents complete in the current transaction V3
  commit 50d9aa99bd35c77200e0e3dd7a72274f8304701f

CC: <stable@vger.kernel.org> # 3.19, 3.18 and 3.17
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---

V2: No code changes, re-worded commit message and diagram.
    CC'ed stable too.

V3: No code changes, only adjusted commit message and its
    diagram to be more accurate.

 fs/btrfs/ordered-data.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index 534544e..87c8976 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -454,7 +454,7 @@ void btrfs_get_logged_extents(struct inode *inode,
 			break;
 		if (!list_empty(&ordered->log_list))
 			continue;
-		if (test_bit(BTRFS_ORDERED_LOGGED, &ordered->flags))
+		if (test_and_set_bit(BTRFS_ORDERED_LOGGED, &ordered->flags))
 			continue;
 		list_add(&ordered->log_list, logged_list);
 		atomic_inc(&ordered->refs);
@@ -470,6 +470,7 @@ void btrfs_put_logged_extents(struct list_head *logged_list)
 		ordered = list_first_entry(logged_list,
 					   struct btrfs_ordered_extent,
 					   log_list);
+		clear_bit(BTRFS_ORDERED_LOGGED, &ordered->flags);
 		list_del_init(&ordered->log_list);
 		btrfs_put_ordered_extent(ordered);
 	}
@@ -511,8 +512,7 @@ void btrfs_wait_logged_extents(struct btrfs_trans_handle *trans,
 		wait_event(ordered->wait, test_bit(BTRFS_ORDERED_IO_DONE,
 						   &ordered->flags));
 
-		if (!test_and_set_bit(BTRFS_ORDERED_LOGGED, &ordered->flags))
-			list_add_tail(&ordered->trans_list, &trans->ordered);
+		list_add_tail(&ordered->trans_list, &trans->ordered);
 		spin_lock_irq(&log->log_extents_lock[index]);
 	}
 	spin_unlock_irq(&log->log_extents_lock[index]);
-- 
2.1.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v4] Btrfs: fix fsync race leading to ordered extent memory leaks
  2015-02-06 12:34 [PATCH] Btrfs: fix fsync race leading to ordered extent memory leaks Filipe Manana
  2015-02-06 22:09 ` [PATCH v2] " Filipe Manana
  2015-02-07 18:57 ` [PATCH v3] " Filipe Manana
@ 2015-02-08  0:45 ` Filipe Manana
  2015-02-09 17:17 ` [PATCH v5] " Filipe Manana
  3 siblings, 0 replies; 5+ messages in thread
From: Filipe Manana @ 2015-02-08  0:45 UTC (permalink / raw)
  To: linux-btrfs; +Cc: stable, Filipe Manana

We can have multiple fsync operations against the same file during the
same transaction and they can collect the same ordered extents while they
don't complete (still accessible from the inode's ordered tree). If this
happens, those ordered extents will never get their reference counts
decremented to 0, leading to memory leaks and inode leaks (an iput for an
ordered extent's inode is scheduled only when the ordered extent's refcount
drops to 0). The following sequence diagram explains this race:

         CPU 1                                         CPU 2

btrfs_sync_file()

                                                 btrfs_sync_file()

  mutex_lock(inode->i_mutex)
  btrfs_log_inode()
    btrfs_get_logged_extents()
      --> collects ordered extent X
      --> increments ordered
          extent X's refcount
    btrfs_submit_logged_extents()
  mutex_unlock(inode->i_mutex)

                                                   mutex_lock(inode->i_mutex)
  btrfs_sync_log()
     btrfs_wait_logged_extents()
       --> list_del_init(&ordered->log_list)
                                                     btrfs_log_inode()
                                                       btrfs_get_logged_extents()
                                                         --> Adds ordered extent X
                                                             to logged_list because
                                                             at this point:
                                                             list_empty(&ordered->log_list)
                                                             && test_bit(BTRFS_ORDERED_LOGGED,
                                                                         &ordered->flags) == 0
                                                         --> Increments ordered extent
                                                             X's refcount
       --> check if ordered extent's io is
           finished or not, start it if
           necessary and wait for it to finish
       --> sets bit BTRFS_ORDERED_LOGGED
           on ordered extent X's flags
           and adds it to trans->ordered
  btrfs_sync_log() finishes

                                                       btrfs_submit_logged_extents()
                                                     btrfs_log_inode() finishes
                                                   mutex_unlock(inode->i_mutex)

btrfs_sync_file() finishes

                                                   btrfs_sync_log()
                                                      btrfs_wait_logged_extents()
                                                        --> Sees ordered extent X has the
                                                            bit BTRFS_ORDERED_LOGGED set in
                                                            its flags
                                                        --> X's refcount is untouched
                                                   btrfs_sync_log() finishes

                                                 btrfs_sync_file() finishes

btrfs_commit_transaction()
  --> called by transaction kthread for e.g.
  btrfs_wait_pending_ordered()
    --> waits for ordered extent X to
        complete
    --> decrements ordered extent X's
        refcount by 1 only, corresponding
        to the increment done by the fsync
        task ran by CPU 1

In the scenario of the above diagram, after the transaction commit,
the ordered extent will remain with a refcount of 1 forever, leaking
the ordered extent structure and preventing the i_count of its inode
from ever decreasing to 0, since the delayed iput is scheduled only
when the ordered extent's refcount drops to 0, preventing the inode
from ever being evicted by the VFS.

Fix this by using the flag BTRFS_ORDERED_LOGGED differently. Use it to
mean that an ordered extent is already being processed by an fsync call,
which will attach it to the current transaction, preventing it from being
collected by subsequent fsync operations against the same inode.

This race was introduced with the following change (added in 3.19 and
backported to stable 3.18 and 3.17):

  Btrfs: make sure logged extents complete in the current transaction V3
  commit 50d9aa99bd35c77200e0e3dd7a72274f8304701f

CC: <stable@vger.kernel.org> # 3.19, 3.18 and 3.17
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---

V2: No code changes, re-worded commit message and diagram.
    CC'ed stable too.

V3: No code changes, only adjusted commit message and its
    diagram to be more accurate.

V4: Simplified code by removing unnecessary code.
    Updated comment and diagram.

 fs/btrfs/ordered-data.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index 534544e..157cc54 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -452,9 +452,7 @@ void btrfs_get_logged_extents(struct inode *inode,
 			continue;
 		if (entry_end(ordered) <= start)
 			break;
-		if (!list_empty(&ordered->log_list))
-			continue;
-		if (test_bit(BTRFS_ORDERED_LOGGED, &ordered->flags))
+		if (test_and_set_bit(BTRFS_ORDERED_LOGGED, &ordered->flags))
 			continue;
 		list_add(&ordered->log_list, logged_list);
 		atomic_inc(&ordered->refs);
@@ -511,8 +509,7 @@ void btrfs_wait_logged_extents(struct btrfs_trans_handle *trans,
 		wait_event(ordered->wait, test_bit(BTRFS_ORDERED_IO_DONE,
 						   &ordered->flags));
 
-		if (!test_and_set_bit(BTRFS_ORDERED_LOGGED, &ordered->flags))
-			list_add_tail(&ordered->trans_list, &trans->ordered);
+		list_add_tail(&ordered->trans_list, &trans->ordered);
 		spin_lock_irq(&log->log_extents_lock[index]);
 	}
 	spin_unlock_irq(&log->log_extents_lock[index]);
-- 
2.1.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v5] Btrfs: fix fsync race leading to ordered extent memory leaks
  2015-02-06 12:34 [PATCH] Btrfs: fix fsync race leading to ordered extent memory leaks Filipe Manana
                   ` (2 preceding siblings ...)
  2015-02-08  0:45 ` [PATCH v4] " Filipe Manana
@ 2015-02-09 17:17 ` Filipe Manana
  3 siblings, 0 replies; 5+ messages in thread
From: Filipe Manana @ 2015-02-09 17:17 UTC (permalink / raw)
  To: linux-btrfs; +Cc: stable, Filipe Manana

We can have multiple fsync operations against the same file during the
same transaction and they can collect the same ordered extents while they
don't complete (still accessible from the inode's ordered tree). If this
happens, those ordered extents will never get their reference counts
decremented to 0, leading to memory leaks and inode leaks (an iput for an
ordered extent's inode is scheduled only when the ordered extent's refcount
drops to 0). The following sequence diagram explains this race:

         CPU 1                                         CPU 2

btrfs_sync_file()

                                                 btrfs_sync_file()

  mutex_lock(inode->i_mutex)
  btrfs_log_inode()
    btrfs_get_logged_extents()
      --> collects ordered extent X
      --> increments ordered
          extent X's refcount
    btrfs_submit_logged_extents()
  mutex_unlock(inode->i_mutex)

                                                   mutex_lock(inode->i_mutex)
  btrfs_sync_log()
     btrfs_wait_logged_extents()
       --> list_del_init(&ordered->log_list)
                                                     btrfs_log_inode()
                                                       btrfs_get_logged_extents()
                                                         --> Adds ordered extent X
                                                             to logged_list because
                                                             at this point:
                                                             list_empty(&ordered->log_list)
                                                             && test_bit(BTRFS_ORDERED_LOGGED,
                                                                         &ordered->flags) == 0
                                                         --> Increments ordered extent
                                                             X's refcount
       --> check if ordered extent's io is
           finished or not, start it if
           necessary and wait for it to finish
       --> sets bit BTRFS_ORDERED_LOGGED
           on ordered extent X's flags
           and adds it to trans->ordered
  btrfs_sync_log() finishes

                                                       btrfs_submit_logged_extents()
                                                     btrfs_log_inode() finishes
                                                   mutex_unlock(inode->i_mutex)

btrfs_sync_file() finishes

                                                   btrfs_sync_log()
                                                      btrfs_wait_logged_extents()
                                                        --> Sees ordered extent X has the
                                                            bit BTRFS_ORDERED_LOGGED set in
                                                            its flags
                                                        --> X's refcount is untouched
                                                   btrfs_sync_log() finishes

                                                 btrfs_sync_file() finishes

btrfs_commit_transaction()
  --> called by transaction kthread for e.g.
  btrfs_wait_pending_ordered()
    --> waits for ordered extent X to
        complete
    --> decrements ordered extent X's
        refcount by 1 only, corresponding
        to the increment done by the fsync
        task ran by CPU 1

In the scenario of the above diagram, after the transaction commit,
the ordered extent will remain with a refcount of 1 forever, leaking
the ordered extent structure and preventing the i_count of its inode
from ever decreasing to 0, since the delayed iput is scheduled only
when the ordered extent's refcount drops to 0, preventing the inode
from ever being evicted by the VFS.

Fix this by using the flag BTRFS_ORDERED_LOGGED differently. Use it to
mean that an ordered extent is already being processed by an fsync call,
which will attach it to the current transaction, preventing it from being
collected by subsequent fsync operations against the same inode.

This race was introduced with the following change (added in 3.19 and
backported to stable 3.18 and 3.17):

  Btrfs: make sure logged extents complete in the current transaction V3
  commit 50d9aa99bd35c77200e0e3dd7a72274f8304701f

I ran into this issue while running xfstests/generic/113 in a loop, which
failed about 1 out of 10 runs with the following warning in dmesg:

[ 2612.440038] WARNING: CPU: 4 PID: 22057 at fs/btrfs/disk-io.c:3558 free_fs_root+0x36/0x133 [btrfs]()
[ 2612.442810] Modules linked in: btrfs crc32c_generic xor raid6_pq nfsd auth_rpcgss oid_registry nfs_acl nfs lockd grace fscache sunrpc loop processor parport_pc parport psmouse therma
l_sys i2c_piix4 serio_raw pcspkr evdev microcode button i2c_core ext4 crc16 jbd2 mbcache sd_mod sg sr_mod cdrom virtio_scsi ata_generic virtio_pci ata_piix virtio_ring libata virtio flo
ppy e1000 scsi_mod [last unloaded: btrfs]
[ 2612.452711] CPU: 4 PID: 22057 Comm: umount Tainted: G        W      3.19.0-rc5-btrfs-next-4+ #1
[ 2612.454921] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org 04/01/2014
[ 2612.457709]  0000000000000009 ffff8801342c3c78 ffffffff8142425e ffff88023ec8f2d8
[ 2612.459829]  0000000000000000 ffff8801342c3cb8 ffffffff81045308 ffff880046460000
[ 2612.461564]  ffffffffa036da56 ffff88003d07b000 ffff880046460000 ffff880046460068
[ 2612.463163] Call Trace:
[ 2612.463719]  [<ffffffff8142425e>] dump_stack+0x4c/0x65
[ 2612.464789]  [<ffffffff81045308>] warn_slowpath_common+0xa1/0xbb
[ 2612.466026]  [<ffffffffa036da56>] ? free_fs_root+0x36/0x133 [btrfs]
[ 2612.467247]  [<ffffffff810453c5>] warn_slowpath_null+0x1a/0x1c
[ 2612.468416]  [<ffffffffa036da56>] free_fs_root+0x36/0x133 [btrfs]
[ 2612.469625]  [<ffffffffa036f2a7>] btrfs_drop_and_free_fs_root+0x93/0x9b [btrfs]
[ 2612.471251]  [<ffffffffa036f353>] btrfs_free_fs_roots+0xa4/0xd6 [btrfs]
[ 2612.472536]  [<ffffffff8142612e>] ? wait_for_completion+0x24/0x26
[ 2612.473742]  [<ffffffffa0370bbc>] close_ctree+0x1f3/0x33c [btrfs]
[ 2612.475477]  [<ffffffff81059d1d>] ? destroy_workqueue+0x148/0x1ba
[ 2612.476695]  [<ffffffffa034e3da>] btrfs_put_super+0x19/0x1b [btrfs]
[ 2612.477911]  [<ffffffff81153e53>] generic_shutdown_super+0x73/0xef
[ 2612.479106]  [<ffffffff811540e2>] kill_anon_super+0x13/0x1e
[ 2612.480226]  [<ffffffffa034e1e3>] btrfs_kill_super+0x17/0x23 [btrfs]
[ 2612.481471]  [<ffffffff81154307>] deactivate_locked_super+0x3b/0x50
[ 2612.482686]  [<ffffffff811547a7>] deactivate_super+0x3f/0x43
[ 2612.483791]  [<ffffffff8116b3ed>] cleanup_mnt+0x59/0x78
[ 2612.484842]  [<ffffffff8116b44c>] __cleanup_mnt+0x12/0x14
[ 2612.485900]  [<ffffffff8105d019>] task_work_run+0x8f/0xbc
[ 2612.486960]  [<ffffffff810028d8>] do_notify_resume+0x5a/0x6b
[ 2612.488083]  [<ffffffff81236e5b>] ? trace_hardirqs_on_thunk+0x3a/0x3f
[ 2612.489333]  [<ffffffff8142a17f>] int_signal+0x12/0x17
[ 2612.490353] ---[ end trace 54a960a6bdcb8d93 ]---
[ 2612.557253] VFS: Busy inodes after unmount of sdb. Self-destruct in 5 seconds.  Have a nice day...

Kmemleak confirmed the ordered extent leak (and btrfs inode specific
structures such as delayed nodes):

$ cat /sys/kernel/debug/kmemleak
unreferenced object 0xffff880154290db0 (size 576):
  comm "btrfsck", pid 21980, jiffies 4295542503 (age 1273.412s)
  hex dump (first 32 bytes):
    01 40 00 00 01 00 00 00 b0 1d f1 4e 01 88 ff ff  .@.........N....
    00 00 00 00 00 00 00 00 c8 0d 29 54 01 88 ff ff  ..........)T....
  backtrace:
    [<ffffffff8141d74d>] kmemleak_update_trace+0x4c/0x6a
    [<ffffffff8122f2c0>] radix_tree_node_alloc+0x6d/0x83
    [<ffffffff8122fb26>] __radix_tree_create+0x109/0x190
    [<ffffffff8122fbdd>] radix_tree_insert+0x30/0xac
    [<ffffffffa03b9bde>] btrfs_get_or_create_delayed_node+0x130/0x187 [btrfs]
    [<ffffffffa03bb82d>] btrfs_delayed_delete_inode_ref+0x32/0xac [btrfs]
    [<ffffffffa0379dae>] __btrfs_unlink_inode+0xee/0x288 [btrfs]
    [<ffffffffa037c715>] btrfs_unlink_inode+0x1e/0x40 [btrfs]
    [<ffffffffa037c797>] btrfs_unlink+0x60/0x9b [btrfs]
    [<ffffffff8115d7f0>] vfs_unlink+0x9c/0xed
    [<ffffffff8115f5de>] do_unlinkat+0x12c/0x1fa
    [<ffffffff811601a7>] SyS_unlinkat+0x29/0x2b
    [<ffffffff81429e92>] system_call_fastpath+0x12/0x17
    [<ffffffffffffffff>] 0xffffffffffffffff
unreferenced object 0xffff88014ef11db0 (size 576):
  comm "rm", pid 22009, jiffies 4295542593 (age 1273.052s)
  hex dump (first 32 bytes):
    02 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 c8 1d f1 4e 01 88 ff ff  ...........N....
  backtrace:
    [<ffffffff8141d74d>] kmemleak_update_trace+0x4c/0x6a
    [<ffffffff8122f2c0>] radix_tree_node_alloc+0x6d/0x83
    [<ffffffff8122fb26>] __radix_tree_create+0x109/0x190
    [<ffffffff8122fbdd>] radix_tree_insert+0x30/0xac
    [<ffffffffa03b9bde>] btrfs_get_or_create_delayed_node+0x130/0x187 [btrfs]
    [<ffffffffa03bb82d>] btrfs_delayed_delete_inode_ref+0x32/0xac [btrfs]
    [<ffffffffa0379dae>] __btrfs_unlink_inode+0xee/0x288 [btrfs]
    [<ffffffffa037c715>] btrfs_unlink_inode+0x1e/0x40 [btrfs]
    [<ffffffffa037c797>] btrfs_unlink+0x60/0x9b [btrfs]
    [<ffffffff8115d7f0>] vfs_unlink+0x9c/0xed
    [<ffffffff8115f5de>] do_unlinkat+0x12c/0x1fa
    [<ffffffff811601a7>] SyS_unlinkat+0x29/0x2b
    [<ffffffff81429e92>] system_call_fastpath+0x12/0x17
    [<ffffffffffffffff>] 0xffffffffffffffff
unreferenced object 0xffff8800336feda8 (size 584):
  comm "aio-stress", pid 22031, jiffies 4295543006 (age 1271.400s)
  hex dump (first 32 bytes):
    00 40 3e 00 00 00 00 00 00 00 8f 42 00 00 00 00  .@>........B....
    00 00 01 00 00 00 00 00 00 00 01 00 00 00 00 00  ................
  backtrace:
    [<ffffffff8114eb34>] create_object+0x172/0x29a
    [<ffffffff8141d790>] kmemleak_alloc+0x25/0x41
    [<ffffffff81141ae6>] kmemleak_alloc_recursive.constprop.52+0x16/0x18
    [<ffffffff81145288>] kmem_cache_alloc+0xf7/0x198
    [<ffffffffa0389243>] __btrfs_add_ordered_extent+0x43/0x309 [btrfs]
    [<ffffffffa038968b>] btrfs_add_ordered_extent_dio+0x12/0x14 [btrfs]
    [<ffffffffa03810e2>] btrfs_get_blocks_direct+0x3ef/0x571 [btrfs]
    [<ffffffff81181349>] do_blockdev_direct_IO+0x62a/0xb47
    [<ffffffff8118189a>] __blockdev_direct_IO+0x34/0x36
    [<ffffffffa03776e5>] btrfs_direct_IO+0x16a/0x1e8 [btrfs]
    [<ffffffff81100373>] generic_file_direct_write+0xb8/0x12d
    [<ffffffffa038615c>] btrfs_file_write_iter+0x24b/0x42f [btrfs]
    [<ffffffff8118bb0d>] aio_run_iocb+0x2b7/0x32e
    [<ffffffff8118c99a>] do_io_submit+0x26e/0x2ff
    [<ffffffff8118ca3b>] SyS_io_submit+0x10/0x12
    [<ffffffff81429e92>] system_call_fastpath+0x12/0x17

CC: <stable@vger.kernel.org> # 3.19, 3.18 and 3.17
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---

V2: No code changes, re-worded commit message and diagram.
    CC'ed stable too.

V3: No code changes, only adjusted commit message and its
    diagram to be more accurate.

V4: Simplified code by removing unnecessary code.
    Updated comment and diagram.

V5: Added reproducer information and traces (vfs warning and
    kmemleak). This was missing in this patch and was included
    in the previous patch I just dropped titled as:
    "Btrfs: fix race waiting for ordered extents at transaction commit"

 fs/btrfs/ordered-data.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index 534544e..157cc54 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -452,9 +452,7 @@ void btrfs_get_logged_extents(struct inode *inode,
 			continue;
 		if (entry_end(ordered) <= start)
 			break;
-		if (!list_empty(&ordered->log_list))
-			continue;
-		if (test_bit(BTRFS_ORDERED_LOGGED, &ordered->flags))
+		if (test_and_set_bit(BTRFS_ORDERED_LOGGED, &ordered->flags))
 			continue;
 		list_add(&ordered->log_list, logged_list);
 		atomic_inc(&ordered->refs);
@@ -511,8 +509,7 @@ void btrfs_wait_logged_extents(struct btrfs_trans_handle *trans,
 		wait_event(ordered->wait, test_bit(BTRFS_ORDERED_IO_DONE,
 						   &ordered->flags));
 
-		if (!test_and_set_bit(BTRFS_ORDERED_LOGGED, &ordered->flags))
-			list_add_tail(&ordered->trans_list, &trans->ordered);
+		list_add_tail(&ordered->trans_list, &trans->ordered);
 		spin_lock_irq(&log->log_extents_lock[index]);
 	}
 	spin_unlock_irq(&log->log_extents_lock[index]);
-- 
2.1.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-02-09 17:17 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-06 12:34 [PATCH] Btrfs: fix fsync race leading to ordered extent memory leaks Filipe Manana
2015-02-06 22:09 ` [PATCH v2] " Filipe Manana
2015-02-07 18:57 ` [PATCH v3] " Filipe Manana
2015-02-08  0:45 ` [PATCH v4] " Filipe Manana
2015-02-09 17:17 ` [PATCH v5] " Filipe Manana

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).