| Message ID | 151034311187.59853.17265892705257743788.stgit@djiang5-desk3.ch.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Nov 10, 2017 at 11:45 AM, Dave Jiang <dave.jiang@intel.com> wrote: > The size for mmap needs to be aligned to the region alignment. Add helper > funciton to determine the actual size to be mmap'd. > > Signed-off-by: Dave Jiang <dave.jiang@intel.com> > --- > daxctl/io.c | 23 ++++++++++++++++++++++- > 1 file changed, 22 insertions(+), 1 deletion(-) > > diff --git a/daxctl/io.c b/daxctl/io.c > index 2f8cb4a..97a4169 100644 > --- a/daxctl/io.c > +++ b/daxctl/io.c > @@ -80,9 +80,26 @@ static bool is_stdinout(struct io_dev *io_dev) > io_dev->fd == STDOUT_FILENO) ? true : false; > } > > +static int get_mmap_size(struct io_dev *io_dev, size_t size, size_t *map_size) > +{ > + unsigned long align; > + > + align = ndctl_dax_get_align(io_dev->dax); > + if (align == ULLONG_MAX) > + return -ERANGE; > + > + if (size <= align) > + *map_size = align; > + else > + *map_size = (size / align) * align; We have the ALIGN() macro in util/size.h for this. > + > + return 0; > +} > + > static int setup_device(struct io_dev *io_dev, size_t size) > { > int flags, rc; > + size_t map_size; > > if (is_stdinout(io_dev)) > return 0; > @@ -104,8 +121,12 @@ static int setup_device(struct io_dev *io_dev, size_t size) > if (!io_dev->is_dax) > return 0; > > + rc = get_mmap_size(io_dev, size, &map_size); > + if (rc < 0) > + return rc; > + > 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, map_size, flags, MAP_SHARED, io_dev->fd, 0); Why is offset 0 here? I would expect it to come from the command line arguments and also use ALIGN_DOWN() to align to first aligned offset before the target offset.
On Sun, Nov 12, 2017 at 10:10 AM, Dan Williams <dan.j.williams@intel.com> wrote: > On Fri, Nov 10, 2017 at 11:45 AM, Dave Jiang <dave.jiang@intel.com> wrote: >> The size for mmap needs to be aligned to the region alignment. Add helper >> funciton to determine the actual size to be mmap'd. >> >> Signed-off-by: Dave Jiang <dave.jiang@intel.com> >> --- >> daxctl/io.c | 23 ++++++++++++++++++++++- >> 1 file changed, 22 insertions(+), 1 deletion(-) >> >> diff --git a/daxctl/io.c b/daxctl/io.c >> index 2f8cb4a..97a4169 100644 >> --- a/daxctl/io.c >> +++ b/daxctl/io.c >> @@ -80,9 +80,26 @@ static bool is_stdinout(struct io_dev *io_dev) >> io_dev->fd == STDOUT_FILENO) ? true : false; >> } >> >> +static int get_mmap_size(struct io_dev *io_dev, size_t size, size_t *map_size) >> +{ >> + unsigned long align; >> + >> + align = ndctl_dax_get_align(io_dev->dax); >> + if (align == ULLONG_MAX) >> + return -ERANGE; >> + >> + if (size <= align) >> + *map_size = align; >> + else >> + *map_size = (size / align) * align; > > We have the ALIGN() macro in util/size.h for this. > >> + >> + return 0; >> +} >> + >> static int setup_device(struct io_dev *io_dev, size_t size) >> { >> int flags, rc; >> + size_t map_size; >> >> if (is_stdinout(io_dev)) >> return 0; >> @@ -104,8 +121,12 @@ static int setup_device(struct io_dev *io_dev, size_t size) >> if (!io_dev->is_dax) >> return 0; >> >> + rc = get_mmap_size(io_dev, size, &map_size); >> + if (rc < 0) >> + return rc; >> + >> 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, map_size, flags, MAP_SHARED, io_dev->fd, 0); > > Why is offset 0 here? I would expect it to come from the command line > arguments and also use ALIGN_DOWN() to align to first aligned offset > before the target offset. I think we need some 'daxctl io' unit tests to catch cases like this.
diff --git a/daxctl/io.c b/daxctl/io.c index 2f8cb4a..97a4169 100644 --- a/daxctl/io.c +++ b/daxctl/io.c @@ -80,9 +80,26 @@ static bool is_stdinout(struct io_dev *io_dev) io_dev->fd == STDOUT_FILENO) ? true : false; } +static int get_mmap_size(struct io_dev *io_dev, size_t size, size_t *map_size) +{ + unsigned long align; + + align = ndctl_dax_get_align(io_dev->dax); + if (align == ULLONG_MAX) + return -ERANGE; + + if (size <= align) + *map_size = align; + else + *map_size = (size / align) * align; + + return 0; +} + static int setup_device(struct io_dev *io_dev, size_t size) { int flags, rc; + size_t map_size; if (is_stdinout(io_dev)) return 0; @@ -104,8 +121,12 @@ static int setup_device(struct io_dev *io_dev, size_t size) if (!io_dev->is_dax) return 0; + rc = get_mmap_size(io_dev, size, &map_size); + if (rc < 0) + return rc; + 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, map_size, flags, MAP_SHARED, io_dev->fd, 0); if (io_dev->mmap == MAP_FAILED) { rc = -errno; perror("mmap");
The size for mmap needs to be aligned to the region alignment. Add helper funciton to determine the actual size to be mmap'd. Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- daxctl/io.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)