Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752316AbbEAU1O (ORCPT ); Fri, 1 May 2015 16:27:14 -0400 Received: from g1t5424.austin.hp.com ([15.216.225.54]:34228 "EHLO g1t5424.austin.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751314AbbEAU1K (ORCPT ); Fri, 1 May 2015 16:27:10 -0400 Message-ID: <1430510888.23761.185.camel@misato.fc.hp.com> Subject: Re: [Linux-nvdimm] [PATCH v2 05/20] libnd, nd_acpi: dimm/memory-devices From: Toshi Kani To: Dan Williams Cc: "linux-nvdimm@lists.01.org" , Neil Brown , Greg KH , "Rafael J. Wysocki" , Robert Moore , "linux-kernel@vger.kernel.org" , Linux ACPI Date: Fri, 01 May 2015 14:08:08 -0600 In-Reply-To: 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> <1430507711.23761.182.camel@misato.fc.hp.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.10.4 (3.10.4-4.fc20) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4457 Lines: 84 On Fri, 2015-05-01 at 12:38 -0700, Dan Williams wrote: > On Fri, May 1, 2015 at 12:15 PM, Toshi Kani wrote: > > On Fri, 2015-05-01 at 11:43 -0700, Dan Williams wrote: > >> 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? > > > > Did you forget to answer this question? I am not asking to have a > > label. I am asking if the namespace types can handle it correctly. > > Restating the nd_namespace_label_update() example: > > - namespace_io case: Skip, but a label may still exist. Correct? > > - namespace_blk case: Proceed, but blk does not require a label. > > Ah, ok. This is handled by nd_namespace_attr_visible() only labelled > namespaces have writable sysfs attributes. This would need to be > extended for a label-less BLK namespace type. I prefer not to duplicate each namespace type with and without a label, but I am OK as long as the presence of labels is handled properly. Thanks, -Toshi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/