Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ee15b038 authored by Lars Ellenberg's avatar Lars Ellenberg Committed by Philipp Reisner
Browse files

drbd: fix race on meta-data update, addendum 



addendum to baa33ae4eaa4477b60af7c434c0ddd1d182c1ae7

The race:
    drbd_md_sync()
	if (!test_and_clear_bit(MD_DIRTY, &mdev->flags))
		return;
    ==> RACE with drbd_md_mark_dirty() rearming the timer.
	del_timer(&mdev->md_sync_timer);

    Fixed by moving the del_timer before the test_and_clear_bit.

Additionally only rearm the timer in drbd_md_mark_dirty, if MD_DIRTY was
not already set, reduce the grace period from five to one second, and
add an ifdef'ed debuging aid to find code paths missing an explicit
drbd_md_sync, if any, as those are the only relevant ones for this race.

Signed-off-by: default avatarPhilipp Reisner <philipp.reisner@linbit.com>
Signed-off-by: default avatarLars Ellenberg <lars.ellenberg@linbit.com>
parent 63106d3c

drivers/block/drbd/drbd_int.h

+12 −0
Original line number Diff line number Diff line
@@ -975,6 +975,12 @@ struct drbd_conf { 
			  md_sync_work;

	struct timer_list resync_timer;

	struct timer_list md_sync_timer;

#ifdef DRBD_DEBUG_MD_SYNC

	struct {

		unsigned int line;

		const char* func;

	} last_md_mark_dirty;

#endif



	/* Used after attach while negotiating new disk state. */

	union drbd_state new_state_tmp;
@@ -1253,7 +1259,13 @@ extern void drbd_uuid_set_bm(struct drbd_conf *mdev, u64 val) __must_hold(local) 
extern void drbd_md_set_flag(struct drbd_conf *mdev, int flags) __must_hold(local);

extern void drbd_md_clear_flag(struct drbd_conf *mdev, int flags)__must_hold(local);

extern int drbd_md_test_flag(struct drbd_backing_dev *, int);

#ifndef DRBD_DEBUG_MD_SYNC

extern void drbd_md_mark_dirty(struct drbd_conf *mdev);

#else

#define drbd_md_mark_dirty(m)	drbd_md_mark_dirty_(m, __LINE__ , __func__ )

extern void drbd_md_mark_dirty_(struct drbd_conf *mdev,

		unsigned int line, const char *func);

#endif

extern void drbd_queue_bitmap_io(struct drbd_conf *mdev,

				 int (*io_fn)(struct drbd_conf *),

				 void (*done)(struct drbd_conf *, int),

drivers/block/drbd/drbd_main.c

+19 −5
Original line number Diff line number Diff line
@@ -3402,9 +3402,10 @@ void drbd_md_sync(struct drbd_conf *mdev) 
	sector_t sector;

	int i;



	del_timer(&mdev->md_sync_timer);

	/* timer may be rearmed by drbd_md_mark_dirty() now. */

	if (!test_and_clear_bit(MD_DIRTY, &mdev->flags))

		return;

	del_timer(&mdev->md_sync_timer);



	/* We use here D_FAILED and not D_ATTACHING because we try to write

	 * metadata even if we detach due to a disk failure! */
@@ -3529,12 +3530,22 @@ int drbd_md_read(struct drbd_conf *mdev, struct drbd_backing_dev *bdev) 
 * the meta-data super block. This function sets MD_DIRTY, and starts a

 * timer that ensures that within five seconds you have to call drbd_md_sync().

 */

#ifdef DRBD_DEBUG_MD_SYNC

void drbd_md_mark_dirty_(struct drbd_conf *mdev, unsigned int line, const char *func)

{

	if (!test_and_set_bit(MD_DIRTY, &mdev->flags)) {

		mod_timer(&mdev->md_sync_timer, jiffies + HZ);

		mdev->last_md_mark_dirty.line = line;

		mdev->last_md_mark_dirty.func = func;

	}

}

#else

void drbd_md_mark_dirty(struct drbd_conf *mdev)

{

	set_bit(MD_DIRTY, &mdev->flags);

	mod_timer(&mdev->md_sync_timer, jiffies + 5*HZ);

	if (!test_and_set_bit(MD_DIRTY, &mdev->flags))

		mod_timer(&mdev->md_sync_timer, jiffies + HZ);

}



#endif



static void drbd_uuid_move_history(struct drbd_conf *mdev) __must_hold(local)

{
@@ -3775,8 +3786,11 @@ static void md_sync_timer_fn(unsigned long data) 
static int w_md_sync(struct drbd_conf *mdev, struct drbd_work *w, int unused)

{

	dev_warn(DEV, "md_sync_timer expired! Worker calls drbd_md_sync().\n");

#ifdef DEBUG

	dev_warn(DEV, "last md_mark_dirty: %s:%u\n",

		mdev->last_md_mark_dirty.func, mdev->last_md_mark_dirty.line);

#endif

	drbd_md_sync(mdev);



	return 1;

}