All of lore.kernel.org
 help / color / mirror / Atom feed
* libata: remove the global response buffer
@ 2017-01-10  8:41 Christoph Hellwig
  2017-01-10  8:41 ` [PATCH 1/6] libata: avoid global response buffer in atapi_qc_complete Christoph Hellwig
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Christoph Hellwig @ 2017-01-10  8:41 UTC (permalink / raw)
  To: tj; +Cc: geert, linux-ide

Avoid the need for a global 4k buffer by allocating no response buffer
for commands that don't return data, a small on-stack one for the ATAPI
fixup, or a kmalloced one for all other emulated commands.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/6] libata: avoid global response buffer in atapi_qc_complete
  2017-01-10  8:41 libata: remove the global response buffer Christoph Hellwig
@ 2017-01-10  8:41 ` Christoph Hellwig
  2017-01-10  8:41 ` [PATCH 2/6] libata: move struct ata_scsi_args to libata-scsi.c Christoph Hellwig
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2017-01-10  8:41 UTC (permalink / raw)
  To: tj; +Cc: geert, linux-ide

We only need to look at 4 bytes of the inquiry response for ATAPI
devices.  Instead of using the global ata_scsi_rbuf just use a
a stack buffer.  Also factor the fixup into it's own little helper
function to make it more readable.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/ata/libata-scsi.c | 46 ++++++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 1f863e7..031a77a 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2873,6 +2873,26 @@ static void atapi_request_sense(struct ata_queued_cmd *qc)
 	DPRINTK("EXIT\n");
 }
 
+/*
+ * ATAPI devices typically report zero for their SCSI version, and sometimes
+ * deviate from the spec WRT response data format.  If SCSI version is
+ * reported as zero like normal, then we make the following fixups:
+ *   1) Fake MMC-5 version, to indicate to the Linux scsi midlayer this is a
+ *	modern device.
+ *   2) Ensure response data format / ATAPI information are always correct.
+ */
+static void atapi_fixup_inquiry(struct scsi_cmnd *cmd)
+{
+	u8 buf[4];
+
+	sg_copy_to_buffer(scsi_sglist(cmd), scsi_sg_count(cmd), buf, 4);
+	if (buf[2] == 0) {
+		buf[2] = 0x5;
+		buf[3] = 0x32;
+	}
+	sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd), buf, 4);
+}
+
 static void atapi_qc_complete(struct ata_queued_cmd *qc)
 {
 	struct scsi_cmnd *cmd = qc->scsicmd;
@@ -2927,30 +2947,8 @@ static void atapi_qc_complete(struct ata_queued_cmd *qc)
 		 */
 		ata_gen_passthru_sense(qc);
 	} else {
-		u8 *scsicmd = cmd->cmnd;
-
-		if ((scsicmd[0] == INQUIRY) && ((scsicmd[1] & 0x03) == 0)) {
-			unsigned long flags;
-			u8 *buf;
-
-			buf = ata_scsi_rbuf_get(cmd, true, &flags);
-
-	/* ATAPI devices typically report zero for their SCSI version,
-	 * and sometimes deviate from the spec WRT response data
-	 * format.  If SCSI version is reported as zero like normal,
-	 * then we make the following fixups:  1) Fake MMC-5 version,
-	 * to indicate to the Linux scsi midlayer this is a modern
-	 * device.  2) Ensure response data format / ATAPI information
-	 * are always correct.
-	 */
-			if (buf[2] == 0) {
-				buf[2] = 0x5;
-				buf[3] = 0x32;
-			}
-
-			ata_scsi_rbuf_put(cmd, true, &flags);
-		}
-
+		if (cmd->cmnd[0] == INQUIRY && (cmd->cmnd[1] & 0x03) == 0)
+			atapi_fixup_inquiry(cmd);
 		cmd->result = SAM_STAT_GOOD;
 	}
 
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/6] libata: move struct ata_scsi_args to libata-scsi.c
  2017-01-10  8:41 libata: remove the global response buffer Christoph Hellwig
  2017-01-10  8:41 ` [PATCH 1/6] libata: avoid global response buffer in atapi_qc_complete Christoph Hellwig
@ 2017-01-10  8:41 ` Christoph Hellwig
  2017-01-10  8:41 ` [PATCH 3/6] libata: remove the done allback from ata_scsi_args Christoph Hellwig
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2017-01-10  8:41 UTC (permalink / raw)
  To: tj; +Cc: geert, linux-ide

It's only used in libata-scsi.c, so move it closer to the users.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/ata/libata-scsi.c | 7 +++++++
 drivers/ata/libata.h      | 7 -------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 031a77a..43694f5 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2057,6 +2057,13 @@ static int ata_scsi_translate(struct ata_device *dev, struct scsi_cmnd *cmd,
 		return SCSI_MLQUEUE_HOST_BUSY;
 }
 
+struct ata_scsi_args {
+	struct ata_device	*dev;
+	u16			*id;
+	struct scsi_cmnd	*cmd;
+	void			(*done)(struct scsi_cmnd *);
+};
+
 /**
  *	ata_scsi_rbuf_get - Map response buffer.
  *	@cmd: SCSI command containing buffer to be mapped.
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 8f3a559..9943772 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -31,13 +31,6 @@
 #define DRV_NAME	"libata"
 #define DRV_VERSION	"3.00"	/* must be exactly four chars */
 
-struct ata_scsi_args {
-	struct ata_device	*dev;
-	u16			*id;
-	struct scsi_cmnd	*cmd;
-	void			(*done)(struct scsi_cmnd *);
-};
-
 /* libata-core.c */
 enum {
 	/* flags for ata_dev_read_id() */
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/6] libata: remove the done allback from ata_scsi_args
  2017-01-10  8:41 libata: remove the global response buffer Christoph Hellwig
  2017-01-10  8:41 ` [PATCH 1/6] libata: avoid global response buffer in atapi_qc_complete Christoph Hellwig
  2017-01-10  8:41 ` [PATCH 2/6] libata: move struct ata_scsi_args to libata-scsi.c Christoph Hellwig
@ 2017-01-10  8:41 ` Christoph Hellwig
  2017-01-10  8:41 ` [PATCH 4/6] libata: call ->scsi_done from ata_scsi_simulate Christoph Hellwig
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2017-01-10  8:41 UTC (permalink / raw)
  To: tj; +Cc: geert, linux-ide

It's always the scsi_done callback, and we can get at that easily
in the place where ->done is called.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/ata/libata-scsi.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 43694f5..28e2530 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2061,7 +2061,6 @@ struct ata_scsi_args {
 	struct ata_device	*dev;
 	u16			*id;
 	struct scsi_cmnd	*cmd;
-	void			(*done)(struct scsi_cmnd *);
 };
 
 /**
@@ -2140,7 +2139,7 @@ static void ata_scsi_rbuf_fill(struct ata_scsi_args *args,
 
 	if (rc == 0)
 		cmd->result = SAM_STAT_GOOD;
-	args->done(cmd);
+	cmd->scsi_done(cmd);
 }
 
 /**
@@ -4357,7 +4356,6 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
 	args.dev = dev;
 	args.id = dev->id;
 	args.cmd = cmd;
-	args.done = cmd->scsi_done;
 
 	switch(scsicmd[0]) {
 	case INQUIRY:
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/6] libata: call ->scsi_done from ata_scsi_simulate
  2017-01-10  8:41 libata: remove the global response buffer Christoph Hellwig
                   ` (2 preceding siblings ...)
  2017-01-10  8:41 ` [PATCH 3/6] libata: remove the done allback from ata_scsi_args Christoph Hellwig
@ 2017-01-10  8:41 ` Christoph Hellwig
  2017-01-10  8:41 ` [PATCH 5/6] libata: don't call ata_scsi_rbuf_fill for command without a response buffer Christoph Hellwig
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2017-01-10  8:41 UTC (permalink / raw)
  To: tj; +Cc: geert, linux-ide

We always need to call ->scsi_done after we've finished emulating a
command, so do it in a single place at the end of ata_scsi_simulate.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/ata/libata-scsi.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 28e2530..6078bc2 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -484,13 +484,6 @@ struct device_attribute *ata_common_sdev_attrs[] = {
 };
 EXPORT_SYMBOL_GPL(ata_common_sdev_attrs);
 
-static void ata_scsi_invalid_field(struct ata_device *dev,
-				   struct scsi_cmnd *cmd, u16 field)
-{
-	ata_scsi_set_invalid_field(dev, cmd, field, 0xff);
-	cmd->scsi_done(cmd);
-}
-
 /**
  *	ata_std_bios_param - generic bios head/sector/cylinder calculator used by sd.
  *	@sdev: SCSI device for which BIOS geometry is to be determined
@@ -2139,7 +2132,6 @@ static void ata_scsi_rbuf_fill(struct ata_scsi_args *args,
 
 	if (rc == 0)
 		cmd->result = SAM_STAT_GOOD;
-	cmd->scsi_done(cmd);
 }
 
 /**
@@ -4360,7 +4352,7 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
 	switch(scsicmd[0]) {
 	case INQUIRY:
 		if (scsicmd[1] & 2)		   /* is CmdDt set?  */
-		    ata_scsi_invalid_field(dev, cmd, 1);
+			ata_scsi_set_invalid_field(dev, cmd, 1, 0xff);
 		else if ((scsicmd[1] & 1) == 0)    /* is EVPD clear? */
 			ata_scsi_rbuf_fill(&args, ata_scsiop_inq_std);
 		else switch (scsicmd[2]) {
@@ -4392,7 +4384,7 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
 			}
 			/* Fallthrough */
 		default:
-			ata_scsi_invalid_field(dev, cmd, 2);
+			ata_scsi_set_invalid_field(dev, cmd, 2, 0xff);
 			break;
 		}
 		break;
@@ -4410,7 +4402,7 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
 		if ((scsicmd[1] & 0x1f) == SAI_READ_CAPACITY_16)
 			ata_scsi_rbuf_fill(&args, ata_scsiop_read_cap);
 		else
-			ata_scsi_invalid_field(dev, cmd, 1);
+			ata_scsi_set_invalid_field(dev, cmd, 1, 0xff);
 		break;
 
 	case REPORT_LUNS:
@@ -4420,7 +4412,6 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
 	case REQUEST_SENSE:
 		ata_scsi_set_sense(dev, cmd, 0, 0, 0);
 		cmd->result = (DRIVER_SENSE << 24);
-		cmd->scsi_done(cmd);
 		break;
 
 	/* if we reach this, then writeback caching is disabled,
@@ -4442,23 +4433,24 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
 		if ((tmp8 == 0x4) && (!scsicmd[3]) && (!scsicmd[4]))
 			ata_scsi_rbuf_fill(&args, ata_scsiop_noop);
 		else
-			ata_scsi_invalid_field(dev, cmd, 1);
+			ata_scsi_set_invalid_field(dev, cmd, 1, 0xff);
 		break;
 
 	case MAINTENANCE_IN:
 		if (scsicmd[1] == MI_REPORT_SUPPORTED_OPERATION_CODES)
 			ata_scsi_rbuf_fill(&args, ata_scsiop_maint_in);
 		else
-			ata_scsi_invalid_field(dev, cmd, 1);
+			ata_scsi_set_invalid_field(dev, cmd, 1, 0xff);
 		break;
 
 	/* all other commands */
 	default:
 		ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST, 0x20, 0x0);
 		/* "Invalid command operation code" */
-		cmd->scsi_done(cmd);
 		break;
 	}
+
+	cmd->scsi_done(cmd);
 }
 
 int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht)
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/6] libata: don't call ata_scsi_rbuf_fill for command without a response buffer
  2017-01-10  8:41 libata: remove the global response buffer Christoph Hellwig
                   ` (3 preceding siblings ...)
  2017-01-10  8:41 ` [PATCH 4/6] libata: call ->scsi_done from ata_scsi_simulate Christoph Hellwig
@ 2017-01-10  8:41 ` Christoph Hellwig
  2017-01-10  8:41 ` [PATCH 6/6] libata: switch to dynamic allocation insted of ata_scsi_rbuf Christoph Hellwig
  2017-01-10 15:56 ` libata: remove the global response buffer Tejun Heo
  6 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2017-01-10  8:41 UTC (permalink / raw)
  To: tj; +Cc: geert, linux-ide

No need to copy a zeroed buffer to the caller if the command is defined
to not have a response in the SCSI spec.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/ata/libata-scsi.c | 22 +---------------------
 1 file changed, 1 insertion(+), 21 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 6078bc2..395c859 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2453,23 +2453,6 @@ static unsigned int ata_scsiop_inq_b6(struct ata_scsi_args *args, u8 *rbuf)
 }
 
 /**
- *	ata_scsiop_noop - Command handler that simply returns success.
- *	@args: device IDENTIFY data / SCSI command of interest.
- *	@rbuf: Response buffer, to which simulated SCSI cmd output is sent.
- *
- *	No operation.  Simply returns success to caller, to indicate
- *	that the caller should successfully complete this SCSI command.
- *
- *	LOCKING:
- *	spin_lock_irqsave(host lock)
- */
-static unsigned int ata_scsiop_noop(struct ata_scsi_args *args, u8 *rbuf)
-{
-	VPRINTK("ENTER\n");
-	return 0;
-}
-
-/**
  *	modecpy - Prepare response for MODE SENSE
  *	@dest: output buffer
  *	@src: data being copied
@@ -4425,14 +4408,11 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
 	case SEEK_6:
 	case SEEK_10:
 	case TEST_UNIT_READY:
-		ata_scsi_rbuf_fill(&args, ata_scsiop_noop);
 		break;
 
 	case SEND_DIAGNOSTIC:
 		tmp8 = scsicmd[1] & ~(1 << 3);
-		if ((tmp8 == 0x4) && (!scsicmd[3]) && (!scsicmd[4]))
-			ata_scsi_rbuf_fill(&args, ata_scsiop_noop);
-		else
+		if (tmp8 != 0x4 || scsicmd[3] || scsicmd[4])
 			ata_scsi_set_invalid_field(dev, cmd, 1, 0xff);
 		break;
 
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 6/6] libata: switch to dynamic allocation insted of ata_scsi_rbuf
  2017-01-10  8:41 libata: remove the global response buffer Christoph Hellwig
                   ` (4 preceding siblings ...)
  2017-01-10  8:41 ` [PATCH 5/6] libata: don't call ata_scsi_rbuf_fill for command without a response buffer Christoph Hellwig
@ 2017-01-10  8:41 ` Christoph Hellwig
  2017-01-10 14:40   ` Christoph Hellwig
  2017-01-10 15:56 ` libata: remove the global response buffer Tejun Heo
  6 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2017-01-10  8:41 UTC (permalink / raw)
  To: tj; +Cc: geert, linux-ide

Note of the emulated commands in the pageout/pagein path, so just do
a GFP_NOIO dynamic allocation.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/ata/libata-scsi.c | 122 ++++++++++++++--------------------------------
 1 file changed, 36 insertions(+), 86 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 395c859..785eb382 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -57,9 +57,6 @@
 
 #define ATA_SCSI_RBUF_SIZE	4096
 
-static DEFINE_SPINLOCK(ata_scsi_rbuf_lock);
-static u8 ata_scsi_rbuf[ATA_SCSI_RBUF_SIZE];
-
 typedef unsigned int (*ata_xlat_func_t)(struct ata_queued_cmd *qc);
 
 static struct ata_device *__ata_scsi_find_dev(struct ata_port *ap,
@@ -2057,53 +2054,6 @@ struct ata_scsi_args {
 };
 
 /**
- *	ata_scsi_rbuf_get - Map response buffer.
- *	@cmd: SCSI command containing buffer to be mapped.
- *	@flags: unsigned long variable to store irq enable status
- *	@copy_in: copy in from user buffer
- *
- *	Prepare buffer for simulated SCSI commands.
- *
- *	LOCKING:
- *	spin_lock_irqsave(ata_scsi_rbuf_lock) on success
- *
- *	RETURNS:
- *	Pointer to response buffer.
- */
-static void *ata_scsi_rbuf_get(struct scsi_cmnd *cmd, bool copy_in,
-			       unsigned long *flags)
-{
-	spin_lock_irqsave(&ata_scsi_rbuf_lock, *flags);
-
-	memset(ata_scsi_rbuf, 0, ATA_SCSI_RBUF_SIZE);
-	if (copy_in)
-		sg_copy_to_buffer(scsi_sglist(cmd), scsi_sg_count(cmd),
-				  ata_scsi_rbuf, ATA_SCSI_RBUF_SIZE);
-	return ata_scsi_rbuf;
-}
-
-/**
- *	ata_scsi_rbuf_put - Unmap response buffer.
- *	@cmd: SCSI command containing buffer to be unmapped.
- *	@copy_out: copy out result
- *	@flags: @flags passed to ata_scsi_rbuf_get()
- *
- *	Returns rbuf buffer.  The result is copied to @cmd's buffer if
- *	@copy_back is true.
- *
- *	LOCKING:
- *	Unlocks ata_scsi_rbuf_lock.
- */
-static inline void ata_scsi_rbuf_put(struct scsi_cmnd *cmd, bool copy_out,
-				     unsigned long *flags)
-{
-	if (copy_out)
-		sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd),
-				    ata_scsi_rbuf, ATA_SCSI_RBUF_SIZE);
-	spin_unlock_irqrestore(&ata_scsi_rbuf_lock, *flags);
-}
-
-/**
  *	ata_scsi_rbuf_fill - wrapper for SCSI command simulators
  *	@args: device IDENTIFY data / SCSI command of interest.
  *	@actor: Callback hook for desired SCSI command simulator
@@ -2121,17 +2071,22 @@ static inline void ata_scsi_rbuf_put(struct scsi_cmnd *cmd, bool copy_out,
 static void ata_scsi_rbuf_fill(struct ata_scsi_args *args,
 		unsigned int (*actor)(struct ata_scsi_args *args, u8 *rbuf))
 {
-	u8 *rbuf;
-	unsigned int rc;
 	struct scsi_cmnd *cmd = args->cmd;
-	unsigned long flags;
+	u8 *buf;
 
-	rbuf = ata_scsi_rbuf_get(cmd, false, &flags);
-	rc = actor(args, rbuf);
-	ata_scsi_rbuf_put(cmd, rc == 0, &flags);
+	buf = kzalloc(ATA_SCSI_RBUF_SIZE, GFP_NOIO);
+	if (!buf) {
+		ata_scsi_set_sense(args->dev, cmd, NOT_READY, 0x08, 0);
+		return;
+	}
 
-	if (rc == 0)
+	if (actor(args, buf) == 0) {
+		sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd),
+				    buf, ATA_SCSI_RBUF_SIZE);
 		cmd->result = SAM_STAT_GOOD;
+	}
+
+	kfree(buf);
 }
 
 /**
@@ -3363,24 +3318,17 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
  *
  * Return: Number of bytes copied into sglist.
  */
-static size_t ata_format_dsm_trim_descr(struct scsi_cmnd *cmd, u32 trmax,
+static ssize_t ata_format_dsm_trim_descr(struct scsi_cmnd *cmd, u32 trmax,
 					u64 sector, u32 count)
 {
-	struct scsi_device *sdp = cmd->device;
-	size_t len = sdp->sector_size;
 	size_t r;
 	__le64 *buf;
 	u32 i = 0;
-	unsigned long flags;
 
-	WARN_ON(len > ATA_SCSI_RBUF_SIZE);
-
-	if (len > ATA_SCSI_RBUF_SIZE)
-		len = ATA_SCSI_RBUF_SIZE;
+	buf = kzalloc(cmd->device->sector_size, GFP_NOFS);
+	if (!buf)
+		return -ENOMEM;
 
-	spin_lock_irqsave(&ata_scsi_rbuf_lock, flags);
-	buf = ((void *)ata_scsi_rbuf);
-	memset(buf, 0, len);
 	while (i < trmax) {
 		u64 entry = sector |
 			((u64)(count > 0xffff ? 0xffff : count) << 48);
@@ -3390,9 +3338,9 @@ static size_t ata_format_dsm_trim_descr(struct scsi_cmnd *cmd, u32 trmax,
 		count -= 0xffff;
 		sector += 0xffff;
 	}
-	r = sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd), buf, len);
-	spin_unlock_irqrestore(&ata_scsi_rbuf_lock, flags);
-
+	r = sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd), buf,
+			cmd->device->sector_size);
+	kfree(buf);
 	return r;
 }
 
@@ -3408,16 +3356,15 @@ static size_t ata_format_dsm_trim_descr(struct scsi_cmnd *cmd, u32 trmax,
  *
  * Return: Number of bytes copied into sglist.
  */
-static size_t ata_format_sct_write_same(struct scsi_cmnd *cmd, u64 lba, u64 num)
+static ssize_t ata_format_sct_write_same(struct scsi_cmnd *cmd, u64 lba,
+		u64 num)
 {
-	struct scsi_device *sdp = cmd->device;
-	size_t len = sdp->sector_size;
 	size_t r;
 	u16 *buf;
-	unsigned long flags;
 
-	spin_lock_irqsave(&ata_scsi_rbuf_lock, flags);
-	buf = ((void *)ata_scsi_rbuf);
+	buf = kzalloc(cmd->device->sector_size, GFP_NOIO);
+	if (!buf)
+		return -ENOMEM;
 
 	put_unaligned_le16(0x0002,  &buf[0]); /* SCT_ACT_WRITE_SAME */
 	put_unaligned_le16(0x0101,  &buf[1]); /* WRITE PTRN FG */
@@ -3425,14 +3372,9 @@ static size_t ata_format_sct_write_same(struct scsi_cmnd *cmd, u64 lba, u64 num)
 	put_unaligned_le64(num,     &buf[6]);
 	put_unaligned_le32(0u,      &buf[10]); /* pattern */
 
-	WARN_ON(len > ATA_SCSI_RBUF_SIZE);
-
-	if (len > ATA_SCSI_RBUF_SIZE)
-		len = ATA_SCSI_RBUF_SIZE;
-
-	r = sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd), buf, len);
-	spin_unlock_irqrestore(&ata_scsi_rbuf_lock, flags);
-
+	r = sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd), buf,
+			cmd->device->sector_size);
+	kfree(buf);
 	return r;
 }
 
@@ -3451,7 +3393,7 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
 	struct ata_taskfile *tf = &qc->tf;
 	struct scsi_cmnd *scmd = qc->scsicmd;
 	struct scsi_device *sdp = scmd->device;
-	size_t len = sdp->sector_size;
+	ssize_t len = sdp->sector_size;
 	struct ata_device *dev = qc->dev;
 	const u8 *cdb = scmd->cmnd;
 	u64 block;
@@ -3508,6 +3450,8 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
 	 */
 	if (unmap) {
 		size = ata_format_dsm_trim_descr(scmd, trmax, block, n_block);
+		if (size < 0)
+			goto comm_fail;
 		if (size != len)
 			goto invalid_param_len;
 
@@ -3531,6 +3475,8 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
 		}
 	} else {
 		size = ata_format_sct_write_same(scmd, block, n_block);
+		if (size < 0)
+			goto comm_fail;
 		if (size != len)
 			goto invalid_param_len;
 
@@ -3569,6 +3515,10 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
 	/* "Invalid command operation code" */
 	ata_scsi_set_sense(dev, scmd, ILLEGAL_REQUEST, 0x20, 0x0);
 	return 1;
+comm_fail:
+	/* "Logical unit communication failure" */
+	ata_scsi_set_sense(dev, scmd, NOT_READY, 0x08, 0);
+	return 1;
 }
 
 /**
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 6/6] libata: switch to dynamic allocation insted of ata_scsi_rbuf
  2017-01-10  8:41 ` [PATCH 6/6] libata: switch to dynamic allocation insted of ata_scsi_rbuf Christoph Hellwig
@ 2017-01-10 14:40   ` Christoph Hellwig
  0 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2017-01-10 14:40 UTC (permalink / raw)
  To: tj; +Cc: geert, linux-ide

>  	struct scsi_cmnd *scmd = qc->scsicmd;
>  	struct scsi_device *sdp = scmd->device;
> -	size_t len = sdp->sector_size;
> +	ssize_t len = sdp->sector_size;

Julia Lawall's magic checker tool pointed out that len doesn't have
to change to ssize_t here, but the size variable should instead so that
we can properly handle the ENOMEM case below.

I'll wait for some more feedback on the patches and will eventually
resend with that fix.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: libata: remove the global response buffer
  2017-01-10  8:41 libata: remove the global response buffer Christoph Hellwig
                   ` (5 preceding siblings ...)
  2017-01-10  8:41 ` [PATCH 6/6] libata: switch to dynamic allocation insted of ata_scsi_rbuf Christoph Hellwig
@ 2017-01-10 15:56 ` Tejun Heo
  6 siblings, 0 replies; 9+ messages in thread
From: Tejun Heo @ 2017-01-10 15:56 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: geert, linux-ide

On Tue, Jan 10, 2017 at 09:41:42AM +0100, Christoph Hellwig wrote:
> Avoid the need for a global 4k buffer by allocating no response buffer
> for commands that don't return data, a small on-stack one for the ATAPI
> fixup, or a kmalloced one for all other emulated commands.

Applied 1-5 to libata/for-4.11.  Will wait for the updated version of
the last patch.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-01-10 15:57 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-10  8:41 libata: remove the global response buffer Christoph Hellwig
2017-01-10  8:41 ` [PATCH 1/6] libata: avoid global response buffer in atapi_qc_complete Christoph Hellwig
2017-01-10  8:41 ` [PATCH 2/6] libata: move struct ata_scsi_args to libata-scsi.c Christoph Hellwig
2017-01-10  8:41 ` [PATCH 3/6] libata: remove the done allback from ata_scsi_args Christoph Hellwig
2017-01-10  8:41 ` [PATCH 4/6] libata: call ->scsi_done from ata_scsi_simulate Christoph Hellwig
2017-01-10  8:41 ` [PATCH 5/6] libata: don't call ata_scsi_rbuf_fill for command without a response buffer Christoph Hellwig
2017-01-10  8:41 ` [PATCH 6/6] libata: switch to dynamic allocation insted of ata_scsi_rbuf Christoph Hellwig
2017-01-10 14:40   ` Christoph Hellwig
2017-01-10 15:56 ` libata: remove the global response buffer Tejun Heo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.