From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi0-x230.google.com (mail-oi0-x230.google.com [IPv6:2607:f8b0:4003:c06::230]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 2E4D4226C7C30 for ; Tue, 3 Apr 2018 12:47:15 -0700 (PDT) Received: by mail-oi0-x230.google.com with SMTP id t16-v6so17098117oih.3 for ; Tue, 03 Apr 2018 12:47:15 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20180403193912.GC6556@redhat.com> References: <152246892890.36038.18436540150980653229.stgit@dwillia2-desk3.amr.corp.intel.com> <152246898322.36038.17918469633893320678.stgit@dwillia2-desk3.amr.corp.intel.com> <20180403193912.GC6556@redhat.com> From: Dan Williams Date: Tue, 3 Apr 2018 12:47:14 -0700 Message-ID: Subject: Re: [PATCH v8 10/18] dax, dm: introduce ->fs_{claim, release}() dax_device infrastructure 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: Mike Snitzer Cc: Jan Kara , Matthew Wilcox , linux-nvdimm , david , Linux Kernel Mailing List , linux-xfs , =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= , linux-fsdevel , Christoph Hellwig , Alasdair Kergon List-ID: On Tue, Apr 3, 2018 at 12:39 PM, Mike Snitzer wrote: > On Tue, Apr 03 2018 at 2:24pm -0400, > Dan Williams wrote: > >> On Fri, Mar 30, 2018 at 9:03 PM, Dan Williams wrote: >> > In preparation for allowing filesystems to augment the dev_pagemap >> > associated with a dax_device, add an ->fs_claim() callback. The >> > ->fs_claim() callback is leveraged by the device-mapper dax >> > implementation to iterate all member devices in the map and repeat the >> > claim operation across the array. >> > >> > In order to resolve collisions between filesystem operations and DMA to >> > DAX mapped pages we need a callback when DMA completes. With a callback >> > we can hold off filesystem operations while DMA is in-flight and then >> > resume those operations when the last put_page() occurs on a DMA page. >> > The ->fs_claim() operation arranges for this callback to be registered, >> > although that implementation is saved for a later patch. >> > >> > Cc: Alasdair Kergon >> > Cc: Mike Snitzer >> >> Mike, do these DM touches look ok to you? We need these ->fs_claim() >> / ->fs_release() interfaces for device-mapper to set up filesystem-dax >> infrastructure on all sub-devices whenever a dax-capable DM device is >> mounted. It builds on the device-mapper dax dependency removal >> patches. > > I'd prefer dm_dax_iterate() be renamed to dm_dax_iterate_devices() Ok, I'll fix that up. > But dm_dax_iterate() is weird... it is simply returning the struct > dax_device *dax_dev that is passed: seemingly without actually directly > changing anything about that dax_device (I can infer that you're > claiming the underlying devices, but...) I could at least add a note to see the comment in dm_dax_dev_claim(). The filesystem caller expects to get a dax_dev back or NULL from fs_dax_claim_bdev() if the claim failed. For fs_dax_claim() the return value could simply be bool for pass / fail, but I used dax_dev NULL / not-NULL instead. In the case of device-mapper the claim attempt can't fail for conflicting ownership reasons because the exclusive ownership of the underlying block device is already established by device-mapper before the fs claims the device-mapper dax device. > In general user's of ti->type->iterate_devices can get a result back > (via 'int' return).. you aren't using it that way (and maybe dax will > never have a need to return an answer). But all said, I think I'd > prefer to see dm_dax_iterate_devices() return void. > > But please let me know if I'm missing something, thanks. Oh, yeah, I like that better. I'll just make it return void and have dm_dax_fs_claim() return the dax_dev directly. Thanks Mike! _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm