diff mbox

[v2,1/2] ndctl: daxctl: fix mmap size

Message ID 151059717963.29832.3262165776997767425.stgit@djiang5-desk3.ch.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dave Jiang Nov. 13, 2017, 6:19 p.m. UTC
The size for mmap needs to be aligned to the region alignment. Fix the
mapping size so that it satisfy the alignment requirement.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 daxctl/io.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Dan Williams Nov. 13, 2017, 11:40 p.m. UTC | #1
On Mon, Nov 13, 2017 at 10:19 AM, Dave Jiang <dave.jiang@intel.com> wrote:
> The size for mmap needs to be aligned to the region alignment. Fix the
> mapping size so that it satisfy the alignment requirement.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  daxctl/io.c |   12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/daxctl/io.c b/daxctl/io.c
> index 2f8cb4a..e739d0e 100644
> --- a/daxctl/io.c
> +++ b/daxctl/io.c
> @@ -55,6 +55,7 @@ struct io_dev {
>         struct ndctl_region *region;
>         struct ndctl_dax *dax;
>         uint64_t size;
> +       uint64_t mmap_size;
>  };
>
>  static struct {
> @@ -83,6 +84,7 @@ static bool is_stdinout(struct io_dev *io_dev)
>  static int setup_device(struct io_dev *io_dev, size_t size)
>  {
>         int flags, rc;
> +       unsigned long align;
>
>         if (is_stdinout(io_dev))
>                 return 0;
> @@ -104,8 +106,14 @@ static int setup_device(struct io_dev *io_dev, size_t size)
>         if (!io_dev->is_dax)
>                 return 0;
>
> +       align = ndctl_dax_get_align(io_dev->dax);
> +       if (align == ULLONG_MAX)

Isn't this always false because 'align' is 'unsigned long', not
'unsigned long long'?

Otherwise the rest of this looks good.
Dave Jiang Nov. 13, 2017, 11:49 p.m. UTC | #2
On 11/13/2017 04:40 PM, Dan Williams wrote:
> On Mon, Nov 13, 2017 at 10:19 AM, Dave Jiang <dave.jiang@intel.com> wrote:
>> The size for mmap needs to be aligned to the region alignment. Fix the
>> mapping size so that it satisfy the alignment requirement.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>>  daxctl/io.c |   12 +++++++++++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/daxctl/io.c b/daxctl/io.c
>> index 2f8cb4a..e739d0e 100644
>> --- a/daxctl/io.c
>> +++ b/daxctl/io.c
>> @@ -55,6 +55,7 @@ struct io_dev {
>>         struct ndctl_region *region;
>>         struct ndctl_dax *dax;
>>         uint64_t size;
>> +       uint64_t mmap_size;
>>  };
>>
>>  static struct {
>> @@ -83,6 +84,7 @@ static bool is_stdinout(struct io_dev *io_dev)
>>  static int setup_device(struct io_dev *io_dev, size_t size)
>>  {
>>         int flags, rc;
>> +       unsigned long align;
>>
>>         if (is_stdinout(io_dev))
>>                 return 0;
>> @@ -104,8 +106,14 @@ static int setup_device(struct io_dev *io_dev, size_t size)
>>         if (!io_dev->is_dax)
>>                 return 0;
>>
>> +       align = ndctl_dax_get_align(io_dev->dax);
>> +       if (align == ULLONG_MAX)
> 
> Isn't this always false because 'align' is 'unsigned long', not
> 'unsigned long long'?
> 
> Otherwise the rest of this looks good.
> 

Ok I'll send out v3 if you don't have issues with the other patch.
diff mbox

Patch

diff --git a/daxctl/io.c b/daxctl/io.c
index 2f8cb4a..e739d0e 100644
--- a/daxctl/io.c
+++ b/daxctl/io.c
@@ -55,6 +55,7 @@  struct io_dev {
 	struct ndctl_region *region;
 	struct ndctl_dax *dax;
 	uint64_t size;
+	uint64_t mmap_size;
 };
 
 static struct {
@@ -83,6 +84,7 @@  static bool is_stdinout(struct io_dev *io_dev)
 static int setup_device(struct io_dev *io_dev, size_t size)
 {
 	int flags, rc;
+	unsigned long align;
 
 	if (is_stdinout(io_dev))
 		return 0;
@@ -104,8 +106,14 @@  static int setup_device(struct io_dev *io_dev, size_t size)
 	if (!io_dev->is_dax)
 		return 0;
 
+	align = ndctl_dax_get_align(io_dev->dax);
+	if (align == ULLONG_MAX)
+		return -ERANGE;
+
+	io_dev->mmap_size = ALIGN(size, align);
 	flags = (io_dev->direction == IO_READ) ? PROT_READ : PROT_WRITE;
-	io_dev->mmap = mmap(NULL, size, flags, MAP_SHARED, io_dev->fd, 0);
+	io_dev->mmap = mmap(NULL, io_dev->mmap_size, flags,
+			MAP_SHARED, io_dev->fd, 0);
 	if (io_dev->mmap == MAP_FAILED) {
 		rc = -errno;
 		perror("mmap");
@@ -501,6 +509,8 @@  static void cleanup(void)
 	for (i = 0; i < 2; i++) {
 		if (is_stdinout(&io.dev[i]))
 			continue;
+		if (io.dev[i].mmap_size)
+			munmap(io.dev[i].mmap, io.dev[i].mmap_size);
 		close(io.dev[i].fd);
 	}
 }