* [PATCH 10/13] megaraid_sas: set virt_boundary_mask in the scsi host
From: Christoph Hellwig @ 2019-06-05 19:08 UTC (permalink / raw)
To: Jens Axboe
Cc: Sebastian Ott, Sagi Grimberg, Max Gurtovoy, Bart Van Assche,
Ulf Hansson, Alan Stern, Oliver Neukum, linux-block, linux-rdma,
linux-mmc, linux-nvme, linux-scsi, megaraidlinux.pdl,
MPT-FusionLinux.pdl, linux-hyperv, linux-usb, usb-storage,
linux-kernel
In-Reply-To: <20190605190836.32354-1-hch@lst.de>
This ensures all proper DMA layer handling is taken care of by the
SCSI midlayer. Note that the effect is global, as the IOMMU merging
is based off a paramters in struct device. We could still turn if off
if no PCIe devices are present, but I don't know how to find that out.
Also remove the bogus nomerges flag, merges do take the virt_boundary
into account.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/scsi/megaraid/megaraid_sas_base.c | 46 +++++----------------
drivers/scsi/megaraid/megaraid_sas_fusion.c | 7 ++++
2 files changed, 18 insertions(+), 35 deletions(-)
diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index 3dd1df472dc6..20b3b3f8bc16 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -1870,39 +1870,6 @@ void megasas_set_dynamic_target_properties(struct scsi_device *sdev,
}
}
-/*
- * megasas_set_nvme_device_properties -
- * set nomerges=2
- * set virtual page boundary = 4K (current mr_nvme_pg_size is 4K).
- * set maximum io transfer = MDTS of NVME device provided by MR firmware.
- *
- * MR firmware provides value in KB. Caller of this function converts
- * kb into bytes.
- *
- * e.a MDTS=5 means 2^5 * nvme page size. (In case of 4K page size,
- * MR firmware provides value 128 as (32 * 4K) = 128K.
- *
- * @sdev: scsi device
- * @max_io_size: maximum io transfer size
- *
- */
-static inline void
-megasas_set_nvme_device_properties(struct scsi_device *sdev, u32 max_io_size)
-{
- struct megasas_instance *instance;
- u32 mr_nvme_pg_size;
-
- instance = (struct megasas_instance *)sdev->host->hostdata;
- mr_nvme_pg_size = max_t(u32, instance->nvme_page_size,
- MR_DEFAULT_NVME_PAGE_SIZE);
-
- blk_queue_max_hw_sectors(sdev->request_queue, (max_io_size / 512));
-
- blk_queue_flag_set(QUEUE_FLAG_NOMERGES, sdev->request_queue);
- blk_queue_virt_boundary(sdev->request_queue, mr_nvme_pg_size - 1);
-}
-
-
/*
* megasas_set_static_target_properties -
* Device property set by driver are static and it is not required to be
@@ -1961,8 +1928,10 @@ static void megasas_set_static_target_properties(struct scsi_device *sdev,
max_io_size_kb = le32_to_cpu(instance->tgt_prop->max_io_size_kb);
}
- if (instance->nvme_page_size && max_io_size_kb)
- megasas_set_nvme_device_properties(sdev, (max_io_size_kb << 10));
+ if (instance->nvme_page_size && max_io_size_kb) {
+ blk_queue_max_hw_sectors(sdev->request_queue,
+ (max_io_size_kb << 10) / 512);
+ }
scsi_change_queue_depth(sdev, device_qd);
@@ -6258,6 +6227,7 @@ static int megasas_start_aen(struct megasas_instance *instance)
static int megasas_io_attach(struct megasas_instance *instance)
{
struct Scsi_Host *host = instance->host;
+ u32 nvme_page_size = instance->nvme_page_size;
/*
* Export parameters required by SCSI mid-layer
@@ -6298,6 +6268,12 @@ static int megasas_io_attach(struct megasas_instance *instance)
host->max_lun = MEGASAS_MAX_LUN;
host->max_cmd_len = 16;
+ if (nvme_page_size) {
+ if (nvme_page_size > MR_DEFAULT_NVME_PAGE_SIZE)
+ nvme_page_size = MR_DEFAULT_NVME_PAGE_SIZE;
+ host->virt_boundary_mask = nvme_page_size - 1;
+ }
+
/*
* Notify the mid-layer about the new controller
*/
diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c b/drivers/scsi/megaraid/megaraid_sas_fusion.c
index 4dfa0685a86c..a9ff3a648e7b 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
@@ -1935,6 +1935,13 @@ megasas_is_prp_possible(struct megasas_instance *instance,
build_prp = true;
}
+/*
+ * XXX: All the code following should go away. The block layer guarantees
+ * merging according to the virt boundary. And while we might have had some
+ * issues with that in the past we fixed them, and any new bug should be fixed
+ * in the core code as well.
+ */
+
/*
* Below code detects gaps/holes in IO data buffers.
* What does holes/gaps mean?
--
2.20.1
^ permalink raw reply related
* [PATCH 09/13] IB/srp: set virt_boundary_mask in the scsi host
From: Christoph Hellwig @ 2019-06-05 19:08 UTC (permalink / raw)
To: Jens Axboe
Cc: Sebastian Ott, Sagi Grimberg, Max Gurtovoy, Bart Van Assche,
Ulf Hansson, Alan Stern, Oliver Neukum, linux-block, linux-rdma,
linux-mmc, linux-nvme, linux-scsi, megaraidlinux.pdl,
MPT-FusionLinux.pdl, linux-hyperv, linux-usb, usb-storage,
linux-kernel
In-Reply-To: <20190605190836.32354-1-hch@lst.de>
This ensures all proper DMA layer handling is taken care of by the
SCSI midlayer.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/infiniband/ulp/srp/ib_srp.c | 18 +++---------------
1 file changed, 3 insertions(+), 15 deletions(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index be9ddcad8f28..944fe8eee1ea 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -3061,20 +3061,6 @@ static int srp_target_alloc(struct scsi_target *starget)
return 0;
}
-static int srp_slave_alloc(struct scsi_device *sdev)
-{
- struct Scsi_Host *shost = sdev->host;
- struct srp_target_port *target = host_to_target(shost);
- struct srp_device *srp_dev = target->srp_host->srp_dev;
- struct ib_device *ibdev = srp_dev->dev;
-
- if (!(ibdev->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG))
- blk_queue_virt_boundary(sdev->request_queue,
- ~srp_dev->mr_page_mask);
-
- return 0;
-}
-
static int srp_slave_configure(struct scsi_device *sdev)
{
struct Scsi_Host *shost = sdev->host;
@@ -3277,7 +3263,6 @@ static struct scsi_host_template srp_template = {
.name = "InfiniBand SRP initiator",
.proc_name = DRV_NAME,
.target_alloc = srp_target_alloc,
- .slave_alloc = srp_slave_alloc,
.slave_configure = srp_slave_configure,
.info = srp_target_info,
.queuecommand = srp_queuecommand,
@@ -3812,6 +3797,9 @@ static ssize_t srp_create_target(struct device *dev,
target_host->max_cmd_len = sizeof ((struct srp_cmd *) (void *) 0L)->cdb;
target_host->max_segment_size = ib_dma_max_seg_size(ibdev);
+ if (!(ibdev->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG))
+ target_host->virt_boundary_mask = ~srp_dev->mr_page_mask;
+
target = host_to_target(target_host);
target->net = kobj_ns_grab_current(KOBJ_NS_TYPE_NET);
--
2.20.1
^ permalink raw reply related
* [PATCH 08/13] IB/iser: set virt_boundary_mask in the scsi host
From: Christoph Hellwig @ 2019-06-05 19:08 UTC (permalink / raw)
To: Jens Axboe
Cc: Sebastian Ott, Sagi Grimberg, Max Gurtovoy, Bart Van Assche,
Ulf Hansson, Alan Stern, Oliver Neukum, linux-block, linux-rdma,
linux-mmc, linux-nvme, linux-scsi, megaraidlinux.pdl,
MPT-FusionLinux.pdl, linux-hyperv, linux-usb, usb-storage,
linux-kernel
In-Reply-To: <20190605190836.32354-1-hch@lst.de>
This ensures all proper DMA layer handling is taken care of by the
SCSI midlayer.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/infiniband/ulp/iser/iscsi_iser.c | 35 +++++-------------------
1 file changed, 7 insertions(+), 28 deletions(-)
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 9c185a8dabd3..841b66397a57 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -613,6 +613,7 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
struct Scsi_Host *shost;
struct iser_conn *iser_conn = NULL;
struct ib_conn *ib_conn;
+ struct ib_device *ib_dev;
u32 max_fr_sectors;
shost = iscsi_host_alloc(&iscsi_iser_sht, 0, 0);
@@ -643,16 +644,19 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
}
ib_conn = &iser_conn->ib_conn;
+ ib_dev = ib_conn->device->ib_device;
if (ib_conn->pi_support) {
- u32 sig_caps = ib_conn->device->ib_device->attrs.sig_prot_cap;
+ u32 sig_caps = ib_dev->attrs.sig_prot_cap;
scsi_host_set_prot(shost, iser_dif_prot_caps(sig_caps));
scsi_host_set_guard(shost, SHOST_DIX_GUARD_IP |
SHOST_DIX_GUARD_CRC);
}
- if (iscsi_host_add(shost,
- ib_conn->device->ib_device->dev.parent)) {
+ if (!(ib_dev->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG))
+ shost->virt_boundary_mask = ~MASK_4K;
+
+ if (iscsi_host_add(shost, ib_dev->dev.parent)) {
mutex_unlock(&iser_conn->state_mutex);
goto free_host;
}
@@ -958,30 +962,6 @@ static umode_t iser_attr_is_visible(int param_type, int param)
return 0;
}
-static int iscsi_iser_slave_alloc(struct scsi_device *sdev)
-{
- struct iscsi_session *session;
- struct iser_conn *iser_conn;
- struct ib_device *ib_dev;
-
- mutex_lock(&unbind_iser_conn_mutex);
-
- session = starget_to_session(scsi_target(sdev))->dd_data;
- iser_conn = session->leadconn->dd_data;
- if (!iser_conn) {
- mutex_unlock(&unbind_iser_conn_mutex);
- return -ENOTCONN;
- }
- ib_dev = iser_conn->ib_conn.device->ib_device;
-
- if (!(ib_dev->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG))
- blk_queue_virt_boundary(sdev->request_queue, ~MASK_4K);
-
- mutex_unlock(&unbind_iser_conn_mutex);
-
- return 0;
-}
-
static struct scsi_host_template iscsi_iser_sht = {
.module = THIS_MODULE,
.name = "iSCSI Initiator over iSER",
@@ -994,7 +974,6 @@ static struct scsi_host_template iscsi_iser_sht = {
.eh_device_reset_handler= iscsi_eh_device_reset,
.eh_target_reset_handler = iscsi_eh_recover_target,
.target_alloc = iscsi_target_alloc,
- .slave_alloc = iscsi_iser_slave_alloc,
.proc_name = "iscsi_iser",
.this_id = -1,
.track_queue_depth = 1,
--
2.20.1
^ permalink raw reply related
* [PATCH 07/13] storvsc: set virt_boundary_mask in the scsi host template
From: Christoph Hellwig @ 2019-06-05 19:08 UTC (permalink / raw)
To: Jens Axboe
Cc: Sebastian Ott, Sagi Grimberg, Max Gurtovoy, Bart Van Assche,
Ulf Hansson, Alan Stern, Oliver Neukum, linux-block, linux-rdma,
linux-mmc, linux-nvme, linux-scsi, megaraidlinux.pdl,
MPT-FusionLinux.pdl, linux-hyperv, linux-usb, usb-storage,
linux-kernel
In-Reply-To: <20190605190836.32354-1-hch@lst.de>
This ensures all proper DMA layer handling is taken care of by the
SCSI midlayer.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/scsi/storvsc_drv.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 8472de1007ff..e61051c026f6 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1434,9 +1434,6 @@ static int storvsc_device_configure(struct scsi_device *sdevice)
{
blk_queue_rq_timeout(sdevice->request_queue, (storvsc_timeout * HZ));
- /* Ensure there are no gaps in presented sgls */
- blk_queue_virt_boundary(sdevice->request_queue, PAGE_SIZE - 1);
-
sdevice->no_write_same = 1;
/*
@@ -1709,6 +1706,8 @@ static struct scsi_host_template scsi_driver = {
.this_id = -1,
/* Make sure we dont get a sg segment crosses a page boundary */
.dma_boundary = PAGE_SIZE-1,
+ /* Ensure there are no gaps in presented sgls */
+ .virt_boundary_mask = PAGE_SIZE-1,
.no_write_same = 1,
.track_queue_depth = 1,
};
--
2.20.1
^ permalink raw reply related
* [PATCH 06/13] ufshcd: set max_segment_size in the scsi host template
From: Christoph Hellwig @ 2019-06-05 19:08 UTC (permalink / raw)
To: Jens Axboe
Cc: Sebastian Ott, Sagi Grimberg, Max Gurtovoy, Bart Van Assche,
Ulf Hansson, Alan Stern, Oliver Neukum, linux-block, linux-rdma,
linux-mmc, linux-nvme, linux-scsi, megaraidlinux.pdl,
MPT-FusionLinux.pdl, linux-hyperv, linux-usb, usb-storage,
linux-kernel
In-Reply-To: <20190605190836.32354-1-hch@lst.de>
We need to also mirror the value to the device to ensure IOMMU merging
doesn't undo it, and the SCSI host level parameter will ensure that.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/scsi/ufs/ufshcd.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 8c1c551f2b42..4e524ade489e 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -4586,8 +4586,6 @@ static int ufshcd_slave_configure(struct scsi_device *sdev)
struct request_queue *q = sdev->request_queue;
blk_queue_update_dma_pad(q, PRDT_DATA_BYTE_COUNT_PAD - 1);
- blk_queue_max_segment_size(q, PRDT_DATA_BYTE_COUNT_MAX);
-
return 0;
}
@@ -6990,6 +6988,7 @@ static struct scsi_host_template ufshcd_driver_template = {
.sg_tablesize = SG_ALL,
.cmd_per_lun = UFSHCD_CMD_PER_LUN,
.can_queue = UFSHCD_CAN_QUEUE,
+ .max_segment_size = PRDT_DATA_BYTE_COUNT_MAX,
.max_host_blocked = 1,
.track_queue_depth = 1,
.sdev_groups = ufshcd_driver_groups,
--
2.20.1
^ permalink raw reply related
* [PATCH 05/13] scsi: add a host / host template field for the virt boundary
From: Christoph Hellwig @ 2019-06-05 19:08 UTC (permalink / raw)
To: Jens Axboe
Cc: Sebastian Ott, Sagi Grimberg, Max Gurtovoy, Bart Van Assche,
Ulf Hansson, Alan Stern, Oliver Neukum, linux-block, linux-rdma,
linux-mmc, linux-nvme, linux-scsi, megaraidlinux.pdl,
MPT-FusionLinux.pdl, linux-hyperv, linux-usb, usb-storage,
linux-kernel
In-Reply-To: <20190605190836.32354-1-hch@lst.de>
This allows drivers setting it up easily instead of branching out to
block layer calls in slave_alloc, and ensures the upgraded
max_segment_size setting gets picked up by the DMA layer.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/scsi/hosts.c | 3 +++
drivers/scsi/scsi_lib.c | 3 ++-
include/scsi/scsi_host.h | 3 +++
3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index ff0d8c6a8d0c..55522b7162d3 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -462,6 +462,9 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
else
shost->dma_boundary = 0xffffffff;
+ if (sht->virt_boundary_mask)
+ shost->virt_boundary_mask = sht->virt_boundary_mask;
+
device_initialize(&shost->shost_gendev);
dev_set_name(&shost->shost_gendev, "host%d", shost->host_no);
shost->shost_gendev.bus = &scsi_bus_type;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 65d0a10c76ad..d333bb6b1c59 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1775,7 +1775,8 @@ void __scsi_init_queue(struct Scsi_Host *shost, struct request_queue *q)
dma_set_seg_boundary(dev, shost->dma_boundary);
blk_queue_max_segment_size(q, shost->max_segment_size);
- dma_set_max_seg_size(dev, shost->max_segment_size);
+ blk_queue_virt_boundary(q, shost->virt_boundary_mask);
+ dma_set_max_seg_size(dev, queue_max_segment_size(q));
/*
* Set a reasonable default alignment: The larger of 32-byte (dword),
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index a5fcdad4a03e..cc139dbd71e5 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -369,6 +369,8 @@ struct scsi_host_template {
*/
unsigned long dma_boundary;
+ unsigned long virt_boundary_mask;
+
/*
* This specifies "machine infinity" for host templates which don't
* limit the transfer size. Note this limit represents an absolute
@@ -587,6 +589,7 @@ struct Scsi_Host {
unsigned int max_sectors;
unsigned int max_segment_size;
unsigned long dma_boundary;
+ unsigned long virt_boundary_mask;
/*
* In scsi-mq mode, the number of hardware queues supported by the LLD.
*
--
2.20.1
^ permalink raw reply related
* [PATCH 04/13] mmc: also set max_segment_size in the device
From: Christoph Hellwig @ 2019-06-05 19:08 UTC (permalink / raw)
To: Jens Axboe
Cc: Sebastian Ott, Sagi Grimberg, Max Gurtovoy, Bart Van Assche,
Ulf Hansson, Alan Stern, Oliver Neukum, linux-block, linux-rdma,
linux-mmc, linux-nvme, linux-scsi, megaraidlinux.pdl,
MPT-FusionLinux.pdl, linux-hyperv, linux-usb, usb-storage,
linux-kernel
In-Reply-To: <20190605190836.32354-1-hch@lst.de>
If we only set the max_segment_size on the queue an IOMMU merge might
create bigger segments again, so limit the IOMMU merges as well.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/mmc/core/queue.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index b5b9c6142f08..92900a095796 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -377,6 +377,8 @@ static void mmc_setup_queue(struct mmc_queue *mq, struct mmc_card *card)
blk_queue_max_segment_size(mq->queue,
round_down(host->max_seg_size, block_size));
+ dma_set_max_seg_size(mmc_dev(host), queue_max_segment_size(mq->queue));
+
INIT_WORK(&mq->recovery_work, mmc_mq_recovery_handler);
INIT_WORK(&mq->complete_work, mmc_blk_mq_complete_work);
--
2.20.1
^ permalink raw reply related
* [PATCH 03/13] mtip32xx: also set max_segment_size in the device
From: Christoph Hellwig @ 2019-06-05 19:08 UTC (permalink / raw)
To: Jens Axboe
Cc: Sebastian Ott, Sagi Grimberg, Max Gurtovoy, Bart Van Assche,
Ulf Hansson, Alan Stern, Oliver Neukum, linux-block, linux-rdma,
linux-mmc, linux-nvme, linux-scsi, megaraidlinux.pdl,
MPT-FusionLinux.pdl, linux-hyperv, linux-usb, usb-storage,
linux-kernel
In-Reply-To: <20190605190836.32354-1-hch@lst.de>
If we only set the max_segment_size on the queue an IOMMU merge might
create bigger segments again, so limit the IOMMU merges as well.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/block/mtip32xx/mtip32xx.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
index bacfdac7161c..a14b09ab3a41 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -3676,6 +3676,7 @@ static int mtip_block_initialize(struct driver_data *dd)
blk_queue_physical_block_size(dd->queue, 4096);
blk_queue_max_hw_sectors(dd->queue, 0xffff);
blk_queue_max_segment_size(dd->queue, 0x400000);
+ dma_set_max_seg_size(&dd->pdev->dev, 0x400000);
blk_queue_io_min(dd->queue, 4096);
/* Set the capacity of the device in 512 byte sectors. */
--
2.20.1
^ permalink raw reply related
* [PATCH 02/13] rsxx: don't call dma_set_max_seg_size
From: Christoph Hellwig @ 2019-06-05 19:08 UTC (permalink / raw)
To: Jens Axboe
Cc: Sebastian Ott, Sagi Grimberg, Max Gurtovoy, Bart Van Assche,
Ulf Hansson, Alan Stern, Oliver Neukum, linux-block, linux-rdma,
linux-mmc, linux-nvme, linux-scsi, megaraidlinux.pdl,
MPT-FusionLinux.pdl, linux-hyperv, linux-usb, usb-storage,
linux-kernel
In-Reply-To: <20190605190836.32354-1-hch@lst.de>
This driver does never uses dma_map_sg, so the setting is rather
pointless.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/block/rsxx/core.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/block/rsxx/core.c b/drivers/block/rsxx/core.c
index de9b2d2f8654..76b73ddf8fd7 100644
--- a/drivers/block/rsxx/core.c
+++ b/drivers/block/rsxx/core.c
@@ -767,7 +767,6 @@ static int rsxx_pci_probe(struct pci_dev *dev,
goto failed_enable;
pci_set_master(dev);
- dma_set_max_seg_size(&dev->dev, RSXX_HW_BLK_SIZE);
st = dma_set_mask(&dev->dev, DMA_BIT_MASK(64));
if (st) {
--
2.20.1
^ permalink raw reply related
* [PATCH 01/13] nvme-pci: don't limit DMA segement size
From: Christoph Hellwig @ 2019-06-05 19:08 UTC (permalink / raw)
To: Jens Axboe
Cc: Sebastian Ott, Sagi Grimberg, Max Gurtovoy, Bart Van Assche,
Ulf Hansson, Alan Stern, Oliver Neukum, linux-block, linux-rdma,
linux-mmc, linux-nvme, linux-scsi, megaraidlinux.pdl,
MPT-FusionLinux.pdl, linux-hyperv, linux-usb, usb-storage,
linux-kernel
In-Reply-To: <20190605190836.32354-1-hch@lst.de>
NVMe uses PRPs (or optionally unlimited SGLs) for data transfers and
has no specific limit for a single DMA segement. Limiting the size
will cause problems because the block layer assumes PRP-ish devices
using a virt boundary mask don't have a segment limit. And while this
is true, we also really need to tell the DMA mapping layer about it,
otherwise dma-debug will trip over it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reported-by: Sebastian Ott <sebott@linux.ibm.com>
---
drivers/nvme/host/pci.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index f562154551ce..524d6bd6d095 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2513,6 +2513,12 @@ static void nvme_reset_work(struct work_struct *work)
*/
dev->ctrl.max_hw_sectors = NVME_MAX_KB_SZ << 1;
dev->ctrl.max_segments = NVME_MAX_SEGS;
+
+ /*
+ * Don't limit the IOMMU merged segment size.
+ */
+ dma_set_max_seg_size(dev->dev, 0xffffffff);
+
mutex_unlock(&dev->shutdown_lock);
/*
--
2.20.1
^ permalink raw reply related
* properly communicate queue limits to the DMA layer
From: Christoph Hellwig @ 2019-06-05 19:08 UTC (permalink / raw)
To: Jens Axboe
Cc: Sebastian Ott, Sagi Grimberg, Max Gurtovoy, Bart Van Assche,
Ulf Hansson, Alan Stern, Oliver Neukum, linux-block, linux-rdma,
linux-mmc, linux-nvme, linux-scsi, megaraidlinux.pdl,
MPT-FusionLinux.pdl, linux-hyperv, linux-usb, usb-storage,
linux-kernel
Hi Jens,
we've always had a bit of a problem communicating the block layer
queue limits to the DMA layer, which needs to respect them when
an IOMMU that could merge segments is used. Unfortunately most
drivers don't get this right. Oddly enough we've been mostly
getting away with it, although lately dma-debug has been catching
a few of those issues.
The segment merging fix for devices with PRP-like structures seems
to have escalated this a bit. The first patch fixes the actual
report from Sebastian, while the rest fix every drivers that appears
to have the problem based on a code audit looking for drivers using
blk_queue_max_segment_size, blk_queue_segment_boundary or
blk_queue_virt_boundary and calling dma_map_sg eventually. Note
that for SCSI drivers I've taken the blk_queue_virt_boundary setting
to the SCSI core, similar to how we did it for the other two settings
a while ago. This also deals with the fact that the DMA layer
settings are on a per-device granularity, so the per-device settings
in a few SCSI drivers can't actually work in an IOMMU environment.
It would be nice to eventually pass these limits as arguments to
dma_map_sg, but that is a far too big series for the 5.2 merge
window.
^ permalink raw reply
* Re: [PATCH] RDMA/ucma: Use struct_size() helper
From: Leon Romanovsky @ 2019-06-05 4:29 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Doug Ledford, Jason Gunthorpe, linux-rdma, linux-kernel
In-Reply-To: <20190604154222.GA8938@embeddedor>
On Tue, Jun 04, 2019 at 10:42:22AM -0500, Gustavo A. R. Silva wrote:
> Make use of the struct_size() helper instead of an open-coded version
> in order to avoid any potential type mistakes, in particular in the
> context in which this code is being used.
What does "in particular in the context in which this code is being
used" mean?
>
> So, replace the following form:
>
> sizeof(*resp) + (i * sizeof(struct ib_path_rec_data))
>
> with:
>
> struct_size(resp, path_data, i)
It is already written inside commit itself.
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> drivers/infiniband/core/ucma.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> index 140a338a135f..cbe460076611 100644
> --- a/drivers/infiniband/core/ucma.c
> +++ b/drivers/infiniband/core/ucma.c
> @@ -951,8 +951,7 @@ static ssize_t ucma_query_path(struct ucma_context *ctx,
> }
> }
>
> - if (copy_to_user(response, resp,
> - sizeof(*resp) + (i * sizeof(struct ib_path_rec_data))))
> + if (copy_to_user(response, resp, struct_size(resp, path_data, i)))
> ret = -EFAULT;
>
> kfree(resp);
> --
> 2.21.0
>
^ permalink raw reply
* [PATCH] RDMA/ucma: Use struct_size() helper
From: Gustavo A. R. Silva @ 2019-06-04 15:42 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: linux-rdma, linux-kernel, Gustavo A. R. Silva
Make use of the struct_size() helper instead of an open-coded version
in order to avoid any potential type mistakes, in particular in the
context in which this code is being used.
So, replace the following form:
sizeof(*resp) + (i * sizeof(struct ib_path_rec_data))
with:
struct_size(resp, path_data, i)
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
drivers/infiniband/core/ucma.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
index 140a338a135f..cbe460076611 100644
--- a/drivers/infiniband/core/ucma.c
+++ b/drivers/infiniband/core/ucma.c
@@ -951,8 +951,7 @@ static ssize_t ucma_query_path(struct ucma_context *ctx,
}
}
- if (copy_to_user(response, resp,
- sizeof(*resp) + (i * sizeof(struct ib_path_rec_data))))
+ if (copy_to_user(response, resp, struct_size(resp, path_data, i)))
ret = -EFAULT;
kfree(resp);
--
2.21.0
^ permalink raw reply related
* Re: [PATCH v16 12/16] IB, arm64: untag user pointers in ib_uverbs_(re)reg_mr()
From: Andrey Konovalov @ 2019-06-04 13:09 UTC (permalink / raw)
To: Jason Gunthorpe, Catalin Marinas
Cc: Linux ARM, Linux Memory Management List, LKML, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, open list:KERNEL SELFTEST FRAMEWORK,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Kees Cook, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig <Christian.Koenig@
In-Reply-To: <20190604130207.GD15385@ziepe.ca>
On Tue, Jun 4, 2019 at 3:02 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Tue, Jun 04, 2019 at 02:45:32PM +0200, Andrey Konovalov wrote:
> > On Tue, Jun 4, 2019 at 2:27 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > >
> > > On Tue, Jun 04, 2019 at 02:18:19PM +0200, Andrey Konovalov wrote:
> > > > On Mon, Jun 3, 2019 at 7:46 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > > > >
> > > > > On Mon, Jun 03, 2019 at 06:55:14PM +0200, Andrey Konovalov wrote:
> > > > > > This patch is a part of a series that extends arm64 kernel ABI to allow to
> > > > > > pass tagged user pointers (with the top byte set to something else other
> > > > > > than 0x00) as syscall arguments.
> > > > > >
> > > > > > ib_uverbs_(re)reg_mr() use provided user pointers for vma lookups (through
> > > > > > e.g. mlx4_get_umem_mr()), which can only by done with untagged pointers.
> > > > > >
> > > > > > Untag user pointers in these functions.
> > > > > >
> > > > > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > > > > > drivers/infiniband/core/uverbs_cmd.c | 4 ++++
> > > > > > 1 file changed, 4 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> > > > > > index 5a3a1780ceea..f88ee733e617 100644
> > > > > > +++ b/drivers/infiniband/core/uverbs_cmd.c
> > > > > > @@ -709,6 +709,8 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs)
> > > > > > if (ret)
> > > > > > return ret;
> > > > > >
> > > > > > + cmd.start = untagged_addr(cmd.start);
> > > > > > +
> > > > > > if ((cmd.start & ~PAGE_MASK) != (cmd.hca_va & ~PAGE_MASK))
> > > > > > return -EINVAL;
> > > > >
> > > > > I feel like we shouldn't thave to do this here, surely the cmd.start
> > > > > should flow unmodified to get_user_pages, and gup should untag it?
> > > > >
> > > > > ie, this sort of direction for the IB code (this would be a giant
> > > > > patch, so I didn't have time to write it all, but I think it is much
> > > > > saner):
> > > >
> > > > Hi Jason,
> > > >
> > > > ib_uverbs_reg_mr() passes cmd.start to mlx4_get_umem_mr(), which calls
> > > > find_vma(), which only accepts untagged addresses. Could you explain
> > > > how your patch helps?
> > >
> > > That mlx4 is just a 'weird duck', it is not the normal flow, and I
> > > don't think the core code should be making special consideration for
> > > it.
> >
> > How do you think we should do untagging (or something else) to deal
> > with this 'weird duck' case?
>
> mlx4 should handle it around the call to find_vma like other patches
> do, ideally as part of the cast from a void __user * to the unsigned
> long that find_vma needs
So essentially what we had a few versions ago
(https://lkml.org/lkml/2019/4/30/785) plus changing unsigned longs to
__user * across all IB code? I think the second part is something
that's not related to this series and needs to be done separately. I
can move untagging back to mlx4_get_umem_mr() though.
Catalin, you've initially asked to to move untagging out of
mlx4_get_umem_mr(), do you have any comments on this?
>
> Jason
^ permalink raw reply
* Re: [PATCH v16 12/16] IB, arm64: untag user pointers in ib_uverbs_(re)reg_mr()
From: Jason Gunthorpe @ 2019-06-04 13:02 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Linux ARM, Linux Memory Management List, LKML, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, open list:KERNEL SELFTEST FRAMEWORK,
Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher
In-Reply-To: <CAAeHK+xyqwuJyviGhvU7L1wPZQF7Mf9g2vgKSsYmML3fV6NrXg@mail.gmail.com>
On Tue, Jun 04, 2019 at 02:45:32PM +0200, Andrey Konovalov wrote:
> On Tue, Jun 4, 2019 at 2:27 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> >
> > On Tue, Jun 04, 2019 at 02:18:19PM +0200, Andrey Konovalov wrote:
> > > On Mon, Jun 3, 2019 at 7:46 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > > >
> > > > On Mon, Jun 03, 2019 at 06:55:14PM +0200, Andrey Konovalov wrote:
> > > > > This patch is a part of a series that extends arm64 kernel ABI to allow to
> > > > > pass tagged user pointers (with the top byte set to something else other
> > > > > than 0x00) as syscall arguments.
> > > > >
> > > > > ib_uverbs_(re)reg_mr() use provided user pointers for vma lookups (through
> > > > > e.g. mlx4_get_umem_mr()), which can only by done with untagged pointers.
> > > > >
> > > > > Untag user pointers in these functions.
> > > > >
> > > > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > > > > drivers/infiniband/core/uverbs_cmd.c | 4 ++++
> > > > > 1 file changed, 4 insertions(+)
> > > > >
> > > > > diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> > > > > index 5a3a1780ceea..f88ee733e617 100644
> > > > > +++ b/drivers/infiniband/core/uverbs_cmd.c
> > > > > @@ -709,6 +709,8 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs)
> > > > > if (ret)
> > > > > return ret;
> > > > >
> > > > > + cmd.start = untagged_addr(cmd.start);
> > > > > +
> > > > > if ((cmd.start & ~PAGE_MASK) != (cmd.hca_va & ~PAGE_MASK))
> > > > > return -EINVAL;
> > > >
> > > > I feel like we shouldn't thave to do this here, surely the cmd.start
> > > > should flow unmodified to get_user_pages, and gup should untag it?
> > > >
> > > > ie, this sort of direction for the IB code (this would be a giant
> > > > patch, so I didn't have time to write it all, but I think it is much
> > > > saner):
> > >
> > > Hi Jason,
> > >
> > > ib_uverbs_reg_mr() passes cmd.start to mlx4_get_umem_mr(), which calls
> > > find_vma(), which only accepts untagged addresses. Could you explain
> > > how your patch helps?
> >
> > That mlx4 is just a 'weird duck', it is not the normal flow, and I
> > don't think the core code should be making special consideration for
> > it.
>
> How do you think we should do untagging (or something else) to deal
> with this 'weird duck' case?
mlx4 should handle it around the call to find_vma like other patches
do, ideally as part of the cast from a void __user * to the unsigned
long that find_vma needs
Jason
^ permalink raw reply
* Re: [PATCH v16 12/16] IB, arm64: untag user pointers in ib_uverbs_(re)reg_mr()
From: Andrey Konovalov @ 2019-06-04 12:45 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Linux ARM, Linux Memory Management List, LKML, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, open list:KERNEL SELFTEST FRAMEWORK,
Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher
In-Reply-To: <20190604122714.GA15385@ziepe.ca>
On Tue, Jun 4, 2019 at 2:27 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Tue, Jun 04, 2019 at 02:18:19PM +0200, Andrey Konovalov wrote:
> > On Mon, Jun 3, 2019 at 7:46 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > >
> > > On Mon, Jun 03, 2019 at 06:55:14PM +0200, Andrey Konovalov wrote:
> > > > This patch is a part of a series that extends arm64 kernel ABI to allow to
> > > > pass tagged user pointers (with the top byte set to something else other
> > > > than 0x00) as syscall arguments.
> > > >
> > > > ib_uverbs_(re)reg_mr() use provided user pointers for vma lookups (through
> > > > e.g. mlx4_get_umem_mr()), which can only by done with untagged pointers.
> > > >
> > > > Untag user pointers in these functions.
> > > >
> > > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > > > drivers/infiniband/core/uverbs_cmd.c | 4 ++++
> > > > 1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> > > > index 5a3a1780ceea..f88ee733e617 100644
> > > > +++ b/drivers/infiniband/core/uverbs_cmd.c
> > > > @@ -709,6 +709,8 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs)
> > > > if (ret)
> > > > return ret;
> > > >
> > > > + cmd.start = untagged_addr(cmd.start);
> > > > +
> > > > if ((cmd.start & ~PAGE_MASK) != (cmd.hca_va & ~PAGE_MASK))
> > > > return -EINVAL;
> > >
> > > I feel like we shouldn't thave to do this here, surely the cmd.start
> > > should flow unmodified to get_user_pages, and gup should untag it?
> > >
> > > ie, this sort of direction for the IB code (this would be a giant
> > > patch, so I didn't have time to write it all, but I think it is much
> > > saner):
> >
> > Hi Jason,
> >
> > ib_uverbs_reg_mr() passes cmd.start to mlx4_get_umem_mr(), which calls
> > find_vma(), which only accepts untagged addresses. Could you explain
> > how your patch helps?
>
> That mlx4 is just a 'weird duck', it is not the normal flow, and I
> don't think the core code should be making special consideration for
> it.
How do you think we should do untagging (or something else) to deal
with this 'weird duck' case?
>
> Jason
^ permalink raw reply
* Re: [PATCH v16 12/16] IB, arm64: untag user pointers in ib_uverbs_(re)reg_mr()
From: Jason Gunthorpe @ 2019-06-04 12:27 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Linux ARM, Linux Memory Management List, LKML, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, open list:KERNEL SELFTEST FRAMEWORK,
Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher
In-Reply-To: <CAAeHK+xy-dx4dLDLLj9dRzRNSVG9H5nDPPnjpYF38qKZNNCh_g@mail.gmail.com>
On Tue, Jun 04, 2019 at 02:18:19PM +0200, Andrey Konovalov wrote:
> On Mon, Jun 3, 2019 at 7:46 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> >
> > On Mon, Jun 03, 2019 at 06:55:14PM +0200, Andrey Konovalov wrote:
> > > This patch is a part of a series that extends arm64 kernel ABI to allow to
> > > pass tagged user pointers (with the top byte set to something else other
> > > than 0x00) as syscall arguments.
> > >
> > > ib_uverbs_(re)reg_mr() use provided user pointers for vma lookups (through
> > > e.g. mlx4_get_umem_mr()), which can only by done with untagged pointers.
> > >
> > > Untag user pointers in these functions.
> > >
> > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > > drivers/infiniband/core/uverbs_cmd.c | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> > > index 5a3a1780ceea..f88ee733e617 100644
> > > +++ b/drivers/infiniband/core/uverbs_cmd.c
> > > @@ -709,6 +709,8 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs)
> > > if (ret)
> > > return ret;
> > >
> > > + cmd.start = untagged_addr(cmd.start);
> > > +
> > > if ((cmd.start & ~PAGE_MASK) != (cmd.hca_va & ~PAGE_MASK))
> > > return -EINVAL;
> >
> > I feel like we shouldn't thave to do this here, surely the cmd.start
> > should flow unmodified to get_user_pages, and gup should untag it?
> >
> > ie, this sort of direction for the IB code (this would be a giant
> > patch, so I didn't have time to write it all, but I think it is much
> > saner):
>
> Hi Jason,
>
> ib_uverbs_reg_mr() passes cmd.start to mlx4_get_umem_mr(), which calls
> find_vma(), which only accepts untagged addresses. Could you explain
> how your patch helps?
That mlx4 is just a 'weird duck', it is not the normal flow, and I
don't think the core code should be making special consideration for
it.
Jason
^ permalink raw reply
* Re: [PATCH v16 12/16] IB, arm64: untag user pointers in ib_uverbs_(re)reg_mr()
From: Andrey Konovalov @ 2019-06-04 12:18 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Linux ARM, Linux Memory Management List, LKML, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, open list:KERNEL SELFTEST FRAMEWORK,
Catalin Marinas, Vincenzo Frascino, Will Deacon, Mark Rutland,
Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
Felix Kuehling, Alexander Deucher
In-Reply-To: <20190603174619.GC11474@ziepe.ca>
On Mon, Jun 3, 2019 at 7:46 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Mon, Jun 03, 2019 at 06:55:14PM +0200, Andrey Konovalov wrote:
> > This patch is a part of a series that extends arm64 kernel ABI to allow to
> > pass tagged user pointers (with the top byte set to something else other
> > than 0x00) as syscall arguments.
> >
> > ib_uverbs_(re)reg_mr() use provided user pointers for vma lookups (through
> > e.g. mlx4_get_umem_mr()), which can only by done with untagged pointers.
> >
> > Untag user pointers in these functions.
> >
> > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > drivers/infiniband/core/uverbs_cmd.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> > index 5a3a1780ceea..f88ee733e617 100644
> > +++ b/drivers/infiniband/core/uverbs_cmd.c
> > @@ -709,6 +709,8 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs)
> > if (ret)
> > return ret;
> >
> > + cmd.start = untagged_addr(cmd.start);
> > +
> > if ((cmd.start & ~PAGE_MASK) != (cmd.hca_va & ~PAGE_MASK))
> > return -EINVAL;
>
> I feel like we shouldn't thave to do this here, surely the cmd.start
> should flow unmodified to get_user_pages, and gup should untag it?
>
> ie, this sort of direction for the IB code (this would be a giant
> patch, so I didn't have time to write it all, but I think it is much
> saner):
Hi Jason,
ib_uverbs_reg_mr() passes cmd.start to mlx4_get_umem_mr(), which calls
find_vma(), which only accepts untagged addresses. Could you explain
how your patch helps?
Thanks!
>
> diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
> index 54628ef879f0ce..7b3b736c87c253 100644
> --- a/drivers/infiniband/core/umem.c
> +++ b/drivers/infiniband/core/umem.c
> @@ -193,7 +193,7 @@ EXPORT_SYMBOL(ib_umem_find_best_pgsz);
> * @access: IB_ACCESS_xxx flags for memory being pinned
> * @dmasync: flush in-flight DMA when the memory region is written
> */
> -struct ib_umem *ib_umem_get(struct ib_udata *udata, unsigned long addr,
> +struct ib_umem *ib_umem_get(struct ib_udata *udata, void __user *addr,
> size_t size, int access, int dmasync)
> {
> struct ib_ucontext *context;
> @@ -201,7 +201,7 @@ struct ib_umem *ib_umem_get(struct ib_udata *udata, unsigned long addr,
> struct page **page_list;
> unsigned long lock_limit;
> unsigned long new_pinned;
> - unsigned long cur_base;
> + void __user *cur_base;
> struct mm_struct *mm;
> unsigned long npages;
> int ret;
> diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> index 5a3a1780ceea4d..94389e7f12371f 100644
> --- a/drivers/infiniband/core/uverbs_cmd.c
> +++ b/drivers/infiniband/core/uverbs_cmd.c
> @@ -735,7 +735,8 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs)
> }
> }
>
> - mr = pd->device->ops.reg_user_mr(pd, cmd.start, cmd.length, cmd.hca_va,
> + mr = pd->device->ops.reg_user_mr(pd, u64_to_user_ptr(cmd.start),
> + cmd.length, cmd.hca_va,
> cmd.access_flags,
> &attrs->driver_udata);
> if (IS_ERR(mr)) {
> diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
> index 4d033796dcfcc2..bddbb952082fc5 100644
> --- a/drivers/infiniband/hw/mlx5/mr.c
> +++ b/drivers/infiniband/hw/mlx5/mr.c
> @@ -786,7 +786,7 @@ static int mr_cache_max_order(struct mlx5_ib_dev *dev)
> }
>
> static int mr_umem_get(struct mlx5_ib_dev *dev, struct ib_udata *udata,
> - u64 start, u64 length, int access_flags,
> + void __user *start, u64 length, int access_flags,
> struct ib_umem **umem, int *npages, int *page_shift,
> int *ncont, int *order)
> {
> @@ -1262,8 +1262,8 @@ struct ib_mr *mlx5_ib_reg_dm_mr(struct ib_pd *pd, struct ib_dm *dm,
> attr->access_flags, mode);
> }
>
> -struct ib_mr *mlx5_ib_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
> - u64 virt_addr, int access_flags,
> +struct ib_mr *mlx5_ib_reg_user_mr(struct ib_pd *pd, void __user *start,
> + u64 length, u64 virt_addr, int access_flags,
> struct ib_udata *udata)
> {
> struct mlx5_ib_dev *dev = to_mdev(pd->device);
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index ec6446864b08e9..b3c8eaaa35c760 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -2464,8 +2464,8 @@ struct ib_device_ops {
> struct ib_mr *(*reg_user_mr)(struct ib_pd *pd, u64 start, u64 length,
> u64 virt_addr, int mr_access_flags,
> struct ib_udata *udata);
> - int (*rereg_user_mr)(struct ib_mr *mr, int flags, u64 start, u64 length,
> - u64 virt_addr, int mr_access_flags,
> + int (*rereg_user_mr)(struct ib_mr *mr, int flags, void __user *start,
> + u64 length, u64 virt_addr, int mr_access_flags,
> struct ib_pd *pd, struct ib_udata *udata);
> int (*dereg_mr)(struct ib_mr *mr, struct ib_udata *udata);
> struct ib_mr *(*alloc_mr)(struct ib_pd *pd, enum ib_mr_type mr_type,
^ permalink raw reply
* Re: [PATCH v16 01/16] uaccess: add untagged_addr definition for other arches
From: Andrey Konovalov @ 2019-06-04 11:45 UTC (permalink / raw)
To: Khalid Aziz
Cc: Christoph Hellwig, Linux ARM, Linux Memory Management List, LKML,
amd-gfx, dri-devel, linux-rdma, linux-media, kvm,
open list:KERNEL SELFTEST FRAMEWORK, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Kees Cook, Yishai Hadas, Felix Kuehling
In-Reply-To: <7a687a26-fc3e-2caa-1d6a-464f1f7e684c@oracle.com>
On Mon, Jun 3, 2019 at 8:17 PM Khalid Aziz <khalid.aziz@oracle.com> wrote:
>
> On 6/3/19 11:29 AM, Christoph Hellwig wrote:
> > On Mon, Jun 03, 2019 at 11:24:35AM -0600, Khalid Aziz wrote:
> >> On 6/3/19 11:06 AM, Andrey Konovalov wrote:
> >>> On Mon, Jun 3, 2019 at 7:04 PM Khalid Aziz <khalid.aziz@oracle.com> wrote:
> >>>> Andrey,
> >>>>
> >>>> This patch has now become part of the other patch series Chris Hellwig
> >>>> has sent out -
> >>>> <https://lore.kernel.org/lkml/20190601074959.14036-1-hch@lst.de/>. Can
> >>>> you coordinate with that patch series?
> >>>
> >>> Hi!
> >>>
> >>> Yes, I've seen it. How should I coordinate? Rebase this series on top
> >>> of that one?
> >>
> >> That would be one way to do it. Better yet, separate this patch from
> >> both patch series, make it standalone and then rebase the two patch
> >> series on top of it.
> >
> > I think easiest would be to just ask Linus if he could make an exception
> > and include this trivial prep patch in 5.2-rc.
> >
>
> Andrey,
>
> Would you mind updating the commit log to make it not arm64 specific and
> sending this patch out by itself. We can then ask Linus if he can
> include just this patch in the next rc.
Sure! Just sent it out.
>
> Thanks,
> Khalid
>
^ permalink raw reply
* Re: [PATCH 1/1] net: rds: fix memory leak when unload rds_rdma
From: Yanjun Zhu @ 2019-06-04 6:58 UTC (permalink / raw)
To: santosh.shilimkar, davem, netdev, linux-rdma, rds-devel,
haakon.bugge@oracle.com >> Håkon Bugge
In-Reply-To: <80d57531-761e-9465-23bd-a9b8d9e30e66@oracle.com>
Hi, All
The steps to reproduce this problem. And it is easy to reproduce.
1. boot with 5.2.0-rc2+, and KASAN is also enabled in this kernel
2. run the following scripts:
"
for i in `seq 0 5`
do
rds-ping -c 2 1.1.1.14 -Q $i
done
"
3. run "rmmod rds_rdma"
4. check /var/log/dmesg, the following will appear.
"
BUG rds_ib_incoming (Tainted: G OE ): Objects remaining in
rds_ib_incoming on __kmem_cache_shutdown()
-----------------------------------------------------------------------------
Disabling lock debugging due to kernel taint
INFO: Slab 0x00000000e0306204 objects=32 used=1 fp=0x0000000050936906
flags=0x17fffc000010200
CPU: 20 PID: 6643 Comm: rmmod Tainted: G B OE 5.2.0-rc2+ #1
Hardware name: Oracle Corporation SUN FIRE X4170 M2 SERVER
/ASSY,MOTHERBOARD,X4170, BIOS 08140112 08/03/2016
Call Trace:
dump_stack+0x71/0xab
slab_err+0xad/0xd0
? __kasan_kmalloc+0xd5/0xf0
? kasan_unpoison_shadow+0x31/0x40
__kmem_cache_shutdown+0x17d/0x370
shutdown_cache+0x17/0x130
kmem_cache_destroy+0x1df/0x210
rds_ib_recv_exit+0x11/0x20 [rds_rdma]
rds_ib_exit+0x7a/0x90 [rds_rdma]
__x64_sys_delete_module+0x224/0x2c0
? __ia32_sys_delete_module+0x2c0/0x2c0
do_syscall_64+0x73/0x190
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x7f424d53b457
Code: 73 01 c3 48 8b 0d 19 7a 2c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e
0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 b8 b0 00 00 00 0f 05 <48> 3d 01
f0 ff ff 73 01 c3 48 8b 0d e9 79 2c 00 f7 d8 64 89 01 48
RSP: 002b:00007ffe70e8f048 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
RAX: ffffffffffffffda RBX: 0000000001bc1210 RCX: 00007f424d53b457
RDX: 00007f424d5afbe0 RSI: 0000000000000800 RDI: 0000000001bc1278
RBP: 0000000000000000 R08: 00007f424d804060 R09: 00007f424d5afbe0
R10: 00007ffe70e8ec10 R11: 0000000000000206 R12: 00007ffe70e90879
R13: 0000000000000000 R14: 0000000001bc1210 R15: 0000000001bc1010
INFO: Object 0x000000003c6e3538 @offset=0
kmem_cache_destroy rds_ib_incoming: Slab cache still has objects
CPU: 20 PID: 6643 Comm: rmmod Tainted: G B OE 5.2.0-rc2+ #1
Hardware name: Oracle Corporation SUN FIRE X4170 M2 SERVER
/ASSY,MOTHERBOARD,X4170, BIOS 08140112 08/03/2016
Call Trace:
dump_stack+0x71/0xab
kmem_cache_destroy+0x1fd/0x210
rds_ib_recv_exit+0x11/0x20 [rds_rdma]
rds_ib_exit+0x7a/0x90 [rds_rdma]
__x64_sys_delete_module+0x224/0x2c0
? __ia32_sys_delete_module+0x2c0/0x2c0
do_syscall_64+0x73/0x190
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x7f424d53b457
Code: 73 01 c3 48 8b 0d 19 7a 2c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e
0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 b8 b0 00 00 00 0f 05 <48> 3d 01
f0 ff ff 73 01 c3 48 8b 0d e9 79 2c 00 f7 d8 64 89 01 48
RSP: 002b:00007ffe70e8f048 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
RAX: ffffffffffffffda RBX: 0000000001bc1210 RCX: 00007f424d53b457
RDX: 00007f424d5afbe0 RSI: 0000000000000800 RDI: 0000000001bc1278
RBP: 0000000000000000 R08: 00007f424d804060 R09: 00007f424d5afbe0
R10: 00007ffe70e8ec10 R11: 0000000000000206 R12: 00007ffe70e90879
R13: 0000000000000000 R14: 0000000001bc1210 R15: 0000000001bc1010
=============================================================================
BUG rds_ib_frag (Tainted: G B OE ): Objects remaining in
rds_ib_frag on __kmem_cache_shutdown()
-----------------------------------------------------------------------------
INFO: Slab 0x00000000b70dee4a objects=32 used=1 fp=0x000000001dde4640
flags=0x17fffc000000200
CPU: 21 PID: 6643 Comm: rmmod Tainted: G B OE 5.2.0-rc2+ #1
Hardware name: Oracle Corporation SUN FIRE X4170 M2 SERVER
/ASSY,MOTHERBOARD,X4170, BIOS 08140112 08/03/2016
Call Trace:
dump_stack+0x71/0xab
slab_err+0xad/0xd0
? __kasan_kmalloc+0xd5/0xf0
? kasan_unpoison_shadow+0x31/0x40
__kmem_cache_shutdown+0x17d/0x370
shutdown_cache+0x17/0x130
kmem_cache_destroy+0x1df/0x210
rds_ib_exit+0x7a/0x90 [rds_rdma]
__x64_sys_delete_module+0x224/0x2c0
? __ia32_sys_delete_module+0x2c0/0x2c0
do_syscall_64+0x73/0x190
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x7f424d53b457
Code: 73 01 c3 48 8b 0d 19 7a 2c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e
0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 b8 b0 00 00 00 0f 05 <48> 3d 01
f0 ff ff 73 01 c3 48 8b 0d e9 79 2c 00 f7 d8 64 89 01 48
RSP: 002b:00007ffe70e8f048 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
RAX: ffffffffffffffda RBX: 0000000001bc1210 RCX: 00007f424d53b457
RDX: 00007f424d5afbe0 RSI: 0000000000000800 RDI: 0000000001bc1278
RBP: 0000000000000000 R08: 00007f424d804060 R09: 00007f424d5afbe0
R10: 00007ffe70e8ec10 R11: 0000000000000206 R12: 00007ffe70e90879
R13: 0000000000000000 R14: 0000000001bc1210 R15: 0000000001bc1010
INFO: Object 0x000000001d5f42ef @offset=0
kmem_cache_destroy rds_ib_frag: Slab cache still has objects
CPU: 21 PID: 6643 Comm: rmmod Tainted: G B OE 5.2.0-rc2+ #1
Hardware name: Oracle Corporation SUN FIRE X4170 M2 SERVER
/ASSY,MOTHERBOARD,X4170, BIOS 08140112 08/03/2016
Call Trace:
dump_stack+0x71/0xab
kmem_cache_destroy+0x1fd/0x210
rds_ib_exit+0x7a/0x90 [rds_rdma]
__x64_sys_delete_module+0x224/0x2c0
? __ia32_sys_delete_module+0x2c0/0x2c0
do_syscall_64+0x73/0x190
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x7f424d53b457
Code: 73 01 c3 48 8b 0d 19 7a 2c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e
0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 b8 b0 00 00 00 0f 05 <48> 3d 01
f0 ff ff 73 01 c3 48 8b 0d e9 79 2c 00 f7 d8 64 89 01 48
RSP: 002b:00007ffe70e8f048 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
RAX: ffffffffffffffda RBX: 0000000001bc1210 RCX: 00007f424d53b457
RDX: 00007f424d5afbe0 RSI: 0000000000000800 RDI: 0000000001bc1278
RBP: 0000000000000000 R08: 00007f424d804060 R09: 00007f424d5afbe0
R10: 00007ffe70e8ec10 R11: 0000000000000206 R12: 00007ffe70e90879
R13: 0000000000000000 R14: 0000000001bc1210 R15: 0000000001bc1010
Unregistered RDS/infiniband transport
"
5. after this commit is applied. Repeat the above steps. Check
/var/log/dmesg,
only the following appear.
"
[101287.926043] Unregistered RDS/infiniband transport
"
So IMO, this commit fixes this problem. The root cause is in the commit log.
Zhu Yanjun
On 2019/6/3 20:43, Yanjun Zhu wrote:
> Sorry. Add Håkon Bugge <haakon.bugge@oracle.com>
>
> He told me to notice the memory leak when caches are freed.
>
> Zhu Yanjun
>
> On 2019/6/3 20:48, Zhu Yanjun wrote:
>> When KASAN is enabled, after several rds connections are
>> created, then "rmmod rds_rdma" is run. The following will
>> appear.
>>
>> "
>> BUG rds_ib_incoming (Not tainted): Objects remaining
>> in rds_ib_incoming on __kmem_cache_shutdown()
>>
>> Call Trace:
>> dump_stack+0x71/0xab
>> slab_err+0xad/0xd0
>> __kmem_cache_shutdown+0x17d/0x370
>> shutdown_cache+0x17/0x130
>> kmem_cache_destroy+0x1df/0x210
>> rds_ib_recv_exit+0x11/0x20 [rds_rdma]
>> rds_ib_exit+0x7a/0x90 [rds_rdma]
>> __x64_sys_delete_module+0x224/0x2c0
>> ? __ia32_sys_delete_module+0x2c0/0x2c0
>> do_syscall_64+0x73/0x190
>> entry_SYSCALL_64_after_hwframe+0x44/0xa9
>> "
>> This is rds connection memory leak. The root cause is:
>> When "rmmod rds_rdma" is run, rds_ib_remove_one will call
>> rds_ib_dev_shutdown to drop the rds connections.
>> rds_ib_dev_shutdown will call rds_conn_drop to drop rds
>> connections as below.
>> "
>> rds_conn_path_drop(&conn->c_path[0], false);
>> "
>> In the above, destroy is set to false.
>> void rds_conn_path_drop(struct rds_conn_path *cp, bool destroy)
>> {
>> atomic_set(&cp->cp_state, RDS_CONN_ERROR);
>>
>> rcu_read_lock();
>> if (!destroy && rds_destroy_pending(cp->cp_conn)) {
>> rcu_read_unlock();
>> return;
>> }
>> queue_work(rds_wq, &cp->cp_down_w);
>> rcu_read_unlock();
>> }
>> In the above function, destroy is set to false. rds_destroy_pending
>> is called. This does not move rds connections to ib_nodev_conns.
>> So destroy is set to true to move rds connections to ib_nodev_conns.
>> In rds_ib_unregister_client, flush_workqueue is called to make rds_wq
>> finsh shutdown rds connections. The function rds_ib_destroy_nodev_conns
>> is called to shutdown rds connections finally.
>> Then rds_ib_recv_exit is called to destroy slab.
>>
>> void rds_ib_recv_exit(void)
>> {
>> kmem_cache_destroy(rds_ib_incoming_slab);
>> kmem_cache_destroy(rds_ib_frag_slab);
>> }
>> The above slab memory leak will not occur again.
>>
>> >From tests,
>> 256 rds connections
>> [root@ca-dev14 ~]# time rmmod rds_rdma
>>
>> real 0m16.522s
>> user 0m0.000s
>> sys 0m8.152s
>> 512 rds connections
>> [root@ca-dev14 ~]# time rmmod rds_rdma
>>
>> real 0m32.054s
>> user 0m0.000s
>> sys 0m15.568s
>>
>> To rmmod rds_rdma with 256 rds connections, about 16 seconds are needed.
>> And with 512 rds connections, about 32 seconds are needed.
>> >From ftrace, when one rds connection is destroyed,
>>
>> "
>> 19) | rds_conn_destroy [rds]() {
>> 19) 7.782 us | rds_conn_path_drop [rds]();
>> 15) | rds_shutdown_worker [rds]() {
>> 15) | rds_conn_shutdown [rds]() {
>> 15) 1.651 us | rds_send_path_reset [rds]();
>> 15) 7.195 us | }
>> 15) + 11.434 us | }
>> 19) 2.285 us | rds_cong_remove_conn [rds]();
>> 19) * 24062.76 us | }
>> "
>> So if many rds connections will be destroyed, this function
>> rds_ib_destroy_nodev_conns uses most of time.
>>
>> Suggested-by: Håkon Bugge <haakon.bugge@oracle.com>
>> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
>> ---
>> net/rds/ib.c | 2 +-
>> net/rds/ib_recv.c | 3 +++
>> 2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/rds/ib.c b/net/rds/ib.c
>> index f9baf2d..ec05d91 100644
>> --- a/net/rds/ib.c
>> +++ b/net/rds/ib.c
>> @@ -87,7 +87,7 @@ static void rds_ib_dev_shutdown(struct
>> rds_ib_device *rds_ibdev)
>> spin_lock_irqsave(&rds_ibdev->spinlock, flags);
>> list_for_each_entry(ic, &rds_ibdev->conn_list, ib_node)
>> - rds_conn_drop(ic->conn);
>> + rds_conn_path_drop(&ic->conn->c_path[0], true);
>> spin_unlock_irqrestore(&rds_ibdev->spinlock, flags);
>> }
>> diff --git a/net/rds/ib_recv.c b/net/rds/ib_recv.c
>> index 8946c89..3cae88c 100644
>> --- a/net/rds/ib_recv.c
>> +++ b/net/rds/ib_recv.c
>> @@ -168,6 +168,7 @@ void rds_ib_recv_free_caches(struct
>> rds_ib_connection *ic)
>> list_del(&inc->ii_cache_entry);
>> WARN_ON(!list_empty(&inc->ii_frags));
>> kmem_cache_free(rds_ib_incoming_slab, inc);
>> + atomic_dec(&rds_ib_allocation);
>> }
>> rds_ib_cache_xfer_to_ready(&ic->i_cache_frags);
>> @@ -1057,6 +1058,8 @@ int rds_ib_recv_init(void)
>> void rds_ib_recv_exit(void)
>> {
>> + WARN_ON(atomic_read(&rds_ib_allocation));
>> +
>> kmem_cache_destroy(rds_ib_incoming_slab);
>> kmem_cache_destroy(rds_ib_frag_slab);
>> }
>
^ permalink raw reply
* WARNING: suspicious RCU usage in in_dev_dump_addr
From: syzbot @ 2019-06-03 18:51 UTC (permalink / raw)
To: amitkarwar-Re5JQEeQqe8AvxtiuMwx3w, anshuman.khandual-5wv7dgnIgG8,
axboe-tSWWG44O7X1aa/9Udqfwiw, benedictwong-hpIqsD4AKlfQT0dZR+AlfA,
benve-FYB4Gu1CFyUAvxtiuMwx3w, coreteam-Cap9r6Oaw4JrovVCs/uTlw,
davej-rdkfGonbjUTCLXcRTR1eJlpr/1R2p/CL,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, dbanerje-JqFfY2XvxFXQT0dZR+AlfA,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
dledford-H+wXaHxf7aLQT0dZR+AlfA, doshir-pghWNbHTmq7QT0dZR+AlfA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA,
faisal.latif-ral2JQCrhuEAvxtiuMwx3w, fw-HFFVJYpyMKqzQB+pC5nmwQ,
gbhat-eYqpPyKDWXRBDgjK7y7TUQ,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
gustavo-L1vi/lXTdts+Va1GwOuvDg,
huxinming820-Re5JQEeQqe8AvxtiuMwx3w,
idosch-VPRAkNaXOzVWk0Htik3J/w,
jakub.kicinski-wFxRvT7yatFl57MIdRCFDg, jgg-uk2M96/98Pc,
johannes-cdvu00un1VgdHxzADdlk8Q,
kadlec-K40Dz/62t/MgiyqX0sVFJYdd74u8MsAO,
keescook-F7+t8E8rja9g9hUCZPvPmw, kuznet-v/Mj1YrvjDBInbfyfbPRSQ,
kvalo-sgV2jX0FEOL9JmXXK+q4OQ, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
liuhangbin-Re5JQEeQqe8AvxtiuMwx3w,
lucien.xin-Re5JQEeQqe8AvxtiuMwx3w, matwey-TTlVxmypnbovJsYlp49lxw,
mpe-Gsx/Oe8HsFggBc27wqDAHg, neescoba
Hello,
syzbot found the following crash on:
HEAD commit: b33bc2b8 nexthop: Add entry to MAINTAINERS
git tree: net-next
console output: https://syzkaller.appspot.com/x/log.txt?x=13f46f52a00000
kernel config: https://syzkaller.appspot.com/x/.config?x=1004db091673bbaf
dashboard link: https://syzkaller.appspot.com/bug?extid=bad6e32808a3a97b1515
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11dc685aa00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16229e36a00000
The bug was bisected to:
commit 2638eb8b50cfc16240e0bb080b9afbf541a9b39d
Author: Florian Westphal <fw-HFFVJYpyMKqzQB+pC5nmwQ@public.gmane.org>
Date: Fri May 31 16:27:09 2019 +0000
net: ipv4: provide __rcu annotation for ifa_list
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=170e1a0ea00000
final crash: https://syzkaller.appspot.com/x/report.txt?x=148e1a0ea00000
console output: https://syzkaller.appspot.com/x/log.txt?x=108e1a0ea00000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+bad6e32808a3a97b1515-Pl5Pbv+GP7P466ipTTIvnc23WoclnBCfAL8bYrjMMd8@public.gmane.org
Fixes: 2638eb8b50cf ("net: ipv4: provide __rcu annotation for ifa_list")
=============================
WARNING: suspicious RCU usage
5.2.0-rc2+ #13 Not tainted
-----------------------------
net/ipv4/devinet.c:1766 suspicious rcu_dereference_check() usage!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
1 lock held by syz-executor924/9000:
#0: 0000000087fe3874 (rtnl_mutex){+.+.}, at: netlink_dump+0xe7/0xfb0
net/netlink/af_netlink.c:2208
stack backtrace:
CPU: 0 PID: 9000 Comm: syz-executor924 Not tainted 5.2.0-rc2+ #13
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x172/0x1f0 lib/dump_stack.c:113
lockdep_rcu_suspicious+0x153/0x15d kernel/locking/lockdep.c:5250
in_dev_dump_addr+0x36f/0x3d0 net/ipv4/devinet.c:1766
inet_dump_ifaddr+0xa8f/0xca0 net/ipv4/devinet.c:1826
rtnl_dump_all+0x295/0x490 net/core/rtnetlink.c:3444
netlink_dump+0x558/0xfb0 net/netlink/af_netlink.c:2253
__netlink_dump_start+0x5b1/0x7d0 net/netlink/af_netlink.c:2361
netlink_dump_start include/linux/netlink.h:226 [inline]
rtnetlink_rcv_msg+0x73d/0xb00 net/core/rtnetlink.c:5181
netlink_rcv_skb+0x177/0x450 net/netlink/af_netlink.c:2486
rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5236
netlink_unicast_kernel net/netlink/af_netlink.c:1311 [inline]
netlink_unicast+0x531/0x710 net/netlink/af_netlink.c:1337
netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1926
sock_sendmsg_nosec net/socket.c:652 [inline]
sock_sendmsg+0xd7/0x130 net/socket.c:671
___sys_sendmsg+0x803/0x920 net/socket.c:2292
__sys_sendmsg+0x105/0x1d0 net/socket.c:2330
__do_sys_sendmsg net/socket.c:2339 [inline]
__se_sys_sendmsg net/socket.c:2337 [inline]
__x64_sys_sendmsg+0x78/0xb0 net/socket.c:2337
do_syscall_64+0xfd/0x680 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4402a9
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fffe5f26f18 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 00000000004402a9
RDX: 0000000000000000 RSI: 0000000020000240 RDI: 0000000000000003
RBP: 00000000006ca018 R08: 00000000004002c8 R09: 00000000004002c8
R10:
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH v16 01/16] uaccess: add untagged_addr definition for other arches
From: Khalid Aziz @ 2019-06-03 18:17 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Mark Rutland, kvm, Szabolcs Nagy, Catalin Marinas, Will Deacon,
dri-devel, Linux Memory Management List,
open list:KERNEL SELFTEST FRAMEWORK, Felix Kuehling,
Vincenzo Frascino, Jacob Bramley, Leon Romanovsky, linux-rdma,
amd-gfx, Christoph Hellwig, Jason Gunthorpe, Dmitry Vyukov,
Dave Martin, Evgeniy Stepanov, linux-media, Kevin Brodsky,
Kees Cook, Ruben Ayrapetyan
In-Reply-To: <20190603172916.GA5390@infradead.org>
On 6/3/19 11:29 AM, Christoph Hellwig wrote:
> On Mon, Jun 03, 2019 at 11:24:35AM -0600, Khalid Aziz wrote:
>> On 6/3/19 11:06 AM, Andrey Konovalov wrote:
>>> On Mon, Jun 3, 2019 at 7:04 PM Khalid Aziz <khalid.aziz@oracle.com> wrote:
>>>> Andrey,
>>>>
>>>> This patch has now become part of the other patch series Chris Hellwig
>>>> has sent out -
>>>> <https://lore.kernel.org/lkml/20190601074959.14036-1-hch@lst.de/>. Can
>>>> you coordinate with that patch series?
>>>
>>> Hi!
>>>
>>> Yes, I've seen it. How should I coordinate? Rebase this series on top
>>> of that one?
>>
>> That would be one way to do it. Better yet, separate this patch from
>> both patch series, make it standalone and then rebase the two patch
>> series on top of it.
>
> I think easiest would be to just ask Linus if he could make an exception
> and include this trivial prep patch in 5.2-rc.
>
Andrey,
Would you mind updating the commit log to make it not arm64 specific and
sending this patch out by itself. We can then ask Linus if he can
include just this patch in the next rc.
Thanks,
Khalid
^ permalink raw reply
* Re: [PATCH v16 12/16] IB, arm64: untag user pointers in ib_uverbs_(re)reg_mr()
From: Jason Gunthorpe @ 2019-06-03 17:46 UTC (permalink / raw)
To: Andrey Konovalov
Cc: linux-arm-kernel, linux-mm, linux-kernel, amd-gfx, dri-devel,
linux-rdma, linux-media, kvm, linux-kselftest, Catalin Marinas,
Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
Greg Kroah-Hartman, Kees Cook, Yishai Hadas, Felix Kuehling,
Alexander Deucher, Christian Koenig, Mauro Carvalho Chehab
In-Reply-To: <c829f93b19ad6af1b13be8935ce29baa8e58518f.1559580831.git.andreyknvl@google.com>
On Mon, Jun 03, 2019 at 06:55:14PM +0200, Andrey Konovalov wrote:
> This patch is a part of a series that extends arm64 kernel ABI to allow to
> pass tagged user pointers (with the top byte set to something else other
> than 0x00) as syscall arguments.
>
> ib_uverbs_(re)reg_mr() use provided user pointers for vma lookups (through
> e.g. mlx4_get_umem_mr()), which can only by done with untagged pointers.
>
> Untag user pointers in these functions.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> drivers/infiniband/core/uverbs_cmd.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> index 5a3a1780ceea..f88ee733e617 100644
> +++ b/drivers/infiniband/core/uverbs_cmd.c
> @@ -709,6 +709,8 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs)
> if (ret)
> return ret;
>
> + cmd.start = untagged_addr(cmd.start);
> +
> if ((cmd.start & ~PAGE_MASK) != (cmd.hca_va & ~PAGE_MASK))
> return -EINVAL;
I feel like we shouldn't thave to do this here, surely the cmd.start
should flow unmodified to get_user_pages, and gup should untag it?
ie, this sort of direction for the IB code (this would be a giant
patch, so I didn't have time to write it all, but I think it is much
saner):
diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
index 54628ef879f0ce..7b3b736c87c253 100644
--- a/drivers/infiniband/core/umem.c
+++ b/drivers/infiniband/core/umem.c
@@ -193,7 +193,7 @@ EXPORT_SYMBOL(ib_umem_find_best_pgsz);
* @access: IB_ACCESS_xxx flags for memory being pinned
* @dmasync: flush in-flight DMA when the memory region is written
*/
-struct ib_umem *ib_umem_get(struct ib_udata *udata, unsigned long addr,
+struct ib_umem *ib_umem_get(struct ib_udata *udata, void __user *addr,
size_t size, int access, int dmasync)
{
struct ib_ucontext *context;
@@ -201,7 +201,7 @@ struct ib_umem *ib_umem_get(struct ib_udata *udata, unsigned long addr,
struct page **page_list;
unsigned long lock_limit;
unsigned long new_pinned;
- unsigned long cur_base;
+ void __user *cur_base;
struct mm_struct *mm;
unsigned long npages;
int ret;
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 5a3a1780ceea4d..94389e7f12371f 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -735,7 +735,8 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs)
}
}
- mr = pd->device->ops.reg_user_mr(pd, cmd.start, cmd.length, cmd.hca_va,
+ mr = pd->device->ops.reg_user_mr(pd, u64_to_user_ptr(cmd.start),
+ cmd.length, cmd.hca_va,
cmd.access_flags,
&attrs->driver_udata);
if (IS_ERR(mr)) {
diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
index 4d033796dcfcc2..bddbb952082fc5 100644
--- a/drivers/infiniband/hw/mlx5/mr.c
+++ b/drivers/infiniband/hw/mlx5/mr.c
@@ -786,7 +786,7 @@ static int mr_cache_max_order(struct mlx5_ib_dev *dev)
}
static int mr_umem_get(struct mlx5_ib_dev *dev, struct ib_udata *udata,
- u64 start, u64 length, int access_flags,
+ void __user *start, u64 length, int access_flags,
struct ib_umem **umem, int *npages, int *page_shift,
int *ncont, int *order)
{
@@ -1262,8 +1262,8 @@ struct ib_mr *mlx5_ib_reg_dm_mr(struct ib_pd *pd, struct ib_dm *dm,
attr->access_flags, mode);
}
-struct ib_mr *mlx5_ib_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
- u64 virt_addr, int access_flags,
+struct ib_mr *mlx5_ib_reg_user_mr(struct ib_pd *pd, void __user *start,
+ u64 length, u64 virt_addr, int access_flags,
struct ib_udata *udata)
{
struct mlx5_ib_dev *dev = to_mdev(pd->device);
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index ec6446864b08e9..b3c8eaaa35c760 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -2464,8 +2464,8 @@ struct ib_device_ops {
struct ib_mr *(*reg_user_mr)(struct ib_pd *pd, u64 start, u64 length,
u64 virt_addr, int mr_access_flags,
struct ib_udata *udata);
- int (*rereg_user_mr)(struct ib_mr *mr, int flags, u64 start, u64 length,
- u64 virt_addr, int mr_access_flags,
+ int (*rereg_user_mr)(struct ib_mr *mr, int flags, void __user *start,
+ u64 length, u64 virt_addr, int mr_access_flags,
struct ib_pd *pd, struct ib_udata *udata);
int (*dereg_mr)(struct ib_mr *mr, struct ib_udata *udata);
struct ib_mr *(*alloc_mr)(struct ib_pd *pd, enum ib_mr_type mr_type,
^ permalink raw reply related
* Re: [PATCH v16 01/16] uaccess: add untagged_addr definition for other arches
From: Christoph Hellwig @ 2019-06-03 17:29 UTC (permalink / raw)
To: Khalid Aziz
Cc: Mark Rutland, kvm, Szabolcs Nagy, Catalin Marinas, Will Deacon,
dri-devel, Linux Memory Management List,
open list:KERNEL SELFTEST FRAMEWORK, Felix Kuehling,
Vincenzo Frascino, Jacob Bramley, Leon Romanovsky, linux-rdma,
amd-gfx, Christoph Hellwig, Jason Gunthorpe, Dmitry Vyukov,
Dave Martin, Evgeniy Stepanov, linux-media, Kevin Brodsky,
Kees Cook, Ruben Ayrapetyan
In-Reply-To: <f6711d31-e52c-473a-d7ad-b2d63131d7a5@oracle.com>
On Mon, Jun 03, 2019 at 11:24:35AM -0600, Khalid Aziz wrote:
> On 6/3/19 11:06 AM, Andrey Konovalov wrote:
> > On Mon, Jun 3, 2019 at 7:04 PM Khalid Aziz <khalid.aziz@oracle.com> wrote:
> >> Andrey,
> >>
> >> This patch has now become part of the other patch series Chris Hellwig
> >> has sent out -
> >> <https://lore.kernel.org/lkml/20190601074959.14036-1-hch@lst.de/>. Can
> >> you coordinate with that patch series?
> >
> > Hi!
> >
> > Yes, I've seen it. How should I coordinate? Rebase this series on top
> > of that one?
>
> That would be one way to do it. Better yet, separate this patch from
> both patch series, make it standalone and then rebase the two patch
> series on top of it.
I think easiest would be to just ask Linus if he could make an exception
and include this trivial prep patch in 5.2-rc.
^ permalink raw reply
* Re: [PATCH v16 01/16] uaccess: add untagged_addr definition for other arches
From: Khalid Aziz @ 2019-06-03 17:24 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Mark Rutland, kvm, Szabolcs Nagy, Catalin Marinas, Will Deacon,
dri-devel, Linux Memory Management List,
open list:KERNEL SELFTEST FRAMEWORK, Felix Kuehling,
Vincenzo Frascino, Jacob Bramley, Leon Romanovsky, linux-rdma,
amd-gfx, Christoph Hellwig, Jason Gunthorpe, Dmitry Vyukov,
Dave Martin, Evgeniy Stepanov, linux-media, Kevin Brodsky,
Kees Cook, Ruben Ayrapetyan
In-Reply-To: <CAAeHK+xX2538e674Pz25unkdFPCO_SH0pFwFu=8+DS7RzfYnLQ@mail.gmail.com>
On 6/3/19 11:06 AM, Andrey Konovalov wrote:
> On Mon, Jun 3, 2019 at 7:04 PM Khalid Aziz <khalid.aziz@oracle.com> wrote:
>> Andrey,
>>
>> This patch has now become part of the other patch series Chris Hellwig
>> has sent out -
>> <https://lore.kernel.org/lkml/20190601074959.14036-1-hch@lst.de/>. Can
>> you coordinate with that patch series?
>
> Hi!
>
> Yes, I've seen it. How should I coordinate? Rebase this series on top
> of that one?
That would be one way to do it. Better yet, separate this patch from
both patch series, make it standalone and then rebase the two patch
series on top of it.
--
Khalid
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox