Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1662730 > unrolled thread

[PATCH v3 13/14] filesystem-dax: gate calls to dax_flush() on QUEUE_FLAG_WC

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v3 13/14] filesystem-dax: gate calls to dax_flush() on  QUEUE_FLAG_WC Dan Williams <dan.j.williams@intel.com> - 2017-06-09 22:40 +0200
    Re: [PATCH v3 13/14] filesystem-dax: gate calls to dax_flush() on  QUEUE_FLAG_WC Jan Kara <jack@suse.cz> - 2017-06-14 12:50 +0200
      Re: [PATCH v3 13/14] filesystem-dax: gate calls to dax_flush() on QUEUE_FLAG_WC Dan Williams <dan.j.williams@intel.com> - 2017-06-14 18:50 +0200
    [PATCH v4 13/14] libnvdimm,  pmem: gate cache management on QUEUE_FLAG_WC in pmem_dax_flush() Dan Williams <dan.j.williams@intel.com> - 2017-06-15 01:20 +0200
      Re: [PATCH v4 13/14] libnvdimm, pmem: gate cache management on  QUEUE_FLAG_WC in pmem_dax_flush() Jan Kara <jack@suse.cz> - 2017-06-15 10:10 +0200
    Re: [PATCH v3 13/14] filesystem-dax: gate calls to dax_flush() on         QUEUE_FLAG_WC Christoph Hellwig <hch@lst.de> - 2017-06-18 10:50 +0200
      Re: [PATCH v3 13/14] filesystem-dax: gate calls to dax_flush() on QUEUE_FLAG_WC Dan Williams <dan.j.williams@intel.com> - 2017-06-19 04:10 +0200

#1662730 — [PATCH v3 13/14] filesystem-dax: gate calls to dax_flush() on QUEUE_FLAG_WC

Some platforms arrange for cpu caches to be flushed on power-fail. On
those platforms there is no requirement that the kernel track and flush
potentially dirty cache lines. Given that we still insert entries into
the radix for locking purposes this patch only disables the cache flush
loop, not the dirty tracking.

Userspace can override the default cache setting via the block device
queue "write_cache" attribute in sysfs.

Cc: Jan Kara <jack@suse.cz>
Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Matthew Wilcox <mawilcox@microsoft.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 fs/dax.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/dax.c b/fs/dax.c
index 6d8699feae2e..c3140343ff7e 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -783,7 +783,8 @@ static int dax_writeback_one(struct block_device *bdev,
 	}
 
 	dax_mapping_entry_mkclean(mapping, index, pfn_t_to_pfn(pfn));
-	dax_flush(dax_dev, pgoff, kaddr, size);
+	if (test_bit(QUEUE_FLAG_WC, &bdev->bd_queue->queue_flags))
+		dax_flush(dax_dev, pgoff, kaddr, size);
 	/*
 	 * After we have flushed the cache, we can clear the dirty tag. There
 	 * cannot be new dirty data in the pfn after the flush has completed as
@@ -975,7 +976,8 @@ int __dax_zero_page_range(struct block_device *bdev,
 			return rc;
 		}
 		memset(kaddr + offset, 0, size);
-		dax_flush(dax_dev, pgoff, kaddr + offset, size);
+		if (test_bit(QUEUE_FLAG_WC, &bdev->bd_queue->queue_flags))
+			dax_flush(dax_dev, pgoff, kaddr + offset, size);
 		dax_read_unlock(id);
 	}
 	return 0;

[toc] | [next] | [standalone]

#1665687

On Fri 09-06-17 13:24:56, Dan Williams wrote:
> Some platforms arrange for cpu caches to be flushed on power-fail. On
> those platforms there is no requirement that the kernel track and flush
> potentially dirty cache lines. Given that we still insert entries into
> the radix for locking purposes this patch only disables the cache flush
> loop, not the dirty tracking.
> 
> Userspace can override the default cache setting via the block device
> queue "write_cache" attribute in sysfs.
> 
> Cc: Jan Kara <jack@suse.cz>
> Cc: Jeff Moyer <jmoyer@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Matthew Wilcox <mawilcox@microsoft.com>
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

...

> -	dax_flush(dax_dev, pgoff, kaddr, size);
> +	if (test_bit(QUEUE_FLAG_WC, &bdev->bd_queue->queue_flags))
> +		dax_flush(dax_dev, pgoff, kaddr, size);

IMHO the check belongs into dax_flush() similarly as blkdev_issue_flush()
takes silently handles whether the flush is actually needed or not.

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[toc] | [prev] | [next] | [standalone]

#1666068 — Re: [PATCH v3 13/14] filesystem-dax: gate calls to dax_flush() on QUEUE_FLAG_WC

On Wed, Jun 14, 2017 at 3:46 AM, Jan Kara <jack@suse.cz> wrote:
> On Fri 09-06-17 13:24:56, Dan Williams wrote:
>> Some platforms arrange for cpu caches to be flushed on power-fail. On
>> those platforms there is no requirement that the kernel track and flush
>> potentially dirty cache lines. Given that we still insert entries into
>> the radix for locking purposes this patch only disables the cache flush
>> loop, not the dirty tracking.
>>
>> Userspace can override the default cache setting via the block device
>> queue "write_cache" attribute in sysfs.
>>
>> Cc: Jan Kara <jack@suse.cz>
>> Cc: Jeff Moyer <jmoyer@redhat.com>
>> Cc: Christoph Hellwig <hch@lst.de>
>> Cc: Matthew Wilcox <mawilcox@microsoft.com>
>> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>
> ...
>
>> -     dax_flush(dax_dev, pgoff, kaddr, size);
>> +     if (test_bit(QUEUE_FLAG_WC, &bdev->bd_queue->queue_flags))
>> +             dax_flush(dax_dev, pgoff, kaddr, size);
>
> IMHO the check belongs into dax_flush() similarly as blkdev_issue_flush()
> takes silently handles whether the flush is actually needed or not.

Looks good, will do.

[toc] | [prev] | [next] | [standalone]

#1666313 — [PATCH v4 13/14] libnvdimm, pmem: gate cache management on QUEUE_FLAG_WC in pmem_dax_flush()

Some platforms arrange for cpu caches to be flushed on power-fail. On
those platforms there is no requirement that the kernel track and flush
potentially dirty cache lines. Given that we still insert entries into
the radix for locking purposes this patch only disables the cache flush
loop, not the dirty tracking.

Userspace can override the default cache setting via the block device
queue "write_cache" attribute in sysfs.

Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Matthew Wilcox <mawilcox@microsoft.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
Changes since v3:
* move the check of QUEUE_FLAG_WC into the pmem driver directly (Jan)

 drivers/nvdimm/pmem.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 06f6c27ec1e9..49938b246a7b 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -244,7 +244,16 @@ static size_t pmem_copy_from_iter(struct dax_device *dax_dev, pgoff_t pgoff,
 static void pmem_dax_flush(struct dax_device *dax_dev, pgoff_t pgoff,
 		void *addr, size_t size)
 {
-	arch_wb_cache_pmem(addr, size);
+	struct pmem_device *pmem = dax_get_private(dax_dev);
+	struct gendisk *disk = pmem->disk;
+	struct request_queue *q = disk->queue;
+
+	/*
+	 * Only perform cache management when the queue has caching
+	 * enabled.
+	 */
+	if (test_bit(QUEUE_FLAG_WC, &q->queue_flags))
+		arch_wb_cache_pmem(addr, size);
 }
 
 static const struct dax_operations pmem_dax_ops = {

[toc] | [prev] | [next] | [standalone]

#1666543 — Re: [PATCH v4 13/14] libnvdimm, pmem: gate cache management on QUEUE_FLAG_WC in pmem_dax_flush()

On Wed 14-06-17 16:11:26, Dan Williams wrote:
> Some platforms arrange for cpu caches to be flushed on power-fail. On
> those platforms there is no requirement that the kernel track and flush
> potentially dirty cache lines. Given that we still insert entries into
> the radix for locking purposes this patch only disables the cache flush
> loop, not the dirty tracking.
> 
> Userspace can override the default cache setting via the block device
> queue "write_cache" attribute in sysfs.
> 
> Cc: Jeff Moyer <jmoyer@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Matthew Wilcox <mawilcox@microsoft.com>
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Looks good. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
> Changes since v3:
> * move the check of QUEUE_FLAG_WC into the pmem driver directly (Jan)
> 
>  drivers/nvdimm/pmem.c |   11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> index 06f6c27ec1e9..49938b246a7b 100644
> --- a/drivers/nvdimm/pmem.c
> +++ b/drivers/nvdimm/pmem.c
> @@ -244,7 +244,16 @@ static size_t pmem_copy_from_iter(struct dax_device *dax_dev, pgoff_t pgoff,
>  static void pmem_dax_flush(struct dax_device *dax_dev, pgoff_t pgoff,
>  		void *addr, size_t size)
>  {
> -	arch_wb_cache_pmem(addr, size);
> +	struct pmem_device *pmem = dax_get_private(dax_dev);
> +	struct gendisk *disk = pmem->disk;
> +	struct request_queue *q = disk->queue;
> +
> +	/*
> +	 * Only perform cache management when the queue has caching
> +	 * enabled.
> +	 */
> +	if (test_bit(QUEUE_FLAG_WC, &q->queue_flags))
> +		arch_wb_cache_pmem(addr, size);
>  }
>  
>  static const struct dax_operations pmem_dax_ops = {
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[toc] | [prev] | [next] | [standalone]

#1668561 — Re: [PATCH v3 13/14] filesystem-dax: gate calls to dax_flush() on QUEUE_FLAG_WC

On Fri, Jun 09, 2017 at 01:24:56PM -0700, Dan Williams wrote:
> Some platforms arrange for cpu caches to be flushed on power-fail. On
> those platforms there is no requirement that the kernel track and flush
> potentially dirty cache lines. Given that we still insert entries into
> the radix for locking purposes this patch only disables the cache flush
> loop, not the dirty tracking.
> 
> Userspace can override the default cache setting via the block device
> queue "write_cache" attribute in sysfs.

NAK.  Please stop using the block infrastructure for dax values.  Have
your own flag and sysfs file in the dax infrastructure and only propagate
it to the block layer for the block devices using dax.

[toc] | [prev] | [next] | [standalone]

#1668711 — Re: [PATCH v3 13/14] filesystem-dax: gate calls to dax_flush() on QUEUE_FLAG_WC

On Sun, Jun 18, 2017 at 1:45 AM, Christoph Hellwig <hch@lst.de> wrote:
> On Fri, Jun 09, 2017 at 01:24:56PM -0700, Dan Williams wrote:
>> Some platforms arrange for cpu caches to be flushed on power-fail. On
>> those platforms there is no requirement that the kernel track and flush
>> potentially dirty cache lines. Given that we still insert entries into
>> the radix for locking purposes this patch only disables the cache flush
>> loop, not the dirty tracking.
>>
>> Userspace can override the default cache setting via the block device
>> queue "write_cache" attribute in sysfs.
>
> NAK.  Please stop using the block infrastructure for dax values.  Have
> your own flag and sysfs file in the dax infrastructure and only propagate
> it to the block layer for the block devices using dax.

Ok, that makes sense.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web