From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 4D56320336AA7 for ; Wed, 6 Jun 2018 11:16:21 -0700 (PDT) Date: Wed, 6 Jun 2018 12:16:19 -0600 From: Ross Zwisler Subject: Re: [PATCH v3 2/4] libnvdimm: unconditionally deep flush on *sync Message-ID: <20180606181619.GA13076@linux.intel.com> References: <20180606164515.25677-1-ross.zwisler@linux.intel.com> <20180606164515.25677-2-ross.zwisler@linux.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-nvdimm-bounces@lists.01.org Sender: "Linux-nvdimm" To: Dan Williams Cc: Linux Kernel Mailing List , linux-nvdimm List-ID: On Wed, Jun 06, 2018 at 10:57:59AM -0700, Dan Williams wrote: > On Wed, Jun 6, 2018 at 9:45 AM, Ross Zwisler > wrote: > > Prior to this commit we would only do a "deep flush" (have nvdimm_flush() > > write to each of the flush hints for a region) in response to an > > msync/fsync/sync call if the nvdimm_has_cache() returned true at the time > > we were setting up the request queue. This happens due to the write cache > > value passed in to blk_queue_write_cache(), which then causes the block > > layer to send down BIOs with REQ_FUA and REQ_PREFLUSH set. We do have a > > "write_cache" sysfs entry for namespaces, i.e.: > > > > /sys/bus/nd/devices/pfn0.1/block/pmem0/dax/write_cache > > > > which can be used to control whether or not the kernel thinks a given > > namespace has a write cache, but this didn't modify the deep flush behavior > > that we set up when the driver was initialized. Instead, it only modified > > whether or not DAX would flush CPU caches via dax_flush() in response to > > *sync calls. > > > > Simplify this by making the *sync deep flush always happen, regardless of > > the write cache setting of a namespace. The DAX CPU cache flushing will > > still be controlled the write_cache setting of the namespace. > > > > Signed-off-by: Ross Zwisler > > Suggested-by: Dan Williams > > Looks, good. I believe we want this one and ["PATCH v3 4/4] libnvdimm: > don't flush power-fail protected CPU caches" marked for -stable and > tagged with: > > Fixes: 5fdf8e5ba566 ("libnvdimm: re-enable deep flush for pmem devices > via fsync()") > > ...any concerns with that? Nope, sounds good. Can you fix that up when you apply, or would it be helpful for me to send another revision with those tags? _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 479AF606DD Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752282AbeFFSQW (ORCPT + 25 others); Wed, 6 Jun 2018 14:16:22 -0400 Received: from mga07.intel.com ([134.134.136.100]:42900 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751812AbeFFSQV (ORCPT ); Wed, 6 Jun 2018 14:16:21 -0400 X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,484,1520924400"; d="scan'208";a="57090536" Date: Wed, 6 Jun 2018 12:16:19 -0600 From: Ross Zwisler To: Dan Williams Cc: Ross Zwisler , Linux Kernel Mailing List , Dave Jiang , linux-nvdimm Subject: Re: [PATCH v3 2/4] libnvdimm: unconditionally deep flush on *sync Message-ID: <20180606181619.GA13076@linux.intel.com> References: <20180606164515.25677-1-ross.zwisler@linux.intel.com> <20180606164515.25677-2-ross.zwisler@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.2 (2017-12-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jun 06, 2018 at 10:57:59AM -0700, Dan Williams wrote: > On Wed, Jun 6, 2018 at 9:45 AM, Ross Zwisler > wrote: > > Prior to this commit we would only do a "deep flush" (have nvdimm_flush() > > write to each of the flush hints for a region) in response to an > > msync/fsync/sync call if the nvdimm_has_cache() returned true at the time > > we were setting up the request queue. This happens due to the write cache > > value passed in to blk_queue_write_cache(), which then causes the block > > layer to send down BIOs with REQ_FUA and REQ_PREFLUSH set. We do have a > > "write_cache" sysfs entry for namespaces, i.e.: > > > > /sys/bus/nd/devices/pfn0.1/block/pmem0/dax/write_cache > > > > which can be used to control whether or not the kernel thinks a given > > namespace has a write cache, but this didn't modify the deep flush behavior > > that we set up when the driver was initialized. Instead, it only modified > > whether or not DAX would flush CPU caches via dax_flush() in response to > > *sync calls. > > > > Simplify this by making the *sync deep flush always happen, regardless of > > the write cache setting of a namespace. The DAX CPU cache flushing will > > still be controlled the write_cache setting of the namespace. > > > > Signed-off-by: Ross Zwisler > > Suggested-by: Dan Williams > > Looks, good. I believe we want this one and ["PATCH v3 4/4] libnvdimm: > don't flush power-fail protected CPU caches" marked for -stable and > tagged with: > > Fixes: 5fdf8e5ba566 ("libnvdimm: re-enable deep flush for pmem devices > via fsync()") > > ...any concerns with that? Nope, sounds good. Can you fix that up when you apply, or would it be helpful for me to send another revision with those tags?