From patchwork Thu Mar 26 17:02:17 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boaz Harrosh X-Patchwork-Id: 6101121 Return-Path: X-Original-To: patchwork-linux-nvdimm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 21C089F2A9 for ; Thu, 26 Mar 2015 17:02:24 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0DE472041A for ; Thu, 26 Mar 2015 17:02:23 +0000 (UTC) Received: from ml01.01.org (ml01.01.org [198.145.21.10]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B58972041E for ; Thu, 26 Mar 2015 17:02:21 +0000 (UTC) Received: from ml01.vlan14.01.org (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id A45CC81307; Thu, 26 Mar 2015 10:02:21 -0700 (PDT) X-Original-To: linux-nvdimm@ml01.01.org Delivered-To: linux-nvdimm@ml01.01.org Received: from mail-wg0-f50.google.com (mail-wg0-f50.google.com [74.125.82.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 959EF80DBD for ; Thu, 26 Mar 2015 10:02:20 -0700 (PDT) Received: by wgs2 with SMTP id 2so71680796wgs.1 for ; Thu, 26 Mar 2015 10:02:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=dx+FMq93Nq2G686FLWL00YslYUgHqnxIXaR7l8VzQ28=; b=b53SBobm7iuOFYbUUoavMYi15EDMueEztxajOKJ49qtKYhN4aYwdWSo2VbNJUQPpRH Qk4oH8lp7rkmGWU2+v3AkpibRBwi6uSfL/HJ9i4RvbqrleHttiTPG7IsUqcWGQZf9acr +piaPwxwY79f+pJ99idd9K7fth0pAhw4Lfdqv2r7FYvCs4NFqg1WZinEhalxOpEPu7Cu 1Vcphi1j1HqK/HtnI8LF0Sp/cr/vDzwPN+N0eDjDW1NPU7hp1mKaUrnz9KO6A5HS0Dta 1eRfHADtNBmHMcj30EBfDhgGpEabsQw6alJjMzN0TuzYBxrUO/QBPrwPsniXjYwKxu+t ZUKg== X-Gm-Message-State: ALoCoQl/WyO7O1yggvhm0gAf8bloFqaUUEZawY4d9hCDh34Oy9cL9VB5fgtLIxShlnhduX1EQ/u4 X-Received: by 10.180.90.34 with SMTP id bt2mr6581499wib.43.1427389339278; Thu, 26 Mar 2015 10:02:19 -0700 (PDT) Received: from [10.0.0.5] ([207.232.55.62]) by mx.google.com with ESMTPSA id fs8sm26131039wib.8.2015.03.26.10.02.17 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 26 Mar 2015 10:02:18 -0700 (PDT) Message-ID: <55143B99.7060407@plexistor.com> Date: Thu, 26 Mar 2015 19:02:17 +0200 From: Boaz Harrosh User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Christoph Hellwig , linux-nvdimm@ml01.01.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, x86@kernel.org References: <1427358764-6126-1-git-send-email-hch@lst.de> <55143A8B.2060304@plexistor.com> In-Reply-To: <55143A8B.2060304@plexistor.com> Cc: axboe@kernel.dk Subject: [Linux-nvdimm] [PATCH] SQUASHME: Streamline pmem.c X-BeenThere: linux-nvdimm@lists.01.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Linux-nvdimm developer list." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-nvdimm-bounces@lists.01.org Sender: "Linux-nvdimm" X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Christoph why did you choose the fat and ugly version of pmem.c beats me. Anyway, here are the cleanups you need on top of your pmem patch. Among other it does: * Remove getgeo. It is not needed for modern fdisk and was never needed for libgparted and cfdisk. * remove 89 lines of code to do a single memcpy. The reason this was so in brd (done badly BTW) is because destination memory is page-by-page based. With pmem we have the destination contiguous so we can do any size, in one go. * Remove SECTOR_SHIFT. It is defined in 6 other places in the Kernel. I do not like a new one. 9 is used through out, including block core. I do not like pmem to blasphemy more than needed. * More style stuff ... Please squash into your initial submission Signed-off-by: Boaz Harrosh Reviewed-by: Ross Zwisler --- drivers/block/pmem.c | 137 +++++++++++---------------------------------------- 1 file changed, 28 insertions(+), 109 deletions(-) diff --git a/drivers/block/pmem.c b/drivers/block/pmem.c index 545b13b..5a57a06 100644 --- a/drivers/block/pmem.c +++ b/drivers/block/pmem.c @@ -11,7 +11,7 @@ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for * more details. * - * This driver is heavily based on drivers/block/brd.c. + * This driver's skeleton is based on drivers/block/brd.c. * Copyright (C) 2007 Nick Piggin * Copyright (C) 2007 Novell Inc. */ @@ -24,11 +24,6 @@ #include #include #include - -#define SECTOR_SHIFT 9 -#define PAGE_SECTORS_SHIFT (PAGE_SHIFT - SECTOR_SHIFT) -#define PAGE_SECTORS (1 << PAGE_SECTORS_SHIFT) - #define PMEM_MINORS 16 struct pmem_device { @@ -44,100 +39,17 @@ struct pmem_device { static int pmem_major; static atomic_t pmem_index; -static int pmem_getgeo(struct block_device *bd, struct hd_geometry *geo) -{ - /* some standard values */ - geo->heads = 1 << 6; - geo->sectors = 1 << 5; - geo->cylinders = get_capacity(bd->bd_disk) >> 11; - return 0; -} - -/* - * direct translation from (pmem,sector) => void* - * We do not require that sector be page aligned. - * The return value will point to the beginning of the page containing the - * given sector, not to the sector itself. - */ -static void *pmem_lookup_pg_addr(struct pmem_device *pmem, sector_t sector) -{ - size_t page_offset = sector >> PAGE_SECTORS_SHIFT; - size_t offset = page_offset << PAGE_SHIFT; - - BUG_ON(offset >= pmem->size); - return pmem->virt_addr + offset; -} - -/* sector must be page aligned */ -static unsigned long pmem_lookup_pfn(struct pmem_device *pmem, sector_t sector) -{ - size_t page_offset = sector >> PAGE_SECTORS_SHIFT; - - BUG_ON(sector & (PAGE_SECTORS - 1)); - return (pmem->phys_addr >> PAGE_SHIFT) + page_offset; -} - -/* - * sector is not required to be page aligned. - * n is at most a single page, but could be less. - */ -static void copy_to_pmem(struct pmem_device *pmem, const void *src, - sector_t sector, size_t n) -{ - void *dst; - unsigned int offset = (sector & (PAGE_SECTORS - 1)) << SECTOR_SHIFT; - size_t copy; - - BUG_ON(n > PAGE_SIZE); - - copy = min_t(size_t, n, PAGE_SIZE - offset); - dst = pmem_lookup_pg_addr(pmem, sector); - memcpy(dst + offset, src, copy); - - if (copy < n) { - src += copy; - sector += copy >> SECTOR_SHIFT; - copy = n - copy; - dst = pmem_lookup_pg_addr(pmem, sector); - memcpy(dst, src, copy); - } -} - -/* - * sector is not required to be page aligned. - * n is at most a single page, but could be less. - */ -static void copy_from_pmem(void *dst, struct pmem_device *pmem, - sector_t sector, size_t n) -{ - void *src; - unsigned int offset = (sector & (PAGE_SECTORS - 1)) << SECTOR_SHIFT; - size_t copy; - - BUG_ON(n > PAGE_SIZE); - - copy = min_t(size_t, n, PAGE_SIZE - offset); - src = pmem_lookup_pg_addr(pmem, sector); - - memcpy(dst, src + offset, copy); - - if (copy < n) { - dst += copy; - sector += copy >> SECTOR_SHIFT; - copy = n - copy; - src = pmem_lookup_pg_addr(pmem, sector); - memcpy(dst, src, copy); - } -} - static void pmem_do_bvec(struct pmem_device *pmem, struct page *page, unsigned int len, unsigned int off, int rw, sector_t sector) { void *mem = kmap_atomic(page); + size_t pmem_off = sector << 9; + + BUG_ON(pmem_off >= pmem->size); if (rw == READ) { - copy_from_pmem(mem + off, pmem, sector, len); + memcpy(mem + off, pmem->virt_addr + pmem_off, len); flush_dcache_page(page); } else { /* @@ -145,7 +57,7 @@ static void pmem_do_bvec(struct pmem_device *pmem, struct page *page, * NVDIMMs are actually durable before returning. */ flush_dcache_page(page); - copy_to_pmem(pmem, mem + off, sector, len); + memcpy(pmem->virt_addr + pmem_off, mem + off, len); } kunmap_atomic(mem); @@ -161,25 +73,32 @@ static void pmem_make_request(struct request_queue *q, struct bio *bio) struct bvec_iter iter; int err = 0; - sector = bio->bi_iter.bi_sector; - if (bio_end_sector(bio) > get_capacity(bdev->bd_disk)) { + if (unlikely(bio_end_sector(bio) > get_capacity(bdev->bd_disk))) { err = -EIO; goto out; } - BUG_ON(bio->bi_rw & REQ_DISCARD); + if (WARN_ON(bio->bi_rw & REQ_DISCARD)) { + err = -EINVAL; + goto out; + } rw = bio_rw(bio); if (rw == READA) rw = READ; + sector = bio->bi_iter.bi_sector; bio_for_each_segment(bvec, bio, iter) { - unsigned int len = bvec.bv_len; - - BUG_ON(len > PAGE_SIZE); - pmem_do_bvec(pmem, bvec.bv_page, len, - bvec.bv_offset, rw, sector); - sector += len >> SECTOR_SHIFT; + /* NOTE: There is a legend saying that bv_len might be + * bigger than PAGE_SIZE in the case that bv_page points to + * a physical contiguous PFN set. But for us it is fine because + * it means the Kernel virtual mapping is also contiguous. And + * on the pmem side we are always contiguous both virtual and + * physical + */ + pmem_do_bvec(pmem, bvec.bv_page, bvec.bv_len, bvec.bv_offset, + rw, sector); + sector += bvec.bv_len >> 9; } out: @@ -200,21 +119,21 @@ static long pmem_direct_access(struct block_device *bdev, sector_t sector, void **kaddr, unsigned long *pfn, long size) { struct pmem_device *pmem = bdev->bd_disk->private_data; + size_t offset = sector << 9; - if (!pmem) + if (unlikely(!pmem)) return -ENODEV; - *kaddr = pmem_lookup_pg_addr(pmem, sector); - *pfn = pmem_lookup_pfn(pmem, sector); + *kaddr = pmem->virt_addr + offset; + *pfn = (pmem->phys_addr + offset) >> PAGE_SHIFT; - return pmem->size - (sector * 512); + return pmem->size - offset; } static const struct block_device_operations pmem_fops = { .owner = THIS_MODULE, .rw_page = pmem_rw_page, .direct_access = pmem_direct_access, - .getgeo = pmem_getgeo, }; /* pmem->phys_addr and pmem->size need to be set. @@ -307,7 +226,7 @@ static int pmem_probe(struct platform_device *pdev) disk->flags = GENHD_FL_EXT_DEVT; sprintf(disk->disk_name, "pmem%d", idx); disk->driverfs_dev = &pdev->dev; - set_capacity(disk, pmem->size >> SECTOR_SHIFT); + set_capacity(disk, pmem->size >> 9); pmem->pmem_disk = disk; add_disk(disk);