From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 In-Reply-To: <1430504355.23761.153.camel@misato.fc.hp.com> References: <20150428181203.35812.60474.stgit@dwillia2-desk3.amr.corp.intel.com> <20150428182439.35812.81191.stgit@dwillia2-desk3.amr.corp.intel.com> <1430502493.23761.145.camel@misato.fc.hp.com> <1430504355.23761.153.camel@misato.fc.hp.com> Date: Fri, 1 May 2015 11:43:12 -0700 Message-ID: Subject: Re: [Linux-nvdimm] [PATCH v2 05/20] libnd, nd_acpi: dimm/memory-devices From: Dan Williams Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org To: Toshi Kani Cc: "linux-nvdimm@lists.01.org" , Neil Brown , Greg KH , "Rafael J. Wysocki" , Robert Moore , "linux-kernel@vger.kernel.org" , Linux ACPI List-ID: On Fri, May 1, 2015 at 11:19 AM, Toshi Kani wrote: > On Fri, 2015-05-01 at 11:22 -0700, Dan Williams wrote: >> On Fri, May 1, 2015 at 10:48 AM, Toshi Kani wrote: >> > On Tue, 2015-04-28 at 14:24 -0400, Dan Williams wrote: >> >> Register the memory devices described in the nfit as libnd 'dimm' >> >> devices on an nd bus. The kernel assigned device id for dimms is >> >> dynamic. If userspace needs a more static identifier it should consult >> >> a provider-specific attribute. In the case where NFIT is the provider, >> >> the 'nmemX/nfit/handle' or 'nmemX/nfit/serial' attributes may be used >> >> for this purpose. >> > : >> >> + >> >> +static int nd_acpi_register_dimms(struct acpi_nfit_desc *acpi_desc) >> >> +{ >> >> + struct nfit_mem *nfit_mem; >> >> + >> >> + list_for_each_entry(nfit_mem, &acpi_desc->dimms, list) { >> >> + struct nd_dimm *nd_dimm; >> >> + unsigned long flags = 0; >> >> + u32 nfit_handle; >> >> + >> >> + nfit_handle = __to_nfit_memdev(nfit_mem)->nfit_handle; >> >> + nd_dimm = nd_acpi_dimm_by_handle(acpi_desc, nfit_handle); >> >> + if (nd_dimm) { >> >> + /* >> >> + * If for some reason we find multiple DCRs the >> >> + * first one wins >> >> + */ >> >> + dev_err(acpi_desc->dev, "duplicate DCR detected: %s\n", >> >> + nd_dimm_name(nd_dimm)); >> >> + continue; >> >> + } >> >> + >> >> + if (nfit_mem->bdw && nfit_mem->memdev_pmem) >> >> + flags |= NDD_ALIASING; >> > >> > Does this check work for a NVDIMM card which has multiple pmem regions >> > with label info, but does not have any bdw region configured? >> >> If you have multiple pmem regions then you don't have aliasing and >> don't need a label. You'll get an nd_namespace_io per region. >> >> > The code assumes that namespace_pmem (NDD_ALIASING) and namespace_blk >> > have label info. There may be an NVDIMM card with a single blk region >> > without label info. >> >> I'd really like to suggest that labels are only for resolving aliasing >> and that if you have a BLK-only NVDIMM you'll get an automatic >> namespace created the same as a PMEM-only. Partitioning is always >> there to provide sub-divisions of a namespace. The only reason to >> support multiple BLK-namespaces per-region is to give each a different >> sector size. I may eventually need to relent on this position, but >> I'd really like to understand the use case for requiring labels when >> aliasing is not present as it seems like a waste to me. > > By looking at the callers of is_namespace_pmem() and is_namespace_blk(), > such as nd_namespace_label_update(), I am concerned that the namespace > types are also used for indicating the presence a label. Is it OK for > nd_namespace_label_update() to do nothing when there is no aliasing? > >> > Instead of using the namespace types to assume the label info, how about >> > adding a flag to indicate the presence of the label info? This avoids >> > the separation of namespace_io and namespace_pmem for the same pmem >> > driver. >> >> To what benefit? > > Why do they need to be separated? Having alias or not should not make > the pmem namespace different. The intent is to maximize the number of devices that can be immediately attached to nd_pmem and nd_blk without user intervention. nd_namespace_io is a pmem namespace where the boundaries are 100% described by the NFIT / parent-region. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751879AbbEASnS (ORCPT ); Fri, 1 May 2015 14:43:18 -0400 Received: from mail-wg0-f47.google.com ([74.125.82.47]:33718 "EHLO mail-wg0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751722AbbEASnP (ORCPT ); Fri, 1 May 2015 14:43:15 -0400 MIME-Version: 1.0 In-Reply-To: <1430504355.23761.153.camel@misato.fc.hp.com> References: <20150428181203.35812.60474.stgit@dwillia2-desk3.amr.corp.intel.com> <20150428182439.35812.81191.stgit@dwillia2-desk3.amr.corp.intel.com> <1430502493.23761.145.camel@misato.fc.hp.com> <1430504355.23761.153.camel@misato.fc.hp.com> Date: Fri, 1 May 2015 11:43:12 -0700 Message-ID: Subject: Re: [Linux-nvdimm] [PATCH v2 05/20] libnd, nd_acpi: dimm/memory-devices From: Dan Williams To: Toshi Kani Cc: "linux-nvdimm@lists.01.org" , Neil Brown , Greg KH , "Rafael J. Wysocki" , Robert Moore , "linux-kernel@vger.kernel.org" , Linux ACPI Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, May 1, 2015 at 11:19 AM, Toshi Kani wrote: > On Fri, 2015-05-01 at 11:22 -0700, Dan Williams wrote: >> On Fri, May 1, 2015 at 10:48 AM, Toshi Kani wrote: >> > On Tue, 2015-04-28 at 14:24 -0400, Dan Williams wrote: >> >> Register the memory devices described in the nfit as libnd 'dimm' >> >> devices on an nd bus. The kernel assigned device id for dimms is >> >> dynamic. If userspace needs a more static identifier it should consult >> >> a provider-specific attribute. In the case where NFIT is the provider, >> >> the 'nmemX/nfit/handle' or 'nmemX/nfit/serial' attributes may be used >> >> for this purpose. >> > : >> >> + >> >> +static int nd_acpi_register_dimms(struct acpi_nfit_desc *acpi_desc) >> >> +{ >> >> + struct nfit_mem *nfit_mem; >> >> + >> >> + list_for_each_entry(nfit_mem, &acpi_desc->dimms, list) { >> >> + struct nd_dimm *nd_dimm; >> >> + unsigned long flags = 0; >> >> + u32 nfit_handle; >> >> + >> >> + nfit_handle = __to_nfit_memdev(nfit_mem)->nfit_handle; >> >> + nd_dimm = nd_acpi_dimm_by_handle(acpi_desc, nfit_handle); >> >> + if (nd_dimm) { >> >> + /* >> >> + * If for some reason we find multiple DCRs the >> >> + * first one wins >> >> + */ >> >> + dev_err(acpi_desc->dev, "duplicate DCR detected: %s\n", >> >> + nd_dimm_name(nd_dimm)); >> >> + continue; >> >> + } >> >> + >> >> + if (nfit_mem->bdw && nfit_mem->memdev_pmem) >> >> + flags |= NDD_ALIASING; >> > >> > Does this check work for a NVDIMM card which has multiple pmem regions >> > with label info, but does not have any bdw region configured? >> >> If you have multiple pmem regions then you don't have aliasing and >> don't need a label. You'll get an nd_namespace_io per region. >> >> > The code assumes that namespace_pmem (NDD_ALIASING) and namespace_blk >> > have label info. There may be an NVDIMM card with a single blk region >> > without label info. >> >> I'd really like to suggest that labels are only for resolving aliasing >> and that if you have a BLK-only NVDIMM you'll get an automatic >> namespace created the same as a PMEM-only. Partitioning is always >> there to provide sub-divisions of a namespace. The only reason to >> support multiple BLK-namespaces per-region is to give each a different >> sector size. I may eventually need to relent on this position, but >> I'd really like to understand the use case for requiring labels when >> aliasing is not present as it seems like a waste to me. > > By looking at the callers of is_namespace_pmem() and is_namespace_blk(), > such as nd_namespace_label_update(), I am concerned that the namespace > types are also used for indicating the presence a label. Is it OK for > nd_namespace_label_update() to do nothing when there is no aliasing? > >> > Instead of using the namespace types to assume the label info, how about >> > adding a flag to indicate the presence of the label info? This avoids >> > the separation of namespace_io and namespace_pmem for the same pmem >> > driver. >> >> To what benefit? > > Why do they need to be separated? Having alias or not should not make > the pmem namespace different. The intent is to maximize the number of devices that can be immediately attached to nd_pmem and nd_blk without user intervention. nd_namespace_io is a pmem namespace where the boundaries are 100% described by the NFIT / parent-region.