All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boaz Harrosh <boaz@plexistor.com>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-nvdimm@ml01.01.org, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, x86@kernel.org,
	ross.zwisler@linux.intel.com, axboe@kernel.dk
Subject: [PATCH 3/6] SQUASHME: pmem: Streamline pmem driver
Date: Tue, 31 Mar 2015 16:25:35 +0300	[thread overview]
Message-ID: <551AA04F.60209@plexistor.com> (raw)
In-Reply-To: <551A9EB3.8000605@plexistor.com>


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.

[v2]
Remove the BUG_ON checks on out of range IO.
The core already does these checks and I did not see these
checks done in other drivers.

Signed-off-by: Boaz Harrosh <boaz@plexistor.com>
---
 drivers/block/pmem.c | 112 ++++++++++-----------------------------------------
 1 file changed, 22 insertions(+), 90 deletions(-)

diff --git a/drivers/block/pmem.c b/drivers/block/pmem.c
index dcb524f..6a45fd5 100644
--- a/drivers/block/pmem.c
+++ b/drivers/block/pmem.c
@@ -44,91 +44,15 @@ struct pmem_device {
 static int pmem_major;
 static atomic_t pmem_index;
 
-/*
- * 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;
 
 	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 {
 		/*
@@ -136,7 +60,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);
@@ -152,25 +76,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)) {
 		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:
@@ -191,14 +122,15 @@ 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)
 		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 = {
-- 
1.9.3



  parent reply	other threads:[~2015-03-31 13:25 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-26  8:32 another pmem variant V2 Christoph Hellwig
2015-03-26  8:32 ` [PATCH 1/3] pmem: Initial version of persistent memory driver Christoph Hellwig
2015-03-26 14:12   ` [Linux-nvdimm] " Dan Williams
2015-03-26 14:35     ` Christoph Hellwig
2015-03-26 21:37       ` Ross Zwisler
2015-03-26 14:52     ` Boaz Harrosh
2015-03-26 15:59       ` Dan Williams
2015-03-26  8:32 ` [PATCH 2/3] x86: add a is_e820_ram() helper Christoph Hellwig
2015-03-26  9:02   ` Ingo Molnar
2015-03-26  9:34     ` Christoph Hellwig
2015-03-26 10:04       ` Ingo Molnar
2015-03-26 10:19         ` Christoph Hellwig
2015-03-26 10:28           ` Ingo Molnar
2015-03-26 10:29             ` Christoph Hellwig
2015-03-26 15:49       ` Boaz Harrosh
2015-03-26 16:02         ` [Linux-nvdimm] " Dan Williams
2015-03-26 16:07           ` Boaz Harrosh
2015-03-26 16:43         ` Christoph Hellwig
2015-03-26 18:46           ` Elliott, Robert (Server Storage)
2015-03-26 19:25             ` [Linux-nvdimm] " Dan Williams
2015-03-26 20:53           ` Ross Zwisler
2015-03-26 22:59       ` Yinghai Lu
2015-03-27  8:10         ` Christoph Hellwig
2015-03-26  8:32 ` [PATCH 3/3] x86: add support for the non-standard protected e820 type Christoph Hellwig
2015-03-26 16:57 ` another pmem variant V2 Boaz Harrosh
2015-03-26 17:02   ` [PATCH] SQUASHME: Streamline pmem.c Boaz Harrosh
2015-03-26 17:23     ` Christoph Hellwig
2015-03-26 22:17     ` Ross Zwisler
2015-03-26 22:22     ` Ross Zwisler
2015-03-26 23:31     ` [Linux-nvdimm] " Dan Williams
2015-03-31 13:44       ` Boaz Harrosh
2015-03-26 17:18   ` another pmem variant V2 Christoph Hellwig
2015-03-26 17:31     ` Boaz Harrosh
2015-03-26 18:38       ` Christoph Hellwig
2015-03-31  9:25   ` Christoph Hellwig
2015-03-31 10:25     ` Boaz Harrosh
2015-03-31 10:31       ` Boaz Harrosh
2015-03-31 14:21       ` [RFC] SQUASHME: pmem: Split up pmem_probe from pmem_alloc Boaz Harrosh
2015-03-31 16:10         ` Christoph Hellwig
2015-03-31 16:08       ` another pmem variant V2 Christoph Hellwig
2015-03-31 13:18     ` [SQUASHME 0/6] Streamline of Initial pmem submission Boaz Harrosh
2015-03-31 13:23       ` [PATCH 1/6] SQUASHME: Don't let e820_PMEM sections Boaz Harrosh
2015-03-31 17:16         ` [Linux-nvdimm] " Brooks, Adam J
2015-03-31 13:24       ` [PATCH 2/6] SQUASHME: pmem: Remove getgeo Boaz Harrosh
2015-03-31 13:25       ` Boaz Harrosh [this message]
2015-03-31 13:27       ` [PATCH 4/6] SQUSHME: pmem: Micro cleaning Boaz Harrosh
2015-03-31 15:17         ` [Linux-nvdimm] " Dan Williams
2015-03-31 15:24           ` Boaz Harrosh
2015-03-31 15:30             ` Dan Williams
2015-03-31 15:43               ` Boaz Harrosh
2015-03-31 19:40                 ` Matthew Wilcox
2015-03-31 13:28       ` [PATCH 5/6] SQUASHME: pmem: Remove SECTOR_SHIFT Boaz Harrosh
2015-03-31 13:33       ` [PATCH 6/6] SQUASHME: pmem: Remove "... based on brd.c" + Copyright Boaz Harrosh
2015-03-31 15:14     ` another pmem variant V2 Boaz Harrosh
2015-03-31 16:16       ` Christoph Hellwig
2015-03-31 16:44         ` Ingo Molnar
2015-03-31 17:24           ` Christoph Hellwig
2015-03-31 17:33             ` [Linux-nvdimm] " Dan Williams
2015-04-01  7:50               ` Ingo Molnar
2015-04-01  8:06                 ` Boaz Harrosh
2015-04-01 12:49         ` Boaz Harrosh
2015-03-31 22:11 ` Elliott, Robert (Server Storage)
2015-04-01  7:26   ` Christoph Hellwig
2015-04-02 15:11     ` Elliott, Robert (Server Storage)
2015-04-02 16:41       ` Christoph Hellwig
2015-04-02 18:03         ` Ingo Molnar
2015-04-02 18:03           ` Ingo Molnar
2015-04-01 19:33 ` Elliott, Robert (Server Storage)
2015-04-02  9:37   ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=551AA04F.60209@plexistor.com \
    --to=boaz@plexistor.com \
    --cc=axboe@kernel.dk \
    --cc=hch@lst.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@ml01.01.org \
    --cc=ross.zwisler@linux.intel.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.