| Message ID | 167537140762.3268840.2926966718345830138.stgit@dwillia2-xfh.jf.intel.com |
|---|---|
| State | Accepted |
| Commit | 06419eb6e8622baab05601e4c1007642b14dd4ef |
| Headers | show |
| Series | daxctl: Fix memblock enumeration off-by-one | expand |
On 2/3/2023 7:56 AM, Dan Williams wrote: > A memblock is an inclusive memory range. Bound the search by the last > address in the memory block. > > Found by wondering why an offline 32-block (at 128MB == 4GB) range was > reported as 33 blocks with one online. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > daxctl/lib/libdaxctl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c > index 5703992f5b88..d990479d8585 100644 > --- a/daxctl/lib/libdaxctl.c > +++ b/daxctl/lib/libdaxctl.c > @@ -1477,7 +1477,7 @@ static int memblock_in_dev(struct daxctl_memory *mem, const char *memblock) > err(ctx, "%s: Unable to determine resource\n", devname); > return -EACCES; > } > - dev_end = dev_start + daxctl_dev_get_size(dev); > + dev_end = dev_start + daxctl_dev_get_size(dev) - 1; > > memblock_size = daxctl_memory_get_block_size(mem); > if (!memblock_size) { Might this address the bug I reported? Regards - Eliot Moss
Eliot Moss wrote: > On 2/3/2023 7:56 AM, Dan Williams wrote: > > A memblock is an inclusive memory range. Bound the search by the last > > address in the memory block. > > > > Found by wondering why an offline 32-block (at 128MB == 4GB) range was > > reported as 33 blocks with one online. > > > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > > --- > > daxctl/lib/libdaxctl.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c > > index 5703992f5b88..d990479d8585 100644 > > --- a/daxctl/lib/libdaxctl.c > > +++ b/daxctl/lib/libdaxctl.c > > @@ -1477,7 +1477,7 @@ static int memblock_in_dev(struct daxctl_memory *mem, const char *memblock) > > err(ctx, "%s: Unable to determine resource\n", devname); > > return -EACCES; > > } > > - dev_end = dev_start + daxctl_dev_get_size(dev); > > + dev_end = dev_start + daxctl_dev_get_size(dev) - 1; > > > > memblock_size = daxctl_memory_get_block_size(mem); > > if (!memblock_size) { > > Might this address the bug I reported? This one? http://lore.kernel.org/r/558d0ff1-4658-a11b-5a6d-0be0a3a6799c@cs.umass.edu I don't think so, that one seems to have something to do with the file extent layout that causes fs/dax.c to fallback to 4K mappings.
On 2/3/2023 11:11 AM, Dan Williams wrote: > Eliot Moss wrote: >> On 2/3/2023 7:56 AM, Dan Williams wrote: >>> A memblock is an inclusive memory range. Bound the search by the last >>> address in the memory block. >>> >>> Found by wondering why an offline 32-block (at 128MB == 4GB) range was >>> reported as 33 blocks with one online. >>> >>> Signed-off-by: Dan Williams <dan.j.williams@intel.com> >>> --- >>> daxctl/lib/libdaxctl.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c >>> index 5703992f5b88..d990479d8585 100644 >>> --- a/daxctl/lib/libdaxctl.c >>> +++ b/daxctl/lib/libdaxctl.c >>> @@ -1477,7 +1477,7 @@ static int memblock_in_dev(struct daxctl_memory *mem, const char *memblock) >>> err(ctx, "%s: Unable to determine resource\n", devname); >>> return -EACCES; >>> } >>> - dev_end = dev_start + daxctl_dev_get_size(dev); >>> + dev_end = dev_start + daxctl_dev_get_size(dev) - 1; >>> >>> memblock_size = daxctl_memory_get_block_size(mem); >>> if (!memblock_size) { >> >> Might this address the bug I reported? > > This one? > > http://lore.kernel.org/r/558d0ff1-4658-a11b-5a6d-0be0a3a6799c@cs.umass.edu > > I don't think so, that one seems to have something to do with the file > extent layout that causes fs/dax.c to fallback to 4K mappings. That was the one - I think you're right; it's not immediately related. EM
On Thu, 2023-02-02 at 12:56 -0800, Dan Williams wrote: > A memblock is an inclusive memory range. Bound the search by the last > address in the memory block. > > Found by wondering why an offline 32-block (at 128MB == 4GB) range was > reported as 33 blocks with one online. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > daxctl/lib/libdaxctl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Good find! Applied, thanks. > > diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c > index 5703992f5b88..d990479d8585 100644 > --- a/daxctl/lib/libdaxctl.c > +++ b/daxctl/lib/libdaxctl.c > @@ -1477,7 +1477,7 @@ static int memblock_in_dev(struct daxctl_memory *mem, const char *memblock) > err(ctx, "%s: Unable to determine resource\n", devname); > return -EACCES; > } > - dev_end = dev_start + daxctl_dev_get_size(dev); > + dev_end = dev_start + daxctl_dev_get_size(dev) - 1; > > memblock_size = daxctl_memory_get_block_size(mem); > if (!memblock_size) { >
diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c index 5703992f5b88..d990479d8585 100644 --- a/daxctl/lib/libdaxctl.c +++ b/daxctl/lib/libdaxctl.c @@ -1477,7 +1477,7 @@ static int memblock_in_dev(struct daxctl_memory *mem, const char *memblock) err(ctx, "%s: Unable to determine resource\n", devname); return -EACCES; } - dev_end = dev_start + daxctl_dev_get_size(dev); + dev_end = dev_start + daxctl_dev_get_size(dev) - 1; memblock_size = daxctl_memory_get_block_size(mem); if (!memblock_size) {
A memblock is an inclusive memory range. Bound the search by the last address in the memory block. Found by wondering why an offline 32-block (at 128MB == 4GB) range was reported as 33 blocks with one online. Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- daxctl/lib/libdaxctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)