* [PATCH 1/2] libnvdimm, pfn: use size is enough
@ 2019-01-22 2:48 Wei Yang
2019-01-22 2:48 ` [PATCH 2/2] libnvdimm, pfn: use PAGE_SIZE to calculate npfns Wei Yang
2019-01-23 1:28 ` [PATCH 1/2] libnvdimm, pfn: use size is enough Dan Williams
0 siblings, 2 replies; 7+ messages in thread
From: Wei Yang @ 2019-01-22 2:48 UTC (permalink / raw)
To: linux-nvdimm; +Cc: zwisler
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@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
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] libnvdimm, pfn: use PAGE_SIZE to calculate npfns
2019-01-22 2:48 [PATCH 1/2] libnvdimm, pfn: use size is enough Wei Yang
@ 2019-01-22 2:48 ` Wei Yang
2019-01-23 1:27 ` Dan Williams
2019-01-23 1:28 ` [PATCH 1/2] libnvdimm, pfn: use size is enough Dan Williams
1 sibling, 1 reply; 7+ messages in thread
From: Wei Yang @ 2019-01-22 2:48 UTC (permalink / raw)
To: linux-nvdimm; +Cc: zwisler
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@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
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] libnvdimm, pfn: use PAGE_SIZE to calculate npfns
2019-01-22 2:48 ` [PATCH 2/2] libnvdimm, pfn: use PAGE_SIZE to calculate npfns Wei Yang
@ 2019-01-23 1:27 ` Dan Williams
2019-01-23 6:40 ` Wei Yang
0 siblings, 1 reply; 7+ messages in thread
From: Dan Williams @ 2019-01-23 1:27 UTC (permalink / raw)
To: Wei Yang; +Cc: Ross Zwisler, linux-nvdimm
On Mon, Jan 21, 2019 at 6:49 PM Wei Yang <richardw.yang@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.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] libnvdimm, pfn: use PAGE_SIZE to calculate npfns
2019-01-23 1:27 ` Dan Williams
@ 2019-01-23 6:40 ` Wei Yang
0 siblings, 0 replies; 7+ messages in thread
From: Wei Yang @ 2019-01-23 6:40 UTC (permalink / raw)
To: Dan Williams; +Cc: Ross Zwisler, linux-nvdimm
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@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
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] libnvdimm, pfn: use size is enough
2019-01-22 2:48 [PATCH 1/2] libnvdimm, pfn: use size is enough Wei Yang
2019-01-22 2:48 ` [PATCH 2/2] libnvdimm, pfn: use PAGE_SIZE to calculate npfns Wei Yang
@ 2019-01-23 1:28 ` Dan Williams
2019-01-23 2:38 ` Wei Yang
2019-02-13 1:27 ` Wei Yang
1 sibling, 2 replies; 7+ messages in thread
From: Dan Williams @ 2019-01-23 1:28 UTC (permalink / raw)
To: Wei Yang; +Cc: Ross Zwisler, linux-nvdimm
On Mon, Jan 21, 2019 at 6:49 PM Wei Yang <richardw.yang@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@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@vger.kernel.org>
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/2] libnvdimm, pfn: use size is enough
2019-01-23 1:28 ` [PATCH 1/2] libnvdimm, pfn: use size is enough Dan Williams
@ 2019-01-23 2:38 ` Wei Yang
2019-02-13 1:27 ` Wei Yang
1 sibling, 0 replies; 7+ messages in thread
From: Wei Yang @ 2019-01-23 2:38 UTC (permalink / raw)
To: Dan Williams; +Cc: Ross Zwisler, linux-nvdimm
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@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@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@vger.kernel.org>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
--
Wei Yang
Help you, Help me
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] libnvdimm, pfn: use size is enough
2019-01-23 1:28 ` [PATCH 1/2] libnvdimm, pfn: use size is enough Dan Williams
2019-01-23 2:38 ` Wei Yang
@ 2019-02-13 1:27 ` Wei Yang
1 sibling, 0 replies; 7+ messages in thread
From: Wei Yang @ 2019-02-13 1:27 UTC (permalink / raw)
To: Dan Williams; +Cc: Ross Zwisler, linux-nvdimm
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@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@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@vger.kernel.org>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
--
Wei Yang
Help you, Help me
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-02-13 1:27 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-22 2:48 [PATCH 1/2] libnvdimm, pfn: use size is enough Wei Yang
2019-01-22 2:48 ` [PATCH 2/2] libnvdimm, pfn: use PAGE_SIZE to calculate npfns Wei Yang
2019-01-23 1:27 ` Dan Williams
2019-01-23 6:40 ` Wei Yang
2019-01-23 1:28 ` [PATCH 1/2] libnvdimm, pfn: use size is enough Dan Williams
2019-01-23 2:38 ` Wei Yang
2019-02-13 1:27 ` Wei Yang
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.