| Message ID | 20180917213554.987-7-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Make ABORT and LUN RESET handling synchronous | expand |
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?
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.
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?
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.
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; }
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(-)