always handle REQ_BLOCK_PC requests in common code
Subsystems (2)
PATCH 1/1 always handle REQ_BLOCK_PC requests in common code
LLDDs should never see REQ_BLOCK_PC requests, we can handle them just fine in the core code. There is a small behaviour change in that some check in sr's rw_intr are bypassed, but I consider the old behaviour a bug.
Mike found this cleanup ooportunity and provdided early patches, so all the credit goes to him, even if I redid the patches from scratch beause that was easier than forward-porting the old patches.
drivers/scsi/scsi_lib.c +8 -8
@@ -1247,7 +1247,7 @@return -EOPNOTSUPP ; } -static void scsi_generic_done (struct scsi_cmnd *cmd) +static void scsi_blk_pc_done (struct scsi_cmnd *cmd) {BUG_ON (!blk_pc_request (cmd->request));/*
@@ -1259,7 +1259,7 @@scsi_io_completion (cmd, cmd->bufflen,0 ); } -void scsi_setup_blk_pc_cmnd (struct scsi_cmnd *cmd) +static void scsi_setup_blk_pc_cmnd (struct scsi_cmnd *cmd) {struct request *req = cmd->request;
@@ -1276,8 +1276,8 @@ cmd->transfersize = req->data_len; cmd->allowed = req->retries; cmd->timeout_per_command = req->timeout; + cmd->done = scsi_blk_pc_done; } -EXPORT_SYMBOL_GPL (scsi_setup_blk_pc_cmnd);static int scsi_prep_fn (struct request_queue *q,struct request *req) {
@@ -1374,7 +1374,6 @@* happening now. */ if (req->flags & (REQ_CMD |REQ_BLOCK_PC )) { -struct scsi_driver *drv;int ret;/*
@@ -1406,16 +1405,17 @@/* * Initialize the actual SCSI command for this request. */ -if (req->rq_disk) { +if (req->flags &REQ_BLOCK_PC ) { +scsi_setup_blk_pc_cmnd (cmd); + }else if (req->rq_disk) { +struct scsi_driver *drv; + drv = *(struct scsi_driver **)req->rq_disk->private_data;if (unlikely (!drv->init_command (cmd))) {scsi_release_buffers (cmd);scsi_put_command (cmd); goto kill; } - }else { -scsi_setup_blk_pc_cmnd (cmd); - cmd->done = scsi_generic_done; } }
drivers/scsi/sd.c +5 -37
62 lines not on this page · raw message
drivers/scsi/sr.c +0 -21
@@ -237,8 +237,6 @@case ILLEGAL_REQUEST :if (!(SCpnt->sense_buffer[0 ] &0x90 ))break ; -if (!blk_fs_request (SCpnt->request)) -break ; error_sector = (SCpnt->sense_buffer[3 ] <<24 ) | (SCpnt->sense_buffer[4 ] <<16 ) | (SCpnt->sense_buffer[5] << 8) |
@@ -317,23 +315,6 @@ }/* -* these are already setup, just copy cdb basically -*/ -if (SCpnt->request->flags &REQ_BLOCK_PC ) { -scsi_setup_blk_pc_cmnd (SCpnt); - -if (SCpnt->timeout_per_command) - timeout = SCpnt->timeout_per_command; - - goto queue; - } - -if (!(SCpnt->request->flags &REQ_CMD )) { -blk_dump_rq_flags (SCpnt->request,"sr unsup command" ); -return 0 ; - } - -/* * we do lazy blocksize switching (when reading XA sectors, * see CDROMREADMODE2 ioctl) */
@@ -421,8 +402,6 @@*/ SCpnt->transfersize = cd->device->sector_size; SCpnt->underflow = this_count <<9 ; - -queue: SCpnt->allowed =MAX_RETRIES ; SCpnt->timeout_per_command = timeout;
drivers/scsi/st.c +0 -25
@@ -193,7 +193,6 @@static int st_probe (struct device *);static int st_remove (struct device *); -static int st_init_command (struct scsi_cmnd *);static void do_create_driverfs_files (void );static void do_remove_driverfs_files (void );
@@ -206,7 +205,6 @@ .probe = st_probe, .remove = st_remove, }, - .init_command = st_init_command, };static int st_compression (struct scsi_tape *,int );
@@ -4180,29 +4178,6 @@return ; } -static void st_intr (struct scsi_cmnd *SCpnt) -{ -/* -* The caller should be checking the request's errors -* value. -*/ -scsi_io_completion (SCpnt, SCpnt->bufflen,0 ); -} - -/* -* st_init_command: only called via the scsi_cmd_ioctl (block SG_IO) -* interface for REQ_BLOCK_PC commands. -*/ -static int st_init_command (struct scsi_cmnd *SCpnt) -{ -if (!(SCpnt->request->flags &REQ_BLOCK_PC )) -return 0 ; - -scsi_setup_blk_pc_cmnd (SCpnt); - SCpnt->done = st_intr; -return 1 ; -} -static int __init init_st (void ) {validate_options ();
include/scsi/scsi_cmnd.h +0 -1
@@ -151,6 +151,5 @@extern void scsi_put_command (struct scsi_cmnd *);extern void scsi_io_completion (struct scsi_cmnd *,unsigned int ,unsigned int );extern void scsi_finish_command (struct scsi_cmnd *cmd); -extern void scsi_setup_blk_pc_cmnd (struct scsi_cmnd *cmd);#endif /* _SCSI_SCSI_CMND_H */
Quoted 8 lines · Christoph Hellwig
LLDDs should never see
REQ_BLOCK_PCrequests, we can handle them just fine in the core code. There is a small behaviour change in that some check in sr'srw_intrare bypassed, but I consider the old behaviour a bug.Mike found this cleanup ooportunity and provdided early patches, so all the credit goes to him, even if I redid the patches from scratch beause that was easier than forward-porting the old patches.
The point of the hacks in the sd driver was to make sure SG_IO ioctl commands issued via a sd device node would not get caught up in the sd driver's error processing (e.g. retries on MEDIUM ERRORs). Is there a clean path back for such errors to the SG_IO ioctl and hence the user space?
The same applies to most of the error processing done in the midlayer, with TASK SET FULL being a bit of a grey area.
Doug Gilbert
Quoted 10 lines · Douglas Gilbert
Quoted 8 lines · Christoph Hellwig
LLDDs should never see
REQ_BLOCK_PCrequests, we can handle them just fine in the core code. There is a small behaviour change in that some check in sr'srw_intrare bypassed, but I consider the old behaviour a bug.Mike found this cleanup ooportunity and provdided early patches, so all the credit goes to him, even if I redid the patches from scratch beause that was easier than forward-porting the old patches.
Quoted 6 lines
The point of the hacks in the sd driver was to make sure
SG_IOioctl commands issued via a sd device node would not get caught up in the sd driver's error processing (e.g. retries on MEDIUM ERRORs). Is there a clean path back for such errors to theSG_IOioctl and hence the user space?
Yeah. scsi_io_completion deals specially with REQ_BLOCK_PC requests, so we don't do these retries on MEDIUM ERRORs. sg even before my patch directly handed off these requests to scsi_io_completion, and didn't do any own handling of errors for them. There's zero change in behaviour for sd and st. Only sr did some error processesing previously which was wrong.