Re: [PATCH] aha1542: convert to DMA mapping API
On Wed, Aug 08, 2018 at 10:30:19PM +0200, Ondrej Zary wrote:
> Then it crashes with null-pointer dereference in aha1542_reset.
Must be the dma_unmap. Updated patch below:
---
>From 98a9c770430cde972923838b990b2b9cf7b95816 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig
Date: Thu, 2 Aug 2018 14:22:46 +0200
Subject: aha1542: convert to DMA mapping API
aha1542 is one of the last users of the legacy isa_*_to_bus APIs, which
also isn't portable enough. Convert it to the proper DMA mapping API.
Signed-off-by: Christoph Hellwig
---
drivers/scsi/aha1542.c | 126 +
1 file changed, 91 insertions(+), 35 deletions(-)
diff --git a/drivers/scsi/aha1542.c b/drivers/scsi/aha1542.c
index 41add33e3f1f..d09e19dde177 100644
--- a/drivers/scsi/aha1542.c
+++ b/drivers/scsi/aha1542.c
@@ -58,8 +58,15 @@ struct aha1542_hostdata {
int aha1542_last_mbi_used;
int aha1542_last_mbo_used;
struct scsi_cmnd *int_cmds[AHA1542_MAILBOXES];
- struct mailbox mb[2 * AHA1542_MAILBOXES];
- struct ccb ccb[AHA1542_MAILBOXES];
+ struct mailbox *mb;
+ dma_addr_t mb_handle;
+ struct ccb *ccb;
+ dma_addr_t ccb_handle;
+};
+
+struct aha1542_cmd {
+ struct chain *chain;
+ dma_addr_t chain_handle;
};
static inline void aha1542_intr_reset(u16 base)
@@ -233,6 +240,21 @@ static int aha1542_test_port(struct Scsi_Host *sh)
return 1;
}
+static void aha1542_free_cmd(struct scsi_cmnd *cmd)
+{
+ struct aha1542_cmd *acmd = scsi_cmd_priv(cmd);
+ struct device *dev = cmd->device->host->dma_dev;
+ size_t len = scsi_sg_count(cmd) * sizeof(struct chain);
+
+ if (acmd->chain) {
+ dma_unmap_single(dev, acmd->chain_handle, len, DMA_TO_DEVICE);
+ kfree(acmd->chain);
+ }
+
+ acmd->chain = NULL;
+ scsi_dma_unmap(cmd);
+}
+
static irqreturn_t aha1542_interrupt(int irq, void *dev_id)
{
struct Scsi_Host *sh = dev_id;
@@ -303,7 +325,7 @@ static irqreturn_t aha1542_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
};
- mbo = (scsi2int(mb[mbi].ccbptr) - (isa_virt_to_bus([0]))) /
sizeof(struct ccb);
+ mbo = (scsi2int(mb[mbi].ccbptr) - aha1542->ccb_handle) /
sizeof(struct ccb);
mbistatus = mb[mbi].status;
mb[mbi].status = 0;
aha1542->aha1542_last_mbi_used = mbi;
@@ -331,8 +353,7 @@ static irqreturn_t aha1542_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
}
my_done = tmp_cmd->scsi_done;
- kfree(tmp_cmd->host_scribble);
- tmp_cmd->host_scribble = NULL;
+ aha1542_free_cmd(tmp_cmd);
/* Fetch the sense data, and tuck it away, in the required
slot. The
Adaptec automatically fetches it, and there is no guarantee
that
we will still have it in the cdb when we come back */
@@ -369,6 +390,7 @@ static irqreturn_t aha1542_interrupt(int irq, void *dev_id)
static int aha1542_queuecommand(struct Scsi_Host *sh, struct scsi_cmnd *cmd)
{
+ struct aha1542_cmd *acmd = scsi_cmd_priv(cmd);
struct aha1542_hostdata *aha1542 = shost_priv(sh);
u8 direction;
u8 target = cmd->device->id;
@@ -378,7 +400,6 @@ static int aha1542_queuecommand(struct Scsi_Host *sh,
struct scsi_cmnd *cmd)
int mbo, sg_count;
struct mailbox *mb = aha1542->mb;
struct ccb *ccb = aha1542->ccb;
- struct chain *cptr;
if (*cmd->cmnd == REQUEST_SENSE) {
/* Don't do the command - we have the sense data already */
@@ -398,15 +419,17 @@ static int aha1542_queuecommand(struct Scsi_Host *sh,
struct scsi_cmnd *cmd)
print_hex_dump_bytes("command: ", DUMP_PREFIX_NONE, cmd->cmnd,
cmd->cmd_len);
}
#endif
- if (bufflen) { /* allocate memory before taking host_lock */
- sg_count = scsi_sg_count(cmd);
- cptr = kmalloc_array(sg_count, sizeof(*cptr),
-GFP_KERNEL | GFP_DMA);
- if (!cptr)
- return SCSI_MLQUEUE_HOST_BUSY;
- } else {
- sg_count = 0;
- cptr = NULL;
+ sg_count = scsi_dma_map(cmd);
+ if (sg_count) {
+ size_t len = sg_count * sizeof(struct chain);
+
+ acmd->chain = kmalloc(len, GFP_DMA);
+ if (!acmd->chain)
+ goto out_unmap;
+ acmd->chain_handle = dma_map_single(sh->dma_dev, acmd->chain,
+ len, DMA_TO_DEVICE);
+ if (dma_mapping_error(sh->dma_dev, acmd->chain_handle))
+ goto out_free_chain;
}
/* Use the outgoing mailboxes in a round-robin fashion, because this
@@ -437,7 +460,8 @@ static int aha1542_queuecommand(struct Scsi_Host *sh,
Re: [PATCH] mpt3sas: correct reset of smid while clearing scsi tracker
Tomas, > Fixes: 2b548dd3790f95c2e174d51c2c8ada71b6505b4e > scsi: mpt3sas: Fix calltrace observed while running IO & reset > > Maybe both patches could be merged. Merged the fix into the patch above. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v2] scsi: aic94xx: fix an error code in aic94xx_init()
Dan, > We accidentally return success instead of -ENOMEM on this error path. Applied to 4.19/scsi-queue, thanks! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH] scsi: aic94xx: fix an error code in aic94xx_init()
Looks good, Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Re: [PATCH] scsi: aic94xx: fix an error code in aic94xx_init()
On 08/08/2018 12:56, Dan Carpenter wrote:
We accidentally return success instead of -ENODEV on this error path.
Sorry to nitpick, but - as I see - the only way for
sas_domain_attach_transport() to fail is if the kzalloc() in
sas_attach_transport() fails, so should this be -ENOMEM? Other drivers
return this error code for this scenario.
Thanks,
John
Fixes: 2908d778ab3e ("[SCSI] aic94xx: new driver")
Signed-off-by: Dan Carpenter
diff --git a/drivers/scsi/aic94xx/aic94xx_init.c
b/drivers/scsi/aic94xx/aic94xx_init.c
index 80e5b283fd81..cb8191afc1dc 100644
--- a/drivers/scsi/aic94xx/aic94xx_init.c
+++ b/drivers/scsi/aic94xx/aic94xx_init.c
@@ -1030,8 +1030,10 @@ static int __init aic94xx_init(void)
aic94xx_transport_template =
sas_domain_attach_transport(_transport_functions);
- if (!aic94xx_transport_template)
+ if (!aic94xx_transport_template) {
+ err = -ENODEV;
goto out_destroy_caches;
+ }
err = pci_register_driver(_pci_driver);
if (err)
Re: [PATCH] aha1542: convert to DMA mapping API
On Tue, Aug 07, 2018 at 07:30:17PM +0200, Ondrej Zary wrote:
> On Tuesday 07 August 2018 09:29:59 Christoph Hellwig wrote:
> > Looks like the dma allocation is too late. Updated version below:
> >
> > ...
>
> Crashes a bit later now:
Next try:
---
>From 9dd7a3eb89d1cc72c001daeb80e44de7c5fb5b00 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig
Date: Thu, 2 Aug 2018 14:22:46 +0200
Subject: aha1542: convert to DMA mapping API
aha1542 is one of the last users of the legacy isa_*_to_bus APIs, which
also isn't portable enough. Convert it to the proper DMA mapping API.
Signed-off-by: Christoph Hellwig
---
drivers/scsi/aha1542.c | 112 -
1 file changed, 77 insertions(+), 35 deletions(-)
diff --git a/drivers/scsi/aha1542.c b/drivers/scsi/aha1542.c
index 41add33e3f1f..fb9c8fdb5a09 100644
--- a/drivers/scsi/aha1542.c
+++ b/drivers/scsi/aha1542.c
@@ -58,8 +58,15 @@ struct aha1542_hostdata {
int aha1542_last_mbi_used;
int aha1542_last_mbo_used;
struct scsi_cmnd *int_cmds[AHA1542_MAILBOXES];
- struct mailbox mb[2 * AHA1542_MAILBOXES];
- struct ccb ccb[AHA1542_MAILBOXES];
+ struct mailbox *mb;
+ dma_addr_t mb_handle;
+ struct ccb *ccb;
+ dma_addr_t ccb_handle;
+};
+
+struct aha1542_cmd {
+ struct chain *chain;
+ dma_addr_t chain_handle;
};
static inline void aha1542_intr_reset(u16 base)
@@ -233,6 +240,17 @@ static int aha1542_test_port(struct Scsi_Host *sh)
return 1;
}
+static void aha1542_free_cmd(struct scsi_cmnd *cmd)
+{
+ struct aha1542_cmd *acmd = scsi_cmd_priv(cmd);
+ struct device *dev = cmd->device->host->dma_dev;
+
+ dma_free_coherent(dev, scsi_sg_count(cmd) * sizeof(struct chain),
+ acmd->chain, acmd->chain_handle);
+ acmd->chain = NULL;
+ scsi_dma_unmap(cmd);
+}
+
static irqreturn_t aha1542_interrupt(int irq, void *dev_id)
{
struct Scsi_Host *sh = dev_id;
@@ -303,7 +321,7 @@ static irqreturn_t aha1542_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
};
- mbo = (scsi2int(mb[mbi].ccbptr) - (isa_virt_to_bus([0]))) /
sizeof(struct ccb);
+ mbo = (scsi2int(mb[mbi].ccbptr) - aha1542->ccb_handle) /
sizeof(struct ccb);
mbistatus = mb[mbi].status;
mb[mbi].status = 0;
aha1542->aha1542_last_mbi_used = mbi;
@@ -331,8 +349,7 @@ static irqreturn_t aha1542_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
}
my_done = tmp_cmd->scsi_done;
- kfree(tmp_cmd->host_scribble);
- tmp_cmd->host_scribble = NULL;
+ aha1542_free_cmd(tmp_cmd);
/* Fetch the sense data, and tuck it away, in the required
slot. The
Adaptec automatically fetches it, and there is no guarantee
that
we will still have it in the cdb when we come back */
@@ -369,6 +386,7 @@ static irqreturn_t aha1542_interrupt(int irq, void *dev_id)
static int aha1542_queuecommand(struct Scsi_Host *sh, struct scsi_cmnd *cmd)
{
+ struct aha1542_cmd *acmd = scsi_cmd_priv(cmd);
struct aha1542_hostdata *aha1542 = shost_priv(sh);
u8 direction;
u8 target = cmd->device->id;
@@ -378,7 +396,6 @@ static int aha1542_queuecommand(struct Scsi_Host *sh,
struct scsi_cmnd *cmd)
int mbo, sg_count;
struct mailbox *mb = aha1542->mb;
struct ccb *ccb = aha1542->ccb;
- struct chain *cptr;
if (*cmd->cmnd == REQUEST_SENSE) {
/* Don't do the command - we have the sense data already */
@@ -398,15 +415,17 @@ static int aha1542_queuecommand(struct Scsi_Host *sh,
struct scsi_cmnd *cmd)
print_hex_dump_bytes("command: ", DUMP_PREFIX_NONE, cmd->cmnd,
cmd->cmd_len);
}
#endif
- if (bufflen) { /* allocate memory before taking host_lock */
- sg_count = scsi_sg_count(cmd);
- cptr = kmalloc_array(sg_count, sizeof(*cptr),
-GFP_KERNEL | GFP_DMA);
- if (!cptr)
+ sg_count = scsi_dma_map(cmd);
+ if (sg_count) {
+ acmd->chain = dma_alloc_coherent(sh->dma_dev,
+ sg_count * sizeof(struct chain),
+ >chain_handle, GFP_DMA);
+ if (!acmd->chain) {
+ scsi_dma_unmap(cmd);
return SCSI_MLQUEUE_HOST_BUSY;
+ }
} else {
- sg_count = 0;
- cptr = NULL;
+ acmd->chain = NULL;
}
/* Use the outgoing mailboxes in a round-robin fashion, because this
@@ -437,7 +456,8 @@ static int aha1542_queuecommand(struct Scsi_Host *sh,
struct scsi_cmnd *cmd)
shost_printk(KERN_DEBUG, sh, "Sending command (%d %p)...", mbo,
cmd->scsi_done);
#endif
-
Re: [PATCH v3] mpt3sas: correct reset of smid while clearing scsi tracker
On Wed, Aug 8, 2018 at 1:27 AM, Bart Van Assche wrote: > On Tue, 2018-08-07 at 21:46 +0530, Sreekanth Reddy wrote: >> [Sreekanth] In the patch description I mentioned that I have done a >> manual mistake and I am correcting with this patch. >> >> Hope I have answered all of your quires. > > Not yet unfortunately. Why do you consider the current implementation a > wrong? In function mpt3sas_base_clear_st(), st->cb_idx = 0xFF; st->direct_io = 0; st->smid = 0; atomic_set(>chain_lookup[st->smid - 1].chain_offset, 0); In the above code we can see that smid is reset to zero before resetting the chain_offset to zero in chain_lookup[] table for corresponding IO request entry (i.e. smid -1 th entry). So in the above code driver is logically resetting the chain_offset to zero for the -1th entry of the chain_lookup[] table for all the IO requests, which I am say it wrong. I am saying that above code should be like below, st->cb_idx = 0xFF; st->direct_io = 0; atomic_set(>chain_lookup[st->smid - 1].chain_offset, 0); st->smid = 0; I.e. reset the smid variable to zero only after resetting the chain_offset for the corresponding IO request (i.e. for smid -1 th entry) of the chain_lookup[] table. > Why is the patch at the start of this e-mail thread considered a > fix? Which behavior changes due to this patch? If this patch is a bug fix, > how can the bug be triggered and why is this patch considered the right way > to fix that bug? See while posting the patch "mpt3sas: Fix calltrace observed while running IO & reset" in hurry I made a manual mistake that I added "st->smid = 0" line before "atomic_set(>chain_lookup[st->smid - 1].chain_offset, 0);" line in mpt3sas_base_clear_st() function instead I should have added "st->smid = 0" line after the atomic set line. Which I am correcting my mistake with this patch. Thanks, Sreekanth > > Thanks, > > Bart. >
Re: [PATCH] qla2xxx: Fix issue reported by static checker for qla2x00_els_dcmd2_sp_done()
On Tue, 2018-08-07 at 20:39 -0700, Himanshu Madhani wrote: > From: Quinn Tran To me this seems to be a real bug fix rather than a patch that only suppresses a static checker complaint. If that is the case, please mention this in the patch subject. Thanks, Bart.
Re: [PATCH] scsi: aic94xx: fix an error code in aic94xx_init()
On Wed, Aug 08, 2018 at 03:16:57PM +0100, John Garry wrote: > On 08/08/2018 12:56, Dan Carpenter wrote: > > We accidentally return success instead of -ENODEV on this error path. > > Sorry to nitpick, but - as I see - the only way for > sas_domain_attach_transport() to fail is if the kzalloc() in > sas_attach_transport() fails, so should this be -ENOMEM? Other drivers > return this error code for this scenario. > Huh... I was so sure I looked up other callers to see what people normally return... Anyway, you're right. Let me resend. regards, dan carpenter
Re: [PATCH] mpt3sas: correct reset of smid while clearing scsi tracker
On Wed, 2018-07-18 at 01:22 -0400, Sreekanth Reddy wrote: > In mpt3sas_base_clear_st() function smid value is reseted in wrong line, > i.e. driver should reset smid value to zero after decrementing chain_offset > counter in chain_lookup table but in current code, driver is resetting smid > value before decrementing the chain_offset counter. which we are correcting > with this patch. > > Signed-off-by: Sreekanth Reddy > --- > drivers/scsi/mpt3sas/mpt3sas_base.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c > b/drivers/scsi/mpt3sas/mpt3sas_base.c > index 902610d..94b939b 100644 > --- a/drivers/scsi/mpt3sas/mpt3sas_base.c > +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c > @@ -3283,8 +3283,8 @@ void mpt3sas_base_clear_st(struct MPT3SAS_ADAPTER *ioc, > return; > st->cb_idx = 0xFF; > st->direct_io = 0; > - st->smid = 0; > atomic_set(>chain_lookup[st->smid - 1].chain_offset, 0); > + st->smid = 0; > } > > /** Reviewed-by: Bart Van Assche
[PATCH v2] scsi: aic94xx: fix an error code in aic94xx_init()
We accidentally return success instead of -ENOMEM on this error path.
Fixes: 2908d778ab3e ("[SCSI] aic94xx: new driver")
Signed-off-by: Dan Carpenter
---
v2: return -ENOMEM instead of -ENODEV
diff --git a/drivers/scsi/aic94xx/aic94xx_init.c
b/drivers/scsi/aic94xx/aic94xx_init.c
index 80e5b283fd81..cb8191afc1dc 100644
--- a/drivers/scsi/aic94xx/aic94xx_init.c
+++ b/drivers/scsi/aic94xx/aic94xx_init.c
@@ -1030,8 +1030,10 @@ static int __init aic94xx_init(void)
aic94xx_transport_template =
sas_domain_attach_transport(_transport_functions);
- if (!aic94xx_transport_template)
+ if (!aic94xx_transport_template) {
+ err = -ENOMEM;
goto out_destroy_caches;
+ }
err = pci_register_driver(_pci_driver);
if (err)
Re: [PATCH v2] scsi: aic94xx: fix an error code in aic94xx_init()
On 08/08/2018 15:29, Dan Carpenter wrote:
We accidentally return success instead of -ENOMEM on this error path.
Fixes: 2908d778ab3e ("[SCSI] aic94xx: new driver")
Signed-off-by: Dan Carpenter
thanks
Reviewed-by: John Garry
---
v2: return -ENOMEM instead of -ENODEV
diff --git a/drivers/scsi/aic94xx/aic94xx_init.c
b/drivers/scsi/aic94xx/aic94xx_init.c
index 80e5b283fd81..cb8191afc1dc 100644
--- a/drivers/scsi/aic94xx/aic94xx_init.c
+++ b/drivers/scsi/aic94xx/aic94xx_init.c
@@ -1030,8 +1030,10 @@ static int __init aic94xx_init(void)
aic94xx_transport_template =
sas_domain_attach_transport(_transport_functions);
- if (!aic94xx_transport_template)
+ if (!aic94xx_transport_template) {
+ err = -ENOMEM;
goto out_destroy_caches;
+ }
err = pci_register_driver(_pci_driver);
if (err)
.
Re: [PATCH v2 2/8] scsi: ufs: Add ufs-bsg module
On Sun, 2018-08-05 at 14:39 +0300, Avri Altman wrote: > A LLD companion for the ufs scsi transport. This description is misleading. How about changing this into "Add a bsg endpoint that supports UPIUs"? diff --git a/drivers/scsi/ufs/ufs_bsg.c b/drivers/scsi/ufs/ufs_bsg.c > new file mode 100644 > index 000..6f1941b > --- /dev/null > +++ b/drivers/scsi/ufs/ufs_bsg.c > @@ -0,0 +1,127 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * SCSI transport companion for UFS HBA This description is misleading too. Please consider to change this into something like "bsg endpoint that supports UPIUs". Thanks, Bart.
Re: [PATCH] aha1542: convert to DMA mapping API
On Wed, Aug 08, 2018 at 06:26:25PM +0200, Ondrej Zary wrote:
> Looks like you attached wrong file (same as the previous one).
This should be better:
---
>From afa1c18bd98168fb690d905fc84a2db6f0d617b6 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig
Date: Thu, 2 Aug 2018 14:22:46 +0200
Subject: aha1542: convert to DMA mapping API
aha1542 is one of the last users of the legacy isa_*_to_bus APIs, which
also isn't portable enough. Convert it to the proper DMA mapping API.
Signed-off-by: Christoph Hellwig
---
drivers/scsi/aha1542.c | 126 +
1 file changed, 89 insertions(+), 37 deletions(-)
diff --git a/drivers/scsi/aha1542.c b/drivers/scsi/aha1542.c
index 41add33e3f1f..d5385eb3a7a4 100644
--- a/drivers/scsi/aha1542.c
+++ b/drivers/scsi/aha1542.c
@@ -58,8 +58,15 @@ struct aha1542_hostdata {
int aha1542_last_mbi_used;
int aha1542_last_mbo_used;
struct scsi_cmnd *int_cmds[AHA1542_MAILBOXES];
- struct mailbox mb[2 * AHA1542_MAILBOXES];
- struct ccb ccb[AHA1542_MAILBOXES];
+ struct mailbox *mb;
+ dma_addr_t mb_handle;
+ struct ccb *ccb;
+ dma_addr_t ccb_handle;
+};
+
+struct aha1542_cmd {
+ struct chain *chain;
+ dma_addr_t chain_handle;
};
static inline void aha1542_intr_reset(u16 base)
@@ -233,6 +240,18 @@ static int aha1542_test_port(struct Scsi_Host *sh)
return 1;
}
+static void aha1542_free_cmd(struct scsi_cmnd *cmd)
+{
+ struct aha1542_cmd *acmd = scsi_cmd_priv(cmd);
+ struct device *dev = cmd->device->host->dma_dev;
+ size_t len = scsi_sg_count(cmd) * sizeof(struct chain);
+
+ dma_unmap_single(dev, acmd->chain_handle, len, DMA_TO_DEVICE);
+ kfree(acmd->chain);
+ acmd->chain = NULL;
+ scsi_dma_unmap(cmd);
+}
+
static irqreturn_t aha1542_interrupt(int irq, void *dev_id)
{
struct Scsi_Host *sh = dev_id;
@@ -303,7 +322,7 @@ static irqreturn_t aha1542_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
};
- mbo = (scsi2int(mb[mbi].ccbptr) - (isa_virt_to_bus([0]))) /
sizeof(struct ccb);
+ mbo = (scsi2int(mb[mbi].ccbptr) - aha1542->ccb_handle) /
sizeof(struct ccb);
mbistatus = mb[mbi].status;
mb[mbi].status = 0;
aha1542->aha1542_last_mbi_used = mbi;
@@ -331,8 +350,7 @@ static irqreturn_t aha1542_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
}
my_done = tmp_cmd->scsi_done;
- kfree(tmp_cmd->host_scribble);
- tmp_cmd->host_scribble = NULL;
+ aha1542_free_cmd(tmp_cmd);
/* Fetch the sense data, and tuck it away, in the required
slot. The
Adaptec automatically fetches it, and there is no guarantee
that
we will still have it in the cdb when we come back */
@@ -369,6 +387,7 @@ static irqreturn_t aha1542_interrupt(int irq, void *dev_id)
static int aha1542_queuecommand(struct Scsi_Host *sh, struct scsi_cmnd *cmd)
{
+ struct aha1542_cmd *acmd = scsi_cmd_priv(cmd);
struct aha1542_hostdata *aha1542 = shost_priv(sh);
u8 direction;
u8 target = cmd->device->id;
@@ -378,7 +397,6 @@ static int aha1542_queuecommand(struct Scsi_Host *sh,
struct scsi_cmnd *cmd)
int mbo, sg_count;
struct mailbox *mb = aha1542->mb;
struct ccb *ccb = aha1542->ccb;
- struct chain *cptr;
if (*cmd->cmnd == REQUEST_SENSE) {
/* Don't do the command - we have the sense data already */
@@ -398,15 +416,17 @@ static int aha1542_queuecommand(struct Scsi_Host *sh,
struct scsi_cmnd *cmd)
print_hex_dump_bytes("command: ", DUMP_PREFIX_NONE, cmd->cmnd,
cmd->cmd_len);
}
#endif
- if (bufflen) { /* allocate memory before taking host_lock */
- sg_count = scsi_sg_count(cmd);
- cptr = kmalloc_array(sg_count, sizeof(*cptr),
-GFP_KERNEL | GFP_DMA);
- if (!cptr)
- return SCSI_MLQUEUE_HOST_BUSY;
- } else {
- sg_count = 0;
- cptr = NULL;
+ sg_count = scsi_dma_map(cmd);
+ if (sg_count) {
+ size_t len = sg_count * sizeof(struct chain);
+
+ acmd->chain = kmalloc(len, GFP_DMA);
+ if (!acmd->chain)
+ goto out_unmap;
+ acmd->chain_handle = dma_map_single(sh->dma_dev, acmd->chain,
+ len, DMA_TO_DEVICE);
+ if (dma_mapping_error(sh->dma_dev, acmd->chain_handle))
+ goto out_free_chain;
}
/* Use the outgoing mailboxes in a round-robin fashion, because this
@@ -437,7 +457,8 @@ static int aha1542_queuecommand(struct Scsi_Host *sh,
struct scsi_cmnd *cmd)
shost_printk(KERN_DEBUG, sh, "Sending command
Re: [PATCH v2 6/8] scsi: ufs: Add API to execute raw upiu commands
On Sun, 2018-08-05 at 14:39 +0300, Avri Altman wrote: > + lrbp->command_type = (hba->ufs_version == UFSHCI_VERSION_10 || > + hba->ufs_version == UFSHCI_VERSION_11) ? > + UTP_CMD_TYPE_DEV_MANAGE : > + UTP_CMD_TYPE_UFS_STORAGE; Parentheses are not needed around the logical or expression because logical or has a higher precedence than ?: so please leave the parentheses out. Thanks, Bart.
Re: [PATCH] aha1542: convert to DMA mapping API
On Wednesday 08 August 2018 11:14:33 Christoph Hellwig wrote: > On Tue, Aug 07, 2018 at 07:30:17PM +0200, Ondrej Zary wrote: > > On Tuesday 07 August 2018 09:29:59 Christoph Hellwig wrote: > > > Looks like the dma allocation is too late. Updated version below: > > > > > > ... > > > > Crashes a bit later now: > > Next try: > > --- > From 9dd7a3eb89d1cc72c001daeb80e44de7c5fb5b00 Mon Sep 17 00:00:00 2001 > From: Christoph Hellwig > Date: Thu, 2 Aug 2018 14:22:46 +0200 > Subject: aha1542: convert to DMA mapping API Looks like you attached wrong file (same as the previous one). -- Ondrej Zary
Re: [PATCH v2 7/8] scsi: ufs-bsg: Add support for raw upiu in ufs_bsg_request()
On Sun, 2018-08-05 at 14:39 +0300, Avri Altman wrote: > + if (ret || !(*desc_len)) > + return -EINVAL; No parentheses around *desc_len please. Thanks, Bart.
Re: [PATCH v2 1/8] scsi: Add ufs transport class
On Sun, 2018-08-05 at 14:39 +0300, Avri Altman wrote: > A “ufs-port” is purely a software object. Evidently, the function > template takes no port as an argument, as the driver has no concept > of "port". We only need it as a hanging point in the bsg device tree, > so maybe a more appropriate term would be: "ufs-bsg-dev-node". > > The ufs-port has a pretty lean structure. This is because we are > using upiu transactions that contains the outmost detailed info, > so we don't really need complex constructs to support it. > > The transport will keep track of its ufs-ports via its scsi host. Since no port concept has been defined in the UFS standard, can the port concept be left out? Have you considered to attach the bsg queue directly to the UFS SCSI host? There are two struct device instances in each Scsi_Host structure, namely shost_gendev and shost_dev. I think the former corresponds to /sys/bus/scsi/devices/host and the latter corresponds to /sys/class/scsi_host/host. The bsg queue probably can be attached to the latter. Several SCSI drivers already add additional sysfs attributes to the /sys/class/scsi_host/host. Thanks, Bart.
[PATCH] scsi: aic94xx: fix an error code in aic94xx_init()
We accidentally return success instead of -ENODEV on this error path.
Fixes: 2908d778ab3e ("[SCSI] aic94xx: new driver")
Signed-off-by: Dan Carpenter
diff --git a/drivers/scsi/aic94xx/aic94xx_init.c
b/drivers/scsi/aic94xx/aic94xx_init.c
index 80e5b283fd81..cb8191afc1dc 100644
--- a/drivers/scsi/aic94xx/aic94xx_init.c
+++ b/drivers/scsi/aic94xx/aic94xx_init.c
@@ -1030,8 +1030,10 @@ static int __init aic94xx_init(void)
aic94xx_transport_template =
sas_domain_attach_transport(_transport_functions);
- if (!aic94xx_transport_template)
+ if (!aic94xx_transport_template) {
+ err = -ENODEV;
goto out_destroy_caches;
+ }
err = pci_register_driver(_pci_driver);
if (err)
Re: [PATCH] qla2xxx: Fix issue reported by static checker for qla2x00_els_dcmd2_sp_done()
Hi Bart, > On Aug 8, 2018, at 8:10 AM, Bart Van Assche wrote: > > External Email > > On Tue, 2018-08-07 at 20:39 -0700, Himanshu Madhani wrote: >> From: Quinn Tran > > To me this seems to be a real bug fix rather than a patch that only suppresses > a static checker complaint. If that is the case, please mention this in the > patch subject. > We have not run into this issue exposing this bug yet, until Static checker complained about it. But Yes, this is bug in the code where we should return after freeing sp. > Thanks, > > Bart. > Thanks, - Himanshu
[PATCH] target/iblock: split T10 PI SGL across command bios
When T10 PI is enabled on a backing device for the iblock backstore, the
PI SGL for the entire command is attached to the first bio only. This
works fine if the command is covered by a single bio, but results in
integrity verification errors for the other bios in a multi-bio command.
Split the PI SGL across the bios in the command, so each bip contains
the protection information for the data in the bio. In a multi-bio
command, store where we left off in the PI SGL so we know where to start
with the next bio.
Signed-off-by: Greg Edwards
---
This patch depends on commit 359f642700f2 ("block: move
bio_integrity_{intervals,bytes} into blkdev.h") in Jens' block for-next branch.
drivers/target/target_core_iblock.c | 60 +
1 file changed, 44 insertions(+), 16 deletions(-)
diff --git a/drivers/target/target_core_iblock.c
b/drivers/target/target_core_iblock.c
index ce1321a5cb7b..d3ab83282f61 100644
--- a/drivers/target/target_core_iblock.c
+++ b/drivers/target/target_core_iblock.c
@@ -635,7 +635,8 @@ static ssize_t iblock_show_configfs_dev_params(struct
se_device *dev, char *b)
}
static int
-iblock_alloc_bip(struct se_cmd *cmd, struct bio *bio)
+iblock_alloc_bip(struct se_cmd *cmd, struct bio *bio,
+struct scatterlist **sgl, unsigned int *skip)
{
struct se_device *dev = cmd->se_dev;
struct blk_integrity *bi;
@@ -643,6 +644,7 @@ iblock_alloc_bip(struct se_cmd *cmd, struct bio *bio)
struct iblock_dev *ib_dev = IBLOCK_DEV(dev);
struct scatterlist *sg;
int i, rc;
+ unsigned int size, nr_pages, len, off;
bi = bdev_get_integrity(ib_dev->ibd_bd);
if (!bi) {
@@ -650,32 +652,52 @@ iblock_alloc_bip(struct se_cmd *cmd, struct bio *bio)
return -ENODEV;
}
- bip = bio_integrity_alloc(bio, GFP_NOIO, cmd->t_prot_nents);
+ nr_pages = min_t(unsigned int, cmd->t_prot_nents, BIO_MAX_PAGES);
+ bip = bio_integrity_alloc(bio, GFP_NOIO, nr_pages);
if (IS_ERR(bip)) {
pr_err("Unable to allocate bio_integrity_payload\n");
return PTR_ERR(bip);
}
- bip->bip_iter.bi_size = (cmd->data_length / dev->dev_attrib.block_size)
*
-dev->prot_length;
- bip->bip_iter.bi_sector = bio->bi_iter.bi_sector;
+ bip->bip_iter.bi_size = bio_integrity_bytes(bi, bio_sectors(bio));
+ bip_set_seed(bip, bio->bi_iter.bi_sector);
pr_debug("IBLOCK BIP Size: %u Sector: %llu\n", bip->bip_iter.bi_size,
(unsigned long long)bip->bip_iter.bi_sector);
- for_each_sg(cmd->t_prot_sg, sg, cmd->t_prot_nents, i) {
+ size = bip->bip_iter.bi_size;
+ for_each_sg(*sgl, sg, nr_pages, i) {
- rc = bio_integrity_add_page(bio, sg_page(sg), sg->length,
- sg->offset);
- if (rc != sg->length) {
+ len = sg->length - *skip;
+ off = sg->offset + *skip;
+
+ if (*skip)
+ *skip = 0;
+
+ if (len > size) {
+ len = size;
+ *skip = len;
+ }
+
+ rc = bio_integrity_add_page(bio, sg_page(sg), len, off);
+ if (rc != len) {
pr_err("bio_integrity_add_page() failed; %d\n", rc);
return -ENOMEM;
}
pr_debug("Added bio integrity page: %p length: %d offset; %d\n",
-sg_page(sg), sg->length, sg->offset);
+sg_page(sg), len, off);
+
+ size -= len;
+ if (!size)
+ break;
}
+ if (*skip == 0)
+ *sgl = sg_next(sg);
+ else
+ *sgl = sg;
+
return 0;
}
@@ -686,12 +708,12 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist
*sgl, u32 sgl_nents,
struct se_device *dev = cmd->se_dev;
sector_t block_lba = target_to_linux_sector(dev, cmd->t_task_lba);
struct iblock_req *ibr;
- struct bio *bio, *bio_start;
+ struct bio *bio;
struct bio_list list;
- struct scatterlist *sg;
+ struct scatterlist *sg, *sg_prot = cmd->t_prot_sg;
u32 sg_num = sgl_nents;
- unsigned bio_cnt;
- int i, op, op_flags = 0;
+ unsigned int bio_cnt, skip = 0;
+ int i, rc, op, op_flags = 0;
if (data_direction == DMA_TO_DEVICE) {
struct iblock_dev *ib_dev = IBLOCK_DEV(dev);
@@ -726,7 +748,6 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist
*sgl, u32 sgl_nents,
if (!bio)
goto fail_free_ibr;
- bio_start = bio;
bio_list_init();
bio_list_add(, bio);
@@ -741,6 +762,13 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist
*sgl, u32 sgl_nents,
*/
while (bio_add_page(bio, sg_page(sg),
