6:48 p.m.
When trying to see whether current nd_region intersects with others, we
have already calculated the *size* to be expanded to SECTION size.
So just pass size is enough.
Signed-off-by: Wei Yang <richardw.yang(a)linux.intel.com>
---
drivers/nvdimm/pfn_devs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
index becf0bb481b3..5eca050b3660 100644
--- a/drivers/nvdimm/pfn_devs.c
+++ b/drivers/nvdimm/pfn_devs.c
@@ -686,7 +686,7 @@ static void trim_pfn_device(struct nd_pfn *nd_pfn, u32 *start_pad, u32
*end_trun
if (region_intersects(start, size, IORESOURCE_SYSTEM_RAM,
IORES_DESC_NONE) == REGION_MIXED
|| !IS_ALIGNED(end, nd_pfn->align)
- || nd_region_conflict(nd_region, start, size + adjust))
+ || nd_region_conflict(nd_region, start, size))
*end_trunc = end - phys_pmem_align_down(nd_pfn, end);
}
--
2.19.1
6:48 p.m.
New subject: [PATCH 2/2] libnvdimm, pfn: use PAGE_SIZE to calculate npfns
There are two places to calculate npfns in nd_pfn_init(), while they use
difference size to calculate.
Use PAGE_SIZE would be more proper for calculation.
Signed-off-by: Wei Yang <richardw.yang(a)linux.intel.com>
---
drivers/nvdimm/pfn_devs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
index 5eca050b3660..383f01bedd40 100644
--- a/drivers/nvdimm/pfn_devs.c
+++ b/drivers/nvdimm/pfn_devs.c
@@ -770,7 +770,7 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn)
return -ENXIO;
}
- npfns = (size - offset - start_pad - end_trunc) / SZ_4K;
+ npfns = (size - offset - start_pad - end_trunc) / PAGE_SIZE;
pfn_sb->mode = cpu_to_le32(nd_pfn->mode);
pfn_sb->dataoff = cpu_to_le64(offset);
pfn_sb->npfns = cpu_to_le64(npfns);
--
2.19.1
5:27 p.m.
New subject: [PATCH 2/2] libnvdimm, pfn: use PAGE_SIZE to calculate npfns
On Mon, Jan 21, 2019 at 6:49 PM Wei Yang <richardw.yang(a)linux.intel.com> wrote:
There are two places to calculate npfns in nd_pfn_init(), while they use
difference size to calculate.
Use PAGE_SIZE would be more proper for calculation.
No, this would make the kernel have different output based on
PAGE_SIZE. It should be possible for a PAGE_SIZE==64K system to create
a valid info-block for PAGE_SIZE==4K system. This would need to encode
the PAGE_SIZE in the info-block if it were to ever support non-4K
PAGE_SIZE systems.
Another consideration is that a PAGE_SIZE==4K infoblock is compatible
with a PAGE_SIZE==64K system. All that happens is that the memmap
reserve area is oversized and portions go unused. The reverse is not
true.
10:40 p.m.
New subject: [PATCH 2/2] libnvdimm, pfn: use PAGE_SIZE to calculate npfns
On Tue, Jan 22, 2019 at 05:27:29PM -0800, Dan Williams wrote:
On Mon, Jan 21, 2019 at 6:49 PM Wei Yang
<richardw.yang(a)linux.intel.com> wrote:
>
> There are two places to calculate npfns in nd_pfn_init(), while they use
> difference size to calculate.
>
> Use PAGE_SIZE would be more proper for calculation.
No, this would make the kernel have different output based on
PAGE_SIZE. It should be possible for a PAGE_SIZE==64K system to create
a valid info-block for PAGE_SIZE==4K system. This would need to encode
the PAGE_SIZE in the info-block if it were to ever support non-4K
PAGE_SIZE systems.
I am confused.
Generally, npfns is used in two functions: nd_pfn_init() and
__nvdimm_setup_pfn().
The code flow looks like this:
nvdimm_setup_pfn()
nd_pfn_init()
npfns = SECTION_ALIGN(trim_size / PAGE_SIZE)
offset = start + npfns * page_struct
npfns = (trim_size - offset) / SZ_4K
pfn_sb->npfns
__nvdimm_setup_pfn()
nd_pfn->npfns = pfn_sb->npfns
or
nd_pfn->npfns = (trim_size - offset) / PAGE_SIZE
My questions are:
1. offset = start + npfns * page_struct
This means the number of page struct we reserve in meta-data area is
calculated with PAGE_SIZE page.
But we set pfn_sb->npfns = (trim_size - offset) / SZ_4K, which
means we tell hardware the number of pages is calculated with 4K size.
Would this be a conflict? Or at least we need to reserve more meta-data
area to hold page struct?
2. When mode == PFN_MODE_PMEM and PAGE_SIZE == 64K,
nd_pfn->pfn_sb->npfns is sure to be bigger than nd_pfn->npfns. Because
nd_pfn->pfn_sb->npfns = (trim_size - offset) / 4K
nd_pfn->npfns = (trim_size - offset) / 64K
If we are sure for the final result, why we would like to have this
calculation again?
Another consideration is that a PAGE_SIZE==4K infoblock is compatible
with a PAGE_SIZE==64K system. All that happens is that the memmap
reserve area is oversized and portions go unused. The reverse is not
true.
The oversized memap reserve area is SECTION size aligned? If so, it looks we
took that into consideration when we calculate offset.
--
Wei Yang
Help you, Help me
5:28 p.m.
On Mon, Jan 21, 2019 at 6:49 PM Wei Yang <richardw.yang(a)linux.intel.com> wrote:
When trying to see whether current nd_region intersects with others, we
have already calculated the *size* to be expanded to SECTION size.
So just pass size is enough.
Signed-off-by: Wei Yang <richardw.yang(a)linux.intel.com>
---
drivers/nvdimm/pfn_devs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
index becf0bb481b3..5eca050b3660 100644
--- a/drivers/nvdimm/pfn_devs.c
+++ b/drivers/nvdimm/pfn_devs.c
@@ -686,7 +686,7 @@ static void trim_pfn_device(struct nd_pfn *nd_pfn, u32 *start_pad,
u32 *end_trun
if (region_intersects(start, size, IORESOURCE_SYSTEM_RAM,
IORES_DESC_NONE) == REGION_MIXED
|| !IS_ALIGNED(end, nd_pfn->align)
- || nd_region_conflict(nd_region, start, size + adjust))
+ || nd_region_conflict(nd_region, start, size))
Good catch, thanks. I fixed up the changelog a bit and applied this:
libnvdimm, pfn: Fix over-trim in trim_pfn_device()
When trying to see whether current nd_region intersects with others,
trim_pfn_device() has already calculated the *size* to be expanded to
SECTION size.
Do not double append 'adjust' to 'size' when calculating whether the
end
of a region collides with the next pmem region.
Fixes: ae86cbfef381 "libnvdimm, pfn: Pad pfn namespaces relative
to other regions"
Cc: <stable(a)vger.kernel.org>
Signed-off-by: Wei Yang <richardw.yang(a)linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams(a)intel.com>
6:38 p.m.
On Tue, Jan 22, 2019 at 05:28:39PM -0800, Dan Williams wrote:
On Mon, Jan 21, 2019 at 6:49 PM Wei Yang
<richardw.yang(a)linux.intel.com> wrote:
>
> When trying to see whether current nd_region intersects with others, we
> have already calculated the *size* to be expanded to SECTION size.
>
> So just pass size is enough.
>
> Signed-off-by: Wei Yang <richardw.yang(a)linux.intel.com>
> ---
> drivers/nvdimm/pfn_devs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
> index becf0bb481b3..5eca050b3660 100644
> --- a/drivers/nvdimm/pfn_devs.c
> +++ b/drivers/nvdimm/pfn_devs.c
> @@ -686,7 +686,7 @@ static void trim_pfn_device(struct nd_pfn *nd_pfn, u32
*start_pad, u32 *end_trun
> if (region_intersects(start, size, IORESOURCE_SYSTEM_RAM,
> IORES_DESC_NONE) == REGION_MIXED
> || !IS_ALIGNED(end, nd_pfn->align)
> - || nd_region_conflict(nd_region, start, size + adjust))
> + || nd_region_conflict(nd_region, start, size))
Good catch, thanks. I fixed up the changelog a bit and applied this:
libnvdimm, pfn: Fix over-trim in trim_pfn_device()
When trying to see whether current nd_region intersects with others,
trim_pfn_device() has already calculated the *size* to be expanded to
SECTION size.
Do not double append 'adjust' to 'size' when calculating whether the
end
of a region collides with the next pmem region.
Looks much better :-)
Thanks
Fixes: ae86cbfef381 "libnvdimm, pfn: Pad pfn namespaces relative
to other regions"
Cc: <stable(a)vger.kernel.org>
Signed-off-by: Wei Yang <richardw.yang(a)linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams(a)intel.com>
--
Wei Yang
Help you, Help me
5:27 p.m.
On Tue, Jan 22, 2019 at 05:28:39PM -0800, Dan Williams wrote:
On Mon, Jan 21, 2019 at 6:49 PM Wei Yang
<richardw.yang(a)linux.intel.com> wrote:
>
> When trying to see whether current nd_region intersects with others, we
> have already calculated the *size* to be expanded to SECTION size.
>
> So just pass size is enough.
>
> Signed-off-by: Wei Yang <richardw.yang(a)linux.intel.com>
> ---
> drivers/nvdimm/pfn_devs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
> index becf0bb481b3..5eca050b3660 100644
> --- a/drivers/nvdimm/pfn_devs.c
> +++ b/drivers/nvdimm/pfn_devs.c
> @@ -686,7 +686,7 @@ static void trim_pfn_device(struct nd_pfn *nd_pfn, u32
*start_pad, u32 *end_trun
> if (region_intersects(start, size, IORESOURCE_SYSTEM_RAM,
> IORES_DESC_NONE) == REGION_MIXED
> || !IS_ALIGNED(end, nd_pfn->align)
> - || nd_region_conflict(nd_region, start, size + adjust))
> + || nd_region_conflict(nd_region, start, size))
Hi, Dan,
I got a question about the trim on start.
We check the alignment of nd_pfn->align on end, while we don't do this for
start. I lost why we would like to have this behavior.
Would we align start with nd_pfn->align too?
Good catch, thanks. I fixed up the changelog a bit and applied this:
libnvdimm, pfn: Fix over-trim in trim_pfn_device()
When trying to see whether current nd_region intersects with others,
trim_pfn_device() has already calculated the *size* to be expanded to
SECTION size.
Do not double append 'adjust' to 'size' when calculating whether the
end
of a region collides with the next pmem region.
Fixes: ae86cbfef381 "libnvdimm, pfn: Pad pfn namespaces relative
to other regions"
Cc: <stable(a)vger.kernel.org>
Signed-off-by: Wei Yang <richardw.yang(a)linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams(a)intel.com>
--
Wei Yang
Help you, Help me
476
days inactive
498
days old
6 comments
2 participants
participants (2)
-
Dan Williams -
Wei Yang