| Message ID | 20190122024810.4448-2-richardw.yang@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] libnvdimm, pfn: use size is enough | expand |
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.
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.
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);
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(-)