[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160622223842.GA34512@redhat.com>
Date: Wed, 22 Jun 2016 18:38:42 -0400
From: Mike Snitzer <snitzer@...hat.com>
To: "Kani, Toshimitsu" <toshi.kani@....com>
Cc: "dan.j.williams@...el.com" <dan.j.williams@...el.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"sandeen@...hat.com" <sandeen@...hat.com>,
"linux-nvdimm@...1.01.org" <linux-nvdimm@...1.01.org>,
"agk@...hat.com" <agk@...hat.com>,
"linux-raid@...r.kernel.org" <linux-raid@...r.kernel.org>,
"viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
"axboe@...nel.dk" <axboe@...nel.dk>, "axboe@...com" <axboe@...com>,
"ross.zwisler@...ux.intel.com" <ross.zwisler@...ux.intel.com>,
"dm-devel@...hat.com" <dm-devel@...hat.com>
Subject: Re: [PATCH 0/6] Support DAX for device-mapper dm-linear devices
On Wed, Jun 22 2016 at 4:16P -0400,
Kani, Toshimitsu <toshi.kani@....com> wrote:
> On Wed, 2016-06-22 at 12:15 -0700, Dan Williams wrote:
> > On Wed, Jun 22, 2016 at 10:44 AM, Kani, Toshimitsu <toshi.kani@....com>
> > wrote:
> > > On Tue, 2016-06-21 at 14:17 -0400, Mike Snitzer wrote:
> > > >
> > > > On Tue, Jun 21 2016 at 11:44am -0400,
> > > > Kani, Toshimitsu <toshi.kani@....com> wrote:
> > > > >
> > > > > On Tue, 2016-06-21 at 09:41 -0400, Mike Snitzer wrote:
> > > > > > On Mon, Jun 20 2016 at 6:22pm -0400,
> > > > > > Mike Snitzer <snitzer@...hat.com> wrote:
> > > > > > I'm now wondering if we'd be better off setting a new QUEUE_FLAG_DAX
> > > > > > rather than establish GENHD_FL_DAX on the genhd?
> > > > > >
> > > > > > It'd be quite a bit easier to allow upper layers (e.g. XFS and ext4)
> > > > > > to check for a queue flag.
> > > > >
> > > > > I think GENHD_FL_DAX is more appropriate since DAX does not use a
> > > > > request queue, except for protecting the underlining device being
> > > > > disabled while direct_access() is called (b2e0d1625e19).
> > > >
> > > > The devices in question have a request_queue. All bio-based device have
> > > > a request_queue.
> > >
> > > DAX-capable devices have two operation modes, bio-based and DAX. I agree
> > > that bio-based operation is associated with a request queue, and its
> > > capabilities should be set to it. DAX, on the other hand, is rather
> > > independent from a request queue.
> > >
> > > > I don't have a big problem with GENHD_FL_DAX. Just wanted to point out
> > > > that such block device capabilities are generally advertised in terms of
> > > > a QUEUE_FLAG.
> > >
> > > I do not have a strong opinion, but feel a bit odd to associate DAX to a
> > > request queue.
> >
> > Given that we do not support dax to a raw block device [1] it seems a
> > gendisk flag is more misleading than request_queue flag that specifies
> > what requests can be made of the device.
> >
> > [1]: acc93d30d7d4 Revert "block: enable dax for raw block devices"
>
> Oh, I see. I will change to use request_queue flag.
I implemented the block patch for this yesterday but based on your
feedback I stopped there (didn't carry the change through to the DM core
and DM linear -- can easily do so tomorrow though).
Feel free to use this as a starting point and fix/extend and add a
proper header:
>From e88736ce322f248157da6c7d402e940adafffa1e Mon Sep 17 00:00:00 2001
From: Mike Snitzer <snitzer@...hat.com>
Date: Tue, 21 Jun 2016 12:23:29 -0400
Subject: [PATCH] block: add QUEUE_FLAG_DAX for devices to advertise their DAX support
Signed-off-by: Mike Snitzer <snitzer@...hat.com>
---
drivers/block/brd.c | 3 +++
drivers/nvdimm/pmem.c | 1 +
drivers/s390/block/dcssblk.c | 1 +
fs/block_dev.c | 5 +++--
include/linux/blkdev.h | 2 ++
5 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index f5b0d6f..13eee12 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -508,6 +508,9 @@ static struct brd_device *brd_alloc(int i)
brd->brd_queue->limits.discard_granularity = PAGE_SIZE;
blk_queue_max_discard_sectors(brd->brd_queue, UINT_MAX);
brd->brd_queue->limits.discard_zeroes_data = 1;
+#ifdef CONFIG_BLK_DEV_RAM_DAX
+ queue_flag_set_unlocked(QUEUE_FLAG_DAX, brd->brd_queue);
+#endif
queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, brd->brd_queue);
disk = brd->brd_disk = alloc_disk(max_part);
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 608fc44..53b701b 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -283,6 +283,7 @@ static int pmem_attach_disk(struct device *dev,
blk_queue_max_hw_sectors(q, UINT_MAX);
blk_queue_bounce_limit(q, BLK_BOUNCE_ANY);
queue_flag_set_unlocked(QUEUE_FLAG_NONROT, q);
+ queue_flag_set_unlocked(QUEUE_FLAG_DAX, q);
q->queuedata = pmem;
disk = alloc_disk_node(0, nid);
diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c
index bed53c4..093e9e1 100644
--- a/drivers/s390/block/dcssblk.c
+++ b/drivers/s390/block/dcssblk.c
@@ -618,6 +618,7 @@ dcssblk_add_store(struct device *dev, struct device_attribute *attr, const char
dev_info->gd->driverfs_dev = &dev_info->dev;
blk_queue_make_request(dev_info->dcssblk_queue, dcssblk_make_request);
blk_queue_logical_block_size(dev_info->dcssblk_queue, 4096);
+ queue_flag_set_unlocked(QUEUE_FLAG_DAX, dev_info->dcssblk_queue);
seg_byte_size = (dev_info->end - dev_info->start + 1);
set_capacity(dev_info->gd, seg_byte_size >> 9); // size in sectors
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 71ccab1..9bcb3a9 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -484,6 +484,7 @@ long bdev_direct_access(struct block_device *bdev, struct blk_dax_ctl *dax)
sector_t sector = dax->sector;
long avail, size = dax->size;
const struct block_device_operations *ops = bdev->bd_disk->fops;
+ struct request_queue *q = bdev_get_queue(bdev);
/*
* The device driver is allowed to sleep, in order to make the
@@ -493,7 +494,7 @@ long bdev_direct_access(struct block_device *bdev, struct blk_dax_ctl *dax)
if (size < 0)
return size;
- if (!ops->direct_access)
+ if (!blk_queue_dax(q) || !ops->direct_access)
return -EOPNOTSUPP;
if ((sector + DIV_ROUND_UP(size, 512)) >
part_nr_sects_read(bdev->bd_part))
@@ -1287,7 +1288,7 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
bdev->bd_disk = disk;
bdev->bd_queue = disk->queue;
bdev->bd_contains = bdev;
- if (IS_ENABLED(CONFIG_BLK_DEV_DAX) && disk->fops->direct_access)
+ if (IS_ENABLED(CONFIG_BLK_DEV_DAX) && blk_queue_dax(disk->queue))
bdev->bd_inode->i_flags = S_DAX;
else
bdev->bd_inode->i_flags = 0;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 9746d22..d5cb326 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -505,6 +505,7 @@ struct request_queue {
#define QUEUE_FLAG_WC 23 /* Write back caching */
#define QUEUE_FLAG_FUA 24 /* device supports FUA writes */
#define QUEUE_FLAG_FLUSH_NQ 25 /* flush not queueuable */
+#define QUEUE_FLAG_DAX 26 /* device supports DAX */
#define QUEUE_FLAG_DEFAULT ((1 << QUEUE_FLAG_IO_STAT) | \
(1 << QUEUE_FLAG_STACKABLE) | \
@@ -594,6 +595,7 @@ static inline void queue_flag_clear(unsigned int flag, struct request_queue *q)
#define blk_queue_discard(q) test_bit(QUEUE_FLAG_DISCARD, &(q)->queue_flags)
#define blk_queue_secdiscard(q) (blk_queue_discard(q) && \
test_bit(QUEUE_FLAG_SECDISCARD, &(q)->queue_flags))
+#define blk_queue_dax(q) test_bit(QUEUE_FLAG_DAX, &(q)->queue_flags)
#define blk_noretry_request(rq) \
((rq)->cmd_flags & (REQ_FAILFAST_DEV|REQ_FAILFAST_TRANSPORT| \
--
2.7.4 (Apple Git-66)
Powered by blists - more mailing lists