On 02/14/12 11:34, Jun'ichi Nomura wrote:
> While scsi_device is propery refcounted object,
> q->queuedata is set to NULL by scsi_remove_device() asynchronously.
> So every reader of scsi_device's q->queuedata should always check it.
>
> Though I don't have a machine to actually test it,
> the following patch should remove such places.
>
> Index: linux-3.3/drivers/scsi/scsi_lib.c
> ===================================================================
> --- linux-3.3.orig/drivers/scsi/scsi_lib.c 2012-02-01 06:31:54.000000000 +0900
> +++ linux-3.3/drivers/scsi/scsi_lib.c 2012-02-14 19:12:57.641216402 +0900
> @@ -1214,10 +1214,8 @@ int scsi_prep_state_check(struct scsi_de
> }
> EXPORT_SYMBOL(scsi_prep_state_check);
>
> -int scsi_prep_return(struct request_queue *q, struct request *req, int ret)
> +int scsi_prep_return(struct request_queue *q, struct scsi_device *sdev, struct request *req, int ret)
> {
> - struct scsi_device *sdev = q->queuedata;
> -
> switch (ret) {
> case BLKPREP_KILL:
> req->errors = DID_NO_CONNECT << 16;
> @@ -1251,9 +1249,11 @@ int scsi_prep_fn(struct request_queue *q
> struct scsi_device *sdev = q->queuedata;
> int ret = BLKPREP_KILL;
>
> + if (!sdev)
> + return ret;
> if (req->cmd_type == REQ_TYPE_BLOCK_PC)
> ret = scsi_setup_blk_pc_cmnd(sdev, req);
> - return scsi_prep_return(q, req, ret);
> + return scsi_prep_return(q, sdev, req, ret);
> }
> EXPORT_SYMBOL(scsi_prep_fn);
>
> Index: linux-3.3/drivers/scsi/sd.c
> ===================================================================
> --- linux-3.3.orig/drivers/scsi/sd.c 2012-02-13 13:02:14.000000000 +0900
> +++ linux-3.3/drivers/scsi/sd.c 2012-02-14 19:15:18.224212107 +0900
> @@ -653,6 +653,9 @@ static int sd_prep_fn(struct request_que
> int ret, host_dif;
> unsigned char protect;
>
> + if (!sdp)
> + return BLKPREP_KILL;
> +
> /*
> * Discard request come in as REQ_TYPE_FS but we turn them into
> * block PC requests to make life easier.
> @@ -905,7 +908,7 @@ static int sd_prep_fn(struct request_que
> */
> ret = BLKPREP_OK;
> out:
> - return scsi_prep_return(q, rq, ret);
> + return scsi_prep_return(q, sdp, rq, ret);
> }
>
> /**
> Index: linux-3.3/drivers/scsi/sr.c
> ===================================================================
> --- linux-3.3.orig/drivers/scsi/sr.c 2012-02-01 06:31:54.000000000 +0900
> +++ linux-3.3/drivers/scsi/sr.c 2012-02-14 19:15:59.001211143 +0900
> @@ -372,6 +372,9 @@ static int sr_prep_fn(struct request_que
> struct scsi_device *sdp = q->queuedata;
> int ret;
>
> + if (!sdp)
> + return BLKPREP_KILL;
> +
> if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
> ret = scsi_setup_blk_pc_cmnd(sdp, rq);
> goto out;
> @@ -503,7 +506,7 @@ static int sr_prep_fn(struct request_que
> */
> ret = BLKPREP_OK;
> out:
> - return scsi_prep_return(q, rq, ret);
> + return scsi_prep_return(q, sdp, rq, ret);
> }
>
> static int sr_block_open(struct block_device *bdev, fmode_t mode)
> Index: linux-3.3/include/scsi/scsi_driver.h
> ===================================================================
> --- linux-3.3.orig/include/scsi/scsi_driver.h 2012-02-01 06:31:54.000000000 +0900
> +++ linux-3.3/include/scsi/scsi_driver.h 2012-02-14 19:16:47.478209843 +0900
> @@ -31,7 +31,7 @@ extern int scsi_register_interface(struc
> int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req);
> int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct request *req);
> int scsi_prep_state_check(struct scsi_device *sdev, struct request *req);
> -int scsi_prep_return(struct request_queue *q, struct request *req, int ret);
> +int scsi_prep_return(struct request_queue *q, struct scsi_device *sdev, struct request *req, int ret);
> int scsi_prep_fn(struct request_queue *, struct request *);
>
> #endif /* _SCSI_SCSI_DRIVER_H */
Now that I've had some more time to think about this: has anyone
considered to hold a reference on the SCSI host instead of the SCSI
device as long as sd_probe_async() is active ? If sd_prep_fn() can ever
see a NULL queuedata pointer then that means that
scsi_host_dev_release() can get invoked while sd_prep_fn() is running.
That doesn't look correct to me.
Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
