From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S939268AbdD0Nph (ORCPT ); Thu, 27 Apr 2017 09:45:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50130 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754102AbdD0Np2 (ORCPT ); Thu, 27 Apr 2017 09:45:28 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 88342C05AD7F Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=jmoyer@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 88342C05AD7F From: Jeff Moyer To: Dan Williams Cc: "linux-nvdimm\@lists.01.org" , Masayoshi Mizuma , Linux ACPI , "linux-kernel\@vger.kernel.org" Subject: Re: [PATCH v3] libnvdimm, region: sysfs trigger for nvdimm_flush() References: <149307780135.7155.11108531648914675756.stgit@dwillia2-desk3.amr.corp.intel.com> <149315140303.23340.14688142799059150805.stgit@dwillia2-desk3.amr.corp.intel.com> X-PGP-KeyID: 1F78E1B4 X-PGP-CertKey: F6FE 280D 8293 F72C 65FD 5A58 1FF8 A7CA 1F78 E1B4 X-PCLoadLetter: What the f**k does that mean? Date: Thu, 27 Apr 2017 09:45:25 -0400 In-Reply-To: (Dan Williams's message of "Wed, 26 Apr 2017 16:00:04 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Thu, 27 Apr 2017 13:45:27 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Dan Williams writes: > On Wed, Apr 26, 2017 at 1:38 PM, Jeff Moyer wrote: >> Dan Williams writes: >> >>> 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. These requests are typically associated with filesystem >>> metadata updates. However, when a namespace is in device-dax mode, >>> userspace (think database metadata) needs another path to perform the >>> same flushing. In other words this is not required to make data >>> persistent, but in the case of metadata it allows for a smaller failure >>> domain in the unlikely event of an ADR failure. >>> >>> The new 'flush' attribute is visible when the individual DIMMs backing a >>> given interleave-set are described by platform firmware. In ACPI terms >>> this is "NVDIMM Region Mapping Structures" and associated "Flush Hint >>> Address Structures". Reads return "1" if the region supports triggering >>> WPQ flushes on all DIMMs. Reads return "0" the flush operation is a >>> platform nop, and in that case the attribute is read-only. >> >> I can make peace with exposing this to userspace, though I am mostly >> against its use. However, sysfs feels like the wrong interface. >> Believe it or not, I'd rather see this implemented as an ioctl. >> >> This isn't a NACK, it's me giving my opinion. Do with it what you will. > > I hate ioctls with a burning passion so I can't get on board with that > change, but perhaps the sentiment behind it is that this is too > visible and too attractive being called "flush" in sysfs? Would a name > more specific to the mechanism make it more palatable? Like > "flush_hint_trigger" or "wpq_drain"? The sentiment is that programs shouldn't have to grovel around in sysfs to do stuff related to an open file descriptor or mapping. I don't take issue with the name. I do worry that something like 'wpq_drain' may be too platform specific, though. The NVM Programming Model specification is going to call this "deep flush", so maybe that will give you some inspiration if you do want to change the name. Cheers, Jeff