From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758202AbbJVW5l (ORCPT ); Thu, 22 Oct 2015 18:57:41 -0400 Received: from mga03.intel.com ([134.134.136.65]:53641 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758139AbbJVW5j (ORCPT ); Thu, 22 Oct 2015 18:57:39 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,184,1444719600"; d="scan'208";a="669882401" From: "Williams, Dan J" To: "jmoyer@redhat.com" CC: "linux-kernel@vger.kernel.org" , "linux-nvdimm@ml01.01.org" , "hch@lst.de" , "akpm@linux-foundation.org" , "axboe@fb.com" , "jack@suse.com" , "david@fromorbit.com" , "jack@suse.cz" Subject: Re: [PATCH v2 2/5] dax: increase granularity of dax_clear_blocks() operations Thread-Topic: [PATCH v2 2/5] dax: increase granularity of dax_clear_blocks() operations Thread-Index: AQHRDQ07go/4DD6ZWkSo3PcnxpznDJ54lVEA Date: Thu, 22 Oct 2015 22:57:37 +0000 Message-ID: <1445554654.17208.14.camel@intel.com> References: <20151022171015.38343.72043.stgit@dwillia2-desk3.amr.corp.intel.com> <20151022171026.38343.6371.stgit@dwillia2-desk3.amr.corp.intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.22.254.140] Content-Type: text/plain; charset="utf-8" Content-ID: <1CBBE9BBC095A64C8084F48B9CD0AD21@intel.com> MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id t9MMvntN005620 On Thu, 2015-10-22 at 17:04 -0400, Jeff Moyer wrote: > Dan Williams writes: > > > dax_clear_blocks is currently performing a cond_resched() after every > > PAGE_SIZE memset. We need not check so frequently, for example md-raid > > only calls cond_resched() at stripe granularity. Also, in preparation > > for introducing a dax_map_atomic() operation that temporarily pins a dax > > mapping move the call to cond_resched() to the outer loop. > > There's nothing wrong with the mechanics here, but why bother? I only > see 1 caller in the kernel, and that caller passes in > 1<i_blkbits for the size (so 1 page or less). Did you plan to > add other callers? I don't see them in this particular patch set. > > Again, I'm not taking issue with the patch, I'm just wondering what > motivated the change. The motivation is the subsequent patch to wrap all touches of pmem within a dax_map_atomic() / dax_unmap_atomic() pairing. If I just do the straightforward conversion of this function to dax_map_atomic() it looks something like this: > diff --git a/fs/dax.c b/fs/dax.c > index 5dc33d788d50..fa2a2a255d3a 100644 > --- a/fs/dax.c > +++ b/fs/dax.c > @@ -40,9 +40,9 @@ int dax_clear_blocks(struct inode *inode, sector_t block, long size) > unsigned long pfn; > long count; > > - count = bdev_direct_access(bdev, sector, &addr, &pfn, size); > - if (count < 0) > - return count; > + addr = __dax_map_atomic(bdev, sector, size, &pfn, &count); > + if (IS_ERR(addr)) > + return PTR_ERR(addr); > BUG_ON(size < count); > while (count > 0) { > unsigned pgsz = PAGE_SIZE - offset_in_page(addr); > @@ -56,6 +56,7 @@ int dax_clear_blocks(struct inode *inode, sector_t block, long size) > sector += pgsz / 512; > cond_resched(); > } > + dax_unmap_atomic(bdev, addr); > } while (size); > > wmb_pmem(); The problem is that intervening call to cond_resched(). I later want to inject an rcu_read_lock()/unlock() pair to allow flushing active dax_map_atomic() usages at driver teardown time [1]. But, I think the patch stands alone as a cleanup outside of that admittedly hidden motivation. [1]: "mm, pmem: devm_memunmap_pages(), truncate and unmap ZONE_DEVICE pages" https://lists.01.org/pipermail/linux-nvdimm/2015-October/002406.html {.n++%ݶw{.n+{G{ayʇڙ,jfhz_(階ݢj"mG?&~iOzv^m ?I