Linux block layer
 help / color / mirror / Atom feed
* Re: block: add a error_count field to struct request
From: Bart Van Assche @ 2017-04-18 22:57 UTC (permalink / raw)
  To: hch@bombadil.infradead.org, axboe@kernel.dk
  Cc: 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: <20170418155229.5977-18-hch@bombadil.infradead.org>

On Tue, 2017-04-18 at 08:52 -0700, Christoph Hellwig wrote:
> From: Christoph Hellwig <hch@lst.de>
>=20
> This is for the legacy floppy and ataflop drivers that currently abuse
> ->errors for this purpose.  It's stashed away in a union to not grow
> the struct size, the other fields are either used by modern drivers
> for different purposes or the I/O scheduler before queing the I/O
> to drivers.
>=20
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  include/linux/blkdev.h | 1 +
>  1 file changed, 1 insertion(+)
>=20
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 5986e1250d7d..e618164462e8 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -175,6 +175,7 @@ struct request {
>  		struct rb_node rb_node;	/* sort/lookup */
>  		struct bio_vec special_vec;
>  		void *completion_data;
> +		int error_count; /* for legacy drivers, don't use */
>  	};
> =20
>  	/*

Hello Christoph,

Both blk-mq and the traditional block layer support a .cmd_size field to
make the block layer core allocate driver-specific data at the end of struc=
t
request. Could that mechanism have been used for the error_count field?

Thanks,

Bart.=

^ permalink raw reply

* Re: blk-mq: remove the error argument to blk_mq_complete_request
From: Bart Van Assche @ 2017-04-18 22:50 UTC (permalink / raw)
  To: hch@bombadil.infradead.org, axboe@kernel.dk
  Cc: 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: <20170418155229.5977-16-hch@bombadil.infradead.org>

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:

static const struct blk_mq_ops rbd_mq_ops =3D {
	.queue_rq	=3D rbd_queue_rq,
	.init_request	=3D rbd_init_request,
};

static const struct blk_mq_ops ubiblock_mq_ops =3D {
	.queue_rq=A0=A0=A0=A0=A0=A0=A0=3D ubiblock_queue_rq,
	.init_request	=3D ubiblock_init_request,
};

Thanks,

Bart.=

^ permalink raw reply

* Re: scsi: introduce a new result field in struct scsi_request
From: Bart Van Assche @ 2017-04-18 22:34 UTC (permalink / raw)
  To: hch@bombadil.infradead.org, axboe@kernel.dk
  Cc: 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: <20170418155229.5977-8-hch@bombadil.infradead.org>

On Tue, 2017-04-18 at 08:52 -0700, Christoph Hellwig wrote:
> --- a/block/bsg.c
> +++ b/block/bsg.c
> @@ -391,13 +391,13 @@ static int blk_complete_sgv4_hdr_rq(struct request =
*rq, struct sg_io_v4 *hdr,
>  	struct scsi_request *req =3D scsi_req(rq);
>  	int ret =3D 0;
> =20
> -	dprintk("rq %p bio %p 0x%x\n", rq, bio, rq->errors);
> +	dprintk("rq %p bio %p 0x%x\n", rq, bio, rq->result);

Hello Christoph,

Did you perhaps intend "req->result" instead of "rq->result"?

> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -229,8 +229,8 @@ void scsi_queue_insert(struct scsi_cmnd *cmd, int rea=
son)
>   * @rq_flags:	flags for ->rq_flags
>   * @resid:	optional residual length
>   *
> - * returns the req->errors value which is the scsi_cmnd result
> - * field.
> + * Returns the scsi_cmnd result field if a command was executed, or a ne=
gative
> + * Linux error code if we didn't get that war.

Did you intend "war" or is that perhaps a typo?

> @@ -1905,7 +1904,7 @@ static int scsi_mq_prep_fn(struct request *req)
>  static void scsi_mq_done(struct scsi_cmnd *cmd)
>  {
>  	trace_scsi_dispatch_cmd_done(cmd);
> -	blk_mq_complete_request(cmd->request, cmd->request->errors);
> +	blk_mq_complete_request(cmd->request, 0);
>  }

Why has cmd->request->errors been changed into 0?

Bart.=

^ permalink raw reply

* Re: block: remove the blk_execute_rq return value
From: Bart Van Assche @ 2017-04-18 22:24 UTC (permalink / raw)
  To: hch@bombadil.infradead.org, axboe@kernel.dk
  Cc: 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: <20170418155229.5977-2-hch@bombadil.infradead.org>

On Tue, 2017-04-18 at 08:52 -0700, Christoph Hellwig wrote:
> diff --git a/drivers/block/paride/pd.c b/drivers/block/paride/pd.c
> index b05e151c9b38..82c6d02193ae 100644
> --- a/drivers/block/paride/pd.c
> +++ b/drivers/block/paride/pd.c
> @@ -747,7 +747,8 @@ static int pd_special_command(struct pd_unit *disk,
> =20
>  	rq->special =3D func;
> =20
> -	err =3D blk_execute_rq(disk->gd->queue, disk->gd, rq, 0);
> +	blk_execute_rq(disk->gd->queue, disk->gd, rq, 0);
> +	err =3D req->errors ? -EIO : 0;
> =20
>  	blk_put_request(rq);
>  	return err;

Hello Christoph,

Sorry that I hadn't noticed this before but shouldn't "req" be changed into
"rq"? Otherwise this patch looks fine to me. If this comment gets addressed
you can add:

Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>

^ permalink raw reply

* Re: nvme-fc: fix status code handling in nvme_fc_fcpio_done
From: James Smart @ 2017-04-18 21:08 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Josef Bacik, Konrad Rzeszutek Wilk, Roger Pau Monné,
	linux-scsi, linux-nvme, linux-block, dm-devel, Christoph Hellwig
In-Reply-To: <20170418155229.5977-3-hch@bombadil.infradead.org>


On 4/18/2017 8:52 AM, Christoph Hellwig wrote:
> From: Christoph Hellwig <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>
> ---
>

look good

-- james

Signed-off-by: James Smart <james.smart@broadcom.com>

^ permalink raw reply

* [PATCH] nbd: set the max segment size to UINT_MAX
From: Josef Bacik @ 2017-04-18 20:22 UTC (permalink / raw)
  To: axboe, nbd-general, linux-block, kernel-team

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.

Signed-off-by: Josef Bacik <jbacik@fb.com>
---
 drivers/block/nbd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index b78f23c..6e592c2 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1469,6 +1469,7 @@ static int nbd_dev_add(int index)
 	queue_flag_clear_unlocked(QUEUE_FLAG_ADD_RANDOM, disk->queue);
 	disk->queue->limits.discard_granularity = 512;
 	blk_queue_max_discard_sectors(disk->queue, UINT_MAX);
+	blk_queue_max_segment_size(disk->queue, UINT_MAX);
 	blk_queue_max_hw_sectors(disk->queue, 65536);
 	disk->queue->limits.max_sectors = 256;
 
-- 
2.7.4

^ permalink raw reply related

* [GIT PULL] (xen) stable/for-jens-4.12
From: Konrad Rzeszutek Wilk @ 2017-04-18 18:31 UTC (permalink / raw)
  To: linux-block, axboe, xen-devel, linux-kernel

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.

Please pull!

 drivers/block/xen-blkfront.c | 3 +++
 1 file changed, 3 insertions(+)

Marc Olson (1):
      blkfront: add uevent for size change

^ permalink raw reply

* Re: RDMA performance comparison: IBNBD, SCST, NVMEoF
From: Bart Van Assche @ 2017-04-18 18:22 UTC (permalink / raw)
  To: linux-block@vger.kernel.org, linux-rdma@vger.kernel.org,
	roman.penyaev@profitbricks.com, mail@fholler.de, sagi@grimberg.me,
	jinpu.wang@profitbricks.com, yun.wang@profitbricks.com,
	hch@lst.de, axboe@kernel.dk, danil.kipnis@profitbricks.com,
	Milind.dumbare@gmail.com, dledford@redhat.com
In-Reply-To: <CAJrWOzAM_A6fwFxTZVo9MmiwWF1d6Bp9zTdBvUfnDFCPY_CmCQ@mail.gmail.com>

On Tue, 2017-04-18 at 19:33 +0200, Roman Penyaev wrote:
> By current email I would like to share some fresh RDMA performance
> results of IBNBD, SCST and NVMEof, based on 4.10 kernel and variety
> of configurations.

Hello Roman,

Thank you for having shared these results. But please do not expect me
to have another look at IBNBD before the design bugs in the driver and
also in the protocol get fixed. The presentation during Vault 2017 made
it clear that the driver does not scale if more than two CPUs submit I/O
simultaneously at the initiator side. The comments Sagi posted should be
addressed but I haven't seen any progress from the IBNBD authors with
regard to these comments ...

See also:
*=A0Danil Kipnis, Infiniband Network Block Device (IBNBD), Vault 2017
(https://vault2017.sched.com/event/9Xjw/infiniband-network-block-device-ibn=
bd-danil-kipnis-profitbricks-gmbh).
* Sagi Grimberg, Re: [RFC PATCH 00/28] INFINIBAND NETWORK BLOCK
DEVICE (IBNBD), March 27th, 2017
(https://www.spinics.net/lists/linux-rdma/msg47879.html).

Best regards,

Bart.=

^ permalink raw reply

* RDMA performance comparison: IBNBD, SCST, NVMEoF
From: Roman Penyaev @ 2017-04-18 17:33 UTC (permalink / raw)
  To: Bart Van Assche, Sagi Grimberg, Doug Ledford, Jens Axboe,
	Christoph Hellwig, Fabian Holler, Milind Dumbare, Michael Wang,
	Danil Kipnis, Jinpu Wang, linux-block, linux-rdma

Hi Bart, Sagi and all,

By current email I would like to share some fresh RDMA performance
results of IBNBD, SCST and NVMEof, based on 4.10 kernel and variety
of configurations.

All fio runs are grouped by the name of a project, crucial config
differencies (e.g. CPU pinning or register_always=3DN) and two testing
modes: MANY-DISKS and MANY-JOBS.  In each group of results amount of
simultaneous fio jobs is increasing starting from 1 up to 128.  E.g.
in MANY-DISKS testing mode 1 fio job is dedicated to 1 disk, where
amount of jobs (and disks) is growing, in its turn, in MANY-JOBS
testing mode each fio job produces IO for the same disk, i.e.:

  MANY-DISKS:
     x1:
         numjobs=3D1
         [job1]
         filename=3D/dev/nvme0n1
     ...
     x128:
         numjobs=3D1
         [job1]
         filename=3D/dev/nvme0n1
         [job2]
         filename=3D/dev/nvme0n2
         ...
         [job128]
         filename=3D/dev/nvme0n128

  MANY-JOBS:
     x1:
         numjobs=3D1
         [job1]
         filename=3D/dev/nvme0n1
     ...
     x128:
         numjobs=3D128
         [job1]
         filename=3D/dev/nvme0n1

Each group of results represents itself as a performance measurement,
which can be easily plotted, taking number of jobs as X axis and iops,
overall IO latencies or anything else extracted from fio json result
files as Y axis.

FIO configurations were generated and saved along with produced fio
json results by the fio-runner.py script [1].  Complete archive with
FIO configs and results can be downloaded here [2].

The following metrics were taken from fio json results:

    write/iops     - IOPS
    write/lat/mean - average latency (=CE=BCs)

Here I would like to present reduced results table taking into account
only runs with CPU pinning in MANY-DISKS testing mode, since CPU pinning
makes more sense in terms of performance and MANY-DISKS and MANY-JOBS
results look very much similar:

write/iops (MANY-DISKS)
      IBNBD_pin   NVME_noreg_pin  NVME_pin    SCST_noreg_pin  SCST_pin
x1    80398.96    75577.24        54110.19    59555.04        48446.05
x2    109018.60   96478.45        69176.77    73925.81        55557.59
x4    169164.56   140558.75       93700.96    75419.91        56294.61
x8    197725.44   159887.33       99773.05    79750.92        55938.84
x16   176782.36   150448.33       99644.05    92964.23        56463.14
x32   139666.00   123198.38       81845.30    81287.98        50590.86
x64   125666.16   82231.77        72117.67    72023.32        45121.17
x128  120253.63   73911.97        65665.08    74642.27        47268.46

write/lat/mean (MANY-DISKS)
      IBNBD_pin   NVME_noreg_pin  NVME_pin    SCST_noreg_pin  SCST_pin
x1    647.78      697.91          1032.97     925.51          1173.04
x2    973.20      1104.38         1612.75     1462.18         2047.11
x4    1279.49     1528.09         2452.22     3188.41         4235.95
x8    2356.92     2929.87         4891.70     6248.85         8907.10
x16   5605.62     6575.70         10046.4     10830.50        17945.57
x32   14489.54    16516.60        24849.16    24984.26        40335.09
x64   32364.39    49481.42        56615.23    56559.02        90590.84
x128  67570.88    110768.70       124249.4    109321.84       171390.00

    * Where suffixes mean:

     _pin   - CPU pinning
     _noreg - modules on initiator side (ib_srp, nvme_rdma) were loaded
              with 'register_always=3DN' param

Complete table results and corresponding graphs are presented on Google
sheet [3].

Conclusion:
    IBNBD outperforms in average by:

                     NVME_noreg_pin  NVME_pin  SCST_noreg_pin  SCST_pin
       iops          41%             72%       61%             155%
       lat/mean      28%             42%       38%             60%

       * Complete tables results [3] were taken into account for average
         percentage calculation.

Test setup is the following:

Initiator and target HW configuration:

    AMD Opteron 6386 SE, 64CPU, 128Gb
    InfiniBand: Mellanox Technologies MT26428
                [ConnectX VPI PCIe 2.0 5GT/s - IB QDR / 10GigE]

Initiator and target SW configuration:

    vanilla Linux 4.10
    + IBNBD patches
    + SCST from https://github.com/bvanassche/scst, master branch

Initiator side:

    IBNBD and NVME: MQ mode
    SRP: default RQ, on attempt to set 'use_blk_mq=3DY' IO hangs.

    FIO generic configuration pattern:

        bssplit=3D512/20:1k/16:2k/9:4k/12:8k/19:16k/10:32k/8:64k/4
        fadvise_hint=3D0
        rw=3Drandrw:2
        direct=3D1
        random_distribution=3Dzipf:1.2
        time_based=3D1
        runtime=3D10
        ioengine=3Dlibaio
        iodepth=3D128
        iodepth_batch_submit=3D128
        iodepth_batch_complete=3D128
        group_reporting

Target side:

    128 null_blk devices with default configuration, opened as blockio.

NVMEoF configuration script [4].
SCST configuration script [5].


Would be great to receive any feedback.  I am open for further perf
tuning and testing with other possible configurations and options.

Thanks.

--
Roman

[1] FIO runner and results extractor script:
    https://drive.google.com/open?id=3D0B8_SivzwHdgSS2RKcmc4bWg0YjA

[2] Archive with FIO configurations and results:
    https://drive.google.com/open?id=3D0B8_SivzwHdgSaDlhMXV6THhoRXc

[3] Google sheet with performance measurements:
    https://drive.google.com/open?id=3D1sCTBKLA5gbhhkgd2USZXY43VL3zLidzdqDe=
ObZn9Edc

[4] NVMEoF configuration:
    https://drive.google.com/open?id=3D0B8_SivzwHdgSTzRjbGtmaVR6LWM

[5] SCST configuration:
    https://drive.google.com/open?id=3D0B8_SivzwHdgSM1B5eGpKWmFJMFk

^ permalink raw reply

* Re: RFC: drop the T10 OSD code and its users
From: Chandy, John @ 2017-04-18 17:06 UTC (permalink / raw)
  To: Boaz Harrosh
  Cc: Christoph Hellwig, martin.petersen@oracle.com,
	trond.myklebust@primarydata.com, axboe@kernel.dk,
	osd-dev@open-osd.org, linux-nfs@vger.kernel.org,
	linux-scsi@vger.kernel.org, linux-block@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <13c543db-dd38-1825-a58d-b4dff99e5f3c@electrozaur.com>

As one of those academics that Boaz was talking about, we do use the OSD dr=
iver for various research projects including support for OSD in parallel fi=
le systems like OrangeFS and Lustre.

John.

> On Apr 18, 2017, at 12:24 PM, Boaz Harrosh <ooo@electrozaur.com> wrote:
>=20
> On 04/12/2017 07:01 PM, Christoph Hellwig wrote:
>=20
> Hi Sir Christoph
>=20
>> 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.
>>=20
>=20
> I understand why osdblk might be a pain, and was broken from day one, and
> should by all means go away ASAP.
>=20
> But exofs should not be bothering anyone, and as far as I know does
> not use any special API's except the osd_uld code of course.
>=20
>> 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.
>>=20
>=20
> Please tell me what are those changes you are talking about? I might be
> able to help in conversion. I guess you mean osd_uld and the Upper SCSI A=
PI.
> Just point me at a tree where osd_uld is broken, and perhaps with a littl=
e
> guidance from you I can do a satisfactory conversion.
>=20
> Is true that no new code went in for a long while, but I still from time
> to time run a setup and test that the all stack, like iscsi-bidi and so o=
n still
> works.
>=20
> That said, yes only a stand alone exofs was tested for a long time, a ful=
l
> pnfs setup is missing any supporting server. So Yes I admit that pnfs-obj=
 is
> getting very rotten. And is most probably broken, on the pnfs side of thi=
ngs.
> [Which I admit makes my little plea kind of mute ;-) ]
>=20
> Every once in a while I get emails from Students basing all kind of inter=
esting
> experiments on top of the exofs and object base storage. So for the sake =
of academics
> and for the sake of a true bidi-stack testing, might we want to evaluate =
what is the
> up coming cost, and what is a minimum set we are willing to keep?
>=20
> Please advise?
>=20
> thanks
> Boaz
>=20
>> These patches are against Jens' block for-next tree as that already
>> has various modifications of the SCSI code.
>>=20
>=20
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH] block: get rid of blk_integrity_revalidate()
From: Ilya Dryomov @ 2017-04-18 16:43 UTC (permalink / raw)
  To: Martin K. Petersen
  Cc: linux-block, Mike Snitzer, ceph-devel, linux-nvme, Jens Axboe,
	dm-devel, Dan Williams, Christoph Hellwig

Commit 25520d55cdb6 ("block: Inline blk_integrity in struct gendisk")
introduced blk_integrity_revalidate(), which seems to assume ownership
of the stable pages flag and unilaterally clears it if no blk_integrity
profile is registered:

    if (bi->profile)
            disk->queue->backing_dev_info->capabilities |=
                    BDI_CAP_STABLE_WRITES;
    else
            disk->queue->backing_dev_info->capabilities &=
                    ~BDI_CAP_STABLE_WRITES;

It's called from revalidate_disk() and rescan_partitions(), making it
impossible to enable stable pages for drivers that support partitions
and don't use blk_integrity: while the call in revalidate_disk() can be
trivially worked around (see zram, which doesn't support partitions and
hence gets away with zram_revalidate_disk()), rescan_partitions() can
be triggered from userspace at any time.  This breaks rbd, where the
ceph messenger is responsible for generating/verifying CRCs.

Since blk_integrity_{un,}register() "must" be used for (un)registering
the integrity profile with the block layer, move BDI_CAP_STABLE_WRITES
setting there.  This way drivers that call blk_integrity_register() and
use integrity infrastructure won't interfere with drivers that don't
but still want stable pages.

Fixes: 25520d55cdb6 ("block: Inline blk_integrity in struct gendisk")
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: stable@vger.kernel.org # 4.4+, needs backporting
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
---
I don't have any integrity-capable disks or nvme separate-metadata
cards, so I couldn't test this as well as I wanted to.  25520d55cdb6
went in with some nvme work, but Martin recalls that it may have had
something to do with integrity-capable dm arrays losing the integrity
capability at runtime.  Previous discussion at

    http://www.spinics.net/lists/ceph-devel/msg35413.html

---
 block/blk-integrity.c     | 19 ++-----------------
 block/partition-generic.c |  1 -
 fs/block_dev.c            |  1 -
 include/linux/genhd.h     |  2 --
 4 files changed, 2 insertions(+), 21 deletions(-)

diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 9f0ff5ba4f84..35c5af1ea068 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -417,7 +417,7 @@ void blk_integrity_register(struct gendisk *disk, struct blk_integrity *template
 	bi->tuple_size = template->tuple_size;
 	bi->tag_size = template->tag_size;
 
-	blk_integrity_revalidate(disk);
+	disk->queue->backing_dev_info->capabilities |= BDI_CAP_STABLE_WRITES;
 }
 EXPORT_SYMBOL(blk_integrity_register);
 
@@ -430,26 +430,11 @@ EXPORT_SYMBOL(blk_integrity_register);
  */
 void blk_integrity_unregister(struct gendisk *disk)
 {
-	blk_integrity_revalidate(disk);
+	disk->queue->backing_dev_info->capabilities &= ~BDI_CAP_STABLE_WRITES;
 	memset(&disk->queue->integrity, 0, sizeof(struct blk_integrity));
 }
 EXPORT_SYMBOL(blk_integrity_unregister);
 
-void blk_integrity_revalidate(struct gendisk *disk)
-{
-	struct blk_integrity *bi = &disk->queue->integrity;
-
-	if (!(disk->flags & GENHD_FL_UP))
-		return;
-
-	if (bi->profile)
-		disk->queue->backing_dev_info->capabilities |=
-			BDI_CAP_STABLE_WRITES;
-	else
-		disk->queue->backing_dev_info->capabilities &=
-			~BDI_CAP_STABLE_WRITES;
-}
-
 void blk_integrity_add(struct gendisk *disk)
 {
 	if (kobject_init_and_add(&disk->integrity_kobj, &integrity_ktype,
diff --git a/block/partition-generic.c b/block/partition-generic.c
index 7afb9907821f..0171a2faad68 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -497,7 +497,6 @@ int rescan_partitions(struct gendisk *disk, struct block_device *bdev)
 
 	if (disk->fops->revalidate_disk)
 		disk->fops->revalidate_disk(disk);
-	blk_integrity_revalidate(disk);
 	check_disk_size_change(disk, bdev);
 	bdev->bd_invalidated = 0;
 	if (!get_capacity(disk) || !(state = check_partition(disk, bdev)))
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 2eca00ec4370..56039dfbc674 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1451,7 +1451,6 @@ int revalidate_disk(struct gendisk *disk)
 
 	if (disk->fops->revalidate_disk)
 		ret = disk->fops->revalidate_disk(disk);
-	blk_integrity_revalidate(disk);
 	bdev = bdget_disk(disk, 0);
 	if (!bdev)
 		return ret;
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 76f39754e7b0..76d6a1cd4153 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -722,11 +722,9 @@ static inline void part_nr_sects_write(struct hd_struct *part, sector_t size)
 #if defined(CONFIG_BLK_DEV_INTEGRITY)
 extern void blk_integrity_add(struct gendisk *);
 extern void blk_integrity_del(struct gendisk *);
-extern void blk_integrity_revalidate(struct gendisk *);
 #else	/* CONFIG_BLK_DEV_INTEGRITY */
 static inline void blk_integrity_add(struct gendisk *disk) { }
 static inline void blk_integrity_del(struct gendisk *disk) { }
-static inline void blk_integrity_revalidate(struct gendisk *disk) { }
 #endif	/* CONFIG_BLK_DEV_INTEGRITY */
 
 #else /* CONFIG_BLOCK */
-- 
2.4.3


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply related

* Re: RFC: drop the T10 OSD code and its users
From: Boaz Harrosh @ 2017-04-18 16:24 UTC (permalink / raw)
  To: Christoph Hellwig, martin.petersen, trond.myklebust, axboe
  Cc: osd-dev, linux-nfs, linux-scsi, linux-block, linux-kernel
In-Reply-To: <20170412160109.10598-1-hch@lst.de>

On 04/12/2017 07:01 PM, Christoph Hellwig wrote:

Hi Sir Christoph

> 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.
> 

I understand why osdblk might be a pain, and was broken from day one, and
should by all means go away ASAP.

But exofs should not be bothering anyone, and as far as I know does
not use any special API's except the osd_uld code of course.

> 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.
> 

Please tell me what are those changes you are talking about? I might be
able to help in conversion. I guess you mean osd_uld and the Upper SCSI API.
Just point me at a tree where osd_uld is broken, and perhaps with a little
guidance from you I can do a satisfactory conversion.

Is true that no new code went in for a long while, but I still from time
to time run a setup and test that the all stack, like iscsi-bidi and so on still
works.

That said, yes only a stand alone exofs was tested for a long time, a full
pnfs setup is missing any supporting server. So Yes I admit that pnfs-obj is
getting very rotten. And is most probably broken, on the pnfs side of things.
[Which I admit makes my little plea kind of mute ;-) ]

Every once in a while I get emails from Students basing all kind of interesting
experiments on top of the exofs and object base storage. So for the sake of academics
and for the sake of a true bidi-stack testing, might we want to evaluate what is the
up coming cost, and what is a minimum set we are willing to keep?

Please advise?

thanks
Boaz

> These patches are against Jens' block for-next tree as that already
> has various modifications of the SCSI code.
> 

^ permalink raw reply

* floppy: switch from req->errors to req->error_count
From: Christoph Hellwig @ 2017-04-18 15:52 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, Christoph Hellwig
In-Reply-To: <20170418155229.5977-1-hch@bombadil.infradead.org>

From: Christoph Hellwig <hch@lst.de>

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/floppy.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index ce102ec47ef2..60d4c7653178 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -2805,8 +2805,10 @@ static int set_next_request(void)
 			fdc_queue = 0;
 		if (q) {
 			current_req = blk_fetch_request(q);
-			if (current_req)
+			if (current_req) {
+				current_req->error_count = 0;
 				break;
+			}
 		}
 	} while (fdc_queue != old_pos);
 
@@ -2866,7 +2868,7 @@ static void redo_fd_request(void)
 		_floppy = floppy_type + DP->autodetect[DRS->probed_format];
 	} else
 		probing = 0;
-	errors = &(current_req->errors);
+	errors = &(current_req->error_count);
 	tmp = make_raw_rw_request();
 	if (tmp < 2) {
 		request_done(tmp);
-- 
2.11.0

^ permalink raw reply related

* block: add a error_count field to struct request
From: Christoph Hellwig @ 2017-04-18 15:52 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, Christoph Hellwig
In-Reply-To: <20170418155229.5977-1-hch@bombadil.infradead.org>

From: Christoph Hellwig <hch@lst.de>

This is for the legacy floppy and ataflop drivers that currently abuse
->errors for this purpose.  It's stashed away in a union to not grow
the struct size, the other fields are either used by modern drivers
for different purposes or the I/O scheduler before queing the I/O
to drivers.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/blkdev.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 5986e1250d7d..e618164462e8 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -175,6 +175,7 @@ struct request {
 		struct rb_node rb_node;	/* sort/lookup */
 		struct bio_vec special_vec;
 		void *completion_data;
+		int error_count; /* for legacy drivers, don't use */
 	};
 
 	/*
-- 
2.11.0

^ permalink raw reply related

* dm rq: don't pass irrelevant error code to blk_mq_complete_request
From: Christoph Hellwig @ 2017-04-18 15:52 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, Christoph Hellwig
In-Reply-To: <20170418155229.5977-1-hch@bombadil.infradead.org>

From: Christoph Hellwig <hch@lst.de>

dm never uses rq->errors, so there is no need to pass an error argument
to blk_mq_complete_request.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/dm-rq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
index e60f1b6845be..1173be21f6f6 100644
--- a/drivers/md/dm-rq.c
+++ b/drivers/md/dm-rq.c
@@ -363,7 +363,7 @@ static void dm_complete_request(struct request *rq, int error)
 	if (!rq->q->mq_ops)
 		blk_complete_request(rq);
 	else
-		blk_mq_complete_request(rq, error);
+		blk_mq_complete_request(rq, 0);
 }
 
 /*
-- 
2.11.0

^ permalink raw reply related

* virtio_blk: don't use req->errors
From: Christoph Hellwig @ 2017-04-18 15:52 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, Christoph Hellwig
In-Reply-To: <20170418155229.5977-1-hch@bombadil.infradead.org>

From: Christoph Hellwig <hch@lst.de>

Remove passing req->errors (which at that point is always 0) to
blk_mq_complete_request, and rely on the virtio status code for the
serial number passthrough request.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 drivers/block/virtio_blk.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index dbc4e80680b1..8378ad480f77 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -175,19 +175,15 @@ static int virtblk_add_req(struct virtqueue *vq, struct virtblk_req *vbr,
 static inline void virtblk_request_done(struct request *req)
 {
 	struct virtblk_req *vbr = blk_mq_rq_to_pdu(req);
-	int error = virtblk_result(vbr);
 
 	switch (req_op(req)) {
 	case REQ_OP_SCSI_IN:
 	case REQ_OP_SCSI_OUT:
 		virtblk_scsi_request_done(req);
 		break;
-	case REQ_OP_DRV_IN:
-		req->errors = (error != 0);
-		break;
 	}
 
-	blk_mq_end_request(req, error);
+	blk_mq_end_request(req, virtblk_result(vbr));
 }
 
 static void virtblk_done(struct virtqueue *vq)
@@ -205,7 +201,7 @@ static void virtblk_done(struct virtqueue *vq)
 		while ((vbr = virtqueue_get_buf(vblk->vqs[qid].vq, &len)) != NULL) {
 			struct request *req = blk_mq_rq_from_pdu(vbr);
 
-			blk_mq_complete_request(req, req->errors);
+			blk_mq_complete_request(req, 0);
 			req_done = true;
 		}
 		if (unlikely(virtqueue_is_broken(vq)))
@@ -311,7 +307,7 @@ static int virtblk_get_id(struct gendisk *disk, char *id_str)
 		goto out;
 
 	blk_execute_rq(vblk->disk->queue, vblk->disk, req, false);
-	err = req->errors ? -EIO : 0;
+	err = virtblk_result(blk_mq_rq_to_pdu(req));
 out:
 	blk_put_request(req);
 	return err;
-- 
2.11.0

^ permalink raw reply related

* swim3: remove (commented out) printing of req->errors
From: Christoph Hellwig @ 2017-04-18 15:52 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, Christoph Hellwig
In-Reply-To: <20170418155229.5977-1-hch@bombadil.infradead.org>

From: Christoph Hellwig <hch@lst.de>

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/swim3.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/block/swim3.c b/drivers/block/swim3.c
index 61b3ffa4f458..ba4809c9bdba 100644
--- a/drivers/block/swim3.c
+++ b/drivers/block/swim3.c
@@ -343,8 +343,8 @@ static void start_request(struct floppy_state *fs)
 			  req->rq_disk->disk_name, req->cmd,
 			  (long)blk_rq_pos(req), blk_rq_sectors(req),
 			  bio_data(req->bio));
-		swim3_dbg("           errors=%d current_nr_sectors=%u\n",
-			  req->errors, blk_rq_cur_sectors(req));
+		swim3_dbg("           current_nr_sectors=%u\n",
+			  blk_rq_cur_sectors(req));
 #endif
 
 		if (blk_rq_pos(req) >= fs->total_secs) {
-- 
2.11.0

^ permalink raw reply related

* nvme: split nvme status from block req->errors
From: Christoph Hellwig @ 2017-04-18 15:52 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, Christoph Hellwig
In-Reply-To: <20170418155229.5977-1-hch@bombadil.infradead.org>

From: Christoph Hellwig <hch@lst.de>

We want our own clearly defined error field for NVMe passthrough commands,
and the request errors field is going away in its current form.

Just store the status and result field in the nvme_request field from
hardirq completion context (using a new helper) and then generate a
Linux errno for the block layer only when we actually need it.

Because we can't overload the status value with a negative error code
for cancelled command we now have a flags filed in struct nvme_request
that contains a bit for this condition.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 drivers/nvme/host/core.c     | 50 +++++++++++++++++++++++++++-----------------
 drivers/nvme/host/fc.c       | 10 ++++-----
 drivers/nvme/host/lightnvm.c |  9 +++++---
 drivers/nvme/host/nvme.h     | 33 +++++++++++++----------------
 drivers/nvme/host/pci.c      | 11 +++++-----
 drivers/nvme/host/rdma.c     |  5 ++---
 drivers/nvme/target/loop.c   |  7 ++-----
 7 files changed, 65 insertions(+), 60 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index d33f829c3ab7..c6f256d74b6b 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -66,11 +66,24 @@ static DEFINE_SPINLOCK(dev_list_lock);
 
 static struct class *nvme_class;
 
+int nvme_error_status(struct request *req)
+{
+	switch (nvme_req(req)->status & 0x7ff) {
+	case NVME_SC_SUCCESS:
+		return 0;
+	case NVME_SC_CAP_EXCEEDED:
+		return -ENOSPC;
+	default:
+		return -EIO;
+	}
+}
+EXPORT_SYMBOL_GPL(nvme_error_status);
+
 static inline bool nvme_req_needs_retry(struct request *req)
 {
 	if (blk_noretry_request(req))
 		return false;
-	if (req->errors & NVME_SC_DNR)
+	if (nvme_req(req)->status & NVME_SC_DNR)
 		return false;
 	if (jiffies - req->start_time >= req->timeout)
 		return false;
@@ -81,23 +94,13 @@ static inline bool nvme_req_needs_retry(struct request *req)
 
 void nvme_complete_rq(struct request *req)
 {
-	int error = 0;
-
-	if (unlikely(req->errors)) {
-		if (nvme_req_needs_retry(req)) {
-			nvme_req(req)->retries++;
-			blk_mq_requeue_request(req,
-					!blk_mq_queue_stopped(req->q));
-			return;
-		}
-
-		if (blk_rq_is_passthrough(req))
-			error = req->errors;
-		else
-			error = nvme_error_status(req->errors);
+	if (unlikely(nvme_req(req)->status && nvme_req_needs_retry(req))) {
+		nvme_req(req)->retries++;
+		blk_mq_requeue_request(req, !blk_mq_queue_stopped(req->q));
+		return;
 	}
 
-	blk_mq_end_request(req, error);
+	blk_mq_end_request(req, nvme_error_status(req));
 }
 EXPORT_SYMBOL_GPL(nvme_complete_rq);
 
@@ -114,7 +117,9 @@ void nvme_cancel_request(struct request *req, void *data, bool reserved)
 	status = NVME_SC_ABORT_REQ;
 	if (blk_queue_dying(req->q))
 		status |= NVME_SC_DNR;
-	blk_mq_complete_request(req, status);
+	nvme_req(req)->status = status;
+	blk_mq_complete_request(req, 0);
+
 }
 EXPORT_SYMBOL_GPL(nvme_cancel_request);
 
@@ -357,6 +362,7 @@ int nvme_setup_cmd(struct nvme_ns *ns, struct request *req,
 
 	if (!(req->rq_flags & RQF_DONTPREP)) {
 		nvme_req(req)->retries = 0;
+		nvme_req(req)->flags = 0;
 		req->rq_flags |= RQF_DONTPREP;
 	}
 
@@ -413,7 +419,10 @@ int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
 	blk_execute_rq(req->q, NULL, req, at_head);
 	if (result)
 		*result = nvme_req(req)->result;
-	ret = req->errors;
+	if (nvme_req(req)->flags & NVME_REQ_CANCELLED)
+		ret = -EINTR;
+	else
+		ret = nvme_req(req)->status;
  out:
 	blk_mq_free_request(req);
 	return ret;
@@ -498,7 +507,10 @@ int __nvme_submit_user_cmd(struct request_queue *q, struct nvme_command *cmd,
 	}
  submit:
 	blk_execute_rq(req->q, disk, req, 0);
-	ret = req->errors;
+	if (nvme_req(req)->flags & NVME_REQ_CANCELLED)
+		ret = -EINTR;
+	else
+		ret = nvme_req(req)->status;
 	if (result)
 		*result = le32_to_cpu(nvme_req(req)->result.u32);
 	if (meta && !ret && !write) {
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index aad7f9c0be32..450733c8cd24 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -1148,6 +1148,7 @@ nvme_fc_fcpio_done(struct nvmefc_fcp_req *req)
 	struct nvme_fc_queue *queue = op->queue;
 	struct nvme_completion *cqe = &op->rsp_iu.cqe;
 	__le16 status = cpu_to_le16(NVME_SC_SUCCESS << 1);
+	union nvme_result result;
 
 	/*
 	 * WARNING:
@@ -1215,7 +1216,7 @@ nvme_fc_fcpio_done(struct nvmefc_fcp_req *req)
 			status = cpu_to_le16(NVME_SC_FC_TRANSPORT_ERROR << 1);
 			goto done;
 		}
-		op->nreq.result.u64 = 0;
+		result.u64 = 0;
 		break;
 
 	case sizeof(struct nvme_fc_ersp_iu):
@@ -1232,7 +1233,7 @@ nvme_fc_fcpio_done(struct nvmefc_fcp_req *req)
 			status = cpu_to_le16(NVME_SC_FC_TRANSPORT_ERROR << 1);
 			goto done;
 		}
-		op->nreq.result = cqe->result;
+		result = cqe->result;
 		status = cqe->status;
 		break;
 
@@ -1243,13 +1244,12 @@ nvme_fc_fcpio_done(struct nvmefc_fcp_req *req)
 
 done:
 	if (!queue->qnum && op->rqno >= AEN_CMDID_BASE) {
-		nvme_complete_async_event(&queue->ctrl->ctrl, status,
-					&op->nreq.result);
+		nvme_complete_async_event(&queue->ctrl->ctrl, status, &result);
 		nvme_fc_ctrl_put(ctrl);
 		return;
 	}
 
-	blk_mq_complete_request(rq, le16_to_cpu(status) >> 1);
+	nvme_end_request(rq, status, result);
 }
 
 static int
diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
index 4b78090518e1..f85e6e57d641 100644
--- a/drivers/nvme/host/lightnvm.c
+++ b/drivers/nvme/host/lightnvm.c
@@ -484,7 +484,7 @@ static void nvme_nvm_end_io(struct request *rq, int error)
 	struct nvm_rq *rqd = rq->end_io_data;
 
 	rqd->ppa_status = nvme_req(rq)->result.u64;
-	rqd->error = error;
+	rqd->error = nvme_req(rq)->status;
 	nvm_end_io(rqd);
 
 	kfree(nvme_req(rq)->cmd);
@@ -669,9 +669,12 @@ static int nvme_nvm_submit_user_cmd(struct request_queue *q,
 
 	wait_for_completion_io(&wait);
 
-	ret = nvme_error_status(rq->errors);
+	if (nvme_req(rq)->flags & NVME_REQ_CANCELLED)
+		ret = -EINTR;
+	else
+		ret = nvme_error_status(rq);
 	if (result)
-		*result = rq->errors & 0x7ff;
+		*result = nvme_req(rq)->status & 0x7ff;
 	if (status)
 		*status = le64_to_cpu(nvme_req(rq)->result.u64);
 
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index a317461c21d3..d7330f75632d 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -21,16 +21,6 @@
 #include <linux/lightnvm.h>
 #include <linux/sed-opal.h>
 
-enum {
-	/*
-	 * Driver internal status code for commands that were cancelled due
-	 * to timeouts or controller shutdown.  The value is negative so
-	 * that it a) doesn't overlap with the unsigned hardware error codes,
-	 * and b) can easily be tested for.
-	 */
-	NVME_SC_CANCELLED		= -EINTR,
-};
-
 extern unsigned char nvme_io_timeout;
 #define NVME_IO_TIMEOUT	(nvme_io_timeout * HZ)
 
@@ -91,6 +81,12 @@ struct nvme_request {
 	struct nvme_command	*cmd;
 	union nvme_result	result;
 	u8			retries;
+	u8			flags;
+	u16			status;
+};
+
+enum {
+	NVME_REQ_CANCELLED		= (1 << 0),
 };
 
 static inline struct nvme_request *nvme_req(struct request *req)
@@ -248,18 +244,17 @@ static inline void nvme_cleanup_cmd(struct request *req)
 	}
 }
 
-static inline int nvme_error_status(u16 status)
+static inline void nvme_end_request(struct request *req, __le16 status,
+		union nvme_result result)
 {
-	switch (status & 0x7ff) {
-	case NVME_SC_SUCCESS:
-		return 0;
-	case NVME_SC_CAP_EXCEEDED:
-		return -ENOSPC;
-	default:
-		return -EIO;
-	}
+	struct nvme_request *rq = nvme_req(req);
+
+	rq->status = le16_to_cpu(status) >> 1;
+	rq->result = result;
+	blk_mq_complete_request(req, 0);
 }
 
+int nvme_error_status(struct request *req);
 void nvme_complete_rq(struct request *req);
 void nvme_cancel_request(struct request *req, void *data, bool reserved);
 bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index f7a9f3fb5fdb..d9e2bd07ed56 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -679,8 +679,7 @@ static void __nvme_process_cq(struct nvme_queue *nvmeq, unsigned int *tag)
 		}
 
 		req = blk_mq_tag_to_rq(*nvmeq->tags, cqe.command_id);
-		nvme_req(req)->result = cqe.result;
-		blk_mq_complete_request(req, le16_to_cpu(cqe.status) >> 1);
+		nvme_end_request(req, cqe.status, cqe.result);
 	}
 
 	if (head == nvmeq->cq_head && phase == nvmeq->cq_phase)
@@ -817,9 +816,9 @@ static void abort_endio(struct request *req, int error)
 {
 	struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
 	struct nvme_queue *nvmeq = iod->nvmeq;
-	u16 status = req->errors;
 
-	dev_warn(nvmeq->dev->ctrl.device, "Abort status: 0x%x", status);
+	dev_warn(nvmeq->dev->ctrl.device,
+		 "Abort status: 0x%x", nvme_req(req)->status);
 	atomic_inc(&nvmeq->dev->ctrl.abort_limit);
 	blk_mq_free_request(req);
 }
@@ -843,7 +842,7 @@ static enum blk_eh_timer_return nvme_timeout(struct request *req, bool reserved)
 			 "I/O %d QID %d timeout, disable controller\n",
 			 req->tag, nvmeq->qid);
 		nvme_dev_disable(dev, false);
-		req->errors = NVME_SC_CANCELLED;
+		nvme_req(req)->flags |= NVME_REQ_CANCELLED;
 		return BLK_EH_HANDLED;
 	}
 
@@ -863,7 +862,7 @@ static enum blk_eh_timer_return nvme_timeout(struct request *req, bool reserved)
 		 * Mark the request as handled, since the inline shutdown
 		 * forces all outstanding requests to complete.
 		 */
-		req->errors = NVME_SC_CANCELLED;
+		nvme_req(req)->flags |= NVME_REQ_CANCELLED;
 		return BLK_EH_HANDLED;
 	}
 
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 4aae363943e3..53b611f9ba5d 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -1178,8 +1178,7 @@ static int nvme_rdma_process_nvme_rsp(struct nvme_rdma_queue *queue,
 	    wc->ex.invalidate_rkey == req->mr->rkey)
 		req->mr->need_inval = false;
 
-	req->req.result = cqe->result;
-	blk_mq_complete_request(rq, le16_to_cpu(cqe->status) >> 1);
+	nvme_end_request(rq, cqe->status, cqe->result);
 	return ret;
 }
 
@@ -1416,7 +1415,7 @@ nvme_rdma_timeout(struct request *rq, bool reserved)
 	nvme_rdma_error_recovery(req->queue->ctrl);
 
 	/* fail with DNR on cmd timeout */
-	rq->errors = NVME_SC_ABORT_REQ | NVME_SC_DNR;
+	nvme_req(rq)->status = NVME_SC_ABORT_REQ | NVME_SC_DNR;
 
 	return BLK_EH_HANDLED;
 }
diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index 33b431e4eec3..8260ee1f8e48 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -124,7 +124,6 @@ static void nvme_loop_queue_response(struct nvmet_req *req)
 				&cqe->result);
 	} else {
 		struct request *rq;
-		struct nvme_loop_iod *iod;
 
 		rq = blk_mq_tag_to_rq(nvme_loop_tagset(queue), cqe->command_id);
 		if (!rq) {
@@ -134,9 +133,7 @@ static void nvme_loop_queue_response(struct nvmet_req *req)
 			return;
 		}
 
-		iod = blk_mq_rq_to_pdu(rq);
-		iod->nvme_req.result = cqe->result;
-		blk_mq_complete_request(rq, le16_to_cpu(cqe->status) >> 1);
+		nvme_end_request(rq, cqe->status, cqe->result);
 	}
 }
 
@@ -157,7 +154,7 @@ nvme_loop_timeout(struct request *rq, bool reserved)
 	schedule_work(&iod->queue->ctrl->reset_work);
 
 	/* fail with DNR on admin cmd timeout */
-	rq->errors = NVME_SC_ABORT_REQ | NVME_SC_DNR;
+	nvme_req(rq)->status = NVME_SC_ABORT_REQ | NVME_SC_DNR;
 
 	return BLK_EH_HANDLED;
 }
-- 
2.11.0

^ permalink raw reply related

* kill req->errors V2
From: Christoph Hellwig @ 2017-04-18 15:52 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 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

* block: remove the blk_execute_rq return value
From: Christoph Hellwig @ 2017-04-18 15:52 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, Christoph Hellwig
In-Reply-To: <20170418155229.5977-1-hch@bombadil.infradead.org>

From: Christoph Hellwig <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/paride/pd.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 +-
 17 files changed, 36 insertions(+), 29 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/paride/pd.c b/drivers/block/paride/pd.c
index b05e151c9b38..82c6d02193ae 100644
--- a/drivers/block/paride/pd.c
+++ b/drivers/block/paride/pd.c
@@ -747,7 +747,8 @@ static int pd_special_command(struct pd_unit *disk,
 
 	rq->special = func;
 
-	err = blk_execute_rq(disk->gd->queue, disk->gd, rq, 0);
+	blk_execute_rq(disk->gd->queue, disk->gd, rq, 0);
+	err = req->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 ec993573e0a8..5986e1250d7d 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

* block: remove the errors field from struct request
From: Christoph Hellwig @ 2017-04-18 15:52 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, Christoph Hellwig
In-Reply-To: <20170418155229.5977-1-hch@bombadil.infradead.org>

From: Christoph Hellwig <hch@lst.de>

Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 block/blk-core.c             | 14 +-------------
 block/blk-exec.c             |  3 +--
 block/blk-mq.c               | 10 +++-------
 block/blk-timeout.c          |  1 -
 include/linux/blkdev.h       |  2 --
 include/trace/events/block.h | 17 +++++++----------
 kernel/trace/blktrace.c      | 26 ++++++++++++--------------
 7 files changed, 24 insertions(+), 49 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 8654aa0cef6d..48f522a02f54 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1633,7 +1633,6 @@ void init_request_from_bio(struct request *req, struct bio *bio)
 	if (bio->bi_opf & REQ_RAHEAD)
 		req->cmd_flags |= REQ_FAILFAST_MASK;
 
-	req->errors = 0;
 	req->__sector = bio->bi_iter.bi_sector;
 	blk_rq_set_prio(req, rq_ioc(bio));
 	if (ioprio_valid(bio_prio(bio)))
@@ -2567,22 +2566,11 @@ bool blk_update_request(struct request *req, int error, unsigned int nr_bytes)
 {
 	int total_bytes;
 
-	trace_block_rq_complete(req->q, req, nr_bytes);
+	trace_block_rq_complete(req, error, nr_bytes);
 
 	if (!req->bio)
 		return false;
 
-	/*
-	 * For fs requests, rq is just carrier of independent bio's
-	 * and each partial completion should be handled separately.
-	 * Reset per-request error on each partial completion.
-	 *
-	 * TODO: tj: This is too subtle.  It would be better to let
-	 * low level drivers do what they see fit.
-	 */
-	if (!blk_rq_is_passthrough(req))
-		req->errors = 0;
-
 	if (error && !blk_rq_is_passthrough(req) &&
 	    !(req->rq_flags & RQF_QUIET)) {
 		char *error_type;
diff --git a/block/blk-exec.c b/block/blk-exec.c
index afa383248c7c..a9451e3b8587 100644
--- a/block/blk-exec.c
+++ b/block/blk-exec.c
@@ -69,8 +69,7 @@ void blk_execute_rq_nowait(struct request_queue *q, struct gendisk *bd_disk,
 
 	if (unlikely(blk_queue_dying(q))) {
 		rq->rq_flags |= RQF_QUIET;
-		rq->errors = -ENXIO;
-		__blk_end_request_all(rq, rq->errors);
+		__blk_end_request_all(rq, -ENXIO);
 		spin_unlock_irq(q->queue_lock);
 		return;
 	}
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 1f98db082da3..f5e3cf153ad7 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -213,7 +213,6 @@ void blk_mq_rq_ctx_init(struct request_queue *q, struct blk_mq_ctx *ctx,
 #endif
 	rq->special = NULL;
 	/* tag was already set */
-	rq->errors = 0;
 	rq->extra_len = 0;
 
 	INIT_LIST_HEAD(&rq->timeout_list);
@@ -624,8 +623,7 @@ void blk_mq_abort_requeue_list(struct request_queue *q)
 
 		rq = list_first_entry(&rq_list, struct request, queuelist);
 		list_del_init(&rq->queuelist);
-		rq->errors = -EIO;
-		blk_mq_end_request(rq, rq->errors);
+		blk_mq_end_request(rq, -EIO);
 	}
 }
 EXPORT_SYMBOL(blk_mq_abort_requeue_list);
@@ -1032,8 +1030,7 @@ bool blk_mq_dispatch_rq_list(struct request_queue *q, struct list_head *list)
 			pr_err("blk-mq: bad return on queue: %d\n", ret);
 		case BLK_MQ_RQ_QUEUE_ERROR:
 			errors++;
-			rq->errors = -EIO;
-			blk_mq_end_request(rq, rq->errors);
+			blk_mq_end_request(rq, -EIO);
 			break;
 		}
 
@@ -1484,8 +1481,7 @@ static void __blk_mq_try_issue_directly(struct request *rq, blk_qc_t *cookie,
 
 	if (ret == BLK_MQ_RQ_QUEUE_ERROR) {
 		*cookie = BLK_QC_T_NONE;
-		rq->errors = -EIO;
-		blk_mq_end_request(rq, rq->errors);
+		blk_mq_end_request(rq, -EIO);
 		return;
 	}
 
diff --git a/block/blk-timeout.c b/block/blk-timeout.c
index a30441a200c0..cbff183f3d9f 100644
--- a/block/blk-timeout.c
+++ b/block/blk-timeout.c
@@ -89,7 +89,6 @@ static void blk_rq_timed_out(struct request *req)
 		ret = q->rq_timed_out_fn(req);
 	switch (ret) {
 	case BLK_EH_HANDLED:
-		/* Can we use req->errors here? */
 		__blk_complete_request(req);
 		break;
 	case BLK_EH_RESET_TIMER:
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index e618164462e8..d40ac947ae26 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -220,8 +220,6 @@ struct request {
 
 	void *special;		/* opaque pointer available for LLD use */
 
-	int errors;
-
 	unsigned int extra_len;	/* length of alignment and padding */
 
 	unsigned long deadline;
diff --git a/include/trace/events/block.h b/include/trace/events/block.h
index 99ed69fad041..d0dbe60d8a6d 100644
--- a/include/trace/events/block.h
+++ b/include/trace/events/block.h
@@ -80,7 +80,6 @@ TRACE_EVENT(block_rq_requeue,
 		__field(  dev_t,	dev			)
 		__field(  sector_t,	sector			)
 		__field(  unsigned int,	nr_sector		)
-		__field(  int,		errors			)
 		__array(  char,		rwbs,	RWBS_LEN	)
 		__dynamic_array( char,	cmd,	1		)
 	),
@@ -89,7 +88,6 @@ TRACE_EVENT(block_rq_requeue,
 		__entry->dev	   = rq->rq_disk ? disk_devt(rq->rq_disk) : 0;
 		__entry->sector    = blk_rq_trace_sector(rq);
 		__entry->nr_sector = blk_rq_trace_nr_sectors(rq);
-		__entry->errors    = rq->errors;
 
 		blk_fill_rwbs(__entry->rwbs, rq->cmd_flags, blk_rq_bytes(rq));
 		__get_str(cmd)[0] = '\0';
@@ -99,13 +97,13 @@ TRACE_EVENT(block_rq_requeue,
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->rwbs, __get_str(cmd),
 		  (unsigned long long)__entry->sector,
-		  __entry->nr_sector, __entry->errors)
+		  __entry->nr_sector, 0)
 );
 
 /**
  * block_rq_complete - block IO operation completed by device driver
- * @q: queue containing the block operation request
  * @rq: block operations request
+ * @error: status code
  * @nr_bytes: number of completed bytes
  *
  * The block_rq_complete tracepoint event indicates that some portion
@@ -116,16 +114,15 @@ TRACE_EVENT(block_rq_requeue,
  */
 TRACE_EVENT(block_rq_complete,
 
-	TP_PROTO(struct request_queue *q, struct request *rq,
-		 unsigned int nr_bytes),
+	TP_PROTO(struct request *rq, int error, unsigned int nr_bytes),
 
-	TP_ARGS(q, rq, nr_bytes),
+	TP_ARGS(rq, error, nr_bytes),
 
 	TP_STRUCT__entry(
 		__field(  dev_t,	dev			)
 		__field(  sector_t,	sector			)
 		__field(  unsigned int,	nr_sector		)
-		__field(  int,		errors			)
+		__field(  int,		error			)
 		__array(  char,		rwbs,	RWBS_LEN	)
 		__dynamic_array( char,	cmd,	1		)
 	),
@@ -134,7 +131,7 @@ TRACE_EVENT(block_rq_complete,
 		__entry->dev	   = rq->rq_disk ? disk_devt(rq->rq_disk) : 0;
 		__entry->sector    = blk_rq_pos(rq);
 		__entry->nr_sector = nr_bytes >> 9;
-		__entry->errors    = rq->errors;
+		__entry->error     = error;
 
 		blk_fill_rwbs(__entry->rwbs, rq->cmd_flags, nr_bytes);
 		__get_str(cmd)[0] = '\0';
@@ -144,7 +141,7 @@ TRACE_EVENT(block_rq_complete,
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->rwbs, __get_str(cmd),
 		  (unsigned long long)__entry->sector,
-		  __entry->nr_sector, __entry->errors)
+		  __entry->nr_sector, __entry->error)
 );
 
 DECLARE_EVENT_CLASS(block_rq,
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index 9f3624dadb09..bd8ae8d5ae9c 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -690,8 +690,8 @@ void blk_trace_shutdown(struct request_queue *q)
 
 /**
  * blk_add_trace_rq - Add a trace for a request oriented action
- * @q:		queue the io is for
  * @rq:		the source request
+ * @error:	return status to log
  * @nr_bytes:	number of completed bytes
  * @what:	the action
  *
@@ -699,10 +699,10 @@ void blk_trace_shutdown(struct request_queue *q)
  *     Records an action against a request. Will log the bio offset + size.
  *
  **/
-static void blk_add_trace_rq(struct request_queue *q, struct request *rq,
+static void blk_add_trace_rq(struct request *rq, int error,
 			     unsigned int nr_bytes, u32 what)
 {
-	struct blk_trace *bt = q->blk_trace;
+	struct blk_trace *bt = rq->q->blk_trace;
 
 	if (likely(!bt))
 		return;
@@ -713,34 +713,32 @@ static void blk_add_trace_rq(struct request_queue *q, struct request *rq,
 		what |= BLK_TC_ACT(BLK_TC_FS);
 
 	__blk_add_trace(bt, blk_rq_trace_sector(rq), nr_bytes, req_op(rq),
-			rq->cmd_flags, what, rq->errors, 0, NULL);
+			rq->cmd_flags, what, error, 0, NULL);
 }
 
 static void blk_add_trace_rq_insert(void *ignore,
 				    struct request_queue *q, struct request *rq)
 {
-	blk_add_trace_rq(q, rq, blk_rq_bytes(rq), BLK_TA_INSERT);
+	blk_add_trace_rq(rq, 0, blk_rq_bytes(rq), BLK_TA_INSERT);
 }
 
 static void blk_add_trace_rq_issue(void *ignore,
 				   struct request_queue *q, struct request *rq)
 {
-	blk_add_trace_rq(q, rq, blk_rq_bytes(rq), BLK_TA_ISSUE);
+	blk_add_trace_rq(rq, 0, blk_rq_bytes(rq), BLK_TA_ISSUE);
 }
 
 static void blk_add_trace_rq_requeue(void *ignore,
 				     struct request_queue *q,
 				     struct request *rq)
 {
-	blk_add_trace_rq(q, rq, blk_rq_bytes(rq), BLK_TA_REQUEUE);
+	blk_add_trace_rq(rq, 0, blk_rq_bytes(rq), BLK_TA_REQUEUE);
 }
 
-static void blk_add_trace_rq_complete(void *ignore,
-				      struct request_queue *q,
-				      struct request *rq,
-				      unsigned int nr_bytes)
+static void blk_add_trace_rq_complete(void *ignore, struct request *rq,
+			int error, unsigned int nr_bytes)
 {
-	blk_add_trace_rq(q, rq, nr_bytes, BLK_TA_COMPLETE);
+	blk_add_trace_rq(rq, error, nr_bytes, BLK_TA_COMPLETE);
 }
 
 /**
@@ -935,7 +933,7 @@ static void blk_add_trace_rq_remap(void *ignore,
 	r.sector_from = cpu_to_be64(from);
 
 	__blk_add_trace(bt, blk_rq_pos(rq), blk_rq_bytes(rq),
-			rq_data_dir(rq), 0, BLK_TA_REMAP, !!rq->errors,
+			rq_data_dir(rq), 0, BLK_TA_REMAP, 0,
 			sizeof(r), &r);
 }
 
@@ -960,7 +958,7 @@ void blk_add_driver_data(struct request_queue *q,
 		return;
 
 	__blk_add_trace(bt, blk_rq_trace_sector(rq), blk_rq_bytes(rq), 0, 0,
-				BLK_TA_DRV_DATA, rq->errors, len, data);
+				BLK_TA_DRV_DATA, 0, len, data);
 }
 EXPORT_SYMBOL_GPL(blk_add_driver_data);
 
-- 
2.11.0

^ permalink raw reply related

* blktrace: remove the unused block_rq_abort tracepoint
From: Christoph Hellwig @ 2017-04-18 15:52 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, Christoph Hellwig
In-Reply-To: <20170418155229.5977-1-hch@bombadil.infradead.org>

From: Christoph Hellwig <hch@lst.de>

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/trace/events/block.h | 44 ++++++++++----------------------------------
 kernel/trace/blktrace.c      |  9 ---------
 2 files changed, 10 insertions(+), 43 deletions(-)

diff --git a/include/trace/events/block.h b/include/trace/events/block.h
index a88ed13446ff..99ed69fad041 100644
--- a/include/trace/events/block.h
+++ b/include/trace/events/block.h
@@ -61,7 +61,16 @@ DEFINE_EVENT(block_buffer, block_dirty_buffer,
 	TP_ARGS(bh)
 );
 
-DECLARE_EVENT_CLASS(block_rq_with_error,
+/**
+ * block_rq_requeue - place block IO request back on a queue
+ * @q: queue holding operation
+ * @rq: block IO operation request
+ *
+ * The block operation request @rq is being placed back into queue
+ * @q.  For some reason the request was not completed and needs to be
+ * put back in the queue.
+ */
+TRACE_EVENT(block_rq_requeue,
 
 	TP_PROTO(struct request_queue *q, struct request *rq),
 
@@ -94,39 +103,6 @@ DECLARE_EVENT_CLASS(block_rq_with_error,
 );
 
 /**
- * block_rq_abort - abort block operation request
- * @q: queue containing the block operation request
- * @rq: block IO operation request
- *
- * Called immediately after pending block IO operation request @rq in
- * queue @q is aborted. The fields in the operation request @rq
- * can be examined to determine which device and sectors the pending
- * operation would access.
- */
-DEFINE_EVENT(block_rq_with_error, block_rq_abort,
-
-	TP_PROTO(struct request_queue *q, struct request *rq),
-
-	TP_ARGS(q, rq)
-);
-
-/**
- * block_rq_requeue - place block IO request back on a queue
- * @q: queue holding operation
- * @rq: block IO operation request
- *
- * The block operation request @rq is being placed back into queue
- * @q.  For some reason the request was not completed and needs to be
- * put back in the queue.
- */
-DEFINE_EVENT(block_rq_with_error, block_rq_requeue,
-
-	TP_PROTO(struct request_queue *q, struct request *rq),
-
-	TP_ARGS(q, rq)
-);
-
-/**
  * block_rq_complete - block IO operation completed by device driver
  * @q: queue containing the block operation request
  * @rq: block operations request
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index b2058a7f94bd..9f3624dadb09 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -716,12 +716,6 @@ static void blk_add_trace_rq(struct request_queue *q, struct request *rq,
 			rq->cmd_flags, what, rq->errors, 0, NULL);
 }
 
-static void blk_add_trace_rq_abort(void *ignore,
-				   struct request_queue *q, struct request *rq)
-{
-	blk_add_trace_rq(q, rq, blk_rq_bytes(rq), BLK_TA_ABORT);
-}
-
 static void blk_add_trace_rq_insert(void *ignore,
 				    struct request_queue *q, struct request *rq)
 {
@@ -974,8 +968,6 @@ static void blk_register_tracepoints(void)
 {
 	int ret;
 
-	ret = register_trace_block_rq_abort(blk_add_trace_rq_abort, NULL);
-	WARN_ON(ret);
 	ret = register_trace_block_rq_insert(blk_add_trace_rq_insert, NULL);
 	WARN_ON(ret);
 	ret = register_trace_block_rq_issue(blk_add_trace_rq_issue, NULL);
@@ -1028,7 +1020,6 @@ static void blk_unregister_tracepoints(void)
 	unregister_trace_block_rq_requeue(blk_add_trace_rq_requeue, NULL);
 	unregister_trace_block_rq_issue(blk_add_trace_rq_issue, NULL);
 	unregister_trace_block_rq_insert(blk_add_trace_rq_insert, NULL);
-	unregister_trace_block_rq_abort(blk_add_trace_rq_abort, NULL);
 
 	tracepoint_synchronize_unregister();
 }
-- 
2.11.0

^ permalink raw reply related

* pd: remove bogus check for req->errors
From: Christoph Hellwig @ 2017-04-18 15:52 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, Christoph Hellwig
In-Reply-To: <20170418155229.5977-1-hch@bombadil.infradead.org>

From: Christoph Hellwig <hch@lst.de>

The driver never sets req->errors
---
 drivers/block/paride/pd.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/block/paride/pd.c b/drivers/block/paride/pd.c
index 82c6d02193ae..3b0ab214fe74 100644
--- a/drivers/block/paride/pd.c
+++ b/drivers/block/paride/pd.c
@@ -739,7 +739,6 @@ 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))
@@ -748,10 +747,9 @@ static int pd_special_command(struct pd_unit *disk,
 	rq->special = func;
 
 	blk_execute_rq(disk->gd->queue, disk->gd, rq, 0);
-	err = req->errors ? -EIO : 0;
 
 	blk_put_request(rq);
-	return err;
+	return 0;
 }
 
 /* kernel glue structures */
-- 
2.11.0

^ permalink raw reply related

* ataflop: switch from req->errors to req->error_count
From: Christoph Hellwig @ 2017-04-18 15:52 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, Christoph Hellwig
In-Reply-To: <20170418155229.5977-1-hch@bombadil.infradead.org>

From: Christoph Hellwig <hch@lst.de>

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/ataflop.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index 2104b1b4ccda..fa69ecd52cb5 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -617,12 +617,12 @@ static void fd_error( void )
 	if (!fd_request)
 		return;
 
-	fd_request->errors++;
-	if (fd_request->errors >= MAX_ERRORS) {
+	fd_request->error_count++;
+	if (fd_request->error_count >= MAX_ERRORS) {
 		printk(KERN_ERR "fd%d: too many errors.\n", SelectedDrive );
 		fd_end_request_cur(-EIO);
 	}
-	else if (fd_request->errors == RECALIBRATE_ERRORS) {
+	else if (fd_request->error_count == RECALIBRATE_ERRORS) {
 		printk(KERN_WARNING "fd%d: recalibrating\n", SelectedDrive );
 		if (SelectedDrive != -1)
 			SUD.track = -1;
@@ -1386,7 +1386,7 @@ static void setup_req_params( int drive )
 	ReqData = ReqBuffer + 512 * ReqCnt;
 
 	if (UseTrackbuffer)
-		read_track = (ReqCmd == READ && fd_request->errors == 0);
+		read_track = (ReqCmd == READ && fd_request->error_count == 0);
 	else
 		read_track = 0;
 
@@ -1409,8 +1409,10 @@ static struct request *set_next_request(void)
 			fdc_queue = 0;
 		if (q) {
 			rq = blk_fetch_request(q);
-			if (rq)
+			if (rq) {
+				rq->error_count = 0;
 				break;
+			}
 		}
 	} while (fdc_queue != old_pos);
 
-- 
2.11.0

^ permalink raw reply related

* blk-mq: simplify __blk_mq_complete_request
From: Christoph Hellwig @ 2017-04-18 15:52 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, Christoph Hellwig
In-Reply-To: <20170418155229.5977-1-hch@bombadil.infradead.org>

From: Christoph Hellwig <hch@lst.de>

Merge blk_mq_ipi_complete_request and blk_mq_stat_add into their only
caller.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-mq.c | 25 ++++++++-----------------
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 0c19df66f5a0..1f98db082da3 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -406,12 +406,19 @@ static void __blk_mq_complete_request_remote(void *data)
 	rq->q->softirq_done_fn(rq);
 }
 
-static void blk_mq_ipi_complete_request(struct request *rq)
+static void __blk_mq_complete_request(struct request *rq)
 {
 	struct blk_mq_ctx *ctx = rq->mq_ctx;
 	bool shared = false;
 	int cpu;
 
+	if (rq->internal_tag != -1)
+		blk_mq_sched_completed_request(rq);
+	if (rq->rq_flags & RQF_STATS) {
+		blk_mq_poll_stats_start(rq->q);
+		blk_stat_add(rq);
+	}
+
 	if (!test_bit(QUEUE_FLAG_SAME_COMP, &rq->q->queue_flags)) {
 		rq->q->softirq_done_fn(rq);
 		return;
@@ -432,22 +439,6 @@ static void blk_mq_ipi_complete_request(struct request *rq)
 	put_cpu();
 }
 
-static void blk_mq_stat_add(struct request *rq)
-{
-	if (rq->rq_flags & RQF_STATS) {
-		blk_mq_poll_stats_start(rq->q);
-		blk_stat_add(rq);
-	}
-}
-
-static void __blk_mq_complete_request(struct request *rq)
-{
-	if (rq->internal_tag != -1)
-		blk_mq_sched_completed_request(rq);
-	blk_mq_stat_add(rq);
-	blk_mq_ipi_complete_request(rq);
-}
-
 /**
  * blk_mq_complete_request - end I/O on a request
  * @rq:		the request being processed
-- 
2.11.0

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox