remove most req->special users v2
Subsystems (4)
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.
Changes since v1:
- fix the retries counter in skd to actually work.
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.
--- drivers/scsi/fnic/fnic_scsi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/scsi/fnic/fnic_scsi.c +2 -2
@@ -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); }
Signature · 1 line
2.19.1
PATCH 2/6 nullb: remove leftover legacy request code
In reply to Christoph Hellwignull_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.
--- drivers/block/null_blk_main.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
drivers/block/null_blk_main.c +3 -6
@@ -642,14 +642,11 @@ static void null_cmd_end_timer(struct nullb_cmd *cmd)hrtimer_start (&cmd->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, };
Signature · 1 line
2.19.1
PATCH 3/6 skd_main: don't use req->special
In reply to Christoph HellwigAdd a retries field to the internal request structure instead, which gets set to zero on the first submission.
--- drivers/block/skd_main.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/block/skd_main.c +7 -1
@@ -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 (++skreq->retries <SKD_MAX_RETRIES ) {skd_log_skreq (skdev, skreq,"retry" );blk_mq_requeue_request (req, true);break ;
Signature · 1 line
2.19.1
PATCH 4/6 aoe: replace ->special use with private data in the request
In reply to Christoph HellwigMakes the code a whole lot better to read..
--- 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(-)
drivers/block/aoe/aoe.h +4 -0
@@ -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;
drivers/block/aoe/aoeblk.c +1 -0
@@ -387,6 +387,7 @@ aoeblk_gdalloc(void *vp) set = &d->tag_set; set->ops = &aoeblk_mq_ops; + set->cmd_size =sizeof (struct aoe_req ); set->nr_hw_queues =1 ; set->queue_depth =128 ; set->numa_node =NUMA_NO_NODE ;
drivers/block/aoe/aoecmd.c +9 -18
@@ -822,17 +822,6 @@ rexmit_timer(struct timer_list *timer)spin_unlock_irqrestore (&d->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 ); }
drivers/block/aoe/aoedev.c +6 -5
@@ -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 ); }
Signature · 1 line
2.19.1
PATCH 5/6 pd: replace ->special use with private data in the request
In reply to Christoph Hellwig--- drivers/block/paride/pd.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-)
drivers/block/paride/pd.c +25 -5
@@ -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 (&disk->tag_set, &pd_mq_ops,2 , -BLK_MQ_F_SHOULD_MERGE |BLK_MQ_F_BLOCKING ); +memset (&disk->tag_set,0 ,sizeof (disk->tag_set)); + disk->tag_set.ops = &pd_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 (&disk->tag_set)) +return ; + + p->queue =blk_mq_init_queue (&disk->tag_set);if (IS_ERR (p->queue)) { +blk_mq_free_tag_set (&disk->tag_set); p->queue =NULL ;return ; }
Signature · 1 line
2.19.1
PATCH 6/6 ide: don't use req->special
In reply to Christoph HellwigJust replace it with a field of the same name in struct ide_req.
--- 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(-)
drivers/ide/ide-atapi.c +2 -2
@@ -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 ;
drivers/ide/ide-cd.c +2 -2
@@ -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) {
drivers/ide/ide-devsets.c +2 -2
@@ -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 *)&scsi_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 *)&scsi_req (rq)->cmd[1 ]);if (err)
drivers/ide/ide-disk.c +3 -3
@@ -434,8 +434,8 @@ static bool idedisk_prep_rq(ide_drive_t *drive, struct request *rq)if (req_op (rq) !=REQ_OP_FLUSH )return true; -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 bool idedisk_prep_rq(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 true;
drivers/ide/ide-eh.c +1 -1
@@ -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);
drivers/ide/ide-floppy.c +1 -1
@@ -276,7 +276,7 @@ static ide_startstop_t ide_floppy_do_request(ide_drive_t *drive,switch (ide_req (rq)->type) {case ATA_PRIV_MISC :case ATA_PRIV_SENSE : - pc = (struct ide_atapi_pc *)rq->special; + pc = (struct ide_atapi_pc *)ide_req (rq)->special;break ;default :BUG ();
drivers/ide/ide-io.c +9 -5
@@ -111,7 +111,7 @@ void ide_complete_cmd(ide_drive_t *drive, struct ide_cmd *cmd, u8 stat, u8 err) }if (rq &&ata_taskfile_request (rq)) { -struct ide_cmd *orig_cmd = rq->special; +struct ide_cmd *orig_cmd =ide_req (rq)->special;if (cmd->tf_flags &IDE_TFLAG_DYN )kfree (orig_cmd);
@@ -261,7 +261,7 @@ EXPORT_SYMBOL_GPL(ide_init_sg_cmd);static ide_startstop_t execute_drive_cmd (ide_drive_t *drive,struct request *rq) { -struct ide_cmd *cmd = rq->special; +struct ide_cmd *cmd =ide_req (rq)->special;if (cmd) {if (cmd->protocol ==ATA_PROT_PIO ) {
@@ -352,7 +352,7 @@ static ide_startstop_t start_request (ide_drive_t *drive, struct request *rq)if (ata_taskfile_request (rq))return execute_drive_cmd (drive, rq);else if (ata_pm_request (rq)) { -struct ide_pm_state *pm = rq->special; +struct ide_pm_state *pm =ide_req (rq)->special;#ifdef DEBUG_PM printk ("%s: start_power_step(step: %d)\n" , drive->name, pm->pm_step);
@@ -460,16 +460,20 @@ blk_status_t ide_queue_rq(struct blk_mq_hw_ctx *hctx,ide_drive_t *drive = hctx->queue->queuedata;ide_hwif_t *hwif = drive->hwif;struct ide_host *host = hwif->host; -struct request *rq =NULL ; +struct request *rq = bd->rq;ide_startstop_t startstop; +if (!(rq->rq_flags &RQF_DONTPREP )) { + rq->rq_flags |=RQF_DONTPREP ; +ide_req (rq)->special =NULL ; + } +/* HLD do_request() callback might sleep, make sure it's okay */ might_sleep ();if (ide_lock_host (host, hwif))return BLK_STS_DEV_RESOURCE ; - rq = bd->rq;blk_mq_start_request (rq);spin_lock_irq (&hwif->lock);
drivers/ide/ide-park.c +2 -2
@@ -36,7 +36,7 @@ static void issue_park_cmd(ide_drive_t *drive, unsigned long timeout)scsi_req (rq)->cmd[0 ] =REQ_PARK_HEADS ;scsi_req (rq)->cmd_len =1 ;ide_req (rq)->type =ATA_PRIV_MISC ; - rq->special = &timeout; +ide_req (rq)->special = &timeout;blk_execute_rq (q,NULL , rq,1 ); rc =scsi_req (rq)->result ? -EIO :0 ;blk_put_request (rq);
@@ -67,7 +67,7 @@ ide_startstop_t ide_do_park_unpark(ide_drive_t *drive, struct request *rq)memset (&cmd,0 ,sizeof (cmd));if (scsi_req (rq)->cmd[0 ] ==REQ_PARK_HEADS ) { - drive->sleep = *(unsigned long *)rq->special; + drive->sleep = *(unsigned long *)ide_req (rq)->special; drive->dev_flags |=IDE_DFLAG_SLEEPING ; tf->command =ATA_CMD_IDLEIMMEDIATE ; tf->feature =0x44 ;
drivers/ide/ide-pm.c +6 -6
@@ -21,7 +21,7 @@ int generic_ide_suspend(struct device *dev, pm_message_t mesg)memset (&rqpm,0 ,sizeof (rqpm)); rq =blk_get_request (drive->queue,REQ_OP_DRV_IN ,0 );ide_req (rq)->type =ATA_PRIV_PM_SUSPEND ; - rq->special = &rqpm; +ide_req (rq)->special = &rqpm; rqpm.pm_step =IDE_PM_START_SUSPEND ;if (mesg.event ==PM_EVENT_PRETHAW ) mesg.event =PM_EVENT_FREEZE ;
@@ -82,7 +82,7 @@ int generic_ide_resume(struct device *dev)memset (&rqpm,0 ,sizeof (rqpm)); rq =blk_get_request (drive->queue,REQ_OP_DRV_IN ,BLK_MQ_REQ_PREEMPT );ide_req (rq)->type =ATA_PRIV_PM_RESUME ; - rq->special = &rqpm; +ide_req (rq)->special = &rqpm; rqpm.pm_step =IDE_PM_START_RESUME ; rqpm.pm_state =PM_EVENT_ON ;
@@ -101,7 +101,7 @@ int generic_ide_resume(struct device *dev)void ide_complete_power_step (ide_drive_t *drive,struct request *rq) { -struct ide_pm_state *pm = rq->special; +struct ide_pm_state *pm =ide_req (rq)->special;#ifdef DEBUG_PM printk(KERN_INFO "%s: complete_power_step(step: %d)\n",
@@ -131,7 +131,7 @@ void ide_complete_power_step(ide_drive_t *drive, struct request *rq)ide_startstop_t ide_start_power_step (ide_drive_t *drive,struct request *rq) { -struct ide_pm_state *pm = rq->special; +struct ide_pm_state *pm =ide_req (rq)->special;struct ide_cmd cmd = { };switch (pm->pm_step) {
@@ -203,7 +203,7 @@ ide_startstop_t ide_start_power_step(ide_drive_t *drive, struct request *rq)void ide_complete_pm_rq (ide_drive_t *drive,struct request *rq) {struct request_queue *q = drive->queue; -struct ide_pm_state *pm = rq->special; +struct ide_pm_state *pm =ide_req (rq)->special;unsigned long flags;ide_complete_power_step (drive, rq);
@@ -228,7 +228,7 @@ void ide_complete_pm_rq(ide_drive_t *drive, struct request *rq)void ide_check_pm_state (ide_drive_t *drive,struct request *rq) { -struct ide_pm_state *pm = rq->special; +struct ide_pm_state *pm =ide_req (rq)->special; if (blk_rq_is_private(rq) && ide_req(rq)->type == ATA_PRIV_PM_SUSPEND &&
drivers/ide/ide-tape.c +1 -1
@@ -639,7 +639,7 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive, goto out; }if (req->cmd[13 ] &REQ_IDETAPE_PC1 ) { - pc = (struct ide_atapi_pc *)rq->special; + pc = (struct ide_atapi_pc *)ide_req (rq)->special; req->cmd[13 ] &= ~(REQ_IDETAPE_PC1 ); req->cmd[13 ] |=REQ_IDETAPE_PC2 ; goto out;
drivers/ide/ide-taskfile.c +1 -1
@@ -440,7 +440,7 @@ int ide_raw_taskfile(ide_drive_t *drive, struct ide_cmd *cmd, u8 *buf, goto put_req; } - rq->special = cmd; +ide_req (rq)->special = cmd; cmd->rq = rq;blk_execute_rq (drive->queue,NULL , rq,0 );
include/linux/ide.h +1 -0
@@ -50,6 +50,7 @@ struct ide_request {struct scsi_request sreq;u8 sense[SCSI_SENSE_BUFFERSIZE ];u8 type; +void *special; };static inline struct ide_request *ide_req (struct request *rq)
Signature · 1 line
2.19.1
Quoted 7 lines · Christoph Hellwig
Try to get rid of the
req->specialusers 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.
Applied, thanks.
Signature · 1 line
Jens Axboe