Commit a9501d74 authored by Song Liu's avatar Song Liu Committed by Shaohua Li

md/r5cache: fix io_unit handling in r5l_log_endio()

In r5l_log_endio(), once log->io_list_lock is released, the io unit
may be accessed (or even freed) by other threads. Current code
doesn't handle the io_unit properly, which leads to potential race
conditions.

This patch solves this race condition by:

1. Add a pending_stripe count flush_payload. Multiple flush_payloads
   are counted as only one pending_stripe. Flag has_flush_payload is
   added to show whether the io unit has flush_payload;
2. In r5l_log_endio(), check flags has_null_flush and
   has_flush_payload with log->io_list_lock held. After the lock
   is released, this IO unit is only accessed when we know the
   pending_stripe counter cannot be zeroed by other threads.
Signed-off-by: default avatarSong Liu <songliubraving@fb.com>
Signed-off-by: default avatarShaohua Li <shli@fb.com>
parent b44886c5
...@@ -238,7 +238,8 @@ struct r5l_io_unit { ...@@ -238,7 +238,8 @@ struct r5l_io_unit {
unsigned int has_flush:1; /* include flush request */ unsigned int has_flush:1; /* include flush request */
unsigned int has_fua:1; /* include fua request */ unsigned int has_fua:1; /* include fua request */
unsigned int has_null_flush:1; /* include empty flush request */ unsigned int has_null_flush:1; /* include null flush request */
unsigned int has_flush_payload:1; /* include flush payload */
/* /*
* io isn't sent yet, flush/fua request can only be submitted till it's * io isn't sent yet, flush/fua request can only be submitted till it's
* the first IO in running_ios list * the first IO in running_ios list
...@@ -571,6 +572,8 @@ static void r5l_log_endio(struct bio *bio) ...@@ -571,6 +572,8 @@ static void r5l_log_endio(struct bio *bio)
struct r5l_io_unit *io_deferred; struct r5l_io_unit *io_deferred;
struct r5l_log *log = io->log; struct r5l_log *log = io->log;
unsigned long flags; unsigned long flags;
bool has_null_flush;
bool has_flush_payload;
if (bio->bi_status) if (bio->bi_status)
md_error(log->rdev->mddev, log->rdev); md_error(log->rdev->mddev, log->rdev);
...@@ -580,6 +583,16 @@ static void r5l_log_endio(struct bio *bio) ...@@ -580,6 +583,16 @@ static void r5l_log_endio(struct bio *bio)
spin_lock_irqsave(&log->io_list_lock, flags); spin_lock_irqsave(&log->io_list_lock, flags);
__r5l_set_io_unit_state(io, IO_UNIT_IO_END); __r5l_set_io_unit_state(io, IO_UNIT_IO_END);
/*
* if the io doesn't not have null_flush or flush payload,
* it is not safe to access it after releasing io_list_lock.
* Therefore, it is necessary to check the condition with
* the lock held.
*/
has_null_flush = io->has_null_flush;
has_flush_payload = io->has_flush_payload;
if (log->need_cache_flush && !list_empty(&io->stripe_list)) if (log->need_cache_flush && !list_empty(&io->stripe_list))
r5l_move_to_end_ios(log); r5l_move_to_end_ios(log);
else else
...@@ -600,18 +613,22 @@ static void r5l_log_endio(struct bio *bio) ...@@ -600,18 +613,22 @@ static void r5l_log_endio(struct bio *bio)
if (log->need_cache_flush) if (log->need_cache_flush)
md_wakeup_thread(log->rdev->mddev->thread); md_wakeup_thread(log->rdev->mddev->thread);
if (io->has_null_flush) { /* finish flush only io_unit and PAYLOAD_FLUSH only io_unit */
if (has_null_flush) {
struct bio *bi; struct bio *bi;
WARN_ON(bio_list_empty(&io->flush_barriers)); WARN_ON(bio_list_empty(&io->flush_barriers));
while ((bi = bio_list_pop(&io->flush_barriers)) != NULL) { while ((bi = bio_list_pop(&io->flush_barriers)) != NULL) {
bio_endio(bi); bio_endio(bi);
atomic_dec(&io->pending_stripe); if (atomic_dec_and_test(&io->pending_stripe)) {
__r5l_stripe_write_finished(io);
return;
} }
} }
}
/* finish flush only io_unit and PAYLOAD_FLUSH only io_unit */ /* decrease pending_stripe for flush payload */
if (atomic_read(&io->pending_stripe) == 0) if (has_flush_payload)
if (atomic_dec_and_test(&io->pending_stripe))
__r5l_stripe_write_finished(io); __r5l_stripe_write_finished(io);
} }
...@@ -881,6 +898,11 @@ static void r5l_append_flush_payload(struct r5l_log *log, sector_t sect) ...@@ -881,6 +898,11 @@ static void r5l_append_flush_payload(struct r5l_log *log, sector_t sect)
payload->size = cpu_to_le32(sizeof(__le64)); payload->size = cpu_to_le32(sizeof(__le64));
payload->flush_stripes[0] = cpu_to_le64(sect); payload->flush_stripes[0] = cpu_to_le64(sect);
io->meta_offset += meta_size; io->meta_offset += meta_size;
/* multiple flush payloads count as one pending_stripe */
if (!io->has_flush_payload) {
io->has_flush_payload = 1;
atomic_inc(&io->pending_stripe);
}
mutex_unlock(&log->io_mutex); mutex_unlock(&log->io_mutex);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment