2021-10-19 07:55:03

by Christoph Hellwig

[permalink] [raw]
Subject: remove QUEUE_FLAG_SCSI_PASSTHROUGH v2

Hi all,

this series removes the QUEUE_FLAG_SCSI_PASSTHROUGH and thus the last
remaining SCSI passthrough concept from the block layer.

The changes to support pktcdvd are a bit ugly, but I can't think of
anything better (except for removing the driver entirely).
If we'd want to support packet writing today it would probably live
entirely inside the sr driver.

Changes since v1:
- use an extra local variable in sd_get_unique_id to make sure we
always return the right length
- add an enum and a comment to better document ->get_unique_id
- spelling fixes

Diffstat:
block/blk-core.c | 9 --
block/blk-mq-debugfs.c | 1
block/bsg-lib.c | 32 +++----
drivers/block/Kconfig | 2
drivers/block/pktcdvd.c | 7 +
drivers/scsi/scsi_bsg.c | 4
drivers/scsi/scsi_error.c | 2
drivers/scsi/scsi_ioctl.c | 4
drivers/scsi/scsi_lib.c | 27 ++++--
drivers/scsi/scsi_scan.c | 1
drivers/scsi/sd.c | 39 +++++++++
drivers/scsi/sg.c | 4
drivers/scsi/sr.c | 2
drivers/scsi/st.c | 2
drivers/target/target_core_pscsi.c | 3
fs/nfsd/Kconfig | 1
fs/nfsd/blocklayout.c | 158 +++++++++----------------------------
fs/nfsd/nfs4layouts.c | 5 -
include/linux/blk-mq.h | 5 -
include/linux/blkdev.h | 14 ++-
include/scsi/scsi_cmnd.h | 3
21 files changed, 148 insertions(+), 177 deletions(-)


2021-10-19 07:55:05

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 6/7] block: remove the initialize_rq_fn blk_mq_ops method

Entirely unused now.

Signed-off-by: Christoph Hellwig <[email protected]>
Reviewed-by: Hannes Reinecke <[email protected]>
---
block/blk-core.c | 9 +--------
include/linux/blk-mq.h | 5 -----
2 files changed, 1 insertion(+), 13 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index d0c2e11411d03..52a460d0aeb2a 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -606,16 +606,9 @@ EXPORT_SYMBOL(blk_get_queue);
struct request *blk_get_request(struct request_queue *q, unsigned int op,
blk_mq_req_flags_t flags)
{
- struct request *req;
-
WARN_ON_ONCE(op & REQ_NOWAIT);
WARN_ON_ONCE(flags & ~(BLK_MQ_REQ_NOWAIT | BLK_MQ_REQ_PM));
-
- req = blk_mq_alloc_request(q, op, flags);
- if (!IS_ERR(req) && q->mq_ops->initialize_rq_fn)
- q->mq_ops->initialize_rq_fn(req);
-
- return req;
+ return blk_mq_alloc_request(q, op, flags);
}
EXPORT_SYMBOL(blk_get_request);

diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 656fe34bdb6cd..649be3f21d740 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -565,11 +565,6 @@ struct blk_mq_ops {
void (*exit_request)(struct blk_mq_tag_set *set, struct request *,
unsigned int);

- /**
- * @initialize_rq_fn: Called from inside blk_get_request().
- */
- void (*initialize_rq_fn)(struct request *rq);
-
/**
* @cleanup_rq: Called before freeing one request which isn't completed
* yet, and usually for freeing the driver private data.
--
2.30.2

2021-10-19 07:55:09

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 5/7] scsi: add a scsi_alloc_request helper

Add a new helper that calls blk_get_request and initializes the
scsi_request to avoid the indirect call through ->.initialize_rq_fn.

Note that this makes the pktcdvd driver depend on the SCSI core, but
given that only SCSI devices support SCSI passthrough requests that
is not a functional change.

Signed-off-by: Christoph Hellwig <[email protected]>
Reviewed-by: Hannes Reinecke <[email protected]>
---
drivers/block/Kconfig | 2 +-
drivers/block/pktcdvd.c | 2 +-
drivers/scsi/scsi_bsg.c | 4 ++--
drivers/scsi/scsi_error.c | 2 +-
drivers/scsi/scsi_ioctl.c | 4 ++--
drivers/scsi/scsi_lib.c | 19 +++++++++++++------
drivers/scsi/sg.c | 4 ++--
drivers/scsi/sr.c | 2 +-
drivers/scsi/st.c | 2 +-
drivers/target/target_core_pscsi.c | 3 +--
include/scsi/scsi_cmnd.h | 3 +++
11 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index ab3e37aa1830c..9151e8ffba1cf 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -304,8 +304,8 @@ config BLK_DEV_RAM_SIZE
config CDROM_PKTCDVD
tristate "Packet writing on CD/DVD media (DEPRECATED)"
depends on !UML
+ depends on SCSI
select CDROM
- select SCSI_COMMON
help
Note: This driver is deprecated and will be removed from the
kernel in the near future!
diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index cb52cce6fb039..ea2262ec76d2c 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -703,7 +703,7 @@ static int pkt_generic_packet(struct pktcdvd_device *pd, struct packet_command *
struct request *rq;
int ret = 0;

- rq = blk_get_request(q, (cgc->data_direction == CGC_DATA_WRITE) ?
+ rq = scsi_alloc_request(q, (cgc->data_direction == CGC_DATA_WRITE) ?
REQ_OP_DRV_OUT : REQ_OP_DRV_IN, 0);
if (IS_ERR(rq))
return PTR_ERR(rq);
diff --git a/drivers/scsi/scsi_bsg.c b/drivers/scsi/scsi_bsg.c
index 81c3853a2a800..551727a6f6941 100644
--- a/drivers/scsi/scsi_bsg.c
+++ b/drivers/scsi/scsi_bsg.c
@@ -25,8 +25,8 @@ static int scsi_bsg_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,
return -EOPNOTSUPP;
}

- rq = blk_get_request(q, hdr->dout_xfer_len ?
- REQ_OP_DRV_OUT : REQ_OP_DRV_IN, 0);
+ rq = scsi_alloc_request(q, hdr->dout_xfer_len ?
+ REQ_OP_DRV_OUT : REQ_OP_DRV_IN, 0);
if (IS_ERR(rq))
return PTR_ERR(rq);
rq->timeout = timeout;
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index b6c86cce57bfa..71d027b94be40 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1998,7 +1998,7 @@ static void scsi_eh_lock_door(struct scsi_device *sdev)
struct request *req;
struct scsi_request *rq;

- req = blk_get_request(sdev->request_queue, REQ_OP_DRV_IN, 0);
+ req = scsi_alloc_request(sdev->request_queue, REQ_OP_DRV_IN, 0);
if (IS_ERR(req))
return;
rq = scsi_req(req);
diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
index 6ff2207bd45a0..0078975e3c07c 100644
--- a/drivers/scsi/scsi_ioctl.c
+++ b/drivers/scsi/scsi_ioctl.c
@@ -438,7 +438,7 @@ static int sg_io(struct scsi_device *sdev, struct gendisk *disk,
at_head = 1;

ret = -ENOMEM;
- rq = blk_get_request(sdev->request_queue, writing ?
+ rq = scsi_alloc_request(sdev->request_queue, writing ?
REQ_OP_DRV_OUT : REQ_OP_DRV_IN, 0);
if (IS_ERR(rq))
return PTR_ERR(rq);
@@ -561,7 +561,7 @@ static int sg_scsi_ioctl(struct request_queue *q, struct gendisk *disk,

}

- rq = blk_get_request(q, in_len ? REQ_OP_DRV_OUT : REQ_OP_DRV_IN, 0);
+ rq = scsi_alloc_request(q, in_len ? REQ_OP_DRV_OUT : REQ_OP_DRV_IN, 0);
if (IS_ERR(rq)) {
err = PTR_ERR(rq);
goto error_free_buffer;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 30f7d0b4eb732..a0f801fc8943b 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -216,7 +216,7 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
struct scsi_request *rq;
int ret;

- req = blk_get_request(sdev->request_queue,
+ req = scsi_alloc_request(sdev->request_queue,
data_direction == DMA_TO_DEVICE ?
REQ_OP_DRV_OUT : REQ_OP_DRV_IN,
rq_flags & RQF_PM ? BLK_MQ_REQ_PM : 0);
@@ -1079,9 +1079,6 @@ EXPORT_SYMBOL(scsi_alloc_sgtables);
* This function initializes the members of struct scsi_cmnd that must be
* initialized before request processing starts and that won't be
* reinitialized if a SCSI command is requeued.
- *
- * Called from inside blk_get_request() for pass-through requests and from
- * inside scsi_init_command() for filesystem requests.
*/
static void scsi_initialize_rq(struct request *rq)
{
@@ -1098,6 +1095,18 @@ static void scsi_initialize_rq(struct request *rq)
cmd->retries = 0;
}

+struct request *scsi_alloc_request(struct request_queue *q,
+ unsigned int op, blk_mq_req_flags_t flags)
+{
+ struct request *rq;
+
+ rq = blk_get_request(q, op, flags);
+ if (!IS_ERR(rq))
+ scsi_initialize_rq(rq);
+ return rq;
+}
+EXPORT_SYMBOL_GPL(scsi_alloc_request);
+
/*
* Only called when the request isn't completed by SCSI, and not freed by
* SCSI
@@ -1864,7 +1873,6 @@ static const struct blk_mq_ops scsi_mq_ops_no_commit = {
#endif
.init_request = scsi_mq_init_request,
.exit_request = scsi_mq_exit_request,
- .initialize_rq_fn = scsi_initialize_rq,
.cleanup_rq = scsi_cleanup_rq,
.busy = scsi_mq_lld_busy,
.map_queues = scsi_map_queues,
@@ -1894,7 +1902,6 @@ static const struct blk_mq_ops scsi_mq_ops = {
#endif
.init_request = scsi_mq_init_request,
.exit_request = scsi_mq_exit_request,
- .initialize_rq_fn = scsi_initialize_rq,
.cleanup_rq = scsi_cleanup_rq,
.busy = scsi_mq_lld_busy,
.map_queues = scsi_map_queues,
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 3c98f08dc25d9..85f57ac0b844e 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1718,13 +1718,13 @@ sg_start_req(Sg_request *srp, unsigned char *cmd)
*
* With scsi-mq enabled, there are a fixed number of preallocated
* requests equal in number to shost->can_queue. If all of the
- * preallocated requests are already in use, then blk_get_request()
+ * preallocated requests are already in use, then scsi_alloc_request()
* will sleep until an active command completes, freeing up a request.
* Although waiting in an asynchronous interface is less than ideal, we
* do not want to use BLK_MQ_REQ_NOWAIT here because userspace might
* not expect an EWOULDBLOCK from this condition.
*/
- rq = blk_get_request(q, hp->dxfer_direction == SG_DXFER_TO_DEV ?
+ rq = scsi_alloc_request(q, hp->dxfer_direction == SG_DXFER_TO_DEV ?
REQ_OP_DRV_OUT : REQ_OP_DRV_IN, 0);
if (IS_ERR(rq)) {
kfree(long_cmdp);
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 115f7ef7a5def..7c4d9a9647999 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -967,7 +967,7 @@ static int sr_read_cdda_bpc(struct cdrom_device_info *cdi, void __user *ubuf,
struct bio *bio;
int ret;

- rq = blk_get_request(disk->queue, REQ_OP_DRV_IN, 0);
+ rq = scsi_alloc_request(disk->queue, REQ_OP_DRV_IN, 0);
if (IS_ERR(rq))
return PTR_ERR(rq);
req = scsi_req(rq);
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 9933722acfd96..1275299f61597 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -543,7 +543,7 @@ static int st_scsi_execute(struct st_request *SRpnt, const unsigned char *cmd,
int err = 0;
struct scsi_tape *STp = SRpnt->stp;

- req = blk_get_request(SRpnt->stp->device->request_queue,
+ req = scsi_alloc_request(SRpnt->stp->device->request_queue,
data_direction == DMA_TO_DEVICE ?
REQ_OP_DRV_OUT : REQ_OP_DRV_IN, 0);
if (IS_ERR(req))
diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
index 75ef52f008ff6..b5705a2bd7618 100644
--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -980,11 +980,10 @@ pscsi_execute_cmd(struct se_cmd *cmd)
memcpy(pt->pscsi_cdb, cmd->t_task_cdb,
scsi_command_size(cmd->t_task_cdb));

- req = blk_get_request(pdv->pdv_sd->request_queue,
+ req = scsi_alloc_request(pdv->pdv_sd->request_queue,
cmd->data_direction == DMA_TO_DEVICE ?
REQ_OP_DRV_OUT : REQ_OP_DRV_IN, 0);
if (IS_ERR(req)) {
- pr_err("PSCSI: blk_get_request() failed\n");
ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
goto fail;
}
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index eaf04c9a1dfcb..31078063afac2 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -396,4 +396,7 @@ static inline unsigned scsi_transfer_length(struct scsi_cmnd *scmd)
extern void scsi_build_sense(struct scsi_cmnd *scmd, int desc,
u8 key, u8 asc, u8 ascq);

+struct request *scsi_alloc_request(struct request_queue *q,
+ unsigned int op, blk_mq_req_flags_t flags);
+
#endif /* _SCSI_SCSI_CMND_H */
--
2.30.2

2021-10-19 07:55:10

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 7/7] block: remove QUEUE_FLAG_SCSI_PASSTHROUGH

Export scsi_device_from_queue for use with pktcdvd and use that instead
of the otherwise unused QUEUE_FLAG_SCSI_PASSTHROUGH queue flag.

Signed-off-by: Christoph Hellwig <[email protected]>
Reviewed-by: Hannes Reinecke <[email protected]>
---
block/blk-mq-debugfs.c | 1 -
drivers/block/pktcdvd.c | 5 ++++-
drivers/scsi/scsi_lib.c | 8 ++++++++
drivers/scsi/scsi_scan.c | 1 -
include/linux/blkdev.h | 3 ---
5 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 68ca5d21cda77..a317f05de466a 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -124,7 +124,6 @@ static const char *const blk_queue_flag_name[] = {
QUEUE_FLAG_NAME(STATS),
QUEUE_FLAG_NAME(POLL_STATS),
QUEUE_FLAG_NAME(REGISTERED),
- QUEUE_FLAG_NAME(SCSI_PASSTHROUGH),
QUEUE_FLAG_NAME(QUIESCED),
QUEUE_FLAG_NAME(PCI_P2PDMA),
QUEUE_FLAG_NAME(ZONE_RESETALL),
diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index ea2262ec76d2c..cacf64eedad87 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -2536,6 +2536,7 @@ static int pkt_new_dev(struct pktcdvd_device *pd, dev_t dev)
int i;
char b[BDEVNAME_SIZE];
struct block_device *bdev;
+ struct scsi_device *sdev;

if (pd->pkt_dev == dev) {
pkt_err(pd, "recursive setup not allowed\n");
@@ -2559,10 +2560,12 @@ static int pkt_new_dev(struct pktcdvd_device *pd, dev_t dev)
bdev = blkdev_get_by_dev(dev, FMODE_READ | FMODE_NDELAY, NULL);
if (IS_ERR(bdev))
return PTR_ERR(bdev);
- if (!blk_queue_scsi_passthrough(bdev_get_queue(bdev))) {
+ sdev = scsi_device_from_queue(bdev->bd_disk->queue);
+ if (!sdev) {
blkdev_put(bdev, FMODE_READ | FMODE_NDELAY);
return -EINVAL;
}
+ put_device(&sdev->sdev_gendev);

/* This is safe, since we have a reference from open(). */
__module_get(THIS_MODULE);
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index a0f801fc8943b..9823b65d15368 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1967,6 +1967,14 @@ struct scsi_device *scsi_device_from_queue(struct request_queue *q)

return sdev;
}
+/*
+ * pktcdvd should have been integrated into the SCSI layers, but for historical
+ * reasons like the old IDE driver it isn't. This export allows it to safely
+ * probe if a given device is a SCSI one and only attach to that.
+ */
+#ifdef CONFIG_CDROM_PKTCDVD_MODULE
+EXPORT_SYMBOL_GPL(scsi_device_from_queue);
+#endif

/**
* scsi_block_requests - Utility function used by low-level drivers to prevent
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index fe22191522a3b..2808c0cb57114 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -280,7 +280,6 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
sdev->request_queue = q;
q->queuedata = sdev;
__scsi_init_queue(sdev->host, q);
- blk_queue_flag_set(QUEUE_FLAG_SCSI_PASSTHROUGH, q);
WARN_ON_ONCE(!blk_get_queue(q));

depth = sdev->host->cmd_per_lun ?: 1;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 6e1c6fbdee0b5..0e811a9642d7f 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -356,7 +356,6 @@ struct request_queue {
#define QUEUE_FLAG_STATS 20 /* track IO start and completion times */
#define QUEUE_FLAG_POLL_STATS 21 /* collecting stats for hybrid polling */
#define QUEUE_FLAG_REGISTERED 22 /* queue has been registered to a disk */
-#define QUEUE_FLAG_SCSI_PASSTHROUGH 23 /* queue supports SCSI commands */
#define QUEUE_FLAG_QUIESCED 24 /* queue has been quiesced */
#define QUEUE_FLAG_PCI_P2PDMA 25 /* device supports PCI p2p requests */
#define QUEUE_FLAG_ZONE_RESETALL 26 /* supports Zone Reset All */
@@ -390,8 +389,6 @@ bool blk_queue_flag_test_and_set(unsigned int flag, struct request_queue *q);
#define blk_queue_secure_erase(q) \
(test_bit(QUEUE_FLAG_SECERASE, &(q)->queue_flags))
#define blk_queue_dax(q) test_bit(QUEUE_FLAG_DAX, &(q)->queue_flags)
-#define blk_queue_scsi_passthrough(q) \
- test_bit(QUEUE_FLAG_SCSI_PASSTHROUGH, &(q)->queue_flags)
#define blk_queue_pci_p2pdma(q) \
test_bit(QUEUE_FLAG_PCI_P2PDMA, &(q)->queue_flags)
#ifdef CONFIG_BLK_RQ_ALLOC_TIME
--
2.30.2

2021-10-20 04:09:40

by Martin K. Petersen

[permalink] [raw]
Subject: Re: remove QUEUE_FLAG_SCSI_PASSTHROUGH v2


Christoph,

> The changes to support pktcdvd are a bit ugly, but I can't think of
> anything better (except for removing the driver entirely). If we'd
> want to support packet writing today it would probably live entirely
> inside the sr driver.

Yeah, I agree.

Anyway. No major objections from me. Not sure whether it makes most
sense for this to go through block or scsi?

--
Martin K. Petersen Oracle Linux Engineering

2021-10-20 05:34:14

by Christoph Hellwig

[permalink] [raw]
Subject: Re: remove QUEUE_FLAG_SCSI_PASSTHROUGH v2

On Wed, Oct 20, 2021 at 12:05:24AM -0400, Martin K. Petersen wrote:
>
> Christoph,
>
> > The changes to support pktcdvd are a bit ugly, but I can't think of
> > anything better (except for removing the driver entirely). If we'd
> > want to support packet writing today it would probably live entirely
> > inside the sr driver.
>
> Yeah, I agree.
>
> Anyway. No major objections from me. Not sure whether it makes most
> sense for this to go through block or scsi?

I'm not sure either, but either tree is fine with me.

2021-10-20 14:55:07

by Jens Axboe

[permalink] [raw]
Subject: Re: remove QUEUE_FLAG_SCSI_PASSTHROUGH v2

On 10/19/21 11:33 PM, Christoph Hellwig wrote:
> On Wed, Oct 20, 2021 at 12:05:24AM -0400, Martin K. Petersen wrote:
>>
>> Christoph,
>>
>>> The changes to support pktcdvd are a bit ugly, but I can't think of
>>> anything better (except for removing the driver entirely). If we'd
>>> want to support packet writing today it would probably live entirely
>>> inside the sr driver.
>>
>> Yeah, I agree.
>>
>> Anyway. No major objections from me. Not sure whether it makes most
>> sense for this to go through block or scsi?
>
> I'm not sure either, but either tree is fine with me.

Looks fine to me, outside of the spelling error in patch 1. I can set
up a topic branch for this one.

Christoph, can you do a resend with the enum naming fixed?

--
Jens Axboe