From patchwork Wed Apr 15 12:31:10 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mikulas Patocka X-Patchwork-Id: 11491147 X-Patchwork-Delegate: snitzer@redhat.com Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id D555E912 for ; Wed, 15 Apr 2020 12:31:26 +0000 (UTC) Received: from us-smtp-delivery-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.81]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 96E9A20767 for ; Wed, 15 Apr 2020 12:31:26 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="XLzXk76e" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 96E9A20767 Authentication-Results: mail.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=dm-devel-bounces@redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1586953885; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=YETl2uX+mbZfcSEEe7ZkTRxrVKhfKsjexWSqb9eKg0E=; b=XLzXk76ekMkH4oJDNSDZF1zCFKwEErH5kC/5divXaDpZjX2Ke2iSDhjE62gKT+v97pHwKu TcJ1B0JxB47Y8o3XslGkSfRoNqL4x4OFFxZuN1YsWLEto7vddS4iyRioqwacDV894ARIhC ytBTaolnDVWH7c7tHyWvxpHmesHcpJY= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-391-kL6hd3_jPJGuutIo-nCaAw-1; Wed, 15 Apr 2020 08:31:23 -0400 X-MC-Unique: kL6hd3_jPJGuutIo-nCaAw-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id EA80819057A4; Wed, 15 Apr 2020 12:31:18 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 6A951A63D6; Wed, 15 Apr 2020 12:31:18 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 254DB18363CF; Wed, 15 Apr 2020 12:31:17 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 03FCVGPQ030115 for ; Wed, 15 Apr 2020 08:31:16 -0400 Received: by smtp.corp.redhat.com (Postfix) id 2FB78C0DB9; Wed, 15 Apr 2020 12:31:16 +0000 (UTC) Delivered-To: dm-devel@redhat.com Received: from file01.intranet.prod.int.rdu2.redhat.com (file01.intranet.prod.int.rdu2.redhat.com [10.11.5.7]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 65D99116D7D; Wed, 15 Apr 2020 12:31:10 +0000 (UTC) Received: from file01.intranet.prod.int.rdu2.redhat.com (localhost [127.0.0.1]) by file01.intranet.prod.int.rdu2.redhat.com (8.14.4/8.14.4) with ESMTP id 03FCVAf7026372; Wed, 15 Apr 2020 08:31:10 -0400 Received: from localhost (mpatocka@localhost) by file01.intranet.prod.int.rdu2.redhat.com (8.14.4/8.14.4/Submit) with ESMTP id 03FCVAp7026368; Wed, 15 Apr 2020 08:31:10 -0400 X-Authentication-Warning: file01.intranet.prod.int.rdu2.redhat.com: mpatocka owned process doing -bs Date: Wed, 15 Apr 2020 08:31:10 -0400 (EDT) From: Mikulas Patocka X-X-Sender: mpatocka@file01.intranet.prod.int.rdu2.redhat.com To: Mike Snitzer In-Reply-To: Message-ID: References: <20200414190515.GA25340@redhat.com> User-Agent: Alpine 2.02 (LRH 1266 2009-07-14) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-loop: dm-devel@redhat.com Cc: dm-devel@redhat.com, David Teigland Subject: [dm-devel] [PATCH v2] dm writecache: fix data corruption when reloading the target X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Here I resubmit it. The reason for this is: we want to read as little data as possible (that's bdev_logical_block_size), however, if the user has a pathological setup where wc->metadata_sectors < bdev_logical_block_size >> SECTOR_SHIFT, we don't want to over-read the allocated memory. Note that if wc->metadata_sectors < bdev_logical_block_size >> SECTOR_SHIFT, it won't work anyway, but I tried to prevent kernel memory corruption. + r = writecache_read_metadata(wc, + min((sector_t)bdev_logical_block_size(wc->ssd_dev->bdev) >> SECTOR_SHIFT, + (sector_t)wc->metadata_sectors)); From: Mikulas Patocka dm writecache: fix data corruption when reloading the target The dm-writecache reads metadata in the target constructor. However, when we reload the target, there could be another active instance running on the same device. This is the sequence of operations when doing a reload: 1. construct new target 2. suspend old target 3. resume new target 4. destroy old target Metadata that were written by the old target between steps 1 and 2 would not be visible by the new target. This patch fixes the data corruption by loading the metadata in the resume handler. Signed-off-by: Mikulas Patocka Cc: stable@vger.kernel.org # v4.18+ Fixes: 48debafe4f2f ("dm: add writecache target") --- drivers/md/dm-writecache.c | 44 ++++++++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 14 deletions(-) -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel Index: linux-2.6/drivers/md/dm-writecache.c =================================================================== --- linux-2.6.orig/drivers/md/dm-writecache.c 2020-04-13 18:27:52.000000000 +0200 +++ linux-2.6/drivers/md/dm-writecache.c 2020-04-15 09:39:27.000000000 +0200 @@ -931,6 +931,24 @@ static int writecache_alloc_entries(stru return 0; } +static int writecache_read_metadata(struct dm_writecache *wc, sector_t n_sectors) +{ + struct dm_io_region region; + struct dm_io_request req; + + region.bdev = wc->ssd_dev->bdev; + region.sector = wc->start_sector; + region.count = n_sectors; + req.bi_op = REQ_OP_READ; + req.bi_op_flags = REQ_SYNC; + req.mem.type = DM_IO_VMA; + req.mem.ptr.vma = (char *)wc->memory_map; + req.client = wc->dm_io; + req.notify.fn = NULL; + + return dm_io(&req, 1, ®ion, NULL); +} + static void writecache_resume(struct dm_target *ti) { struct dm_writecache *wc = ti->private; @@ -941,8 +959,16 @@ static void writecache_resume(struct dm_ wc_lock(wc); - if (WC_MODE_PMEM(wc)) + if (WC_MODE_PMEM(wc)) { persistent_memory_invalidate_cache(wc->memory_map, wc->memory_map_size); + } else { + r = writecache_read_metadata(wc, wc->metadata_sectors); + if (r) { + writecache_error(wc, r, "unable to read metadata: %d", r); + memset((char *)wc->memory_map + offsetof(struct wc_memory_superblock, entries), -1, + (wc->metadata_sectors << SECTOR_SHIFT) - offsetof(struct wc_memory_superblock, entries)); + } + } wc->tree = RB_ROOT; INIT_LIST_HEAD(&wc->lru); @@ -2200,8 +2226,6 @@ invalid_optional: goto bad; } } else { - struct dm_io_region region; - struct dm_io_request req; size_t n_blocks, n_metadata_blocks; uint64_t n_bitmap_bits; @@ -2258,17 +2282,9 @@ invalid_optional: goto bad; } - region.bdev = wc->ssd_dev->bdev; - region.sector = wc->start_sector; - region.count = wc->metadata_sectors; - req.bi_op = REQ_OP_READ; - req.bi_op_flags = REQ_SYNC; - req.mem.type = DM_IO_VMA; - req.mem.ptr.vma = (char *)wc->memory_map; - req.client = wc->dm_io; - req.notify.fn = NULL; - - r = dm_io(&req, 1, ®ion, NULL); + r = writecache_read_metadata(wc, + min((sector_t)bdev_logical_block_size(wc->ssd_dev->bdev) >> SECTOR_SHIFT, + (sector_t)wc->metadata_sectors)); if (r) { ti->error = "Unable to read metadata"; goto bad;