* Re: [PATCH 2/8] nowait aio: Introduce RWF_NOWAIT
From: Goldwyn Rodrigues @ 2017-04-19 10:30 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-fsdevel, jack, linux-block, linux-btrfs, linux-ext4,
linux-xfs, sagi, avi, axboe, linux-api, willy, tom.leiming
In-Reply-To: <20170419063930.GB20053@infradead.org>
On 04/19/2017 01:39 AM, Christoph Hellwig wrote:
>
>> @@ -1593,6 +1593,11 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
>> }
>>
>> req->common.ki_flags |= iocb_rw_flags(iocb->aio_rw_flags);
>> + if ((req->common.ki_flags & IOCB_NOWAIT) &&
>> + !(req->common.ki_flags & IOCB_DIRECT)) {
>> + ret = -EINVAL;
>> + goto out_put_req;
>> + }
>
> Wrong indentation. Also I think this should be EOPNOTSUPP here.
>
Do we plan to add support for nowait in buffered I/O in the future? It
is just too complicated. EINVAL suits best in this case.
--
Goldwyn
^ permalink raw reply
* Re: [PATCH 2/8] nowait aio: Introduce RWF_NOWAIT
From: Jan Kara @ 2017-04-19 11:03 UTC (permalink / raw)
To: Goldwyn Rodrigues
Cc: Christoph Hellwig, linux-fsdevel, jack, linux-block, linux-btrfs,
linux-ext4, linux-xfs, sagi, avi, axboe, linux-api, willy,
tom.leiming
In-Reply-To: <2d816e3e-3625-7ebd-8550-68a7cc9c1a2e@suse.de>
On Wed 19-04-17 05:30:24, Goldwyn Rodrigues wrote:
>
>
> On 04/19/2017 01:39 AM, Christoph Hellwig wrote:
> >
> >> @@ -1593,6 +1593,11 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
> >> }
> >>
> >> req->common.ki_flags |= iocb_rw_flags(iocb->aio_rw_flags);
> >> + if ((req->common.ki_flags & IOCB_NOWAIT) &&
> >> + !(req->common.ki_flags & IOCB_DIRECT)) {
> >> + ret = -EINVAL;
> >> + goto out_put_req;
> >> + }
> >
> > Wrong indentation. Also I think this should be EOPNOTSUPP here.
> >
>
> Do we plan to add support for nowait in buffered I/O in the future? It
> is just too complicated. EINVAL suits best in this case.
Well, it may be complicated in the current implementation but IMO it makes
sense to ask for such functionality (see e.g. patches for non-blocking
buffered reads that were flying around year or two ago). So I agree with
Christoph that EOPNOTSUPP is more logical.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [GIT PULL] (xen) stable/for-jens-4.12
From: Jens Axboe @ 2017-04-19 14:14 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, linux-block, xen-devel, linux-kernel
In-Reply-To: <20170418183123.GA26293@char.us.oracle.com>
On 04/18/2017 12:31 PM, Konrad Rzeszutek Wilk wrote:
> Hey Jens,
>
> Please git pull the following branch:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git stable/for-jens-4.12
>
> which is based on your 'for-4.12/block' branch. It has one fix - to emit an
> uevent whenever the size of the guest disk image changes.
Pulled, thanks.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH] nbd: set the max segment size to UINT_MAX
From: Jens Axboe @ 2017-04-19 14:16 UTC (permalink / raw)
To: Josef Bacik, axboe, nbd-general, linux-block, kernel-team
In-Reply-To: <1492546971-2682-1-git-send-email-jbacik@fb.com>
On 04/18/2017 02:22 PM, Josef Bacik wrote:
> NBD doesn't care about limiting the segment size, let the user push the
> largest bio's they want. This allows us to control the request size
> solely through max_sectors_kb.
Thanks Josef, applied for 4.12.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH V4 00/16] Introduce the BFQ I/O scheduler
From: Jens Axboe @ 2017-04-19 14:33 UTC (permalink / raw)
To: Paolo Valente, Tejun Heo
Cc: Fabio Checconi, Arianna Avanzini, linux-block, Linux-Kernal,
Ulf Hansson, Linus Walleij, broonie
In-Reply-To: <62E939C2-CBA8-4D33-AEEF-380616616E52@linaro.org>
On 04/19/2017 03:23 AM, Paolo Valente wrote:
>
>> Il giorno 12 apr 2017, alle ore 18:23, Paolo Valente <paolo.valente@linaro.org> ha scritto:
>>
>> Hi,
>> new patch series, addressing (both) issues raised by Bart [1], and
>> with block/Makefile fixed as suggested by Bart [2].
>>
>
> Hi Jens,
> apparently no complain of any sort on this last series. Do you think
> we could make it for 4.12, or shall we aim at 4.13?
We may as well queue it up now, I don't think there's much point in
deferring an extra cycle.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH] block: Make writeback throttling defaults consistent for SQ devices
From: Jens Axboe @ 2017-04-19 14:34 UTC (permalink / raw)
To: Jan Kara; +Cc: linux-block, Bart Van Assche
In-Reply-To: <20170419093327.14986-1-jack@suse.cz>
On 04/19/2017 03:33 AM, Jan Kara wrote:
> When CFQ is used as an elevator, it disables writeback throttling
> because they don't play well together. Later when a different elevator
> is chosen for the device, writeback throttling doesn't get enabled
> again as it should. Make sure CFQ enables writeback throttling (if it
> should be enabled by default) when we switch from it to another IO
> scheduler.
Thanks Jan, added.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH 4/5] block: Inline blk_rq_set_prio()
From: Jens Axboe @ 2017-04-19 14:38 UTC (permalink / raw)
To: Christoph Hellwig, Bart Van Assche; +Cc: linux-block, Matias Bjørling
In-Reply-To: <20170419062051.GD6412@lst.de>
On 04/19/2017 12:20 AM, Christoph Hellwig wrote:
>> + req->ioprio = ioprio_valid(bio_prio(bio)) ? bio_prio(bio) : ioc ?
>> + ioc->ioprio : IOPRIO_PRIO_VALUE(IOPRIO_CLASS_NONE, 0);
>
> I think this would be a tad cleaner with a traditional if / else if / else
> chain, e.g.
>
> if (ioprio_valid(bio_prio(bio)))
> req->ioprio = bio_prio(bio);
> else if (ioc)
> req->ioprio = ioc->ioprio;
> else
> req->ioprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_NONE, 0);
Agree, I hate ternaries with a vengeance, and a doubly nested one is
almost a shooting offense.
Bart, care to respin with this fixed and the GPL export modification?
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH 26/27] scsi: sd: Separate zeroout and discard command choices
From: Paolo Bonzini @ 2017-04-19 14:56 UTC (permalink / raw)
To: Christoph Hellwig, axboe, martin.petersen, agk, snitzer, shli,
philipp.reisner, lars.ellenberg
Cc: linux-block, linux-scsi, drbd-dev, dm-devel, linux-raid
In-Reply-To: <20170405172125.22600-27-hch@lst.de>
On 05/04/2017 19:21, Christoph Hellwig wrote:
> +static ssize_t
> +zeroing_mode_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct scsi_disk *sdkp = to_scsi_disk(dev);
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return -EACCES;
> +
> +
> + if (!strncmp(buf, zeroing_mode[SD_ZERO_WRITE], 20))
> + sdkp->zeroing_mode = SD_ZERO_WRITE;
> + else if (!strncmp(buf, zeroing_mode[SD_ZERO_WS], 20))
> + sdkp->zeroing_mode = SD_ZERO_WS;
> + else if (!strncmp(buf, zeroing_mode[SD_ZERO_WS16_UNMAP], 20))
> + sdkp->zeroing_mode = SD_ZERO_WS16_UNMAP;
> + else if (!strncmp(buf, zeroing_mode[SD_ZERO_WS10_UNMAP], 20))
> + sdkp->zeroing_mode = SD_ZERO_WS10_UNMAP;
Should this be conditional on lbprz, lbpws, lbpws10 and max_ws_blocks?
Thanks,
Paolo
^ permalink raw reply
* Re: RFC: drop the T10 OSD code and its users
From: Jens Axboe @ 2017-04-19 15:11 UTC (permalink / raw)
To: Christoph Hellwig
Cc: ooo, martin.petersen, trond.myklebust, osd-dev, linux-nfs,
linux-scsi, linux-block, linux-kernel
In-Reply-To: <20170412160109.10598-1-hch@lst.de>
On Wed, Apr 12 2017, Christoph Hellwig wrote:
> The only real user of the T10 OSD protocol, the pNFS object layout
> driver never went to the point of having shipping products, and the
> other two users (osdblk and exofs) were simple example of it's usage.
>
> The code has been mostly unmaintained for years and is getting in the
> way of block / SCSI changes, so I think it's finally time to drop it.
>
> These patches are against Jens' block for-next tree as that already
I've added 1/4 for now, as that one should obviously go. If we have
consensus to kill the rest, I'm all for it.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH 5/8] nowait aio: return on congested block device
From: Goldwyn Rodrigues @ 2017-04-19 15:21 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-fsdevel, jack, linux-block, linux-btrfs, linux-ext4,
linux-xfs, sagi, avi, axboe, linux-api, willy, tom.leiming,
Goldwyn Rodrigues
In-Reply-To: <20170419064505.GD20053@infradead.org>
On 04/19/2017 01:45 AM, Christoph Hellwig wrote:
>
>> + if (bio->bi_opf & REQ_NOWAIT) {
>> + if (!blk_queue_nowait(q)) {
>> + err = -EOPNOTSUPP;
>> + goto end_io;
>> + }
>> + if (!(bio->bi_opf & REQ_SYNC)) {
>
> I don't understand this check at all..
It is to ensure at the block layer that NOWAIT comes only for DIRECT
calls only. I should probably change it to REQ_SYNC | REQ_IDLE.
>
>> + if (unlikely(!blk_queue_dying(q) && (bio->bi_opf & REQ_NOWAIT)))
>
> Please break lines after 80 characters.
>
>> @@ -119,6 +119,9 @@ struct request *blk_mq_sched_get_request(struct request_queue *q,
>> if (likely(!data->hctx))
>> data->hctx = blk_mq_map_queue(q, data->ctx->cpu);
>>
>> + if (bio && (bio->bi_opf & REQ_NOWAIT))
>> + data->flags |= BLK_MQ_REQ_NOWAIT;
>
> Check the op flag again here.
>
>> +++ b/block/blk-mq.c
>> @@ -1538,6 +1538,8 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
>> rq = blk_mq_sched_get_request(q, bio, bio->bi_opf, &data);
>> if (unlikely(!rq)) {
>> __wbt_done(q->rq_wb, wb_acct);
>> + if (bio && (bio->bi_opf & REQ_NOWAIT))
>> + bio_wouldblock_error(bio);
>
> bio iѕ dereferences unconditionally above, so you can do the same.
>
>> @@ -1662,6 +1664,8 @@ static blk_qc_t blk_sq_make_request(struct request_queue *q, struct bio *bio)
>> rq = blk_mq_sched_get_request(q, bio, bio->bi_opf, &data);
>> if (unlikely(!rq)) {
>> __wbt_done(q->rq_wb, wb_acct);
>> + if (bio && (bio->bi_opf & REQ_NOWAIT))
>> + bio_wouldblock_error(bio);
>
> Same here. Although blk_sq_make_request is gone anyway in the current
> block tree..
>
>> + /* Request queue supports BIO_NOWAIT */
>> + queue_flag_set_unlocked(QUEUE_FLAG_NOWAIT, q);
>
> BIO_NOWAIT is gone. And the comment would not be needed if the
> flag had a more descriptive name, e.g. QUEUE_FLAG_NOWAIT_SUPPORT.
>
> And I think all request based drivers should set the flag implicitly
> as ->queuecommand can't sleep, and ->queue_rq only when it's always
> offloaded to a workqueue when the BLK_MQ_F_BLOCKING flag is set.
>
Yes, Do we have a central point (like a probe() function call?) where
this can be done?
--
Goldwyn
^ permalink raw reply
* Re: [resend PATCH v2 08/33] dcssblk: add dax_operations support
From: Gerald Schaefer @ 2017-04-19 15:31 UTC (permalink / raw)
To: Dan Williams
Cc: linux-nvdimm, linux-kernel, linux-block, dm-devel, linux-fsdevel,
hch
In-Reply-To: <149245617283.10206.846177305206642788.stgit@dwillia2-desk3.amr.corp.intel.com>
On Mon, 17 Apr 2017 12:09:32 -0700
Dan Williams <dan.j.williams@intel.com> wrote:
> Setup a dax_dev to have the same lifetime as the dcssblk block device
> and add a ->direct_access() method that is equivalent to
> dcssblk_direct_access(). Once fs/dax.c has been converted to use
> dax_operations the old dcssblk_direct_access() will be removed.
>
> Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> drivers/s390/block/Kconfig | 1 +
> drivers/s390/block/dcssblk.c | 54 +++++++++++++++++++++++++++++++++++-------
> 2 files changed, 46 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/s390/block/Kconfig b/drivers/s390/block/Kconfig
> index 4a3b62326183..0acb8c2f9475 100644
> --- a/drivers/s390/block/Kconfig
> +++ b/drivers/s390/block/Kconfig
> @@ -14,6 +14,7 @@ config BLK_DEV_XPRAM
>
> config DCSSBLK
> def_tristate m
> + select DAX
> prompt "DCSSBLK support"
> depends on S390 && BLOCK
> help
> diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c
> index 415d10a67b7a..682a9eb4934d 100644
> --- a/drivers/s390/block/dcssblk.c
> +++ b/drivers/s390/block/dcssblk.c
> @@ -18,6 +18,7 @@
> #include <linux/interrupt.h>
> #include <linux/platform_device.h>
> #include <linux/pfn_t.h>
> +#include <linux/dax.h>
> #include <asm/extmem.h>
> #include <asm/io.h>
>
> @@ -30,8 +31,10 @@ static int dcssblk_open(struct block_device *bdev, fmode_t mode);
> static void dcssblk_release(struct gendisk *disk, fmode_t mode);
> static blk_qc_t dcssblk_make_request(struct request_queue *q,
> struct bio *bio);
> -static long dcssblk_direct_access(struct block_device *bdev, sector_t secnum,
> +static long dcssblk_blk_direct_access(struct block_device *bdev, sector_t secnum,
> void **kaddr, pfn_t *pfn, long size);
> +static long dcssblk_dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff,
> + long nr_pages, void **kaddr, pfn_t *pfn);
>
> static char dcssblk_segments[DCSSBLK_PARM_LEN] = "\0";
>
> @@ -40,7 +43,11 @@ static const struct block_device_operations dcssblk_devops = {
> .owner = THIS_MODULE,
> .open = dcssblk_open,
> .release = dcssblk_release,
> - .direct_access = dcssblk_direct_access,
> + .direct_access = dcssblk_blk_direct_access,
> +};
> +
> +static const struct dax_operations dcssblk_dax_ops = {
> + .direct_access = dcssblk_dax_direct_access,
> };
>
> struct dcssblk_dev_info {
> @@ -57,6 +64,7 @@ struct dcssblk_dev_info {
> struct request_queue *dcssblk_queue;
> int num_of_segments;
> struct list_head seg_list;
> + struct dax_device *dax_dev;
> };
>
> struct segment_info {
> @@ -389,6 +397,8 @@ dcssblk_shared_store(struct device *dev, struct device_attribute *attr, const ch
> }
> list_del(&dev_info->lh);
>
> + kill_dax(dev_info->dax_dev);
> + put_dax(dev_info->dax_dev);
> del_gendisk(dev_info->gd);
> blk_cleanup_queue(dev_info->dcssblk_queue);
> dev_info->gd->queue = NULL;
> @@ -525,6 +535,7 @@ dcssblk_add_store(struct device *dev, struct device_attribute *attr, const char
> int rc, i, j, num_of_segments;
> struct dcssblk_dev_info *dev_info;
> struct segment_info *seg_info, *temp;
> + struct dax_device *dax_dev;
> char *local_buf;
> unsigned long seg_byte_size;
>
> @@ -654,6 +665,11 @@ dcssblk_add_store(struct device *dev, struct device_attribute *attr, const char
> if (rc)
> goto put_dev;
>
> + dax_dev = alloc_dax(dev_info, dev_info->gd->disk_name,
> + &dcssblk_dax_ops);
> + if (!dax_dev)
> + goto put_dev;
> +
The returned dax_dev should be stored into dev_info->dax_dev, for later use
by kill/put_dax(). This can also be done directly, so that we don't need the
local dax_dev variable here.
Also, in the error case, a proper rc should be set before going to put_dev,
probably -ENOMEM.
I took a quick look at the patches for the other affected drivers, and it
looks like axonram also has the "missing rc" issue, and brd the "missing
brd->dax_dev init" issue, pmem seems to be fine.
> get_device(&dev_info->dev);
> device_add_disk(&dev_info->dev, dev_info->gd);
>
> @@ -752,6 +768,8 @@ dcssblk_remove_store(struct device *dev, struct device_attribute *attr, const ch
> }
>
> list_del(&dev_info->lh);
> + kill_dax(dev_info->dax_dev);
> + put_dax(dev_info->dax_dev);
> del_gendisk(dev_info->gd);
> blk_cleanup_queue(dev_info->dcssblk_queue);
> dev_info->gd->queue = NULL;
> @@ -883,21 +901,39 @@ dcssblk_make_request(struct request_queue *q, struct bio *bio)
> }
>
> static long
> -dcssblk_direct_access (struct block_device *bdev, sector_t secnum,
> +__dcssblk_direct_access(struct dcssblk_dev_info *dev_info, pgoff_t pgoff,
> + long nr_pages, void **kaddr, pfn_t *pfn)
> +{
> + resource_size_t offset = pgoff * PAGE_SIZE;
> + unsigned long dev_sz;
> +
> + dev_sz = dev_info->end - dev_info->start + 1;
> + *kaddr = (void *) dev_info->start + offset;
> + *pfn = __pfn_to_pfn_t(PFN_DOWN(dev_info->start + offset), PFN_DEV);
> +
> + return (dev_sz - offset) / PAGE_SIZE;
> +}
> +
> +static long
> +dcssblk_blk_direct_access(struct block_device *bdev, sector_t secnum,
> void **kaddr, pfn_t *pfn, long size)
> {
> struct dcssblk_dev_info *dev_info;
> - unsigned long offset, dev_sz;
>
> dev_info = bdev->bd_disk->private_data;
> if (!dev_info)
> return -ENODEV;
> - dev_sz = dev_info->end - dev_info->start + 1;
> - offset = secnum * 512;
> - *kaddr = (void *) dev_info->start + offset;
> - *pfn = __pfn_to_pfn_t(PFN_DOWN(dev_info->start + offset), PFN_DEV);
> + return __dcssblk_direct_access(dev_info, PHYS_PFN(secnum * 512),
> + PHYS_PFN(size), kaddr, pfn) * PAGE_SIZE;
> +}
> +
> +static long
> +dcssblk_dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff,
> + long nr_pages, void **kaddr, pfn_t *pfn)
> +{
> + struct dcssblk_dev_info *dev_info = dax_get_private(dax_dev);
>
> - return dev_sz - offset;
> + return __dcssblk_direct_access(dev_info, pgoff, nr_pages, kaddr, pfn);
> }
>
> static void
>
^ permalink raw reply
* Re: bfq-mq performance comparison to cfq
From: Bart Van Assche @ 2017-04-19 15:43 UTC (permalink / raw)
To: paolo.valente@linaro.org
Cc: linux-kernel@vger.kernel.org, aherrmann@suse.com,
linux-block@vger.kernel.org, axboe@kernel.dk
In-Reply-To: <4C1ABADD-6751-45E4-8DA1-ACA5A9E1379D@linaro.org>
On Wed, 2017-04-19 at 09:02 +0200, Paolo Valente wrote:
> > Il giorno 19 apr 2017, alle ore 07:01, Bart Van Assche <bart.vanassche@=
sandisk.com> ha scritto:
> > What API was used by the Android application to tell the I/O scheduler=
=20
> > to optimize for latency? Do you think that it would be sufficient if th=
e=20
> > application uses the ioprio_set() system call to set the I/O priority t=
o=20
> > IOPRIO_CLASS_RT?
>=20
> That's exactly the hack we are using in our prototype. However, it
> can only be a temporary hack, because it mixes two slightly different
> concepts: 1) the activation of weight raising and other mechanisms for
> reducing latency for the target app, 2) the assignment of a different
> priority class, which (cleanly) means just that processes in a lower
> priority class will be served only when the processes of the target
> app have no pending I/O request. Finding a clean boosting API would
> be one of the main steps to turn our prototype into a usable solution.
Hello Paolo,
Sorry but I do not agree that you call this use of I/O priorities a hack.
I also do not agree that I/O requests submitted by processes in a lower
priority class will only be served by the I/O scheduler when there are no
pending requests in a higher class. It wouldn't be that hard to modify I/O
schedulers that support I/O priorities to avoid the starvation you referred
to. What I expect that will happen is that sooner or later a Linux
distributor will start receiving bug reports about the heuristics for
detecting interactive and streaming applications and that the person who
will work on that bug report will realize that it will be easier to remove
those heuristics from BFQ and to modify streaming applications and the
software that starts interactive applications (e.g. a window manager) to
use a higher I/O priority.
Please also note that what I described above may require to introduce
additional I/O priorities in the Linux kernel next to the existing I/O
priorities RT, BE and NONE and that this may require to map multiple of
these priorities onto the same drive priority.
Bart.=
^ permalink raw reply
* Re: [resend PATCH v2 08/33] dcssblk: add dax_operations support
From: Dan Williams @ 2017-04-19 15:44 UTC (permalink / raw)
To: Gerald Schaefer
Cc: linux-nvdimm@lists.01.org, linux-kernel@vger.kernel.org,
linux-block, dm-devel, linux-fsdevel, Christoph Hellwig
In-Reply-To: <20170419173138.06621e66@thinkpad>
On Wed, Apr 19, 2017 at 8:31 AM, Gerald Schaefer
<gerald.schaefer@de.ibm.com> wrote:
> On Mon, 17 Apr 2017 12:09:32 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
>
>> Setup a dax_dev to have the same lifetime as the dcssblk block device
>> and add a ->direct_access() method that is equivalent to
>> dcssblk_direct_access(). Once fs/dax.c has been converted to use
>> dax_operations the old dcssblk_direct_access() will be removed.
>>
>> Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> ---
>> drivers/s390/block/Kconfig | 1 +
>> drivers/s390/block/dcssblk.c | 54 +++++++++++++++++++++++++++++++++++-------
>> 2 files changed, 46 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/s390/block/Kconfig b/drivers/s390/block/Kconfig
>> index 4a3b62326183..0acb8c2f9475 100644
>> --- a/drivers/s390/block/Kconfig
>> +++ b/drivers/s390/block/Kconfig
>> @@ -14,6 +14,7 @@ config BLK_DEV_XPRAM
>>
>> config DCSSBLK
>> def_tristate m
>> + select DAX
>> prompt "DCSSBLK support"
>> depends on S390 && BLOCK
>> help
>> diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c
>> index 415d10a67b7a..682a9eb4934d 100644
>> --- a/drivers/s390/block/dcssblk.c
>> +++ b/drivers/s390/block/dcssblk.c
>> @@ -18,6 +18,7 @@
>> #include <linux/interrupt.h>
>> #include <linux/platform_device.h>
>> #include <linux/pfn_t.h>
>> +#include <linux/dax.h>
>> #include <asm/extmem.h>
>> #include <asm/io.h>
>>
>> @@ -30,8 +31,10 @@ static int dcssblk_open(struct block_device *bdev, fmode_t mode);
>> static void dcssblk_release(struct gendisk *disk, fmode_t mode);
>> static blk_qc_t dcssblk_make_request(struct request_queue *q,
>> struct bio *bio);
>> -static long dcssblk_direct_access(struct block_device *bdev, sector_t secnum,
>> +static long dcssblk_blk_direct_access(struct block_device *bdev, sector_t secnum,
>> void **kaddr, pfn_t *pfn, long size);
>> +static long dcssblk_dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff,
>> + long nr_pages, void **kaddr, pfn_t *pfn);
>>
>> static char dcssblk_segments[DCSSBLK_PARM_LEN] = "\0";
>>
>> @@ -40,7 +43,11 @@ static const struct block_device_operations dcssblk_devops = {
>> .owner = THIS_MODULE,
>> .open = dcssblk_open,
>> .release = dcssblk_release,
>> - .direct_access = dcssblk_direct_access,
>> + .direct_access = dcssblk_blk_direct_access,
>> +};
>> +
>> +static const struct dax_operations dcssblk_dax_ops = {
>> + .direct_access = dcssblk_dax_direct_access,
>> };
>>
>> struct dcssblk_dev_info {
>> @@ -57,6 +64,7 @@ struct dcssblk_dev_info {
>> struct request_queue *dcssblk_queue;
>> int num_of_segments;
>> struct list_head seg_list;
>> + struct dax_device *dax_dev;
>> };
>>
>> struct segment_info {
>> @@ -389,6 +397,8 @@ dcssblk_shared_store(struct device *dev, struct device_attribute *attr, const ch
>> }
>> list_del(&dev_info->lh);
>>
>> + kill_dax(dev_info->dax_dev);
>> + put_dax(dev_info->dax_dev);
>> del_gendisk(dev_info->gd);
>> blk_cleanup_queue(dev_info->dcssblk_queue);
>> dev_info->gd->queue = NULL;
>> @@ -525,6 +535,7 @@ dcssblk_add_store(struct device *dev, struct device_attribute *attr, const char
>> int rc, i, j, num_of_segments;
>> struct dcssblk_dev_info *dev_info;
>> struct segment_info *seg_info, *temp;
>> + struct dax_device *dax_dev;
>> char *local_buf;
>> unsigned long seg_byte_size;
>>
>> @@ -654,6 +665,11 @@ dcssblk_add_store(struct device *dev, struct device_attribute *attr, const char
>> if (rc)
>> goto put_dev;
>>
>> + dax_dev = alloc_dax(dev_info, dev_info->gd->disk_name,
>> + &dcssblk_dax_ops);
>> + if (!dax_dev)
>> + goto put_dev;
>> +
>
> The returned dax_dev should be stored into dev_info->dax_dev, for later use
> by kill/put_dax(). This can also be done directly, so that we don't need the
> local dax_dev variable here.
>
> Also, in the error case, a proper rc should be set before going to put_dev,
> probably -ENOMEM.
>
> I took a quick look at the patches for the other affected drivers, and it
> looks like axonram also has the "missing rc" issue, and brd the "missing
> brd->dax_dev init" issue, pmem seems to be fine.
Thank you for taking a look. I'll get this fixed up.
^ permalink raw reply
* Re: [PATCH 4/5] block: Inline blk_rq_set_prio()
From: Bart Van Assche @ 2017-04-19 15:54 UTC (permalink / raw)
To: hch@lst.de, axboe@kernel.dk; +Cc: m@bjorling.me, linux-block@vger.kernel.org
In-Reply-To: <352b4d72-d6ed-692b-a3d9-a664f445ad76@kernel.dk>
On Wed, 2017-04-19 at 08:38 -0600, Jens Axboe wrote:
> On 04/19/2017 12:20 AM, Christoph Hellwig wrote:
> > > + req->ioprio =3D ioprio_valid(bio_prio(bio)) ? bio_prio(bio) : ioc ?
> > > + ioc->ioprio : IOPRIO_PRIO_VALUE(IOPRIO_CLASS_NONE, 0);
> >=20
> > I think this would be a tad cleaner with a traditional if / else if / e=
lse
> > chain, e.g.
> >=20
> > if (ioprio_valid(bio_prio(bio)))
> > req->ioprio =3D bio_prio(bio);
> > else if (ioc)
> > req->ioprio =3D ioc->ioprio;
> > else
> > req->ioprio =3D IOPRIO_PRIO_VALUE(IOPRIO_CLASS_NONE, 0);
>=20
> Agree, I hate ternaries with a vengeance, and a doubly nested one is
> almost a shooting offense.
>=20
> Bart, care to respin with this fixed and the GPL export modification?
Hello Christoph and Jens,
Thanks for the review and the feedback. I will make the proposed changes
and repost this patch series.
Bart.=
^ permalink raw reply
* Re: clean up a few end_request variants
From: Jens Axboe @ 2017-04-19 16:26 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-block
In-Reply-To: <20170412101359.7633-1-hch@lst.de>
On 04/12/2017 04:13 AM, Christoph Hellwig wrote:
> Just some trivial cleanup patches that I'd like to offload from
> a big series in the pipe..
Added for 4.12, thanks.
--
Jens Axboe
^ permalink raw reply
* [PATCH] lightnvm: assume 64-bit lba numbers
From: Arnd Bergmann @ 2017-04-19 17:39 UTC (permalink / raw)
To: Matias Bjorling
Cc: Arnd Bergmann, Matias Bjørling, Javier González,
Jens Axboe, linux-block, linux-kernel
The driver uses both u64 and sector_t to refer to offsets, and assigns between the
two. This causes one harmless warning when sector_t is 32-bit:
drivers/lightnvm/pblk-rb.c: In function 'pblk_rb_write_entry_gc':
include/linux/lightnvm.h:215:20: error: large integer implicitly truncated to unsigned type [-Werror=overflow]
drivers/lightnvm/pblk-rb.c:324:22: note: in expansion of macro 'ADDR_EMPTY'
As the driver is already doing this inconsistently, changing the type
won't make it worse and is an easy way to avoid the warning.
Fixes: a4bd217b4326 ("lightnvm: physical block device (pblk) target")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/lightnvm/pblk.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
index c82120ce3be5..11ed7d83f572 100644
--- a/drivers/lightnvm/pblk.h
+++ b/drivers/lightnvm/pblk.h
@@ -119,7 +119,7 @@ struct pblk_w_ctx {
struct bio_list bios; /* Original bios - used for completion
* in REQ_FUA, REQ_FLUSH case
*/
- sector_t lba; /* Logic addr. associated with entry */
+ u64 lba; /* Logic addr. associated with entry */
struct ppa_addr ppa; /* Physic addr. associated with entry */
int flags; /* Write context flags */
};
--
2.9.0
^ permalink raw reply related
* Re: blk-mq: remove the error argument to blk_mq_complete_request
From: hch @ 2017-04-19 17:54 UTC (permalink / raw)
To: Bart Van Assche
Cc: hch@bombadil.infradead.org, axboe@kernel.dk,
linux-block@vger.kernel.org, hch@lst.de, konrad.wilk@oracle.com,
roger.pau@citrix.com, linux-scsi@vger.kernel.org,
linux-nvme@lists.infradead.org, jbacik@fb.com,
james.smart@broadcom.com, dm-devel@redhat.com
In-Reply-To: <1492555816.2689.20.camel@sandisk.com>
On Tue, Apr 18, 2017 at 10:50:18PM +0000, Bart Van Assche wrote:
> On Tue, 2017-04-18 at 08:52 -0700, Christoph Hellwig wrote:
> > Now that we always have a ->complete callback we can remove the direct
> > call to blk_mq_end_request, as well as the error argument to
> > blk_mq_complete_request.
>
> Hello Christoph,
>
> Please add a runtime check that issues a warning early if a .complete
> callback function is missing.
>
> Are you sure that all blk-mq drivers have a .complete callback? In the
> request-errors branch of your block git tree I found the following:
I think the problem is that my above changelog was wrong - we only
require the complete_rq method if the driver calls the
blk_mq_complete_request function. Both rbd and ubiblock don't in
the tree. They probably should, but that's work for a separate
series.
^ permalink raw reply
* Re: [PATCH] lightnvm: assume 64-bit lba numbers
From: Matias Bjørling @ 2017-04-19 18:04 UTC (permalink / raw)
To: Arnd Bergmann, Matias Bjorling
Cc: Javier González, Jens Axboe, linux-block, linux-kernel
In-Reply-To: <20170419173931.3902058-1-arnd@arndb.de>
On 04/19/2017 07:39 PM, Arnd Bergmann wrote:
> The driver uses both u64 and sector_t to refer to offsets, and assigns between the
> two. This causes one harmless warning when sector_t is 32-bit:
>
> drivers/lightnvm/pblk-rb.c: In function 'pblk_rb_write_entry_gc':
> include/linux/lightnvm.h:215:20: error: large integer implicitly truncated to unsigned type [-Werror=overflow]
> drivers/lightnvm/pblk-rb.c:324:22: note: in expansion of macro 'ADDR_EMPTY'
>
> As the driver is already doing this inconsistently, changing the type
> won't make it worse and is an easy way to avoid the warning.
>
> Fixes: a4bd217b4326 ("lightnvm: physical block device (pblk) target")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/lightnvm/pblk.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
> index c82120ce3be5..11ed7d83f572 100644
> --- a/drivers/lightnvm/pblk.h
> +++ b/drivers/lightnvm/pblk.h
> @@ -119,7 +119,7 @@ struct pblk_w_ctx {
> struct bio_list bios; /* Original bios - used for completion
> * in REQ_FUA, REQ_FLUSH case
> */
> - sector_t lba; /* Logic addr. associated with entry */
> + u64 lba; /* Logic addr. associated with entry */
> struct ppa_addr ppa; /* Physic addr. associated with entry */
> int flags; /* Write context flags */
> };
>
Thanks Arnd. Jens, could you pick this up? Thank you
^ permalink raw reply
* Re: [PATCH] lightnvm: assume 64-bit lba numbers
From: Jens Axboe @ 2017-04-19 18:08 UTC (permalink / raw)
To: Matias Bjørling, Arnd Bergmann, Matias Bjorling
Cc: Javier González, linux-block, linux-kernel
In-Reply-To: <0c7f0894-4817-09c4-93b9-8158bbc6a562@cnexlabs.com>
On 04/19/2017 12:04 PM, Matias Bj�rling wrote:
> On 04/19/2017 07:39 PM, Arnd Bergmann wrote:
>> The driver uses both u64 and sector_t to refer to offsets, and assigns between the
>> two. This causes one harmless warning when sector_t is 32-bit:
>>
>> drivers/lightnvm/pblk-rb.c: In function 'pblk_rb_write_entry_gc':
>> include/linux/lightnvm.h:215:20: error: large integer implicitly truncated to unsigned type [-Werror=overflow]
>> drivers/lightnvm/pblk-rb.c:324:22: note: in expansion of macro 'ADDR_EMPTY'
>>
>> As the driver is already doing this inconsistently, changing the type
>> won't make it worse and is an easy way to avoid the warning.
>>
>> Fixes: a4bd217b4326 ("lightnvm: physical block device (pblk) target")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>> drivers/lightnvm/pblk.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
>> index c82120ce3be5..11ed7d83f572 100644
>> --- a/drivers/lightnvm/pblk.h
>> +++ b/drivers/lightnvm/pblk.h
>> @@ -119,7 +119,7 @@ struct pblk_w_ctx {
>> struct bio_list bios; /* Original bios - used for completion
>> * in REQ_FUA, REQ_FLUSH case
>> */
>> - sector_t lba; /* Logic addr. associated with entry */
>> + u64 lba; /* Logic addr. associated with entry */
>> struct ppa_addr ppa; /* Physic addr. associated with entry */
>> int flags; /* Write context flags */
>> };
>>
>
> Thanks Arnd. Jens, could you pick this up? Thank you
Done, thanks Arnd.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH V4 00/16] Introduce the BFQ I/O scheduler
From: Paolo Valente @ 2017-04-19 19:05 UTC (permalink / raw)
To: Jens Axboe
Cc: Tejun Heo, Fabio Checconi, Arianna Avanzini, linux-block,
Linux-Kernal, Ulf Hansson, Linus Walleij, broonie
In-Reply-To: <9270aa70-a40a-68b1-c93c-5229014abb39@kernel.dk>
> Il giorno 19 apr 2017, alle ore 16:33, Jens Axboe <axboe@kernel.dk> ha =
scritto:
>=20
> On 04/19/2017 03:23 AM, Paolo Valente wrote:
>>=20
>>> Il giorno 12 apr 2017, alle ore 18:23, Paolo Valente =
<paolo.valente@linaro.org> ha scritto:
>>>=20
>>> Hi,
>>> new patch series, addressing (both) issues raised by Bart [1], and
>>> with block/Makefile fixed as suggested by Bart [2].
>>>=20
>>=20
>> Hi Jens,
>> apparently no complain of any sort on this last series. Do you think
>> we could make it for 4.12, or shall we aim at 4.13?
>=20
> We may as well queue it up now, I don't think there's much point in
> deferring an extra cycle.
>=20
A little scary, but (of course) fine for me.
Thanks,
Paolo
> --=20
> Jens Axboe
>=20
^ permalink raw reply
* kill req->errors V3
From: Christoph Hellwig @ 2017-04-19 19:26 UTC (permalink / raw)
To: Jens Axboe
Cc: Josef Bacik, James Smart, Konrad Rzeszutek Wilk,
Roger Pau Monné, linux-scsi, linux-nvme, linux-block,
dm-devel
Currently the request structure has an errors field that is used in
various different ways. The oldest drivers use it as an error count,
blk-mq and the generic timeout code assume that it holds a Linux
errno for block completions, and various drivers use it for internal
status values, often overwriting them with Linux errnos later,
that is unless they are processing passthrough requests in which
case they'll leave their errors in it.
This series kills the ->errors field and replaced it with new fields
in the drivers (or an odd hack in a union in struct request for
two bitrotting old drivers).
Also available in the following git tree:
git://git.infradead.org/users/hch/block.git request-errors
Gitweb:
http://git.infradead.org/users/hch/block.git/shortlog/refs/heads/request-errors
Changes since V2;
- reorder one patch to be earlier in the series
- fix the argument to a bsg dprintk
- fix a kerneldoc comment
Changes since V1:
- rebased on top the latest block for-next tree
- fix error handling in nfsd blocklayout
- dropped "scsi: fix fast-fail for non-passthrough requests"
^ permalink raw reply
* kill req->errors V3
From: Christoph Hellwig @ 2017-04-19 19:26 UTC (permalink / raw)
To: Jens Axboe
Cc: Josef Bacik, James Smart, Konrad Rzeszutek Wilk,
Roger Pau Monné, linux-scsi, linux-nvme, linux-block,
dm-devel
Currently the request structure has an errors field that is used in
various different ways. The oldest drivers use it as an error count,
blk-mq and the generic timeout code assume that it holds a Linux
errno for block completions, and various drivers use it for internal
status values, often overwriting them with Linux errnos later,
that is unless they are processing passthrough requests in which
case they'll leave their errors in it.
This series kills the ->errors field and replaced it with new fields
in the drivers (or an odd hack in a union in struct request for
two bitrotting old drivers).
Also available in the following git tree:
git://git.infradead.org/users/hch/block.git request-errors
Gitweb:
http://git.infradead.org/users/hch/block.git/shortlog/refs/heads/request-errors
Changes since V2;
- reorder one patch to be earlier in the series
- fix the argument to a bsg dprintk
- fix a kerneldoc comment
Changes since V1:
- rebased on top the latest block for-next tree
- fix error handling in nfsd blocklayout
- dropped "scsi: fix fast-fail for non-passthrough requests"
^ permalink raw reply
* [PATCH 01/23] pd: don't check blk_execute_rq return value.
From: Christoph Hellwig @ 2017-04-19 19:26 UTC (permalink / raw)
To: Jens Axboe
Cc: Josef Bacik, James Smart, Konrad Rzeszutek Wilk,
Roger Pau Monné, linux-scsi, linux-nvme, linux-block,
dm-devel
In-Reply-To: <20170419192708.22098-1-hch@lst.de>
The driver never sets req->errors, so blk_execute_rq will always return 0.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/block/paride/pd.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/block/paride/pd.c b/drivers/block/paride/pd.c
index b05e151c9b38..7d2402f90978 100644
--- a/drivers/block/paride/pd.c
+++ b/drivers/block/paride/pd.c
@@ -739,18 +739,15 @@ static int pd_special_command(struct pd_unit *disk,
enum action (*func)(struct pd_unit *disk))
{
struct request *rq;
- int err = 0;
rq = blk_get_request(disk->gd->queue, REQ_OP_DRV_IN, __GFP_RECLAIM);
if (IS_ERR(rq))
return PTR_ERR(rq);
rq->special = func;
-
- err = blk_execute_rq(disk->gd->queue, disk->gd, rq, 0);
-
+ blk_execute_rq(disk->gd->queue, disk->gd, rq, 0);
blk_put_request(rq);
- return err;
+ return 0;
}
/* kernel glue structures */
--
2.11.0
^ permalink raw reply related
* [PATCH 02/23] block: remove the blk_execute_rq return value
From: Christoph Hellwig @ 2017-04-19 19:26 UTC (permalink / raw)
To: Jens Axboe
Cc: Josef Bacik, James Smart, Konrad Rzeszutek Wilk,
Roger Pau Monné, linux-scsi, linux-nvme, linux-block,
dm-devel
In-Reply-To: <20170419192708.22098-1-hch@lst.de>
The function only returns -EIO if rq->errors is non-zero, which is not
very useful and lets a large number of callers ignore the return value.
Just let the callers figure out their error themselves.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
---
block/blk-exec.c | 8 +-------
block/scsi_ioctl.c | 3 ++-
drivers/block/virtio_blk.c | 3 ++-
drivers/cdrom/cdrom.c | 3 ++-
drivers/ide/ide-atapi.c | 3 ++-
drivers/ide/ide-cd.c | 3 ++-
drivers/ide/ide-cd_ioctl.c | 3 ++-
drivers/ide/ide-devsets.c | 4 ++--
drivers/ide/ide-disk.c | 3 +--
drivers/ide/ide-ioctls.c | 7 ++++---
drivers/ide/ide-park.c | 3 ++-
drivers/ide/ide-pm.c | 3 ++-
drivers/ide/ide-taskfile.c | 4 ++--
drivers/scsi/osd/osd_initiator.c | 5 ++++-
fs/nfsd/blocklayout.c | 5 +++--
include/linux/blkdev.h | 2 +-
16 files changed, 34 insertions(+), 28 deletions(-)
diff --git a/block/blk-exec.c b/block/blk-exec.c
index 8cd0e9bc8dc8..afa383248c7c 100644
--- a/block/blk-exec.c
+++ b/block/blk-exec.c
@@ -92,11 +92,10 @@ EXPORT_SYMBOL_GPL(blk_execute_rq_nowait);
* Insert a fully prepared request at the back of the I/O scheduler queue
* for execution and wait for completion.
*/
-int blk_execute_rq(struct request_queue *q, struct gendisk *bd_disk,
+void blk_execute_rq(struct request_queue *q, struct gendisk *bd_disk,
struct request *rq, int at_head)
{
DECLARE_COMPLETION_ONSTACK(wait);
- int err = 0;
unsigned long hang_check;
rq->end_io_data = &wait;
@@ -108,10 +107,5 @@ int blk_execute_rq(struct request_queue *q, struct gendisk *bd_disk,
while (!wait_for_completion_io_timeout(&wait, hang_check * (HZ/2)));
else
wait_for_completion_io(&wait);
-
- if (rq->errors)
- err = -EIO;
-
- return err;
}
EXPORT_SYMBOL(blk_execute_rq);
diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index 82a43bb19967..b1352143f12f 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -547,7 +547,8 @@ static int __blk_send_generic(struct request_queue *q, struct gendisk *bd_disk,
scsi_req(rq)->cmd[0] = cmd;
scsi_req(rq)->cmd[4] = data;
scsi_req(rq)->cmd_len = 6;
- err = blk_execute_rq(q, bd_disk, rq, 0);
+ blk_execute_rq(q, bd_disk, rq, 0);
+ err = rq->errors ? -EIO : 0;
blk_put_request(rq);
return err;
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 2d8290169271..eaf99022bdc6 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -310,7 +310,8 @@ static int virtblk_get_id(struct gendisk *disk, char *id_str)
if (err)
goto out;
- err = blk_execute_rq(vblk->disk->queue, vblk->disk, req, false);
+ blk_execute_rq(vblk->disk->queue, vblk->disk, req, false);
+ err = req->errors ? -EIO : 0;
out:
blk_put_request(req);
return err;
diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index 87739649eac2..308501730ab3 100644
--- a/drivers/cdrom/cdrom.c
+++ b/drivers/cdrom/cdrom.c
@@ -2218,7 +2218,8 @@ static int cdrom_read_cdda_bpc(struct cdrom_device_info *cdi, __u8 __user *ubuf,
rq->timeout = 60 * HZ;
bio = rq->bio;
- if (blk_execute_rq(q, cdi->disk, rq, 0)) {
+ blk_execute_rq(q, cdi->disk, rq, 0);
+ if (rq->errors) {
struct request_sense *s = req->sense;
ret = -EIO;
cdi->last_sense = s->sense_key;
diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
index feb30061123b..1524797e1776 100644
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -107,7 +107,8 @@ int ide_queue_pc_tail(ide_drive_t *drive, struct gendisk *disk,
memcpy(scsi_req(rq)->cmd, pc->c, 12);
if (drive->media == ide_tape)
scsi_req(rq)->cmd[13] = REQ_IDETAPE_PC1;
- error = blk_execute_rq(drive->queue, disk, rq, 0);
+ blk_execute_rq(drive->queue, disk, rq, 0);
+ error = rq->errors ? -EIO : 0;
put_req:
blk_put_request(rq);
return error;
diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 74f1b7dc03f7..95c40afa9120 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -452,7 +452,8 @@ int ide_cd_queue_pc(ide_drive_t *drive, const unsigned char *cmd,
}
}
- error = blk_execute_rq(drive->queue, info->disk, rq, 0);
+ blk_execute_rq(drive->queue, info->disk, rq, 0);
+ error = rq->errors ? -EIO : 0;
if (buffer)
*bufflen = scsi_req(rq)->resid_len;
diff --git a/drivers/ide/ide-cd_ioctl.c b/drivers/ide/ide-cd_ioctl.c
index 9fcefbc8425e..f1ab726bd430 100644
--- a/drivers/ide/ide-cd_ioctl.c
+++ b/drivers/ide/ide-cd_ioctl.c
@@ -307,7 +307,8 @@ int ide_cdrom_reset(struct cdrom_device_info *cdi)
scsi_req_init(rq);
ide_req(rq)->type = ATA_PRIV_MISC;
rq->rq_flags = RQF_QUIET;
- ret = blk_execute_rq(drive->queue, cd->disk, rq, 0);
+ blk_execute_rq(drive->queue, cd->disk, rq, 0);
+ ret = rq->errors ? -EIO : 0;
blk_put_request(rq);
/*
* A reset will unlock the door. If it was previously locked,
diff --git a/drivers/ide/ide-devsets.c b/drivers/ide/ide-devsets.c
index a45dda5386e4..eea6a7cb80b5 100644
--- a/drivers/ide/ide-devsets.c
+++ b/drivers/ide/ide-devsets.c
@@ -173,8 +173,8 @@ int ide_devset_execute(ide_drive_t *drive, const struct ide_devset *setting,
*(int *)&scsi_req(rq)->cmd[1] = arg;
rq->special = setting->set;
- if (blk_execute_rq(q, NULL, rq, 0))
- ret = rq->errors;
+ blk_execute_rq(q, NULL, rq, 0);
+ ret = rq->errors;
blk_put_request(rq);
return ret;
diff --git a/drivers/ide/ide-disk.c b/drivers/ide/ide-disk.c
index 186159715b71..7c06237f3479 100644
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -470,7 +470,6 @@ ide_devset_get(multcount, mult_count);
static int set_multcount(ide_drive_t *drive, int arg)
{
struct request *rq;
- int error;
if (arg < 0 || arg > (drive->id[ATA_ID_MAX_MULTSECT] & 0xff))
return -EINVAL;
@@ -484,7 +483,7 @@ static int set_multcount(ide_drive_t *drive, int arg)
drive->mult_req = arg;
drive->special_flags |= IDE_SFLAG_SET_MULTMODE;
- error = blk_execute_rq(drive->queue, NULL, rq, 0);
+ blk_execute_rq(drive->queue, NULL, rq, 0);
blk_put_request(rq);
return (drive->mult_count == arg) ? 0 : -EIO;
diff --git a/drivers/ide/ide-ioctls.c b/drivers/ide/ide-ioctls.c
index 248a3e0ceb46..3e96e531b367 100644
--- a/drivers/ide/ide-ioctls.c
+++ b/drivers/ide/ide-ioctls.c
@@ -128,7 +128,8 @@ static int ide_cmd_ioctl(ide_drive_t *drive, unsigned long arg)
rq = blk_get_request(drive->queue, REQ_OP_DRV_IN, __GFP_RECLAIM);
scsi_req_init(rq);
ide_req(rq)->type = ATA_PRIV_TASKFILE;
- err = blk_execute_rq(drive->queue, NULL, rq, 0);
+ blk_execute_rq(drive->queue, NULL, rq, 0);
+ err = rq->errors ? -EIO : 0;
blk_put_request(rq);
return err;
@@ -227,8 +228,8 @@ static int generic_drive_reset(ide_drive_t *drive)
ide_req(rq)->type = ATA_PRIV_MISC;
scsi_req(rq)->cmd_len = 1;
scsi_req(rq)->cmd[0] = REQ_DRIVE_RESET;
- if (blk_execute_rq(drive->queue, NULL, rq, 1))
- ret = rq->errors;
+ blk_execute_rq(drive->queue, NULL, rq, 1);
+ ret = rq->errors;
blk_put_request(rq);
return ret;
}
diff --git a/drivers/ide/ide-park.c b/drivers/ide/ide-park.c
index 101aed9a61ca..b4f577016f5a 100644
--- a/drivers/ide/ide-park.c
+++ b/drivers/ide/ide-park.c
@@ -37,7 +37,8 @@ static void issue_park_cmd(ide_drive_t *drive, unsigned long timeout)
scsi_req(rq)->cmd_len = 1;
ide_req(rq)->type = ATA_PRIV_MISC;
rq->special = &timeout;
- rc = blk_execute_rq(q, NULL, rq, 1);
+ blk_execute_rq(q, NULL, rq, 1);
+ rc = rq->errors ? -EIO : 0;
blk_put_request(rq);
if (rc)
goto out;
diff --git a/drivers/ide/ide-pm.c b/drivers/ide/ide-pm.c
index ec951be4b0c8..bf513f886f3c 100644
--- a/drivers/ide/ide-pm.c
+++ b/drivers/ide/ide-pm.c
@@ -27,7 +27,8 @@ int generic_ide_suspend(struct device *dev, pm_message_t mesg)
mesg.event = PM_EVENT_FREEZE;
rqpm.pm_state = mesg.event;
- ret = blk_execute_rq(drive->queue, NULL, rq, 0);
+ blk_execute_rq(drive->queue, NULL, rq, 0);
+ ret = rq->errors ? -EIO : 0;
blk_put_request(rq);
if (ret == 0 && ide_port_acpi(hwif)) {
diff --git a/drivers/ide/ide-taskfile.c b/drivers/ide/ide-taskfile.c
index 4c0007cb74e3..78924c7c9478 100644
--- a/drivers/ide/ide-taskfile.c
+++ b/drivers/ide/ide-taskfile.c
@@ -452,8 +452,8 @@ int ide_raw_taskfile(ide_drive_t *drive, struct ide_cmd *cmd, u8 *buf,
rq->special = cmd;
cmd->rq = rq;
- error = blk_execute_rq(drive->queue, NULL, rq, 0);
-
+ blk_execute_rq(drive->queue, NULL, rq, 0);
+ error = rq->errors ? -EIO : 0;
put_req:
blk_put_request(rq);
return error;
diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
index 9d0727b2bdec..44e03c3eb250 100644
--- a/drivers/scsi/osd/osd_initiator.c
+++ b/drivers/scsi/osd/osd_initiator.c
@@ -489,7 +489,10 @@ static void _set_error_resid(struct osd_request *or, struct request *req,
int osd_execute_request(struct osd_request *or)
{
- int error = blk_execute_rq(or->request->q, NULL, or->request, 0);
+ int error;
+
+ blk_execute_rq(or->request->q, NULL, or->request, 0);
+ error = or->request ? -EIO : 0;
_set_error_resid(or, or->request, error);
return error;
diff --git a/fs/nfsd/blocklayout.c b/fs/nfsd/blocklayout.c
index 92b4b41d19d2..9f618b77ffee 100644
--- a/fs/nfsd/blocklayout.c
+++ b/fs/nfsd/blocklayout.c
@@ -242,10 +242,11 @@ static int nfsd4_scsi_identify_device(struct block_device *bdev,
req->cmd[4] = bufflen & 0xff;
req->cmd_len = COMMAND_SIZE(INQUIRY);
- error = blk_execute_rq(rq->q, NULL, rq, 1);
- if (error) {
+ blk_execute_rq(rq->q, NULL, rq, 1);
+ if (rq->errors) {
pr_err("pNFS: INQUIRY 0x83 failed with: %x\n",
rq->errors);
+ error = -EIO;
goto out_put_request;
}
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index fe9c512cc6fa..d8bcd51b8a47 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -969,7 +969,7 @@ extern int blk_rq_map_kern(struct request_queue *, struct request *, void *, uns
extern int blk_rq_map_user_iov(struct request_queue *, struct request *,
struct rq_map_data *, const struct iov_iter *,
gfp_t);
-extern int blk_execute_rq(struct request_queue *, struct gendisk *,
+extern void blk_execute_rq(struct request_queue *, struct gendisk *,
struct request *, int);
extern void blk_execute_rq_nowait(struct request_queue *, struct gendisk *,
struct request *, int, rq_end_io_fn *);
--
2.11.0
^ permalink raw reply related
* [PATCH 03/23] nvme-fc: fix status code handling in nvme_fc_fcpio_done
From: Christoph Hellwig @ 2017-04-19 19:26 UTC (permalink / raw)
To: Jens Axboe
Cc: Josef Bacik, James Smart, Konrad Rzeszutek Wilk,
Roger Pau Monné, linux-scsi, linux-nvme, linux-block,
dm-devel
In-Reply-To: <20170419192708.22098-1-hch@lst.de>
nvme_complete_async_event expects the little endian status code
including the phase bit, and a new completion handler I plan to
introduce will do so as well.
Change the status variable into the little endian format with the
phase bit used in the NVMe CQE to fix / enable this.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
---
drivers/nvme/host/fc.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index fc42172c796a..aad7f9c0be32 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -1147,7 +1147,7 @@ nvme_fc_fcpio_done(struct nvmefc_fcp_req *req)
struct nvme_fc_ctrl *ctrl = op->ctrl;
struct nvme_fc_queue *queue = op->queue;
struct nvme_completion *cqe = &op->rsp_iu.cqe;
- u16 status = NVME_SC_SUCCESS;
+ __le16 status = cpu_to_le16(NVME_SC_SUCCESS << 1);
/*
* WARNING:
@@ -1182,9 +1182,9 @@ nvme_fc_fcpio_done(struct nvmefc_fcp_req *req)
sizeof(op->rsp_iu), DMA_FROM_DEVICE);
if (atomic_read(&op->state) == FCPOP_STATE_ABORTED)
- status = NVME_SC_ABORT_REQ | NVME_SC_DNR;
+ status = cpu_to_le16((NVME_SC_ABORT_REQ | NVME_SC_DNR) << 1);
else if (freq->status)
- status = NVME_SC_FC_TRANSPORT_ERROR;
+ status = cpu_to_le16(NVME_SC_FC_TRANSPORT_ERROR << 1);
/*
* For the linux implementation, if we have an unsuccesful
@@ -1212,7 +1212,7 @@ nvme_fc_fcpio_done(struct nvmefc_fcp_req *req)
*/
if (freq->transferred_length !=
be32_to_cpu(op->cmd_iu.data_len)) {
- status = NVME_SC_FC_TRANSPORT_ERROR;
+ status = cpu_to_le16(NVME_SC_FC_TRANSPORT_ERROR << 1);
goto done;
}
op->nreq.result.u64 = 0;
@@ -1229,15 +1229,15 @@ nvme_fc_fcpio_done(struct nvmefc_fcp_req *req)
freq->transferred_length ||
op->rsp_iu.status_code ||
op->rqno != le16_to_cpu(cqe->command_id))) {
- status = NVME_SC_FC_TRANSPORT_ERROR;
+ status = cpu_to_le16(NVME_SC_FC_TRANSPORT_ERROR << 1);
goto done;
}
op->nreq.result = cqe->result;
- status = le16_to_cpu(cqe->status) >> 1;
+ status = cqe->status;
break;
default:
- status = NVME_SC_FC_TRANSPORT_ERROR;
+ status = cpu_to_le16(NVME_SC_FC_TRANSPORT_ERROR << 1);
goto done;
}
@@ -1249,7 +1249,7 @@ nvme_fc_fcpio_done(struct nvmefc_fcp_req *req)
return;
}
- blk_mq_complete_request(rq, status);
+ blk_mq_complete_request(rq, le16_to_cpu(status) >> 1);
}
static int
--
2.11.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox