All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Snitzer <snitzer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
To: Ross Zwisler <ross.zwisler-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Cc: linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-xfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH v2 3/7] dm: fix test for DAX device support
Date: Fri, 1 Jun 2018 16:46:04 -0400	[thread overview]
Message-ID: <20180601204604.GB1144@redhat.com> (raw)
In-Reply-To: <20180601201924.GA1144-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Fri, Jun 01 2018 at  4:19P -0400,
Mike Snitzer <snitzer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> On Tue, May 29 2018 at  3:51P -0400,
> Ross Zwisler <ross.zwisler-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> 
> > Currently device_supports_dax() just checks to see if the QUEUE_FLAG_DAX
> > flag is set on the device's request queue to decide whether or not the
> > device supports filesystem DAX.  This is insufficient because there are
> > devices like PMEM namespaces in raw mode which have QUEUE_FLAG_DAX set but
> > which don't actually support DAX.
> 
> Isn't that a PMEM bug then?
> 
> What is the point of setting QUEUE_FLAG_DAX if it cannot be trusted?
>  
> > This means that you could create a dm-linear device, for example, where the
> > first part of the dm-linear device was a PMEM namespace in fsdax mode and
> > the second part was a PMEM namespace in raw mode.  Both DM and the
> > filesystem you put on that dm-linear device would think the whole device
> > supports DAX, which would lead to bad behavior once your raw PMEM namespace
> > part using DAX needed struct page for something.
> 
> The PMEM namespace in raw mode shouldn't be setting QUEUE_FLAG_DAX, if
> it didn't then the stacked-up linear DM wouldn't 
> 
> > Fix this by using bdev_dax_supported() like filesystems do at mount time.
> > This checks for raw mode and also performs other tests like checking to
> > make sure the dax_direct_access() path works.
> 
> Sorry "This" does those things where?

I see you meant bdev_dax_supported() does these additional checks.

My previous question stands though.  Why is QUEUE_FLAG_DAX getting set
if the device hasn't already passed these checks?  Shouldn't setting
QUEUE_FLAG_DAX on request_queue depend on bdev_dax_supported() passing?

But looking at the drivers that do set QUEUE_FLAG_DAX: they
don't have the bdev readily available.  Anyway, just strikes me as
bizarre that a driver can set QUEUE_FLAG_DAX without having to have
ensured bdev_dax_supported() passes (even if not programatically, but
that the developer has verified the hooks, et al exist).

But I'll give up on this line of questioning..

My dilemma now is: how do I take these changes without first rebasing
linux-dm.git ontop of Darrick's xfs tree?

I probably should've reviewed faster and been the one to take the entire
set (with appropriate acks obviously).

WARNING: multiple messages have this Message-ID (diff)
From: Mike Snitzer <snitzer@redhat.com>
To: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: linux-nvdimm@lists.01.org, linux-kernel@vger.kernel.org,
	linux-xfs@vger.kernel.org, dm-devel@redhat.com,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v2 3/7] dm: fix test for DAX device support
Date: Fri, 1 Jun 2018 16:46:04 -0400	[thread overview]
Message-ID: <20180601204604.GB1144@redhat.com> (raw)
In-Reply-To: <20180601201924.GA1144@redhat.com>

On Fri, Jun 01 2018 at  4:19P -0400,
Mike Snitzer <snitzer@redhat.com> wrote:

> On Tue, May 29 2018 at  3:51P -0400,
> Ross Zwisler <ross.zwisler@linux.intel.com> wrote:
> 
> > Currently device_supports_dax() just checks to see if the QUEUE_FLAG_DAX
> > flag is set on the device's request queue to decide whether or not the
> > device supports filesystem DAX.  This is insufficient because there are
> > devices like PMEM namespaces in raw mode which have QUEUE_FLAG_DAX set but
> > which don't actually support DAX.
> 
> Isn't that a PMEM bug then?
> 
> What is the point of setting QUEUE_FLAG_DAX if it cannot be trusted?
>  
> > This means that you could create a dm-linear device, for example, where the
> > first part of the dm-linear device was a PMEM namespace in fsdax mode and
> > the second part was a PMEM namespace in raw mode.  Both DM and the
> > filesystem you put on that dm-linear device would think the whole device
> > supports DAX, which would lead to bad behavior once your raw PMEM namespace
> > part using DAX needed struct page for something.
> 
> The PMEM namespace in raw mode shouldn't be setting QUEUE_FLAG_DAX, if
> it didn't then the stacked-up linear DM wouldn't 
> 
> > Fix this by using bdev_dax_supported() like filesystems do at mount time.
> > This checks for raw mode and also performs other tests like checking to
> > make sure the dax_direct_access() path works.
> 
> Sorry "This" does those things where?

I see you meant bdev_dax_supported() does these additional checks.

My previous question stands though.  Why is QUEUE_FLAG_DAX getting set
if the device hasn't already passed these checks?  Shouldn't setting
QUEUE_FLAG_DAX on request_queue depend on bdev_dax_supported() passing?

But looking at the drivers that do set QUEUE_FLAG_DAX: they
don't have the bdev readily available.  Anyway, just strikes me as
bizarre that a driver can set QUEUE_FLAG_DAX without having to have
ensured bdev_dax_supported() passes (even if not programatically, but
that the developer has verified the hooks, et al exist).

But I'll give up on this line of questioning..

My dilemma now is: how do I take these changes without first rebasing
linux-dm.git ontop of Darrick's xfs tree?

I probably should've reviewed faster and been the one to take the entire
set (with appropriate acks obviously).
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

WARNING: multiple messages have this Message-ID (diff)
From: Mike Snitzer <snitzer@redhat.com>
To: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: Toshi Kani <toshi.kani@hpe.com>,
	dm-devel@redhat.com, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-nvdimm@lists.01.org,
	linux-xfs@vger.kernel.org
Subject: Re: [PATCH v2 3/7] dm: fix test for DAX device support
Date: Fri, 1 Jun 2018 16:46:04 -0400	[thread overview]
Message-ID: <20180601204604.GB1144@redhat.com> (raw)
In-Reply-To: <20180601201924.GA1144@redhat.com>

On Fri, Jun 01 2018 at  4:19P -0400,
Mike Snitzer <snitzer@redhat.com> wrote:

> On Tue, May 29 2018 at  3:51P -0400,
> Ross Zwisler <ross.zwisler@linux.intel.com> wrote:
> 
> > Currently device_supports_dax() just checks to see if the QUEUE_FLAG_DAX
> > flag is set on the device's request queue to decide whether or not the
> > device supports filesystem DAX.  This is insufficient because there are
> > devices like PMEM namespaces in raw mode which have QUEUE_FLAG_DAX set but
> > which don't actually support DAX.
> 
> Isn't that a PMEM bug then?
> 
> What is the point of setting QUEUE_FLAG_DAX if it cannot be trusted?
>  
> > This means that you could create a dm-linear device, for example, where the
> > first part of the dm-linear device was a PMEM namespace in fsdax mode and
> > the second part was a PMEM namespace in raw mode.  Both DM and the
> > filesystem you put on that dm-linear device would think the whole device
> > supports DAX, which would lead to bad behavior once your raw PMEM namespace
> > part using DAX needed struct page for something.
> 
> The PMEM namespace in raw mode shouldn't be setting QUEUE_FLAG_DAX, if
> it didn't then the stacked-up linear DM wouldn't 
> 
> > Fix this by using bdev_dax_supported() like filesystems do at mount time.
> > This checks for raw mode and also performs other tests like checking to
> > make sure the dax_direct_access() path works.
> 
> Sorry "This" does those things where?

I see you meant bdev_dax_supported() does these additional checks.

My previous question stands though.  Why is QUEUE_FLAG_DAX getting set
if the device hasn't already passed these checks?  Shouldn't setting
QUEUE_FLAG_DAX on request_queue depend on bdev_dax_supported() passing?

But looking at the drivers that do set QUEUE_FLAG_DAX: they
don't have the bdev readily available.  Anyway, just strikes me as
bizarre that a driver can set QUEUE_FLAG_DAX without having to have
ensured bdev_dax_supported() passes (even if not programatically, but
that the developer has verified the hooks, et al exist).

But I'll give up on this line of questioning..

My dilemma now is: how do I take these changes without first rebasing
linux-dm.git ontop of Darrick's xfs tree?

I probably should've reviewed faster and been the one to take the entire
set (with appropriate acks obviously).

  parent reply	other threads:[~2018-06-01 20:46 UTC|newest]

Thread overview: 119+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-29 19:50 [PATCH v2 0/7] Fix DM DAX handling Ross Zwisler
2018-05-29 19:50 ` Ross Zwisler
2018-05-29 19:50 ` Ross Zwisler
     [not found] ` <20180529195106.14268-1-ross.zwisler-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2018-05-29 19:51   ` [PATCH v2 1/7] fs: allow per-device dax status checking for filesystems Ross Zwisler
2018-05-29 19:51     ` Ross Zwisler
2018-05-29 19:51     ` Ross Zwisler
2018-05-29 19:51   ` [PATCH v2 2/7] dax: change bdev_dax_supported() to support boolean returns Ross Zwisler
2018-05-29 19:51     ` Ross Zwisler
2018-05-29 19:51     ` Ross Zwisler
     [not found]     ` <20180529195106.14268-3-ross.zwisler-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2018-05-29 21:25       ` Darrick J. Wong
2018-05-29 21:25         ` Darrick J. Wong
2018-05-29 21:25         ` Darrick J. Wong
2018-05-29 22:01         ` Ross Zwisler
2018-05-29 22:01           ` Ross Zwisler
     [not found]           ` <20180529220114.GA13948-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2018-05-31 19:13             ` Darrick J. Wong
2018-05-31 19:13               ` Darrick J. Wong
2018-05-31 19:13               ` Darrick J. Wong
2018-05-31 20:34               ` Ross Zwisler
2018-05-31 20:34                 ` Ross Zwisler
2018-05-31 20:34                 ` Ross Zwisler
2018-05-31 20:35               ` Dan Williams
2018-05-31 20:35                 ` Dan Williams
2018-05-31 20:35                 ` Dan Williams
2018-05-31 20:41               ` Ross Zwisler
2018-05-31 20:41                 ` Ross Zwisler
2018-05-31 20:41                 ` Ross Zwisler
2018-05-31 20:52               ` Mike Snitzer
2018-05-31 20:52                 ` Mike Snitzer
2018-05-31 20:52                 ` Mike Snitzer
     [not found]                 ` <20180531205206.GA12681-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2018-05-31 22:26                   ` [dm-devel] " Darrick J. Wong
2018-05-31 22:26                     ` Darrick J. Wong
2018-05-31 22:26                     ` Darrick J. Wong
2018-06-01 20:59                     ` Ross Zwisler
2018-06-01 20:59                       ` Ross Zwisler
2018-06-01 20:59                       ` Ross Zwisler
2018-06-01  1:26               ` Dave Chinner
2018-06-01  1:26                 ` Dave Chinner
2018-06-01  1:57                 ` Dan Williams
2018-06-01  1:57                   ` Dan Williams
     [not found]                   ` <CAPcyv4g6dv9sG9dR-iY+aEfSi2r3ey1dXGbX-nexKsN72t0QMw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-06-01  2:24                     ` Dave Chinner
2018-06-01  2:24                       ` Dave Chinner
2018-06-01  2:24                       ` Dave Chinner
2018-06-01  4:02                       ` Dan Williams
2018-06-01  4:02                         ` Dan Williams
2018-06-01  4:02                         ` Dan Williams
2018-06-03 22:20                         ` Dave Chinner
2018-06-03 22:20                           ` Dave Chinner
2018-06-04  0:25                           ` Dave Chinner
2018-06-04  0:25                             ` Dave Chinner
2018-06-04  0:25                             ` Dave Chinner
2018-06-04  1:48                             ` Dan Williams
2018-06-04  1:48                               ` Dan Williams
2018-06-04  1:48                               ` Dan Williams
     [not found]                               ` <CAPcyv4jL5AKgP3io_hBZxkrKzBZq8wtuqk49L48v=XRiOHdoEw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-06-04 23:40                                 ` Dan Williams
2018-06-04 23:40                                   ` Dan Williams
2018-06-04 23:40                                   ` Dan Williams
     [not found]                                   ` <CAPcyv4iVU3n3G3Vxf9e6cKuCtQtmrm6+R6vS379NyHX6eTZ5Lg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-06-05  0:33                                     ` Mike Snitzer
2018-06-05  0:33                                       ` Mike Snitzer
2018-06-05  0:33                                       ` Mike Snitzer
     [not found]                                       ` <20180605003325.GA6898-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2018-06-05  5:55                                         ` Dave Chinner
2018-06-05  5:55                                           ` Dave Chinner
2018-06-05  5:55                                           ` Dave Chinner
2018-06-05  3:32                                   ` Dan Williams
2018-06-05  3:32                                     ` Dan Williams
2018-05-29 19:51   ` [PATCH v2 3/7] dm: fix test for DAX device support Ross Zwisler
2018-05-29 19:51     ` Ross Zwisler
2018-05-29 19:51     ` Ross Zwisler
     [not found]     ` <20180529195106.14268-4-ross.zwisler-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2018-06-01 20:19       ` Mike Snitzer
2018-06-01 20:19         ` Mike Snitzer
2018-06-01 20:19         ` Mike Snitzer
     [not found]         ` <20180601201924.GA1144-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2018-06-01 20:46           ` Mike Snitzer [this message]
2018-06-01 20:46             ` Mike Snitzer
2018-06-01 20:46             ` Mike Snitzer
     [not found]             ` <20180601204604.GB1144-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2018-06-01 21:11               ` Ross Zwisler
2018-06-01 21:11                 ` Ross Zwisler
2018-06-01 21:11                 ` Ross Zwisler
2018-06-01 21:16               ` Dan Williams
2018-06-01 21:16                 ` Dan Williams
2018-06-01 21:16                 ` Dan Williams
2018-05-29 19:51   ` [PATCH v2 4/7] dm: prevent DAX mounts if not supported Ross Zwisler
2018-05-29 19:51     ` Ross Zwisler
2018-05-29 19:51     ` Ross Zwisler
     [not found]     ` <20180529195106.14268-5-ross.zwisler-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2018-06-01 21:55       ` Mike Snitzer
2018-06-01 21:55         ` Mike Snitzer
2018-06-01 21:55         ` Mike Snitzer
     [not found]         ` <20180601215513.GA18712-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2018-06-04 23:15           ` Ross Zwisler
2018-06-04 23:15             ` Ross Zwisler
2018-06-04 23:15             ` Ross Zwisler
2018-06-20 15:17             ` Mike Snitzer
2018-06-20 15:17               ` Mike Snitzer
2018-06-25 19:20               ` Ross Zwisler
2018-06-25 19:20                 ` Ross Zwisler
2018-05-29 19:51   ` [PATCH v2 5/7] dm: remove DM_TYPE_DAX_BIO_BASED dm_queue_mode Ross Zwisler
2018-05-29 19:51     ` Ross Zwisler
2018-05-29 19:51     ` Ross Zwisler
     [not found]     ` <20180529195106.14268-6-ross.zwisler-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2018-06-01 22:04       ` Mike Snitzer
2018-06-01 22:04         ` Mike Snitzer
2018-06-01 22:04         ` Mike Snitzer
     [not found]         ` <20180601220443.GB18712-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2018-06-04 23:24           ` Ross Zwisler
2018-06-04 23:24             ` Ross Zwisler
2018-06-04 23:24             ` Ross Zwisler
     [not found]             ` <20180604232416.GB10666-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2018-06-04 23:49               ` Kani, Toshi
2018-06-04 23:49                 ` Kani, Toshi
2018-06-04 23:49                 ` Kani, Toshi
2018-06-05  0:46               ` Mike Snitzer
2018-06-05  0:46                 ` Mike Snitzer
2018-06-05  0:46                 ` Mike Snitzer
     [not found]                 ` <20180605004558.GB6898-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2018-06-06 17:24                   ` Ross Zwisler
2018-06-06 17:24                     ` Ross Zwisler
2018-06-06 17:24                     ` Ross Zwisler
     [not found]                     ` <20180606172421.GA2208-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2018-06-06 22:29                       ` Mike Snitzer
2018-06-06 22:29                         ` Mike Snitzer
2018-06-06 22:29                         ` Mike Snitzer
2018-05-29 19:51   ` [PATCH v2 6/7] dm-snap: remove unnecessary direct_access() stub Ross Zwisler
2018-05-29 19:51     ` Ross Zwisler
2018-05-29 19:51     ` Ross Zwisler
2018-05-29 19:51   ` [PATCH v2 7/7] dm-error: " Ross Zwisler
2018-05-29 19:51     ` Ross Zwisler
2018-05-29 19:51     ` Ross Zwisler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180601204604.GB1144@redhat.com \
    --to=snitzer-h+wxahxf7alqt0dzr+alfa@public.gmane.org \
    --cc=dm-devel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org \
    --cc=linux-xfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=ross.zwisler-VuQAYsv1563Yd54FQh9/CA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.