From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 In-Reply-To: References: <20151102042941.6610.27784.stgit@dwillia2-desk3.amr.corp.intel.com> <20151102043053.6610.43948.stgit@dwillia2-desk3.amr.corp.intel.com> <20151103003234.GM10656@dastard> <20151103202041.GG19199@dastard> Date: Wed, 4 Nov 2015 11:23:16 -0800 Message-ID: Subject: Re: [PATCH v3 13/15] block, dax: make dax mappings opt-in by default From: Dan Williams Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org To: Dave Chinner Cc: Jens Axboe , Jan Kara , "linux-nvdimm@lists.01.org" , "linux-kernel@vger.kernel.org" , Ross Zwisler , Christoph Hellwig List-ID: On Tue, Nov 3, 2015 at 3:04 PM, Dan Williams wrote: > On Tue, Nov 3, 2015 at 12:20 PM, Dave Chinner wrote: >> On Mon, Nov 02, 2015 at 11:35:04PM -0800, Dan Williams wrote: >>> On Mon, Nov 2, 2015 at 4:32 PM, Dave Chinner wrote: > [..] >>> Only in the mmap path: >> >> which means blkdev_direct_IO() is now always going to go down the >> dax_do_io() path for any driver with a ->direct_access method rather >> than the direct IO path, regardless of whether DAX is enabled on the >> device or not. >> >> That really seems wrong to me - you've replace explicit "is DAX >> enabled" checks with "is DAX possible" checks, and so DAX paths are >> used regardless of whether DAX is enabled or not. And it's not >> obvious why this is done, nor is it now obvious how DAX interacts >> with the block device. >> >> This really seems like a step backwards to me. > > I think the reason it is not obvious is the original justification for > the bypass as stated in commit bbab37ddc20b "block: Add support for > DAX reads/writes to block devices" was: > > "instead of allocating a DIO and a BIO" > > It turns out it's faster and as far as I can tell semantically > equivalent to the __blockdev_direct_IO() path. The DAX mmap path in > comparison has plenty of sharp edges and semantic differences that > would be avoided by turning off DAX. > > I'm not opposed to also turning off dax_do_io() when S_DAX is clear, > but I don't currently see the point. At the very least I need to add > the above comments to the code, but do you still think opt-in DAX is a > backwards step? I thought of one way dax_do_io() breaks current semantics, it defeats blktrace and i/o stat tracking. I'll restore the existing behavior that gates dax_do_io() on S_DAX. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756204AbbKDTXT (ORCPT ); Wed, 4 Nov 2015 14:23:19 -0500 Received: from mail-wm0-f46.google.com ([74.125.82.46]:36929 "EHLO mail-wm0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750880AbbKDTXS (ORCPT ); Wed, 4 Nov 2015 14:23:18 -0500 MIME-Version: 1.0 In-Reply-To: References: <20151102042941.6610.27784.stgit@dwillia2-desk3.amr.corp.intel.com> <20151102043053.6610.43948.stgit@dwillia2-desk3.amr.corp.intel.com> <20151103003234.GM10656@dastard> <20151103202041.GG19199@dastard> Date: Wed, 4 Nov 2015 11:23:16 -0800 Message-ID: Subject: Re: [PATCH v3 13/15] block, dax: make dax mappings opt-in by default From: Dan Williams To: Dave Chinner Cc: Jens Axboe , Jan Kara , "linux-nvdimm@lists.01.org" , "linux-kernel@vger.kernel.org" , Ross Zwisler , Christoph Hellwig Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Nov 3, 2015 at 3:04 PM, Dan Williams wrote: > On Tue, Nov 3, 2015 at 12:20 PM, Dave Chinner wrote: >> On Mon, Nov 02, 2015 at 11:35:04PM -0800, Dan Williams wrote: >>> On Mon, Nov 2, 2015 at 4:32 PM, Dave Chinner wrote: > [..] >>> Only in the mmap path: >> >> which means blkdev_direct_IO() is now always going to go down the >> dax_do_io() path for any driver with a ->direct_access method rather >> than the direct IO path, regardless of whether DAX is enabled on the >> device or not. >> >> That really seems wrong to me - you've replace explicit "is DAX >> enabled" checks with "is DAX possible" checks, and so DAX paths are >> used regardless of whether DAX is enabled or not. And it's not >> obvious why this is done, nor is it now obvious how DAX interacts >> with the block device. >> >> This really seems like a step backwards to me. > > I think the reason it is not obvious is the original justification for > the bypass as stated in commit bbab37ddc20b "block: Add support for > DAX reads/writes to block devices" was: > > "instead of allocating a DIO and a BIO" > > It turns out it's faster and as far as I can tell semantically > equivalent to the __blockdev_direct_IO() path. The DAX mmap path in > comparison has plenty of sharp edges and semantic differences that > would be avoided by turning off DAX. > > I'm not opposed to also turning off dax_do_io() when S_DAX is clear, > but I don't currently see the point. At the very least I need to add > the above comments to the code, but do you still think opt-in DAX is a > backwards step? I thought of one way dax_do_io() breaks current semantics, it defeats blktrace and i/o stat tracking. I'll restore the existing behavior that gates dax_do_io() on S_DAX.