Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S971463AbdDXHEl (ORCPT ); Mon, 24 Apr 2017 03:04:41 -0400 Received: from mail-wm0-f65.google.com ([74.125.82.65]:33300 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1165887AbdDXHE0 (ORCPT ); Mon, 24 Apr 2017 03:04:26 -0400 MIME-Version: 1.0 In-Reply-To: <20170424143104.4517.61FB500B@jp.fujitsu.com> References: <149281853758.22910.2919981036906495309.stgit@dwillia2-desk3.amr.corp.intel.com> <20170424143104.4517.61FB500B@jp.fujitsu.com> From: Dan Williams Date: Mon, 24 Apr 2017 00:04:23 -0700 X-Google-Sender-Auth: B5DCajVXCRL_qNSDSyvgLNAFHeI Message-ID: Subject: Re: [PATCH] libnvdimm, region: sysfs trigger for nvdimm_flush() To: Masayoshi Mizuma Cc: "linux-nvdimm@lists.01.org" , Linux ACPI , Linux Kernel Mailing List Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2208 Lines: 57 On Sun, Apr 23, 2017 at 10:31 PM, Masayoshi Mizuma wrote: > On Fri, 21 Apr 2017 16:48:57 -0700 Dan Williams wrote: >> The nvdimm_flush() mechanism helps to reduce the impact of an ADR >> (asynchronous-dimm-refresh) failure. The ADR mechanism handles flushing >> platform WPQ (write-pending-queue) buffers when power is removed. The >> nvdimm_flush() mechanism performs that same function on-demand. >> >> When a pmem namespace is associated with a block device, an >> nvdimm_flush() is triggered with every block-layer REQ_FUA, or REQ_FLUSH >> request. However, when a namespace is in device-dax mode, or namespaces >> are disabled, userspace needs another path. >> >> The new 'flush' attribute is visible when it can be determined that the >> interleave-set either does, or does not have DIMMs that expose WPQ-flush >> addresses, "flush-hints" in ACPI NFIT terminology. It returns "1" and >> flushes DIMMs, or returns "0" the flush operation is a platform nop. >> >> Signed-off-by: Dan Williams >> --- >> drivers/nvdimm/region_devs.c | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c >> index 8de5a04644a1..3495b4c23941 100644 >> --- a/drivers/nvdimm/region_devs.c >> +++ b/drivers/nvdimm/region_devs.c >> @@ -255,6 +255,19 @@ static ssize_t size_show(struct device *dev, >> } >> static DEVICE_ATTR_RO(size); >> >> +static ssize_t flush_show(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + struct nd_region *nd_region = to_nd_region(dev); >> + >> + if (nvdimm_has_flush(nd_region)) { > > nvdimm_has_flush() also returns as -ENXIO, so > > if (nvdimm_has_flush(nd_region) == 1) If it returns -ENXIO then region_visible() will hide the attribute. > >> + nvdimm_flush(nd_region); >> + return sprintf(buf, "1\n"); >> + } >> + return sprintf(buf, "0\n"); >> +} >> +static DEVICE_ATTR_RO(flush); >> + > > I think separating show and store is better because > users may only check wheter the device has the flush capability or not. Makes sense, I'll separate. Thanks for the review.