Re: [PATCH] floppy: fix race condition in __floppy_read_block_0()
On Fri, Nov 09, 2018 at 03:58:40PM -0700, Jens Axboe wrote: > LKP recently reported a hang at bootup in the floppy code: > > [ 245.678853] INFO: task mount:580 blocked for more than 120 seconds. > [ 245.679906] Tainted: GT 4.19.0-rc6-00172-ga9f38e1 #1 > [ 245.680959] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables > this message. > [ 245.682181] mount D 6372 580 1 0x0004 > [ 245.683023] Call Trace: > [ 245.683425] __schedule+0x2df/0x570 > [ 245.683975] schedule+0x2d/0x80 > [ 245.684476] schedule_timeout+0x19d/0x330 > [ 245.685090] ? wait_for_common+0xa5/0x170 > [ 245.685735] wait_for_common+0xac/0x170 > [ 245.686339] ? do_sched_yield+0x90/0x90 > [ 245.686935] wait_for_completion+0x12/0x20 > [ 245.687571] __floppy_read_block_0+0xfb/0x150 > [ 245.688244] ? floppy_resume+0x40/0x40 > [ 245.688844] floppy_revalidate+0x20f/0x240 > [ 245.689486] check_disk_change+0x43/0x60 > [ 245.690087] floppy_open+0x1ea/0x360 > [ 245.690653] __blkdev_get+0xb4/0x4d0 > [ 245.691212] ? blkdev_get+0x1db/0x370 > [ 245.691777] blkdev_get+0x1f3/0x370 > [ 245.692351] ? path_put+0x15/0x20 > [ 245.692871] ? lookup_bdev+0x4b/0x90 > [ 245.693539] blkdev_get_by_path+0x3d/0x80 > [ 245.694165] mount_bdev+0x2a/0x190 > [ 245.694695] squashfs_mount+0x10/0x20 > [ 245.695271] ? squashfs_alloc_inode+0x30/0x30 > [ 245.695960] mount_fs+0xf/0x90 > [ 245.696451] vfs_kern_mount+0x43/0x130 > [ 245.697036] do_mount+0x187/0xc40 > [ 245.697563] ? memdup_user+0x28/0x50 > [ 245.698124] ksys_mount+0x60/0xc0 > [ 245.698639] sys_mount+0x19/0x20 > [ 245.699167] do_int80_syscall_32+0x61/0x130 > [ 245.699813] entry_INT80_32+0xc7/0xc7 > > showing that we never complete that read request. The reason is that > the completion setup is racy - it initializes the completion event > AFTER submitting the IO, which means that the IO could complete > before/during the init. If it does, we are passing garbage to > complete() and we may sleep forever waiting for the event to > occur. > > Fixes: 7b7b68bba5ef ("floppy: bail out in open() if drive is not responding > to block0 read") Reviewed-by: Omar Sandoval > Signed-off-by: Jens Axboe > --- > drivers/block/floppy.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c > index a8cfa011c284..fb23578e9a41 100644 > --- a/drivers/block/floppy.c > +++ b/drivers/block/floppy.c > @@ -4148,10 +4148,11 @@ static int __floppy_read_block_0(struct block_device > *bdev, int drive) > bio.bi_end_io = floppy_rb0_cb; > bio_set_op_attrs(, REQ_OP_READ, 0); > > + init_completion(); > + > submit_bio(); > process_fd_request(); > > - init_completion(); > wait_for_completion(); > > __free_page(page); > -- > 2.17.1 > > -- > Jens Axboe >
Re: [PATCH -next] block: remove set but not used variable 'et'
On 11/9/18 7:41 PM, YueHaibing wrote: > Fixes gcc '-Wunused-but-set-variable' warning: > > block/blk-ioc.c: In function 'put_io_context_active': > block/blk-ioc.c:174:24: warning: > variable 'et' set but not used [-Wunused-but-set-variable] > > It not used any more after commit > a1ce35fa4985 ("block: remove dead elevator code") Thanks, applied. -- Jens Axboe
[PATCH -next] block: remove set but not used variable 'et'
Fixes gcc '-Wunused-but-set-variable' warning: block/blk-ioc.c: In function 'put_io_context_active': block/blk-ioc.c:174:24: warning: variable 'et' set but not used [-Wunused-but-set-variable] It not used any more after commit a1ce35fa4985 ("block: remove dead elevator code") Signed-off-by: YueHaibing --- block/blk-ioc.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/block/blk-ioc.c b/block/blk-ioc.c index 007aac6..56755ad 100644 --- a/block/blk-ioc.c +++ b/block/blk-ioc.c @@ -171,7 +171,6 @@ void put_io_context(struct io_context *ioc) */ void put_io_context_active(struct io_context *ioc) { - struct elevator_type *et; unsigned long flags; struct io_cq *icq; @@ -190,7 +189,6 @@ void put_io_context_active(struct io_context *ioc) if (icq->flags & ICQ_EXITED) continue; - et = icq->q->elevator->type; ioc_exit_icq(icq); } spin_unlock_irqrestore(>lock, flags);
[PATCH] floppy: fix race condition in __floppy_read_block_0()
LKP recently reported a hang at bootup in the floppy code: [ 245.678853] INFO: task mount:580 blocked for more than 120 seconds. [ 245.679906] Tainted: GT 4.19.0-rc6-00172-ga9f38e1 #1 [ 245.680959] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 245.682181] mount D 6372 580 1 0x0004 [ 245.683023] Call Trace: [ 245.683425] __schedule+0x2df/0x570 [ 245.683975] schedule+0x2d/0x80 [ 245.684476] schedule_timeout+0x19d/0x330 [ 245.685090] ? wait_for_common+0xa5/0x170 [ 245.685735] wait_for_common+0xac/0x170 [ 245.686339] ? do_sched_yield+0x90/0x90 [ 245.686935] wait_for_completion+0x12/0x20 [ 245.687571] __floppy_read_block_0+0xfb/0x150 [ 245.688244] ? floppy_resume+0x40/0x40 [ 245.688844] floppy_revalidate+0x20f/0x240 [ 245.689486] check_disk_change+0x43/0x60 [ 245.690087] floppy_open+0x1ea/0x360 [ 245.690653] __blkdev_get+0xb4/0x4d0 [ 245.691212] ? blkdev_get+0x1db/0x370 [ 245.691777] blkdev_get+0x1f3/0x370 [ 245.692351] ? path_put+0x15/0x20 [ 245.692871] ? lookup_bdev+0x4b/0x90 [ 245.693539] blkdev_get_by_path+0x3d/0x80 [ 245.694165] mount_bdev+0x2a/0x190 [ 245.694695] squashfs_mount+0x10/0x20 [ 245.695271] ? squashfs_alloc_inode+0x30/0x30 [ 245.695960] mount_fs+0xf/0x90 [ 245.696451] vfs_kern_mount+0x43/0x130 [ 245.697036] do_mount+0x187/0xc40 [ 245.697563] ? memdup_user+0x28/0x50 [ 245.698124] ksys_mount+0x60/0xc0 [ 245.698639] sys_mount+0x19/0x20 [ 245.699167] do_int80_syscall_32+0x61/0x130 [ 245.699813] entry_INT80_32+0xc7/0xc7 showing that we never complete that read request. The reason is that the completion setup is racy - it initializes the completion event AFTER submitting the IO, which means that the IO could complete before/during the init. If it does, we are passing garbage to complete() and we may sleep forever waiting for the event to occur. Fixes: 7b7b68bba5ef ("floppy: bail out in open() if drive is not responding to block0 read") Signed-off-by: Jens Axboe --- drivers/block/floppy.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c index a8cfa011c284..fb23578e9a41 100644 --- a/drivers/block/floppy.c +++ b/drivers/block/floppy.c @@ -4148,10 +4148,11 @@ static int __floppy_read_block_0(struct block_device *bdev, int drive) bio.bi_end_io = floppy_rb0_cb; bio_set_op_attrs(, REQ_OP_READ, 0); + init_completion(); + submit_bio(); process_fd_request(); - init_completion(); wait_for_completion(); __free_page(page); -- 2.17.1 -- Jens Axboe
Re: remove most req->special users
On 11/9/18 11:32 AM, Christoph Hellwig wrote: > Try to get rid of the req->special users so that we can remove this > field. With this series we basically only have the osd / scsi BIDI > code left, which should go away in another series. Outside of the typo in skd_main that Bart pointed out, the rest of it looks fine to me. Can you send a v2 with that fixed? -- Jens Axboe
Re: [GIT PULL] Block fixes for 4.20-rc2
On 11/9/18 3:35 PM, Linus Torvalds wrote: > On Fri, Nov 9, 2018 at 1:29 PM Jens Axboe wrote: >> >> A select set of fixes that should go into this release. This pull >> request contains: > > This is part of a final few "ack" emails, pointing out that there is > now automation in place if you cc lkml in your pull request. > > That automation will parse the pull request and automatically send an > ack when the end result gets merged into mainline, so I'll stop the > manual process. > > But it currently only works on lkml (I think - maybe Konstantin > enabled it for all lists that get tracked by lore.kernel.org), so this > pull request probably won't get an automated response due to just > going to linux-block. > > Just a heads-up. Thanks for the heads-up. I haven't gotten linux-block tracked by lore yet, but will do so. -- Jens Axboe
Re: [GIT PULL] Block fixes for 4.20-rc2
On Fri, Nov 9, 2018 at 1:29 PM Jens Axboe wrote: > > A select set of fixes that should go into this release. This pull > request contains: This is part of a final few "ack" emails, pointing out that there is now automation in place if you cc lkml in your pull request. That automation will parse the pull request and automatically send an ack when the end result gets merged into mainline, so I'll stop the manual process. But it currently only works on lkml (I think - maybe Konstantin enabled it for all lists that get tracked by lore.kernel.org), so this pull request probably won't get an automated response due to just going to linux-block. Just a heads-up. Linus
[GIT PULL] Block fixes for 4.20-rc2
Hi Linus, A select set of fixes that should go into this release. This pull request contains: - Two fixes for an ubd regression, one for missing locking, and one for a missing initialization of a field. The latter was an old latent bug, but it's now visible and triggers (Me, Anton Ivanov) - Set of NVMe fixes via Christoph, but applied manually due to a git tree mixup (Christoph, Sagi) - Fix for a discard split regression, in three patches (Ming) - Update libata git trees (Geert) - SPDX identifier for sata_rcar (Kuninori Morimoto) - Virtual boundary merge fix (Johannes) - Preemptively clear memory we are going to pass to userspace, in case the driver does a short read (Keith) Please pull! git://git.kernel.dk/linux-block.git tags/for-linus-20181109 Anton Ivanov (1): ubd: fix missing initialization of io_req Christoph Hellwig (1): Revert "nvmet-rdma: use a private workqueue for delete" Geert Uytterhoeven (1): MAINTAINERS: Fix remaining pointers to obsolete libata.git Jens Axboe (1): ubd: fix missing lock around request issue Johannes Thumshirn (1): block: respect virtual boundary mask in bvecs Keith Busch (1): block: Clear kernel memory before copying to user Kuninori Morimoto (1): sata_rcar: convert to SPDX identifiers Ming Lei (3): block: make sure discard bio is aligned with logical block size block: cleanup __blkdev_issue_discard() block: make sure writesame bio is aligned with logical block size Sagi Grimberg (2): nvmet: don't try to add ns to p2p map unless it actually uses it nvme: make sure ns head inherits underlying device limits MAINTAINERS | 6 +++--- arch/um/drivers/ubd_kern.c| 12 block/bio.c | 1 + block/blk-lib.c | 26 +++--- block/blk-merge.c | 5 +++-- block/blk.h | 12 +++- drivers/ata/sata_rcar.c | 6 +- drivers/nvme/host/core.c | 4 +++- drivers/nvme/host/multipath.c | 1 + drivers/nvme/target/core.c| 2 +- drivers/nvme/target/rdma.c| 19 --- 11 files changed, 43 insertions(+), 51 deletions(-) -- Jens Axboe
Re: [PATCH 3/6] skd_main: don't use req->special
On 11/9/18 10:32 AM, Christoph Hellwig wrote: Add a retries field to the internal request structure instead, which gets set to zero on the first submission. Signed-off-by: Christoph Hellwig --- drivers/block/skd_main.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/block/skd_main.c b/drivers/block/skd_main.c index a0196477165f..88f29b06d90e 100644 --- a/drivers/block/skd_main.c +++ b/drivers/block/skd_main.c @@ -181,6 +181,7 @@ struct skd_request_context { struct fit_completion_entry_v1 completion; struct fit_comp_error_info err_info; + int retries; blk_status_t status; }; @@ -495,6 +496,11 @@ static blk_status_t skd_mq_queue_rq(struct blk_mq_hw_ctx *hctx, if (unlikely(skdev->state != SKD_DRVR_STATE_ONLINE)) return skd_fail_all(q) ? BLK_STS_IOERR : BLK_STS_RESOURCE; + if (!(req->rq_flags & RQF_DONTPREP)) { + skreq->retries = 0; + req->rq_flags |= RQF_DONTPREP; + } + blk_mq_start_request(req); WARN_ONCE(tag >= skd_max_queue_depth, "%#x > %#x (nr_requests = %lu)\n", @@ -1426,7 +1432,7 @@ static void skd_resolve_req_exception(struct skd_device *skdev, break; case SKD_CHECK_STATUS_REQUEUE_REQUEST: - if ((unsigned long) ++req->special < SKD_MAX_RETRIES) { + if ((unsigned long) skreq->retries < SKD_MAX_RETRIES) { skd_log_skreq(skdev, skreq, "retry"); blk_mq_requeue_request(req, true); break; Hi Christoph, Where has the code been moved to that increments the new skreq->retries counter? Thanks, Bart.
Re: [PATCH] block: remove req->timeout_list
On 11/9/18 11:37 AM, Christoph Hellwig wrote: > Unused now that the legacy request path is gone. Great, missed that. Applied, thanks. -- Jens Axboe
[PATCH] block: remove req->timeout_list
Unused now that the legacy request path is gone. Signed-off-by: Christoph Hellwig --- block/blk-core.c | 1 - block/blk-mq.c | 1 - block/blk-timeout.c| 12 block/blk.h| 2 -- include/linux/blkdev.h | 2 -- 5 files changed, 18 deletions(-) diff --git a/block/blk-core.c b/block/blk-core.c index 3daab9df24e0..fdc0ad2686c4 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -144,7 +144,6 @@ void blk_rq_init(struct request_queue *q, struct request *rq) memset(rq, 0, sizeof(*rq)); INIT_LIST_HEAD(>queuelist); - INIT_LIST_HEAD(>timeout_list); rq->q = q; rq->__sector = (sector_t) -1; INIT_HLIST_NODE(>hash); diff --git a/block/blk-mq.c b/block/blk-mq.c index 4880e13e2394..411be60d0cb6 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -327,7 +327,6 @@ static struct request *blk_mq_rq_ctx_init(struct blk_mq_alloc_data *data, rq->extra_len = 0; rq->__deadline = 0; - INIT_LIST_HEAD(>timeout_list); rq->timeout = 0; rq->end_io = NULL; diff --git a/block/blk-timeout.c b/block/blk-timeout.c index 6428d458072a..006cff4390c0 100644 --- a/block/blk-timeout.c +++ b/block/blk-timeout.c @@ -68,16 +68,6 @@ ssize_t part_timeout_store(struct device *dev, struct device_attribute *attr, #endif /* CONFIG_FAIL_IO_TIMEOUT */ -/* - * blk_delete_timer - Delete/cancel timer for a given function. - * @req: request that we are canceling timer for - * - */ -void blk_delete_timer(struct request *req) -{ - list_del_init(>timeout_list); -} - /** * blk_abort_request -- Request request recovery for the specified command * @req: pointer to the request of interest @@ -123,8 +113,6 @@ void blk_add_timer(struct request *req) struct request_queue *q = req->q; unsigned long expiry; - BUG_ON(!list_empty(>timeout_list)); - /* * Some LLDs, like scsi, peek at the timeout to prevent a * command from being retried forever. diff --git a/block/blk.h b/block/blk.h index 78ae94886acf..41b64e6e101b 100644 --- a/block/blk.h +++ b/block/blk.h @@ -222,8 +222,6 @@ static inline bool bio_integrity_endio(struct bio *bio) unsigned long blk_rq_timeout(unsigned long timeout); void blk_add_timer(struct request *req); -void blk_delete_timer(struct request *); - bool bio_attempt_front_merge(struct request_queue *q, struct request *req, struct bio *bio); diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 9b1f470cc784..dc2a6f625ecb 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -228,8 +228,6 @@ struct request { /* access through blk_rq_set_deadline, blk_rq_deadline */ unsigned long __deadline; - struct list_head timeout_list; - union { struct __call_single_data csd; u64 fifo_time; -- 2.19.1
[PATCH 6/6] ide: don't use req->special
Just replace it with a field of the same name in struct ide_req. Signed-off-by: Christoph Hellwig --- drivers/ide/ide-atapi.c| 4 ++-- drivers/ide/ide-cd.c | 4 ++-- drivers/ide/ide-devsets.c | 4 ++-- drivers/ide/ide-disk.c | 6 +++--- drivers/ide/ide-eh.c | 2 +- drivers/ide/ide-floppy.c | 2 +- drivers/ide/ide-io.c | 14 +- drivers/ide/ide-park.c | 4 ++-- drivers/ide/ide-pm.c | 12 ++-- drivers/ide/ide-tape.c | 2 +- drivers/ide/ide-taskfile.c | 2 +- include/linux/ide.h| 1 + 12 files changed, 31 insertions(+), 26 deletions(-) diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c index 33210bc67618..da58020a144e 100644 --- a/drivers/ide/ide-atapi.c +++ b/drivers/ide/ide-atapi.c @@ -94,7 +94,7 @@ int ide_queue_pc_tail(ide_drive_t *drive, struct gendisk *disk, rq = blk_get_request(drive->queue, REQ_OP_DRV_IN, 0); ide_req(rq)->type = ATA_PRIV_MISC; - rq->special = (char *)pc; + ide_req(rq)->special = pc; if (buf && bufflen) { error = blk_rq_map_kern(drive->queue, rq, buf, bufflen, @@ -244,7 +244,7 @@ int ide_queue_sense_rq(ide_drive_t *drive, void *special) return -ENOMEM; } - sense_rq->special = special; + ide_req(sense_rq)->special = special; drive->sense_rq_armed = false; drive->hwif->rq = NULL; diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c index 4ecaf2ace4cb..fb24093c9aa6 100644 --- a/drivers/ide/ide-cd.c +++ b/drivers/ide/ide-cd.c @@ -211,12 +211,12 @@ static void cdrom_analyze_sense_data(ide_drive_t *drive, static void ide_cd_complete_failed_rq(ide_drive_t *drive, struct request *rq) { /* -* For ATA_PRIV_SENSE, "rq->special" points to the original +* For ATA_PRIV_SENSE, "ide_req(rq)->special" points to the original * failed request. Also, the sense data should be read * directly from rq which might be different from the original * sense buffer if it got copied during mapping. */ - struct request *failed = (struct request *)rq->special; + struct request *failed = ide_req(rq)->special; void *sense = bio_data(rq->bio); if (failed) { diff --git a/drivers/ide/ide-devsets.c b/drivers/ide/ide-devsets.c index f4f8afdf8bbe..f2f93ed40356 100644 --- a/drivers/ide/ide-devsets.c +++ b/drivers/ide/ide-devsets.c @@ -171,7 +171,7 @@ int ide_devset_execute(ide_drive_t *drive, const struct ide_devset *setting, scsi_req(rq)->cmd_len = 5; scsi_req(rq)->cmd[0] = REQ_DEVSET_EXEC; *(int *)_req(rq)->cmd[1] = arg; - rq->special = setting->set; + ide_req(rq)->special = setting->set; blk_execute_rq(q, NULL, rq, 0); ret = scsi_req(rq)->result; @@ -182,7 +182,7 @@ int ide_devset_execute(ide_drive_t *drive, const struct ide_devset *setting, ide_startstop_t ide_do_devset(ide_drive_t *drive, struct request *rq) { - int err, (*setfunc)(ide_drive_t *, int) = rq->special; + int err, (*setfunc)(ide_drive_t *, int) = ide_req(rq)->special; err = setfunc(drive, *(int *)_req(rq)->cmd[1]); if (err) diff --git a/drivers/ide/ide-disk.c b/drivers/ide/ide-disk.c index f8567c8c9dd1..1333d291cb19 100644 --- a/drivers/ide/ide-disk.c +++ b/drivers/ide/ide-disk.c @@ -434,8 +434,8 @@ static int idedisk_prep_fn(ide_drive_t *drive, struct request *rq) if (req_op(rq) != REQ_OP_FLUSH) return BLKPREP_OK; - if (rq->special) { - cmd = rq->special; + if (ide_req(rq)->special) { + cmd = ide_req(rq)->special; memset(cmd, 0, sizeof(*cmd)); } else { cmd = kzalloc(sizeof(*cmd), GFP_ATOMIC); @@ -455,7 +455,7 @@ static int idedisk_prep_fn(ide_drive_t *drive, struct request *rq) rq->cmd_flags &= ~REQ_OP_MASK; rq->cmd_flags |= REQ_OP_DRV_OUT; ide_req(rq)->type = ATA_PRIV_TASKFILE; - rq->special = cmd; + ide_req(rq)->special = cmd; cmd->rq = rq; return BLKPREP_OK; diff --git a/drivers/ide/ide-eh.c b/drivers/ide/ide-eh.c index 47d5f3379748..e1323e058454 100644 --- a/drivers/ide/ide-eh.c +++ b/drivers/ide/ide-eh.c @@ -125,7 +125,7 @@ ide_startstop_t ide_error(ide_drive_t *drive, const char *msg, u8 stat) /* retry only "normal" I/O: */ if (blk_rq_is_passthrough(rq)) { if (ata_taskfile_request(rq)) { - struct ide_cmd *cmd = rq->special; + struct ide_cmd *cmd = ide_req(rq)->special; if (cmd) ide_complete_cmd(drive, cmd, stat, err); diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c index a8df300f949c..780d33ccc5d8 100644 --- a/drivers/ide/ide-floppy.c +++ b/drivers/ide/ide-floppy.c @@ -276,7 +276,7 @@ static ide_startstop_t ide_floppy_do_request(ide_drive_t *drive,
[PATCH 5/6] pd: replace ->special use with private data in the request
Signed-off-by: Christoph Hellwig --- drivers/block/paride/pd.c | 30 +- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/drivers/block/paride/pd.c b/drivers/block/paride/pd.c index ae4971e5d9a8..0ff9b12d0e35 100644 --- a/drivers/block/paride/pd.c +++ b/drivers/block/paride/pd.c @@ -242,6 +242,11 @@ struct pd_unit { static struct pd_unit pd[PD_UNITS]; +struct pd_req { + /* for REQ_OP_DRV_IN: */ + enum action (*func)(struct pd_unit *disk); +}; + static char pd_scratch[512]; /* scratch block buffer */ static char *pd_errs[17] = { "ERR", "INDEX", "ECC", "DRQ", "SEEK", "WRERR", @@ -502,8 +507,9 @@ static enum action do_pd_io_start(void) static enum action pd_special(void) { - enum action (*func)(struct pd_unit *) = pd_req->special; - return func(pd_current); + struct pd_req *req = blk_mq_rq_to_pdu(pd_req); + + return req->func(pd_current); } static int pd_next_buf(void) @@ -767,12 +773,14 @@ static int pd_special_command(struct pd_unit *disk, enum action (*func)(struct pd_unit *disk)) { struct request *rq; + struct pd_req *req; rq = blk_get_request(disk->gd->queue, REQ_OP_DRV_IN, 0); if (IS_ERR(rq)) return PTR_ERR(rq); + req = blk_mq_rq_to_pdu(rq); - rq->special = func; + req->func = func; blk_execute_rq(disk->gd->queue, disk->gd, rq, 0); blk_put_request(rq); return 0; @@ -892,9 +900,21 @@ static void pd_probe_drive(struct pd_unit *disk) disk->gd = p; p->private_data = disk; - p->queue = blk_mq_init_sq_queue(>tag_set, _mq_ops, 2, - BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_BLOCKING); + memset(>tag_set, 0, sizeof(disk->tag_set)); + disk->tag_set.ops = _mq_ops; + disk->tag_set.cmd_size = sizeof(struct pd_req); + disk->tag_set.nr_hw_queues = 1; + disk->tag_set.nr_maps = 1; + disk->tag_set.queue_depth = 2; + disk->tag_set.numa_node = NUMA_NO_NODE; + disk->tag_set.flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_BLOCKING; + + if (blk_mq_alloc_tag_set(>tag_set)) + return; + + p->queue = blk_mq_init_queue(>tag_set); if (IS_ERR(p->queue)) { + blk_mq_free_tag_set(>tag_set); p->queue = NULL; return; } -- 2.19.1
[PATCH 2/6] nullb: remove leftover legacy request code
null_softirq_done_fn is only used for the blk-mq path, so remove the other branch. Also rename the function to better match the method name. Signed-off-by: Christoph Hellwig --- drivers/block/null_blk_main.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c index 9c045bee0985..16ba3db2a015 100644 --- a/drivers/block/null_blk_main.c +++ b/drivers/block/null_blk_main.c @@ -642,14 +642,11 @@ static void null_cmd_end_timer(struct nullb_cmd *cmd) hrtimer_start(>timer, kt, HRTIMER_MODE_REL); } -static void null_softirq_done_fn(struct request *rq) +static void null_complete_rq(struct request *rq) { struct nullb *nullb = rq->q->queuedata; - if (nullb->dev->queue_mode == NULL_Q_MQ) - end_cmd(blk_mq_rq_to_pdu(rq)); - else - end_cmd(rq->special); + end_cmd(blk_mq_rq_to_pdu(rq)); } static struct nullb_page *null_alloc_page(gfp_t gfp_flags) @@ -1357,7 +1354,7 @@ static blk_status_t null_queue_rq(struct blk_mq_hw_ctx *hctx, static const struct blk_mq_ops null_mq_ops = { .queue_rq = null_queue_rq, - .complete = null_softirq_done_fn, + .complete = null_complete_rq, .timeout= null_timeout_rq, }; -- 2.19.1
[PATCH 3/6] skd_main: don't use req->special
Add a retries field to the internal request structure instead, which gets set to zero on the first submission. Signed-off-by: Christoph Hellwig --- drivers/block/skd_main.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/block/skd_main.c b/drivers/block/skd_main.c index a0196477165f..88f29b06d90e 100644 --- a/drivers/block/skd_main.c +++ b/drivers/block/skd_main.c @@ -181,6 +181,7 @@ struct skd_request_context { struct fit_completion_entry_v1 completion; struct fit_comp_error_info err_info; + int retries; blk_status_t status; }; @@ -495,6 +496,11 @@ static blk_status_t skd_mq_queue_rq(struct blk_mq_hw_ctx *hctx, if (unlikely(skdev->state != SKD_DRVR_STATE_ONLINE)) return skd_fail_all(q) ? BLK_STS_IOERR : BLK_STS_RESOURCE; + if (!(req->rq_flags & RQF_DONTPREP)) { + skreq->retries = 0; + req->rq_flags |= RQF_DONTPREP; + } + blk_mq_start_request(req); WARN_ONCE(tag >= skd_max_queue_depth, "%#x > %#x (nr_requests = %lu)\n", @@ -1426,7 +1432,7 @@ static void skd_resolve_req_exception(struct skd_device *skdev, break; case SKD_CHECK_STATUS_REQUEUE_REQUEST: - if ((unsigned long) ++req->special < SKD_MAX_RETRIES) { + if ((unsigned long) skreq->retries < SKD_MAX_RETRIES) { skd_log_skreq(skdev, skreq, "retry"); blk_mq_requeue_request(req, true); break; -- 2.19.1
[PATCH 4/6] aoe: replace ->special use with private data in the request
Makes the code a whole lot better to read.. Signed-off-by: Christoph Hellwig --- drivers/block/aoe/aoe.h| 4 drivers/block/aoe/aoeblk.c | 1 + drivers/block/aoe/aoecmd.c | 27 +-- drivers/block/aoe/aoedev.c | 11 ++- 4 files changed, 20 insertions(+), 23 deletions(-) diff --git a/drivers/block/aoe/aoe.h b/drivers/block/aoe/aoe.h index 7ca76ed2e71a..84d0fcebd6af 100644 --- a/drivers/block/aoe/aoe.h +++ b/drivers/block/aoe/aoe.h @@ -100,6 +100,10 @@ enum { MAX_TAINT = 1000, /* cap on aoetgt taint */ }; +struct aoe_req { + unsigned long nr_bios; +}; + struct buf { ulong nframesout; struct bio *bio; diff --git a/drivers/block/aoe/aoeblk.c b/drivers/block/aoe/aoeblk.c index ed26b7287256..e2c6aae2d636 100644 --- a/drivers/block/aoe/aoeblk.c +++ b/drivers/block/aoe/aoeblk.c @@ -387,6 +387,7 @@ aoeblk_gdalloc(void *vp) set = >tag_set; set->ops = _mq_ops; + set->cmd_size = sizeof(struct aoe_req); set->nr_hw_queues = 1; set->queue_depth = 128; set->numa_node = NUMA_NO_NODE; diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c index bb2fba651bd2..3cf9bc5d8d95 100644 --- a/drivers/block/aoe/aoecmd.c +++ b/drivers/block/aoe/aoecmd.c @@ -822,17 +822,6 @@ rexmit_timer(struct timer_list *timer) spin_unlock_irqrestore(>lock, flags); } -static unsigned long -rqbiocnt(struct request *r) -{ - struct bio *bio; - unsigned long n = 0; - - __rq_for_each_bio(bio, r) - n++; - return n; -} - static void bufinit(struct buf *buf, struct request *rq, struct bio *bio) { @@ -847,6 +836,7 @@ nextbuf(struct aoedev *d) { struct request *rq; struct request_queue *q; + struct aoe_req *req; struct buf *buf; struct bio *bio; @@ -865,7 +855,11 @@ nextbuf(struct aoedev *d) blk_mq_start_request(rq); d->ip.rq = rq; d->ip.nxbio = rq->bio; - rq->special = (void *) rqbiocnt(rq); + + req = blk_mq_rq_to_pdu(rq); + req->nr_bios = 0; + __rq_for_each_bio(bio, rq) + req->nr_bios++; } buf = mempool_alloc(d->bufpool, GFP_ATOMIC); if (buf == NULL) { @@ -1069,16 +1063,13 @@ aoe_end_request(struct aoedev *d, struct request *rq, int fastfail) static void aoe_end_buf(struct aoedev *d, struct buf *buf) { - struct request *rq; - unsigned long n; + struct request *rq = buf->rq; + struct aoe_req *req = blk_mq_rq_to_pdu(rq); if (buf == d->ip.buf) d->ip.buf = NULL; - rq = buf->rq; mempool_free(buf, d->bufpool); - n = (unsigned long) rq->special; - rq->special = (void *) --n; - if (n == 0) + if (--req->nr_bios == 0) aoe_end_request(d, rq, 0); } diff --git a/drivers/block/aoe/aoedev.c b/drivers/block/aoe/aoedev.c index 9063f8efbd3b..5b49f1b33ebe 100644 --- a/drivers/block/aoe/aoedev.c +++ b/drivers/block/aoe/aoedev.c @@ -160,21 +160,22 @@ static void aoe_failip(struct aoedev *d) { struct request *rq; + struct aoe_req *req; struct bio *bio; - unsigned long n; aoe_failbuf(d, d->ip.buf); - rq = d->ip.rq; if (rq == NULL) return; + + req = blk_mq_rq_to_pdu(rq); while ((bio = d->ip.nxbio)) { bio->bi_status = BLK_STS_IOERR; d->ip.nxbio = bio->bi_next; - n = (unsigned long) rq->special; - rq->special = (void *) --n; + req->nr_bios--; } - if ((unsigned long) rq->special == 0) + + if (!req->nr_bios) aoe_end_request(d, rq, 0); } -- 2.19.1
remove most req->special users
Try to get rid of the req->special users so that we can remove this field. With this series we basically only have the osd / scsi BIDI code left, which should go away in another series. Note that the first one is a bug fix for the blk-mq conversion series, I don't think the current fnic code can work, but this version is only compile tested as well.
[PATCH 1/6] fnic: fix fnic_scsi_host_{start,end}_tag
They way these functions abuse ->special to try to store the dummy request looks completely broken, given that it actually stores the original scsi command. Instead switch to ->host_scribble and store the actual dummy command. Signed-off-by: Christoph Hellwig --- drivers/scsi/fnic/fnic_scsi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c index 96acfcecd540..cafbcfb85bfa 100644 --- a/drivers/scsi/fnic/fnic_scsi.c +++ b/drivers/scsi/fnic/fnic_scsi.c @@ -2274,7 +2274,7 @@ fnic_scsi_host_start_tag(struct fnic *fnic, struct scsi_cmnd *sc) return SCSI_NO_TAG; sc->tag = sc->request->tag = dummy->tag; - sc->request->special = sc; + sc->host_scribble = (unsigned char *)dummy; return dummy->tag; } @@ -2286,7 +2286,7 @@ fnic_scsi_host_start_tag(struct fnic *fnic, struct scsi_cmnd *sc) static inline void fnic_scsi_host_end_tag(struct fnic *fnic, struct scsi_cmnd *sc) { - struct request *dummy = sc->request->special; + struct request *dummy = (struct request *)sc->host_scribble; blk_mq_free_request(dummy); } -- 2.19.1
Re: [GIT PULL] nvme fixes for 4.20
On 11/9/18 10:10 AM, Sagi Grimberg wrote: > >>> That is because your for-linus was still 4.20-rc based when >>> I created it. >> >> It's never been 4.20-rc based. After the initial merge, it got based >> on a random commit in the merge window: >> >> commit 3acbd2de6bc3af215c6ed7732dfc097d1e238503 >> Merge: d49f8a52b15b de7d83da84bd >> Author: Linus Torvalds >> Date: Thu Oct 25 09:00:15 2018 -0700 >> >> Merge tag 'sound-4.20-rc1' of >> git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound >> >> and has been ever since. > > That was because I rebased it when your for-linus didn't include Ming's > buggy version of the brd commit. Sorry for the mixup.. No problem, it's pretty trivial to just do manually for cases like this. -- Jens Axboe
Re: [GIT PULL] nvme fixes for 4.20
That is because your for-linus was still 4.20-rc based when I created it. It's never been 4.20-rc based. After the initial merge, it got based on a random commit in the merge window: commit 3acbd2de6bc3af215c6ed7732dfc097d1e238503 Merge: d49f8a52b15b de7d83da84bd Author: Linus Torvalds Date: Thu Oct 25 09:00:15 2018 -0700 Merge tag 'sound-4.20-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound and has been ever since. That was because I rebased it when your for-linus didn't include Ming's buggy version of the brd commit. Sorry for the mixup..
Re: mtip32xx: fixes and cleanups
On 11/9/18 6:48 AM, Christoph Hellwig wrote: > Various low hanging fruit, kicked of by seeing one of the few remaining > req->special users. > > Compile tested only. Looks good to me - as it so happens, I do have a few of these cards. Tested working fine after the patches. -- Jens Axboe
[PATCH blktests] makefile: Add install rule
Add rule to install to a target directory, /usr/local/blktests by default. Signed-off-by: Gwendal Grignou --- Makefile | 9 + src/Makefile | 3 +++ 2 files changed, 12 insertions(+) diff --git a/Makefile b/Makefile index 38b8ad1..d7c2b74 100644 --- a/Makefile +++ b/Makefile @@ -1,9 +1,18 @@ +prefix ?= /usr/local +dest = $(DESTDIR)$(prefix)/blktests + all: $(MAKE) -C src all clean: $(MAKE) -C src clean +install: + install -m755 -d $(dest) + install check $(dest) + cp -R tests common $(dest) + $(MAKE) -C src dest=$(dest) install + # SC2119: "Use foo "$@" if function's $1 should mean script's $1". False # positives on helpers like _init_scsi_debug. SHELLCHECK_EXCLUDE := SC2119 diff --git a/src/Makefile b/src/Makefile index 15c1022..f0ddbb5 100644 --- a/src/Makefile +++ b/src/Makefile @@ -20,6 +20,9 @@ all: $(TARGETS) clean: rm -f $(TARGETS) +install: $(TARGETS) + install $(TARGETS) $(dest) + $(C_TARGETS): %: %.c $(CC) $(CPPFLAGS) $(CFLAGS) -o $@ $^ -- 2.16.4
Re: [PATCH 2/2] sx8: use a per-host tag_set
On 11/9/18 6:51 AM, Christoph Hellwig wrote: > The current sx8 code spends a lot of effort dealing with the fact that > tags are per-host, but there might be multiple queues. Now that the > driver has been converted to blk-mq it can take care of the blk-mq > tag_set concept that has been designed just for that. This is a thing of beauty, it should be on the poster for "why drivers should be as trivial as possible". -- Jens Axboe
Re: [PATCH 3/7] scsi: push blk_status_t up into scsi_setup_{fs,scsi}_cmnd
Look good, Reviewed-by: Johannes Thumshirn -- Johannes ThumshirnSUSE Labs jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
[PATCH 2/2] sx8: use a per-host tag_set
The current sx8 code spends a lot of effort dealing with the fact that tags are per-host, but there might be multiple queues. Now that the driver has been converted to blk-mq it can take care of the blk-mq tag_set concept that has been designed just for that. Signed-off-by: Christoph Hellwig --- drivers/block/sx8.c | 343 1 file changed, 95 insertions(+), 248 deletions(-) diff --git a/drivers/block/sx8.c b/drivers/block/sx8.c index 1f371b9c0954..4478eb7efee0 100644 --- a/drivers/block/sx8.c +++ b/drivers/block/sx8.c @@ -243,7 +243,6 @@ struct carm_port { unsigned intport_no; struct gendisk *disk; struct carm_host*host; - struct blk_mq_tag_set tag_set; /* attached device characteristics */ u64 capacity; @@ -254,13 +253,10 @@ struct carm_port { }; struct carm_request { - unsigned inttag; int n_elem; unsigned intmsg_type; unsigned intmsg_subtype; unsigned intmsg_bucket; - struct request *rq; - struct carm_port*port; struct scatterlist sg[CARM_MAX_REQ_SG]; }; @@ -291,9 +287,6 @@ struct carm_host { unsigned intwait_q_cons; struct request_queue*wait_q[CARM_MAX_WAIT_Q]; - unsigned intn_msgs; - u64 msg_alloc; - struct carm_request req[CARM_MAX_REQ]; void*msg_base; dma_addr_t msg_dma; @@ -478,10 +471,10 @@ static inline dma_addr_t carm_ref_msg_dma(struct carm_host *host, } static int carm_send_msg(struct carm_host *host, -struct carm_request *crq) +struct carm_request *crq, unsigned tag) { void __iomem *mmio = host->mmio; - u32 msg = (u32) carm_ref_msg_dma(host, crq->tag); + u32 msg = (u32) carm_ref_msg_dma(host, tag); u32 cm_bucket = crq->msg_bucket; u32 tmp; int rc = 0; @@ -506,99 +499,24 @@ static int carm_send_msg(struct carm_host *host, return rc; } -static struct carm_request *carm_get_request(struct carm_host *host) -{ - unsigned int i; - - /* obey global hardware limit on S/G entries */ - if (host->hw_sg_used >= (CARM_MAX_HOST_SG - CARM_MAX_REQ_SG)) - return NULL; - - for (i = 0; i < max_queue; i++) - if ((host->msg_alloc & (1ULL << i)) == 0) { - struct carm_request *crq = >req[i]; - crq->port = NULL; - crq->n_elem = 0; - - host->msg_alloc |= (1ULL << i); - host->n_msgs++; - - assert(host->n_msgs <= CARM_MAX_REQ); - sg_init_table(crq->sg, CARM_MAX_REQ_SG); - return crq; - } - - DPRINTK("no request available, returning NULL\n"); - return NULL; -} - -static int carm_put_request(struct carm_host *host, struct carm_request *crq) -{ - assert(crq->tag < max_queue); - - if (unlikely((host->msg_alloc & (1ULL << crq->tag)) == 0)) - return -EINVAL; /* tried to clear a tag that was not active */ - - assert(host->hw_sg_used >= crq->n_elem); - - host->msg_alloc &= ~(1ULL << crq->tag); - host->hw_sg_used -= crq->n_elem; - host->n_msgs--; - - return 0; -} - -static struct carm_request *carm_get_special(struct carm_host *host) -{ - unsigned long flags; - struct carm_request *crq = NULL; - struct request *rq; - int tries = 5000; - - while (tries-- > 0) { - spin_lock_irqsave(>lock, flags); - crq = carm_get_request(host); - spin_unlock_irqrestore(>lock, flags); - - if (crq) - break; - msleep(10); - } - - if (!crq) - return NULL; - - rq = blk_get_request(host->oob_q, REQ_OP_DRV_OUT, 0); - if (IS_ERR(rq)) { - spin_lock_irqsave(>lock, flags); - carm_put_request(host, crq); - spin_unlock_irqrestore(>lock, flags); - return NULL; - } - - crq->rq = rq; - return crq; -} - static int carm_array_info (struct carm_host *host, unsigned int array_idx) { struct carm_msg_ioctl *ioc; - unsigned int idx; u32 msg_data; dma_addr_t msg_dma; struct carm_request *crq; + struct request *rq; int rc; - crq = carm_get_special(host); - if (!crq) { + rq = blk_mq_alloc_request(host->oob_q, REQ_OP_DRV_OUT, 0); + if (IS_ERR(rq)) {
[PATCH 1/2] sx8: cleanup queue and disk allocation / freeing
Make the disk/queue alloc and free helpers per-port by moving the trivial loops into the callers. Signed-off-by: Christoph Hellwig --- drivers/block/sx8.c | 107 1 file changed, 48 insertions(+), 59 deletions(-) diff --git a/drivers/block/sx8.c b/drivers/block/sx8.c index 064b8c5c7a32..1f371b9c0954 100644 --- a/drivers/block/sx8.c +++ b/drivers/block/sx8.c @@ -1499,70 +1499,54 @@ static const struct blk_mq_ops carm_mq_ops = { .queue_rq = carm_queue_rq, }; -static int carm_init_disks(struct carm_host *host) +static int carm_init_disk(struct carm_host *host, unsigned int port_no) { - unsigned int i; - int rc = 0; - - for (i = 0; i < CARM_MAX_PORTS; i++) { - struct gendisk *disk; - struct request_queue *q; - struct carm_port *port; - - port = >port[i]; - port->host = host; - port->port_no = i; - - disk = alloc_disk(CARM_MINORS_PER_MAJOR); - if (!disk) { - rc = -ENOMEM; - break; - } + struct carm_port *port = >port[port_no]; + struct gendisk *disk; + struct request_queue *q; - port->disk = disk; - sprintf(disk->disk_name, DRV_NAME "/%u", - (unsigned int) (host->id * CARM_MAX_PORTS) + i); - disk->major = host->major; - disk->first_minor = i * CARM_MINORS_PER_MAJOR; - disk->fops = _bd_ops; - disk->private_data = port; - - q = blk_mq_init_sq_queue(>tag_set, _mq_ops, -max_queue, BLK_MQ_F_SHOULD_MERGE); - if (IS_ERR(q)) { - rc = PTR_ERR(q); - break; - } - disk->queue = q; - blk_queue_max_segments(q, CARM_MAX_REQ_SG); - blk_queue_segment_boundary(q, CARM_SG_BOUNDARY); + port->host = host; + port->port_no = port_no; - q->queuedata = port; - } + disk = alloc_disk(CARM_MINORS_PER_MAJOR); + if (!disk) + return -ENOMEM; - return rc; + port->disk = disk; + sprintf(disk->disk_name, DRV_NAME "/%u", + (unsigned int)host->id * CARM_MAX_PORTS + port_no); + disk->major = host->major; + disk->first_minor = port_no * CARM_MINORS_PER_MAJOR; + disk->fops = _bd_ops; + disk->private_data = port; + + q = blk_mq_init_sq_queue(>tag_set, _mq_ops, +max_queue, BLK_MQ_F_SHOULD_MERGE); + if (IS_ERR(q)) + return PTR_ERR(q); + disk->queue = q; + blk_queue_max_segments(q, CARM_MAX_REQ_SG); + blk_queue_segment_boundary(q, CARM_SG_BOUNDARY); + + q->queuedata = port; + return 0; } -static void carm_free_disks(struct carm_host *host) +static void carm_free_disk(struct carm_host *host, unsigned int port_no) { - unsigned int i; + struct carm_port *port = >port[port_no]; + struct gendisk *disk = port->disk; - for (i = 0; i < CARM_MAX_PORTS; i++) { - struct carm_port *port = >port[i]; - struct gendisk *disk = port->disk; - - if (disk) { - struct request_queue *q = disk->queue; + if (!disk) + return; - if (disk->flags & GENHD_FL_UP) - del_gendisk(disk); - if (q) { - blk_mq_free_tag_set(>tag_set); - blk_cleanup_queue(q); - } - put_disk(disk); - } + if (disk->flags & GENHD_FL_UP) + del_gendisk(disk); + if (disk->queue) { + blk_mq_free_tag_set(>tag_set); + blk_cleanup_queue(disk->queue); } + put_disk(disk); } static int carm_init_shm(struct carm_host *host) @@ -1667,9 +1651,11 @@ static int carm_init_one (struct pci_dev *pdev, const struct pci_device_id *ent) if (host->flags & FL_DYN_MAJOR) host->major = rc; - rc = carm_init_disks(host); - if (rc) - goto err_out_blkdev_disks; + for (i = 0; i < CARM_MAX_PORTS; i++) { + rc = carm_init_disk(host, i); + if (rc) + goto err_out_blkdev_disks; + } pci_set_master(pdev); @@ -1699,7 +1685,8 @@ static int carm_init_one (struct pci_dev *pdev, const struct pci_device_id *ent) err_out_free_irq: free_irq(pdev->irq, host); err_out_blkdev_disks: - carm_free_disks(host); + for (i = 0; i < CARM_MAX_PORTS; i++) + carm_free_disk(host, i); unregister_blkdev(host->major, host->name); err_out_free_majors: if (host->major == 160) @@ -1724,6
[PATCH 2/9] mtip32xx: merge mtip_submit_request into mtip_queue_rq
Factor out a new is_stopped helper that matches the existing is_se_active helper, and merge the trivial amount of remaining code into the only caller. This also allows better error handling by returning a BLK_STS_* directly instead of explicitly calling blk_mq_end_request, and moving blk_mq_start_request closer to the actual issue to hardware. Signed-off-by: Christoph Hellwig --- drivers/block/mtip32xx/mtip32xx.c | 78 +++ 1 file changed, 28 insertions(+), 50 deletions(-) diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c index 1964af8a187b..75a4ca0210f2 100644 --- a/drivers/block/mtip32xx/mtip32xx.c +++ b/drivers/block/mtip32xx/mtip32xx.c @@ -3534,54 +3534,24 @@ static inline bool is_se_active(struct driver_data *dd) return false; } -/* - * Block layer make request function. - * - * This function is called by the kernel to process a BIO for - * the P320 device. - * - * @queue Pointer to the request queue. Unused other than to obtain - * the driver data structure. - * @rqPointer to the request. - * - */ -static int mtip_submit_request(struct blk_mq_hw_ctx *hctx, struct request *rq) +static inline bool is_stopped(struct driver_data *dd, struct request *rq) { - struct driver_data *dd = hctx->queue->queuedata; - struct mtip_cmd *cmd = blk_mq_rq_to_pdu(rq); - - if (is_se_active(dd)) - return -ENODATA; - - if (unlikely(dd->dd_flag & MTIP_DDF_STOP_IO)) { - if (unlikely(test_bit(MTIP_DDF_REMOVE_PENDING_BIT, - >dd_flag))) { - return -ENXIO; - } - if (unlikely(test_bit(MTIP_DDF_OVER_TEMP_BIT, >dd_flag))) { - return -ENODATA; - } - if (unlikely(test_bit(MTIP_DDF_WRITE_PROTECT_BIT, - >dd_flag) && - rq_data_dir(rq))) { - return -ENODATA; - } - if (unlikely(test_bit(MTIP_DDF_SEC_LOCK_BIT, >dd_flag) || - test_bit(MTIP_DDF_REBUILD_FAILED_BIT, >dd_flag))) - return -ENODATA; - } - - if (req_op(rq) == REQ_OP_DISCARD) { - int err; + if (likely(!(dd->dd_flag & MTIP_DDF_STOP_IO))) + return false; - err = mtip_send_trim(dd, blk_rq_pos(rq), blk_rq_sectors(rq)); - blk_mq_end_request(rq, err ? BLK_STS_IOERR : BLK_STS_OK); - return 0; - } + if (test_bit(MTIP_DDF_REMOVE_PENDING_BIT, >dd_flag)) + return true; + if (test_bit(MTIP_DDF_OVER_TEMP_BIT, >dd_flag)) + return true; + if (test_bit(MTIP_DDF_WRITE_PROTECT_BIT, >dd_flag) && + rq_data_dir(rq)) + return true; + if (test_bit(MTIP_DDF_SEC_LOCK_BIT, >dd_flag)) + return true; + if (test_bit(MTIP_DDF_REBUILD_FAILED_BIT, >dd_flag)) + return true; - /* Issue the read/write. */ - mtip_hw_submit_io(dd, rq, cmd, hctx); - return 0; + return false; } static bool mtip_check_unal_depth(struct blk_mq_hw_ctx *hctx, @@ -3647,8 +3617,9 @@ static blk_status_t mtip_issue_reserved_cmd(struct blk_mq_hw_ctx *hctx, static blk_status_t mtip_queue_rq(struct blk_mq_hw_ctx *hctx, const struct blk_mq_queue_data *bd) { + struct driver_data *dd = hctx->queue->queuedata; struct request *rq = bd->rq; - int ret; + struct mtip_cmd *cmd = blk_mq_rq_to_pdu(rq); mtip_init_cmd_header(rq); @@ -3658,12 +3629,19 @@ static blk_status_t mtip_queue_rq(struct blk_mq_hw_ctx *hctx, if (unlikely(mtip_check_unal_depth(hctx, rq))) return BLK_STS_RESOURCE; + if (is_se_active(dd) || is_stopped(dd, rq)) + return BLK_STS_IOERR; + blk_mq_start_request(rq); - ret = mtip_submit_request(hctx, rq); - if (likely(!ret)) - return BLK_STS_OK; - return BLK_STS_IOERR; + if (req_op(rq) == REQ_OP_DISCARD) { + if (mtip_send_trim(dd, blk_rq_pos(rq), blk_rq_sectors(rq)) < 0) + return BLK_STS_IOERR; + } else { + mtip_hw_submit_io(dd, rq, cmd, hctx); + } + + return BLK_STS_OK; } static void mtip_free_cmd(struct blk_mq_tag_set *set, struct request *rq, -- 2.19.1
[PATCH 6/9] mtip32xx: remove mtip_init_cmd_header
There isn't much need for this helper - we can just calculate the offset for the command header once late in the submission path and fill out the ctba and ctbau fields there. Signed-off-by: Christoph Hellwig --- drivers/block/mtip32xx/mtip32xx.c | 44 +++ drivers/block/mtip32xx/mtip32xx.h | 5 2 files changed, 15 insertions(+), 34 deletions(-) diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c index 9245c325fe3c..6006f7baa1eb 100644 --- a/drivers/block/mtip32xx/mtip32xx.c +++ b/drivers/block/mtip32xx/mtip32xx.c @@ -168,24 +168,6 @@ static bool mtip_check_surprise_removal(struct pci_dev *pdev) return false; /* device present */ } -/* we have to use runtime tag to setup command header */ -static void mtip_init_cmd_header(struct request *rq) -{ - struct driver_data *dd = rq->q->queuedata; - struct mtip_cmd *cmd = blk_mq_rq_to_pdu(rq); - - /* Point the command headers at the command tables. */ - cmd->command_header = dd->port->command_list + - (sizeof(struct mtip_cmd_hdr) * rq->tag); - cmd->command_header_dma = dd->port->command_list_dma + - (sizeof(struct mtip_cmd_hdr) * rq->tag); - - if (test_bit(MTIP_PF_HOST_CAP_64, >port->flags)) - cmd->command_header->ctbau = cpu_to_le32((cmd->command_dma >> 16) >> 16); - - cmd->command_header->ctba = cpu_to_le32(cmd->command_dma & 0x); -} - static struct mtip_cmd *mtip_get_int_command(struct driver_data *dd) { struct request *rq; @@ -197,9 +179,6 @@ static struct mtip_cmd *mtip_get_int_command(struct driver_data *dd) if (IS_ERR(rq)) return NULL; - /* Internal cmd isn't submitted via .queue_rq */ - mtip_init_cmd_header(rq); - return blk_mq_rq_to_pdu(rq); } @@ -2179,6 +2158,8 @@ static void mtip_hw_submit_io(struct driver_data *dd, struct request *rq, struct mtip_cmd *command, struct blk_mq_hw_ctx *hctx) { + struct mtip_cmd_hdr *hdr = + dd->port->command_list + sizeof(struct mtip_cmd_hdr) * rq->tag; struct host_to_dev_fis *fis; struct mtip_port *port = dd->port; int dma_dir = rq_data_dir(rq) == READ ? DMA_FROM_DEVICE : DMA_TO_DEVICE; @@ -2228,9 +2209,11 @@ static void mtip_hw_submit_io(struct driver_data *dd, struct request *rq, fis->device |= 1 << 7; /* Populate the command header */ - command->command_header->opts = - cpu_to_le32((nents << 16) | 5 | AHCI_CMD_PREFETCH); - command->command_header->byte_count = 0; + hdr->ctba = cpu_to_le32(command->command_dma & 0x); + if (test_bit(MTIP_PF_HOST_CAP_64, >port->flags)) + hdr->ctbau = cpu_to_le32((command->command_dma >> 16) >> 16); + hdr->opts = cpu_to_le32((nents << 16) | 5 | AHCI_CMD_PREFETCH); + hdr->byte_count = 0; command->direction = dma_dir; @@ -3577,13 +3560,18 @@ static blk_status_t mtip_issue_reserved_cmd(struct blk_mq_hw_ctx *hctx, struct driver_data *dd = hctx->queue->queuedata; struct mtip_int_cmd *icmd = rq->special; struct mtip_cmd *cmd = blk_mq_rq_to_pdu(rq); + struct mtip_cmd_hdr *hdr = + dd->port->command_list + sizeof(struct mtip_cmd_hdr) * rq->tag; struct mtip_cmd_sg *command_sg; if (mtip_commands_active(dd->port)) return BLK_STS_RESOURCE; + hdr->ctba = cpu_to_le32(cmd->command_dma & 0x); + if (test_bit(MTIP_PF_HOST_CAP_64, >port->flags)) + hdr->ctbau = cpu_to_le32((cmd->command_dma >> 16) >> 16); /* Populate the SG list */ - cmd->command_header->opts = cpu_to_le32(icmd->opts | icmd->fis_len); + hdr->opts = cpu_to_le32(icmd->opts | icmd->fis_len); if (icmd->buf_len) { command_sg = cmd->command + AHCI_CMD_TBL_HDR_SZ; @@ -3592,11 +3580,11 @@ static blk_status_t mtip_issue_reserved_cmd(struct blk_mq_hw_ctx *hctx, command_sg->dba_upper = cpu_to_le32((icmd->buffer >> 16) >> 16); - cmd->command_header->opts |= cpu_to_le32((1 << 16)); + hdr->opts |= cpu_to_le32((1 << 16)); } /* Populate the command header */ - cmd->command_header->byte_count = 0; + hdr->byte_count = 0; blk_mq_start_request(rq); mtip_issue_non_ncq_command(dd->port, rq->tag); @@ -3610,8 +3598,6 @@ static blk_status_t mtip_queue_rq(struct blk_mq_hw_ctx *hctx, struct request *rq = bd->rq; struct mtip_cmd *cmd = blk_mq_rq_to_pdu(rq); - mtip_init_cmd_header(rq); - if (blk_rq_is_passthrough(rq)) return mtip_issue_reserved_cmd(hctx, rq); diff --git a/drivers/block/mtip32xx/mtip32xx.h b/drivers/block/mtip32xx/mtip32xx.h index
[PATCH 1/9] mtip32xx: move the blk_rq_map_sg call to mtip_hw_submit_io
We have all arguments at hand in mtip_hw_submit_io, so keep the rq to sg mapping close to the dma_map_sg call. Signed-off-by: Christoph Hellwig --- drivers/block/mtip32xx/mtip32xx.c | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c index 947aa10107a6..1964af8a187b 100644 --- a/drivers/block/mtip32xx/mtip32xx.c +++ b/drivers/block/mtip32xx/mtip32xx.c @@ -2171,7 +2171,6 @@ static int mtip_hw_ioctl(struct driver_data *dd, unsigned int cmd, * @dd Pointer to the driver data structure. * @startFirst sector to read. * @nsectNumber of sectors to read. - * @nentsNumber of entries in scatter list for the read command. * @tag The tag of this read command. * @callback Pointer to the function that should be called * when the read completes. @@ -2183,7 +2182,7 @@ static int mtip_hw_ioctl(struct driver_data *dd, unsigned int cmd, * None */ static void mtip_hw_submit_io(struct driver_data *dd, struct request *rq, - struct mtip_cmd *command, int nents, + struct mtip_cmd *command, struct blk_mq_hw_ctx *hctx) { struct host_to_dev_fis *fis; @@ -2191,8 +2190,10 @@ static void mtip_hw_submit_io(struct driver_data *dd, struct request *rq, int dma_dir = rq_data_dir(rq) == READ ? DMA_FROM_DEVICE : DMA_TO_DEVICE; u64 start = blk_rq_pos(rq); unsigned int nsect = blk_rq_sectors(rq); + unsigned int nents; /* Map the scatter list for DMA access */ + nents = blk_rq_map_sg(hctx->queue, rq, command->sg); nents = dma_map_sg(>pdev->dev, command->sg, nents, dma_dir); prefetch(>flags); @@ -3548,7 +3549,6 @@ static int mtip_submit_request(struct blk_mq_hw_ctx *hctx, struct request *rq) { struct driver_data *dd = hctx->queue->queuedata; struct mtip_cmd *cmd = blk_mq_rq_to_pdu(rq); - unsigned int nents; if (is_se_active(dd)) return -ENODATA; @@ -3579,11 +3579,8 @@ static int mtip_submit_request(struct blk_mq_hw_ctx *hctx, struct request *rq) return 0; } - /* Create the scatter list for this request. */ - nents = blk_rq_map_sg(hctx->queue, rq, cmd->sg); - /* Issue the read/write. */ - mtip_hw_submit_io(dd, rq, cmd, nents, hctx); + mtip_hw_submit_io(dd, rq, cmd, hctx); return 0; } -- 2.19.1
[PATCH 5/9] mtip32xx: add missing endianess annotations on struct smart_attr
Signed-off-by: Christoph Hellwig --- drivers/block/mtip32xx/mtip32xx.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/block/mtip32xx/mtip32xx.h b/drivers/block/mtip32xx/mtip32xx.h index 0aa1ea210822..e8b4b3d5365a 100644 --- a/drivers/block/mtip32xx/mtip32xx.h +++ b/drivers/block/mtip32xx/mtip32xx.h @@ -172,10 +172,10 @@ enum { struct smart_attr { u8 attr_id; - u16 flags; + __le16 flags; u8 cur; u8 worst; - u32 data; + __le32 data; u8 res[3]; } __packed; -- 2.19.1
[PATCH 9/9] mtip32xxx: use for_each_sg
Use the proper helper instead of manually iterating the scatterlist, which is broken in the presence of chained S/G lists. Signed-off-by: Christoph Hellwig --- drivers/block/mtip32xx/mtip32xx.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c index e99db2c9118f..a4c44db097e0 100644 --- a/drivers/block/mtip32xx/mtip32xx.c +++ b/drivers/block/mtip32xx/mtip32xx.c @@ -1549,11 +1549,11 @@ static inline void fill_command_sg(struct driver_data *dd, int n; unsigned int dma_len; struct mtip_cmd_sg *command_sg; - struct scatterlist *sg = command->sg; + struct scatterlist *sg; command_sg = command->command + AHCI_CMD_TBL_HDR_SZ; - for (n = 0; n < nents; n++) { + for_each_sg(command->sg, sg, nents, n) { dma_len = sg_dma_len(sg); if (dma_len > 0x40) dev_err(>pdev->dev, @@ -1563,7 +1563,6 @@ static inline void fill_command_sg(struct driver_data *dd, command_sg->dba_upper = cpu_to_le32((sg_dma_address(sg) >> 16) >> 16); command_sg++; - sg++; } } -- 2.19.1
[PATCH 7/9] mtip32xx: remove mtip_get_int_command
Merging this function into the only callers makes the code flow easier. Signed-off-by: Christoph Hellwig --- drivers/block/mtip32xx/mtip32xx.c | 24 +++- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c index 6006f7baa1eb..5ae86224b2f5 100644 --- a/drivers/block/mtip32xx/mtip32xx.c +++ b/drivers/block/mtip32xx/mtip32xx.c @@ -168,20 +168,6 @@ static bool mtip_check_surprise_removal(struct pci_dev *pdev) return false; /* device present */ } -static struct mtip_cmd *mtip_get_int_command(struct driver_data *dd) -{ - struct request *rq; - - if (mtip_check_surprise_removal(dd->pdev)) - return NULL; - - rq = blk_mq_alloc_request(dd->queue, REQ_OP_DRV_IN, BLK_MQ_REQ_RESERVED); - if (IS_ERR(rq)) - return NULL; - - return blk_mq_rq_to_pdu(rq); -} - static struct mtip_cmd *mtip_cmd_from_tag(struct driver_data *dd, unsigned int tag) { @@ -1002,12 +988,15 @@ static int mtip_exec_internal_command(struct mtip_port *port, return -EFAULT; } - int_cmd = mtip_get_int_command(dd); - if (!int_cmd) { + if (mtip_check_surprise_removal(dd->pdev)) + return -EFAULT; + + rq = blk_mq_alloc_request(dd->queue, REQ_OP_DRV_IN, BLK_MQ_REQ_RESERVED); + if (IS_ERR(rq)) { dbg_printk(MTIP_DRV_NAME "Unable to allocate tag for PIO cmd\n"); return -EFAULT; } - rq = blk_mq_rq_from_pdu(int_cmd); + rq->special = set_bit(MTIP_PF_IC_ACTIVE_BIT, >flags); @@ -1029,6 +1018,7 @@ static int mtip_exec_internal_command(struct mtip_port *port, } /* Copy the command to the command table */ + int_cmd = blk_mq_rq_to_pdu(rq); memcpy(int_cmd->command, fis, fis_len*4); rq->timeout = timeout; -- 2.19.1
[PATCH 8/9] mtip32xx: don't use req->special
Instead create add to the icmd into struct mtip_cmd which can be unioned with the scatterlist used for the normal I/O path. Signed-off-by: Christoph Hellwig --- drivers/block/mtip32xx/mtip32xx.c | 5 ++--- drivers/block/mtip32xx/mtip32xx.h | 7 ++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c index 5ae86224b2f5..e99db2c9118f 100644 --- a/drivers/block/mtip32xx/mtip32xx.c +++ b/drivers/block/mtip32xx/mtip32xx.c @@ -997,8 +997,6 @@ static int mtip_exec_internal_command(struct mtip_port *port, return -EFAULT; } - rq->special = - set_bit(MTIP_PF_IC_ACTIVE_BIT, >flags); if (fis->command == ATA_CMD_SEC_ERASE_PREP) @@ -1019,6 +1017,7 @@ static int mtip_exec_internal_command(struct mtip_port *port, /* Copy the command to the command table */ int_cmd = blk_mq_rq_to_pdu(rq); + int_cmd->icmd = memcpy(int_cmd->command, fis, fis_len*4); rq->timeout = timeout; @@ -3548,8 +3547,8 @@ static blk_status_t mtip_issue_reserved_cmd(struct blk_mq_hw_ctx *hctx, struct request *rq) { struct driver_data *dd = hctx->queue->queuedata; - struct mtip_int_cmd *icmd = rq->special; struct mtip_cmd *cmd = blk_mq_rq_to_pdu(rq); + struct mtip_int_cmd *icmd = cmd->icmd; struct mtip_cmd_hdr *hdr = dd->port->command_list + sizeof(struct mtip_cmd_hdr) * rq->tag; struct mtip_cmd_sg *command_sg; diff --git a/drivers/block/mtip32xx/mtip32xx.h b/drivers/block/mtip32xx/mtip32xx.h index 63414928f07c..c33f8c3d9fb4 100644 --- a/drivers/block/mtip32xx/mtip32xx.h +++ b/drivers/block/mtip32xx/mtip32xx.h @@ -321,6 +321,8 @@ struct mtip_cmd_sg { }; struct mtip_port; +struct mtip_int_cmd; + /* Structure used to describe a command. */ struct mtip_cmd { void *command; /* ptr to command table entry */ @@ -331,7 +333,10 @@ struct mtip_cmd { int unaligned; /* command is unaligned on 4k boundary */ - struct scatterlist sg[MTIP_MAX_SG]; /* Scatter list entries */ + union { + struct scatterlist sg[MTIP_MAX_SG]; /* Scatter list entries */ + struct mtip_int_cmd *icmd; + }; int retries; /* The number of retries left for this command. */ -- 2.19.1
[PATCH 4/9] mtip32xx: remove __force_bit2int
There is no good excuse not to use proper __le16/32 types. Signed-off-by: Christoph Hellwig --- drivers/block/mtip32xx/mtip32xx.c | 33 --- drivers/block/mtip32xx/mtip32xx.h | 28 -- 2 files changed, 26 insertions(+), 35 deletions(-) diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c index d5277bba0c02..9245c325fe3c 100644 --- a/drivers/block/mtip32xx/mtip32xx.c +++ b/drivers/block/mtip32xx/mtip32xx.c @@ -181,9 +181,9 @@ static void mtip_init_cmd_header(struct request *rq) (sizeof(struct mtip_cmd_hdr) * rq->tag); if (test_bit(MTIP_PF_HOST_CAP_64, >port->flags)) - cmd->command_header->ctbau = __force_bit2int cpu_to_le32((cmd->command_dma >> 16) >> 16); + cmd->command_header->ctbau = cpu_to_le32((cmd->command_dma >> 16) >> 16); - cmd->command_header->ctba = __force_bit2int cpu_to_le32(cmd->command_dma & 0x); + cmd->command_header->ctba = cpu_to_le32(cmd->command_dma & 0x); } static struct mtip_cmd *mtip_get_int_command(struct driver_data *dd) @@ -1459,8 +1459,8 @@ static blk_status_t mtip_send_trim(struct driver_data *dd, unsigned int lba, tlen = (sect_left >= MTIP_MAX_TRIM_ENTRY_LEN ? MTIP_MAX_TRIM_ENTRY_LEN : sect_left); - buf[i].lba = __force_bit2int cpu_to_le32(tlba); - buf[i].range = __force_bit2int cpu_to_le16(tlen); + buf[i].lba = cpu_to_le32(tlba); + buf[i].range = cpu_to_le16(tlen); tlba += tlen; sect_left -= tlen; } @@ -1590,11 +1590,9 @@ static inline void fill_command_sg(struct driver_data *dd, if (dma_len > 0x40) dev_err(>pdev->dev, "DMA segment length truncated\n"); - command_sg->info = __force_bit2int - cpu_to_le32((dma_len-1) & 0x3F); - command_sg->dba = __force_bit2int - cpu_to_le32(sg_dma_address(sg)); - command_sg->dba_upper = __force_bit2int + command_sg->info = cpu_to_le32((dma_len-1) & 0x3F); + command_sg->dba = cpu_to_le32(sg_dma_address(sg)); + command_sg->dba_upper = cpu_to_le32((sg_dma_address(sg) >> 16) >> 16); command_sg++; sg++; @@ -2231,8 +2229,7 @@ static void mtip_hw_submit_io(struct driver_data *dd, struct request *rq, /* Populate the command header */ command->command_header->opts = - __force_bit2int cpu_to_le32( - (nents << 16) | 5 | AHCI_CMD_PREFETCH); + cpu_to_le32((nents << 16) | 5 | AHCI_CMD_PREFETCH); command->command_header->byte_count = 0; command->direction = dma_dir; @@ -3586,20 +3583,16 @@ static blk_status_t mtip_issue_reserved_cmd(struct blk_mq_hw_ctx *hctx, return BLK_STS_RESOURCE; /* Populate the SG list */ - cmd->command_header->opts = -__force_bit2int cpu_to_le32(icmd->opts | icmd->fis_len); + cmd->command_header->opts = cpu_to_le32(icmd->opts | icmd->fis_len); if (icmd->buf_len) { command_sg = cmd->command + AHCI_CMD_TBL_HDR_SZ; - command_sg->info = - __force_bit2int cpu_to_le32((icmd->buf_len-1) & 0x3F); - command_sg->dba = - __force_bit2int cpu_to_le32(icmd->buffer & 0x); + command_sg->info = cpu_to_le32((icmd->buf_len-1) & 0x3F); + command_sg->dba = cpu_to_le32(icmd->buffer & 0x); command_sg->dba_upper = - __force_bit2int cpu_to_le32((icmd->buffer >> 16) >> 16); + cpu_to_le32((icmd->buffer >> 16) >> 16); - cmd->command_header->opts |= - __force_bit2int cpu_to_le32((1 << 16)); + cmd->command_header->opts |= cpu_to_le32((1 << 16)); } /* Populate the command header */ diff --git a/drivers/block/mtip32xx/mtip32xx.h b/drivers/block/mtip32xx/mtip32xx.h index e20e55dab443..0aa1ea210822 100644 --- a/drivers/block/mtip32xx/mtip32xx.h +++ b/drivers/block/mtip32xx/mtip32xx.h @@ -126,8 +126,6 @@ #define MTIP_DFS_MAX_BUF_SIZE 1024 -#define __force_bit2int (unsigned int __force) - enum { /* below are bit numbers in 'flags' defined in mtip_port */ MTIP_PF_IC_ACTIVE_BIT = 0, /* pio/ioctl */ @@ -200,9 +198,9 @@ struct mtip_work { #define MTIP_MAX_TRIM_ENTRY_LEN0xfff8 struct mtip_trim_entry { - u32 lba; /* starting lba of region */ - u16 rsvd; /* unused */ - u16 range; /* # of 512b blocks to trim */ + __le32
mtip32xx: fixes and cleanups
Various low hanging fruit, kicked of by seeing one of the few remaining req->special users. Compile tested only.
[PATCH 3/9] mtip32xx: return a blk_status_t from mtip_send_trim
This allows for better error propagation and simpler code. Signed-off-by: Christoph Hellwig --- drivers/block/mtip32xx/mtip32xx.c | 30 +++--- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c index 75a4ca0210f2..d5277bba0c02 100644 --- a/drivers/block/mtip32xx/mtip32xx.c +++ b/drivers/block/mtip32xx/mtip32xx.c @@ -1423,23 +1423,19 @@ static int mtip_get_smart_attr(struct mtip_port *port, unsigned int id, * @dd pointer to driver_data structure * @lbastarting lba * @len# of 512b sectors to trim - * - * return value - * -ENOMEMOut of dma memory - * -EINVALInvalid parameters passed in, trim not supported - * -EIO Error submitting trim request to hw */ -static int mtip_send_trim(struct driver_data *dd, unsigned int lba, - unsigned int len) +static blk_status_t mtip_send_trim(struct driver_data *dd, unsigned int lba, + unsigned int len) { - int i, rv = 0; u64 tlba, tlen, sect_left; struct mtip_trim_entry *buf; dma_addr_t dma_addr; struct host_to_dev_fis fis; + blk_status_t ret = BLK_STS_OK; + int i; if (!len || dd->trim_supp == false) - return -EINVAL; + return BLK_STS_IOERR; /* Trim request too big */ WARN_ON(len > (MTIP_MAX_TRIM_ENTRY_LEN * MTIP_MAX_TRIM_ENTRIES)); @@ -1454,7 +1450,7 @@ static int mtip_send_trim(struct driver_data *dd, unsigned int lba, buf = dmam_alloc_coherent(>pdev->dev, ATA_SECT_SIZE, _addr, GFP_KERNEL); if (!buf) - return -ENOMEM; + return BLK_STS_RESOURCE; memset(buf, 0, ATA_SECT_SIZE); for (i = 0, sect_left = len, tlba = lba; @@ -1486,10 +1482,10 @@ static int mtip_send_trim(struct driver_data *dd, unsigned int lba, ATA_SECT_SIZE, 0, MTIP_TRIM_TIMEOUT_MS) < 0) - rv = -EIO; + ret = BLK_STS_IOERR; dmam_free_coherent(>pdev->dev, ATA_SECT_SIZE, buf, dma_addr); - return rv; + return ret; } /* @@ -3634,13 +3630,9 @@ static blk_status_t mtip_queue_rq(struct blk_mq_hw_ctx *hctx, blk_mq_start_request(rq); - if (req_op(rq) == REQ_OP_DISCARD) { - if (mtip_send_trim(dd, blk_rq_pos(rq), blk_rq_sectors(rq)) < 0) - return BLK_STS_IOERR; - } else { - mtip_hw_submit_io(dd, rq, cmd, hctx); - } - + if (req_op(rq) == REQ_OP_DISCARD) + return mtip_send_trim(dd, blk_rq_pos(rq), blk_rq_sectors(rq)); + mtip_hw_submit_io(dd, rq, cmd, hctx); return BLK_STS_OK; } -- 2.19.1
Re: [GIT PULL] nvme fixes for 4.20
On 11/9/18 6:20 AM, Christoph Hellwig wrote: > On Fri, Nov 09, 2018 at 06:16:09AM -0700, Jens Axboe wrote: >> On 11/8/18 11:56 PM, Christoph Hellwig wrote: >>> - revert an RDMA commit that didn't help but caused problems >>> - fix another minor P2P fallout >>> - make sure the multipath device inherits the devices limits of >>>the controllers below >>> >>> The following changes since commit 651022382c7f8da46cb4872a545ee1da6d097d2a: >>> >>> Linux 4.20-rc1 (2018-11-04 15:37:52 -0800) >>> >>> are available in the Git repository at: >>> >>> git://git.infradead.org/nvme.git nvme-4.20 >> >> This is based on 4.20-rc1 for some reason, not my for-linus. So I get >> a few thousand extra commits... I pulled the patches and applied >> manually. > > That is because your for-linus was still 4.20-rc based when > I created it. It's never been 4.20-rc based. After the initial merge, it got based on a random commit in the merge window: commit 3acbd2de6bc3af215c6ed7732dfc097d1e238503 Merge: d49f8a52b15b de7d83da84bd Author: Linus Torvalds Date: Thu Oct 25 09:00:15 2018 -0700 Merge tag 'sound-4.20-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound and has been ever since. -- Jens Axboe
Re: [PATCH V2 0/3] block: make sure discard/writesame bio is aligned with logical block size
On 11/6/18 5:55 PM, Ming Lei wrote: > On Tue, Nov 06, 2018 at 04:48:13PM +, Rui Salvaterra wrote: >> On Mon, 5 Nov 2018 at 03:41, Ming Lei wrote: >>> >>> V2 addresses Christoph's comment by introducing bio_allowed_max_sectors(). >>> >>> Ping... >>> >>> Thanks, >>> Ming >> >> Hi, Ming, >> >> Sorry for the delay. I tested your V2 against Linux 4.20-rc1 and >> everything seems fine. FWIW, V2 is also >> >> Tested-by: Rui Salvaterra > > Rui, thanks for your test on V2. > > Jens, what do you think about this patchset? I waffled on this a bit... I generally don't like cleanup patches for fixes that are going into the current series. Those kinds of patches should be as short and sweet as possible, and cleanup part usually ends up being the problematic one, ironically enough. But it looks fine to me, so I've put it in. -- Jens Axboe
Re: [GIT PULL] nvme fixes for 4.20
On Fri, Nov 09, 2018 at 06:16:09AM -0700, Jens Axboe wrote: > On 11/8/18 11:56 PM, Christoph Hellwig wrote: > > - revert an RDMA commit that didn't help but caused problems > > - fix another minor P2P fallout > > - make sure the multipath device inherits the devices limits of > >the controllers below > > > > The following changes since commit 651022382c7f8da46cb4872a545ee1da6d097d2a: > > > > Linux 4.20-rc1 (2018-11-04 15:37:52 -0800) > > > > are available in the Git repository at: > > > > git://git.infradead.org/nvme.git nvme-4.20 > > This is based on 4.20-rc1 for some reason, not my for-linus. So I get > a few thousand extra commits... I pulled the patches and applied > manually. That is because your for-linus was still 4.20-rc based when I created it.
Re: [GIT PULL] nvme fixes for 4.20
On 11/8/18 11:56 PM, Christoph Hellwig wrote: > - revert an RDMA commit that didn't help but caused problems > - fix another minor P2P fallout > - make sure the multipath device inherits the devices limits of >the controllers below > > The following changes since commit 651022382c7f8da46cb4872a545ee1da6d097d2a: > > Linux 4.20-rc1 (2018-11-04 15:37:52 -0800) > > are available in the Git repository at: > > git://git.infradead.org/nvme.git nvme-4.20 This is based on 4.20-rc1 for some reason, not my for-linus. So I get a few thousand extra commits... I pulled the patches and applied manually. -- Jens Axboe