| From 9ae6e8b1c9bbf6874163d1243e393137313762b7 Mon Sep 17 00:00:00 2001 |
| From: Nikos Tsironis <ntsironis@arrikto.com> |
| Date: Tue, 21 Jun 2022 15:24:03 +0300 |
| Subject: dm era: commit metadata in postsuspend after worker stops |
| |
| From: Nikos Tsironis <ntsironis@arrikto.com> |
| |
| commit 9ae6e8b1c9bbf6874163d1243e393137313762b7 upstream. |
| |
| During postsuspend dm-era does the following: |
| |
| 1. Archives the current era |
| 2. Commits the metadata, as part of the RPC call for archiving the |
| current era |
| 3. Stops the worker |
| |
| Until the worker stops, it might write to the metadata again. Moreover, |
| these writes are not flushed to disk immediately, but are cached by the |
| dm-bufio client, which writes them back asynchronously. |
| |
| As a result, the committed metadata of a suspended dm-era device might |
| not be consistent with the in-core metadata. |
| |
| In some cases, this can result in the corruption of the on-disk |
| metadata. Suppose the following sequence of events: |
| |
| 1. Load a new table, e.g. a snapshot-origin table, to a device with a |
| dm-era table |
| 2. Suspend the device |
| 3. dm-era commits its metadata, but the worker does a few more metadata |
| writes until it stops, as part of digesting an archived writeset |
| 4. These writes are cached by the dm-bufio client |
| 5. Load the dm-era table to another device. |
| 6. The new instance of the dm-era target loads the committed, on-disk |
| metadata, which don't include the extra writes done by the worker |
| after the metadata commit. |
| 7. Resume the new device |
| 8. The new dm-era target instance starts using the metadata |
| 9. Resume the original device |
| 10. The destructor of the old dm-era target instance is called and |
| destroys the dm-bufio client, which results in flushing the cached |
| writes to disk |
| 11. These writes might overwrite the writes done by the new dm-era |
| instance, hence corrupting its metadata. |
| |
| Fix this by committing the metadata after the worker stops running. |
| |
| stop_worker uses flush_workqueue to flush the current work. However, the |
| work item may re-queue itself and flush_workqueue doesn't wait for |
| re-queued works to finish. |
| |
| This could result in the worker changing the metadata after they have |
| been committed, or writing to the metadata concurrently with the commit |
| in the postsuspend thread. |
| |
| Use drain_workqueue instead, which waits until the work and all |
| re-queued works finish. |
| |
| Fixes: eec40579d8487 ("dm: add era target") |
| Cc: stable@vger.kernel.org # v3.15+ |
| Signed-off-by: Nikos Tsironis <ntsironis@arrikto.com> |
| Signed-off-by: Mike Snitzer <snitzer@kernel.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/md/dm-era-target.c | 8 +++++++- |
| 1 file changed, 7 insertions(+), 1 deletion(-) |
| |
| --- a/drivers/md/dm-era-target.c |
| +++ b/drivers/md/dm-era-target.c |
| @@ -1398,7 +1398,7 @@ static void start_worker(struct era *era |
| static void stop_worker(struct era *era) |
| { |
| atomic_set(&era->suspended, 1); |
| - flush_workqueue(era->wq); |
| + drain_workqueue(era->wq); |
| } |
| |
| /*---------------------------------------------------------------- |
| @@ -1583,6 +1583,12 @@ static void era_postsuspend(struct dm_ta |
| } |
| |
| stop_worker(era); |
| + |
| + r = metadata_commit(era->md); |
| + if (r) { |
| + DMERR("%s: metadata_commit failed", __func__); |
| + /* FIXME: fail mode */ |
| + } |
| } |
| |
| static int era_preresume(struct dm_target *ti) |