Re: [PATCH 1/8] sd: Rely on the driver core for asynchronous probing

[Hannes Reinecke]

On 1/11/19 5:10 PM, Bart Van Assche wrote:

On Fri, 2019-01-11 at 08:31 +0100, Hannes Reinecke wrote:

Could you please check if this depends on any changes to the async
probing mechanism?


Hi Hannes,

Patches one and two depend on driver core changes. I was assuming that
these were already upstream but apparently that is not yet the case :-(
See also "Re: [driver-core PATCH v9 1/9] driver core: Establish order of
operations for device_add and device_del via bitflag"
https://lore.kernel.org/lkml/0a72b8db91f9151ecc7f215b465ec8e69adc239c.ca...@linux.intel.com/

Please state that in the patch description to avoid backports to 
versions which do not have this patchset included.


Cheers,

Hannes

[PATCH] scsi: csiostor: fix NULL pointer dereference in csio_vport_set_state()

[Varun Prakash]

Assign fc_vport to ln->fc_vport before calling
csio_fcoe_alloc_vnp() to avoid a NULL pointer
dereference in csio_vport_set_state().

ln->fc_vport is dereferenced in csio_vport_set_state().

Signed-off-by: Varun Prakash 
---
 drivers/scsi/csiostor/csio_attr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/csiostor/csio_attr.c 
b/drivers/scsi/csiostor/csio_attr.c
index 8a00403..9bd2bd8 100644
--- a/drivers/scsi/csiostor/csio_attr.c
+++ b/drivers/scsi/csiostor/csio_attr.c
@@ -594,12 +594,12 @@ csio_vport_create(struct fc_vport *fc_vport, bool disable)
}
 
fc_vport_set_state(fc_vport, FC_VPORT_INITIALIZING);
+   ln->fc_vport = fc_vport;
 
if (csio_fcoe_alloc_vnp(hw, ln))
goto error;
 
*(struct csio_lnode **)fc_vport->dd_data = ln;
-   ln->fc_vport = fc_vport;
if (!fc_vport->node_name)
fc_vport->node_name = wwn_to_u64(csio_ln_wwnn(ln));
if (!fc_vport->port_name)
-- 
2.0.2

Re: [PATCH] scsi: ufs: Consider device limitations for dma_mask

[Bjorn Andersson]

On Fri 11 Jan 15:33 PST 2019, Doug Anderson wrote:

> Hi,
> 
> On Fri, Jan 11, 2019 at 2:54 PM Bjorn Andersson
>  wrote:
> >
> > On Qualcomm SDM845 the capabilities of the UFS MEM controller states
> > that it's capable of dealing with 64 bit addresses, but DMA addresses
> > are truncated causing IOMMU faults when trying to issue operations.
> >
> > Limit the DMA mask to that of the device, so that DMA allocations
> > is limited to the range supported by the bus and device and not just
> > following what the controller's capabilities states.
> >
> > Signed-off-by: Bjorn Andersson 
> > ---
> >  drivers/scsi/ufs/ufshcd.c | 13 -
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 9ba7671b84f8..dc0eb59dd46f 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -8151,11 +8151,14 @@ EXPORT_SYMBOL_GPL(ufshcd_dealloc_host);
> >   */
> >  static int ufshcd_set_dma_mask(struct ufs_hba *hba)
> >  {
> > -   if (hba->capabilities & MASK_64_ADDRESSING_SUPPORT) {
> > -   if (!dma_set_mask_and_coherent(hba->dev, DMA_BIT_MASK(64)))
> > -   return 0;
> > -   }
> > -   return dma_set_mask_and_coherent(hba->dev, DMA_BIT_MASK(32));
> > +   u64 dma_mask = dma_get_mask(hba->dev);
> > +
> > +   if (hba->capabilities & MASK_64_ADDRESSING_SUPPORT)
> > +   dma_mask &= DMA_BIT_MASK(64);
> > +   else
> > +   dma_mask &= DMA_BIT_MASK(32);
> 
> Just because I'm annoying like that, I'll point out  that the above is
> a bit on the silly side.  Instead I'd do:
> 
> if (!(hba->capabilities & MASK_64_ADDRESSING_SUPPORT))
> dma_mask &= DMA_BIT_MASK(32);
> 
> AKA: your code is masking a 64-bit variable with a value that is known
> to be 0x, which is kinda a no-op.
> 

You're right, so I took a stab at reworking the patch, but we end up
with something:

u64 dma_mask;

if (!(hba->capabilities & MASK_64_ADDRESSING_SUPPORT)) {
dma_mask = dma_get_mask(hba->dev);
dma_mash &= DMA_BIT_MASK(32);
return dma_set_mask_and_coherent(hba->dev, dma_mask);
}

return 0;
}

Which makes me feel I need a comment here describing that what happens
in the 64-bit case (i.e. nothing). So I think the proposed form is
clearer, even though the compiler is expected to optimize away one of
the branches.

James, Martin, do you have a preference?

> 
> ...other than the nit, this seems sane to me.
> 
> Reviewed-by: Douglas Anderson 
> Tested-by: Douglas Anderson 

Thanks,
Bjorn