[06/17] target/core: Use sg_alloc_table() instead of open-coding it
diff mbox series

Message ID 20180917213554.987-7-bvanassche@acm.org
State New, archived
Headers show
Series
  • Make ABORT and LUN RESET handling synchronous
Related show

Commit Message

Bart Van Assche Sept. 17, 2018, 9:35 p.m. UTC
The purpose of sg_alloc_table() is to allocate and initialize an
sg-list. Use that function instead of open-coding it.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: Nicholas Bellinger <nab@linux-iscsi.org>
Cc: Mike Christie <mchristi@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
---
 drivers/target/target_core_sbc.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Comments

Christoph Hellwig Oct. 5, 2018, 7:46 a.m. UTC | #1
On Mon, Sep 17, 2018 at 02:35:43PM -0700, Bart Van Assche wrote:
> The purpose of sg_alloc_table() is to allocate and initialize an
> sg-list. Use that function instead of open-coding it.

Hmm.  This looks correct, but what is the point if we don't plan
on using chained sg-lists?
Bart Van Assche Oct. 5, 2018, 1:52 p.m. UTC | #2
On 10/5/18 12:46 AM, Christoph Hellwig wrote:
> On Mon, Sep 17, 2018 at 02:35:43PM -0700, Bart Van Assche wrote:
>> The purpose of sg_alloc_table() is to allocate and initialize an
>> sg-list. Use that function instead of open-coding it.
> 
> Hmm.  This looks correct, but what is the point if we don't plan
> on using chained sg-lists?

This patch makes it easier to replace the SCSI target sg-list allocation 
code by (library) code that caches sg-list allocations. I still hope to 
find some time to work on this.

Since the rest of this series does not depend on this patch, I can leave 
it out if you want.

Bart.
Christoph Hellwig Oct. 6, 2018, 11:53 a.m. UTC | #3
On Fri, Oct 05, 2018 at 06:52:12AM -0700, Bart Van Assche wrote:
> On 10/5/18 12:46 AM, Christoph Hellwig wrote:
>> On Mon, Sep 17, 2018 at 02:35:43PM -0700, Bart Van Assche wrote:
>>> The purpose of sg_alloc_table() is to allocate and initialize an
>>> sg-list. Use that function instead of open-coding it.
>>
>> Hmm.  This looks correct, but what is the point if we don't plan
>> on using chained sg-lists?
>
> This patch makes it easier to replace the SCSI target sg-list allocation 
> code by (library) code that caches sg-list allocations. I still hope to 
> find some time to work on this.

Ok.

> Since the rest of this series does not depend on this patch, I can leave it 
> out if you want.

Maybe just update the changelog to explain this better?
Bart Van Assche Oct. 8, 2018, 4:32 p.m. UTC | #4
On Sat, 2018-10-06 at 13:53 +0200, Christoph Hellwig wrote:
> Maybe just update the changelog to explain this better?

OK, I will do that.

Bart.

Patch
diff mbox series

diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index ebac2b49b9c6..689ba6da9a0f 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -453,7 +453,8 @@  static sense_reason_t compare_and_write_callback(struct se_cmd *cmd, bool succes
 						 int *post_ret)
 {
 	struct se_device *dev = cmd->se_dev;
-	struct scatterlist *write_sg = NULL, *sg;
+	struct sg_table write_tbl = { };
+	struct scatterlist *write_sg, *sg;
 	unsigned char *buf = NULL, *addr;
 	struct sg_mapping_iter m;
 	unsigned int offset = 0, len;
@@ -494,14 +495,12 @@  static sense_reason_t compare_and_write_callback(struct se_cmd *cmd, bool succes
 		goto out;
 	}
 
-	write_sg = kmalloc_array(cmd->t_data_nents, sizeof(*write_sg),
-				 GFP_KERNEL);
-	if (!write_sg) {
+	if (sg_alloc_table(&write_tbl, cmd->t_data_nents, GFP_KERNEL) < 0) {
 		pr_err("Unable to allocate compare_and_write sg\n");
 		ret = TCM_OUT_OF_RESOURCES;
 		goto out;
 	}
-	sg_init_table(write_sg, cmd->t_data_nents);
+	write_sg = write_tbl.sgl;
 	/*
 	 * Setup verify and write data payloads from total NumberLBAs.
 	 */
@@ -597,7 +596,7 @@  static sense_reason_t compare_and_write_callback(struct se_cmd *cmd, bool succes
 	 * sbc_compare_and_write() before the original READ I/O submission.
 	 */
 	up(&dev->caw_sem);
-	kfree(write_sg);
+	sg_free_table(&write_tbl);
 	kfree(buf);
 	return ret;
 }