wrong email address in mlx5 patch signature
Hi Roland, There's a typo in Jack's email address which is our mistake, was in V3 9/9, please fix it to be Jack Morgenstein ja...@dev.mellanox.co.il (the error is missing l in mellanox) thanks, Or. -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [infiniband:for-next 769/772] drivers/net/ethernet/mellanox/mlx5/core/cmd.c:894:7-14: WARNING opportunity for memdup_user
On 08/07/2013 12:40, Fengguang Wu wrote: Hi Eli, FYI, there are coccinelle warnings in tree: git://git.kernel.org/pub/scm/linux/kernel/git/roland/infiniband.git for-next head: 1af1abad19f6a40d8822cb7a35736e9e102fade6 commit: 809d3a921f9047bf575488f410ed12b365fe5cd7 [769/772] mlx5: Add driver for Mellanox Connect-IB adapters drivers/net/ethernet/mellanox/mlx5/core/cmd.c:894:7-14: WARNING opportunity for memdup_user -- drivers/infiniband/hw/mlx5/mr.c:445:2-3: Unneeded semicolon drivers/infiniband/hw/mlx5/mr.c:167:2-3: Unneeded semicolon -- drivers/net/ethernet/mellanox/mlx5/core/cmd.c:98:11-19: WARNING opportunity for simple_open, see also structure on line 938 drivers/net/ethernet/mellanox/mlx5/core/cmd.c:98:11-19: WARNING opportunity for simple_open, see also structure on line 718 drivers/net/ethernet/mellanox/mlx5/core/cmd.c:98:11-19: WARNING opportunity for simple_open, see also structure on line 1006 -- drivers/net/ethernet/mellanox/mlx5/core/debugfs.c:145:11-19: WARNING opportunity for simple_open, see also structure on line 470 drivers/net/ethernet/mellanox/mlx5/core/debugfs.c:145:11-19: WARNING opportunity for simple_open, see also structure on line 203 -- drivers/infiniband/hw/mlx5/mr.c:52:11-20: WARNING opportunity for simple_open, see also structure on line 289 drivers/infiniband/hw/mlx5/mr.c:52:11-20: WARNING opportunity for simple_open, see also structure on line 227 Please consider folding the attached diff with suitable fixups :-) Hi Roland, Eli is still OOO, I have reviewed the four patches and they are OK, please roll into the initial version Or. -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [infiniband:for-next 769/772] drivers/net/ethernet/mellanox/mlx5/core/cmd.c:894:7-14: WARNING opportunity for memdup_user
On Mon, Jul 08, 2013 at 12:58:10PM +0300, Or Gerlitz wrote: On 08/07/2013 12:40, Fengguang Wu wrote: Hi Eli, FYI, there are coccinelle warnings in tree: git://git.kernel.org/pub/scm/linux/kernel/git/roland/infiniband.git for-next head: 1af1abad19f6a40d8822cb7a35736e9e102fade6 commit: 809d3a921f9047bf575488f410ed12b365fe5cd7 [769/772] mlx5: Add driver for Mellanox Connect-IB adapters drivers/net/ethernet/mellanox/mlx5/core/cmd.c:894:7-14: WARNING opportunity for memdup_user -- drivers/infiniband/hw/mlx5/mr.c:445:2-3: Unneeded semicolon drivers/infiniband/hw/mlx5/mr.c:167:2-3: Unneeded semicolon -- drivers/net/ethernet/mellanox/mlx5/core/cmd.c:98:11-19: WARNING opportunity for simple_open, see also structure on line 938 drivers/net/ethernet/mellanox/mlx5/core/cmd.c:98:11-19: WARNING opportunity for simple_open, see also structure on line 718 drivers/net/ethernet/mellanox/mlx5/core/cmd.c:98:11-19: WARNING opportunity for simple_open, see also structure on line 1006 -- drivers/net/ethernet/mellanox/mlx5/core/debugfs.c:145:11-19: WARNING opportunity for simple_open, see also structure on line 470 drivers/net/ethernet/mellanox/mlx5/core/debugfs.c:145:11-19: WARNING opportunity for simple_open, see also structure on line 203 -- drivers/infiniband/hw/mlx5/mr.c:52:11-20: WARNING opportunity for simple_open, see also structure on line 289 drivers/infiniband/hw/mlx5/mr.c:52:11-20: WARNING opportunity for simple_open, see also structure on line 227 Please consider folding the attached diff with suitable fixups :-) Hi Roland, Eli is still OOO, I have reviewed the four patches and they are OK, please roll into the initial version Roland, you may need to replace some with \t in the patches. Thanks, Fengguang -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [infiniband:for-next 769/772] drivers/net/ethernet/mellanox/mlx5/core/cmd.c:894:7-14: WARNING opportunity for memdup_user
On 08/07/2013 13:58, Fengguang Wu wrote: Roland, you may need to replace some with \t in the patches. Thanks, Fengguang Roland, I have prepared these patches properly and will send them to you, they can be just squashed into the initial commit or come as add on patches. Or. -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Fixes to issues in mlx5 code found by static checker
All Reported-by Fengguang Wu fengguang...@intel.com Or Gerlitz (2): mlx5: Use simple_open when possible IB/mlx5: Removes unneeded semicolons drivers/infiniband/hw/mlx5/mr.c | 15 --- drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 13 +++-- drivers/net/ethernet/mellanox/mlx5/core/debugfs.c | 15 ++- 3 files changed, 9 insertions(+), 34 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] IB/mlx5: Removes unneeded semicolons
Found by coccinelle Generated by: coccinelle/misc/semicolon.cocci Reported-by: Fengguang Wu fengguang...@intel.com Signed-off-by: Or Gerlitz ogerl...@mellanox.com --- drivers/infiniband/hw/mlx5/mr.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c index 6b41c94..a03999c 100644 --- a/drivers/infiniband/hw/mlx5/mr.c +++ b/drivers/infiniband/hw/mlx5/mr.c @@ -157,7 +157,7 @@ static void remove_keys(struct mlx5_ib_dev *dev, int c, int num) kfree(mr-pas); kfree(mr); } - }; + } } static ssize_t size_write(struct file *filp, const char __user *buf, @@ -435,7 +435,7 @@ static void clean_keys(struct mlx5_ib_dev *dev, int c) kfree(mr-pas); kfree(mr); } - }; + } } static int mlx5_mr_cache_debugfs_init(struct mlx5_ib_dev *dev) -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] mlx5: Use simple_open when possible
If the open entry for char-device just does
file-private_data = inode-i_private;
we can use simple_open instead.
Generated by: coccinelle/api/simple_open.cocci
Reported-by: Fengguang Wu fengguang...@intel.com
Signed-off-by: Or Gerlitz ogerl...@mellanox.com
---
drivers/infiniband/hw/mlx5/mr.c | 11 ++-
drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 13 +++--
drivers/net/ethernet/mellanox/mlx5/core/debugfs.c | 15 ++-
3 files changed, 7 insertions(+), 32 deletions(-)
diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
index 6b76150..6b41c94 100644
--- a/drivers/infiniband/hw/mlx5/mr.c
+++ b/drivers/infiniband/hw/mlx5/mr.c
@@ -49,13 +49,6 @@ static __be64 *mr_align(__be64 *ptr, int align)
return (__be64 *)(((unsigned long)ptr + mask) ~mask);
}
-static int file_open(struct inode *inode, struct file *file)
-{
- file-private_data = inode-i_private;
-
- return 0;
-}
-
static int order2idx(struct mlx5_ib_dev *dev, int order)
{
struct mlx5_mr_cache *cache = dev-cache;
@@ -224,7 +217,7 @@ static ssize_t size_read(struct file *filp, char __user
*buf, size_t count,
static const struct file_operations size_fops = {
.owner = THIS_MODULE,
- .open = file_open,
+ .open = simple_open,
.write = size_write,
.read = size_read,
};
@@ -286,7 +279,7 @@ static ssize_t limit_read(struct file *filp, char __user
*buf, size_t count,
static const struct file_operations limit_fops = {
.owner = THIS_MODULE,
- .open = file_open,
+ .open = simple_open,
.write = limit_write,
.read = limit_read,
};
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
index a0c8941..6e9628b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
@@ -95,13 +95,6 @@ enum {
MLX5_CMD_STAT_BAD_SIZE_OUTS_CQES_ERR= 0x40,
};
-static int dbg_open(struct inode *inode, struct file *file)
-{
- file-private_data = inode-i_private;
-
- return 0;
-}
-
static struct mlx5_cmd_work_ent *alloc_cmd(struct mlx5_cmd *cmd,
struct mlx5_cmd_msg *in,
struct mlx5_cmd_msg *out,
@@ -715,7 +708,7 @@ static ssize_t dbg_write(struct file *filp, const char
__user *buf,
static const struct file_operations fops = {
.owner = THIS_MODULE,
- .open = dbg_open,
+ .open = simple_open,
.write = dbg_write,
};
@@ -935,7 +928,7 @@ static ssize_t data_read(struct file *filp, char __user
*buf, size_t count,
static const struct file_operations dfops = {
.owner = THIS_MODULE,
- .open = dbg_open,
+ .open = simple_open,
.write = data_write,
.read = data_read,
};
@@ -1003,7 +996,7 @@ static ssize_t outlen_write(struct file *filp, const char
__user *buf,
static const struct file_operations olfops = {
.owner = THIS_MODULE,
- .open = dbg_open,
+ .open = simple_open,
.write = outlen_write,
.read = outlen_read,
};
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/debugfs.c
b/drivers/net/ethernet/mellanox/mlx5/core/debugfs.c
index 8acb754..106085b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/debugfs.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/debugfs.c
@@ -142,17 +142,6 @@ void mlx5_eq_debugfs_cleanup(struct mlx5_core_dev *dev)
debugfs_remove_recursive(dev-priv.eq_debugfs);
}
-static int dbg_open(struct inode *inode, struct file *file)
-{
- /*
-* inode.i_private is equal to the data argument passed to
-* debugfs_create_file
-*/
- file-private_data = inode-i_private;
-
- return 0;
-}
-
static ssize_t average_read(struct file *filp, char __user *buf, size_t count,
loff_t *pos)
{
@@ -200,7 +189,7 @@ static ssize_t average_write(struct file *filp, const char
__user *buf,
static const struct file_operations stats_fops = {
.owner = THIS_MODULE,
- .open = dbg_open,
+ .open = simple_open,
.read = average_read,
.write = average_write,
};
@@ -467,7 +456,7 @@ static ssize_t dbg_read(struct file *filp, char __user
*buf, size_t count,
static const struct file_operations fops = {
.owner = THIS_MODULE,
- .open = dbg_open,
+ .open = simple_open,
.read = dbg_read,
};
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] mlx5: Use simple_open when possible
Hi Or,
static const struct file_operations limit_fops = {
.owner = THIS_MODULE,
- .open = file_open,
+ .open = simple_open,
.write = limit_write,
.read = limit_read,
static const struct file_operations dfops = {
.owner = THIS_MODULE,
- .open = dbg_open,
+ .open = simple_open,
.write = data_write,
.read = data_read,
};
@@ -1003,7 +996,7 @@ static ssize_t outlen_write(struct file *filp, const
char __user *buf,
static const struct file_operations olfops = {
.owner = THIS_MODULE,
- .open = dbg_open,
+ .open = simple_open,
.write = outlen_write,
.read = outlen_read,
static const struct file_operations fops = {
.owner = THIS_MODULE,
- .open = dbg_open,
+ .open = simple_open,
.read = dbg_read,
};
The above chunks will need to fix alignments.
Thanks,
Fengguang
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] mlx5: Use simple_open when possible
On 08/07/2013 15:34, Fengguang Wu wrote: The above chunks will need to fix alignments. sure -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V1 0/2] Fixes to issues in mlx5 code found by static checker
All Reported-by Fengguang Wu fengguang...@intel.com changes from V0: - replaced space with tabs in few places Or Gerlitz (2): mlx5: Use simple_open when possible IB/mlx5: Removes unneeded semicolons drivers/infiniband/hw/mlx5/mr.c | 15 --- drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 13 +++-- drivers/net/ethernet/mellanox/mlx5/core/debugfs.c | 15 ++- 3 files changed, 9 insertions(+), 34 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V1 1/2] mlx5: Use simple_open when possible
If the open entry for char-device just does
file-private_data = inode-i_private;
we can use simple_open instead.
Generated by: coccinelle/api/simple_open.cocci
Reported-by: Fengguang Wu fengguang...@intel.com
Signed-off-by: Or Gerlitz ogerl...@mellanox.com
---
drivers/infiniband/hw/mlx5/mr.c | 11 ++-
drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 13 +++--
drivers/net/ethernet/mellanox/mlx5/core/debugfs.c | 15 ++-
3 files changed, 7 insertions(+), 32 deletions(-)
diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
index 6b76150..2b5d336 100644
--- a/drivers/infiniband/hw/mlx5/mr.c
+++ b/drivers/infiniband/hw/mlx5/mr.c
@@ -49,13 +49,6 @@ static __be64 *mr_align(__be64 *ptr, int align)
return (__be64 *)(((unsigned long)ptr + mask) ~mask);
}
-static int file_open(struct inode *inode, struct file *file)
-{
- file-private_data = inode-i_private;
-
- return 0;
-}
-
static int order2idx(struct mlx5_ib_dev *dev, int order)
{
struct mlx5_mr_cache *cache = dev-cache;
@@ -224,7 +217,7 @@ static ssize_t size_read(struct file *filp, char __user
*buf, size_t count,
static const struct file_operations size_fops = {
.owner = THIS_MODULE,
- .open = file_open,
+ .open = simple_open,
.write = size_write,
.read = size_read,
};
@@ -286,7 +279,7 @@ static ssize_t limit_read(struct file *filp, char __user
*buf, size_t count,
static const struct file_operations limit_fops = {
.owner = THIS_MODULE,
- .open = file_open,
+ .open = simple_open,
.write = limit_write,
.read = limit_read,
};
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
index a0c8941..c1c0eef 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
@@ -95,13 +95,6 @@ enum {
MLX5_CMD_STAT_BAD_SIZE_OUTS_CQES_ERR= 0x40,
};
-static int dbg_open(struct inode *inode, struct file *file)
-{
- file-private_data = inode-i_private;
-
- return 0;
-}
-
static struct mlx5_cmd_work_ent *alloc_cmd(struct mlx5_cmd *cmd,
struct mlx5_cmd_msg *in,
struct mlx5_cmd_msg *out,
@@ -715,7 +708,7 @@ static ssize_t dbg_write(struct file *filp, const char
__user *buf,
static const struct file_operations fops = {
.owner = THIS_MODULE,
- .open = dbg_open,
+ .open = simple_open,
.write = dbg_write,
};
@@ -935,7 +928,7 @@ static ssize_t data_read(struct file *filp, char __user
*buf, size_t count,
static const struct file_operations dfops = {
.owner = THIS_MODULE,
- .open = dbg_open,
+ .open = simple_open,
.write = data_write,
.read = data_read,
};
@@ -1003,7 +996,7 @@ static ssize_t outlen_write(struct file *filp, const char
__user *buf,
static const struct file_operations olfops = {
.owner = THIS_MODULE,
- .open = dbg_open,
+ .open = simple_open,
.write = outlen_write,
.read = outlen_read,
};
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/debugfs.c
b/drivers/net/ethernet/mellanox/mlx5/core/debugfs.c
index 8acb754..5e9cf2b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/debugfs.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/debugfs.c
@@ -142,17 +142,6 @@ void mlx5_eq_debugfs_cleanup(struct mlx5_core_dev *dev)
debugfs_remove_recursive(dev-priv.eq_debugfs);
}
-static int dbg_open(struct inode *inode, struct file *file)
-{
- /*
-* inode.i_private is equal to the data argument passed to
-* debugfs_create_file
-*/
- file-private_data = inode-i_private;
-
- return 0;
-}
-
static ssize_t average_read(struct file *filp, char __user *buf, size_t count,
loff_t *pos)
{
@@ -200,7 +189,7 @@ static ssize_t average_write(struct file *filp, const char
__user *buf,
static const struct file_operations stats_fops = {
.owner = THIS_MODULE,
- .open = dbg_open,
+ .open = simple_open,
.read = average_read,
.write = average_write,
};
@@ -467,7 +456,7 @@ static ssize_t dbg_read(struct file *filp, char __user
*buf, size_t count,
static const struct file_operations fops = {
.owner = THIS_MODULE,
- .open = dbg_open,
+ .open = simple_open,
.read = dbg_read,
};
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V1 2/2] IB/mlx5: Remove unneeded semicolons
Found by coccinelle Generated by: coccinelle/misc/semicolon.cocci Reported-by: Fengguang Wu fengguang...@intel.com Signed-off-by: Or Gerlitz ogerl...@mellanox.com --- drivers/infiniband/hw/mlx5/mr.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c index 2b5d336..e2daa8f 100644 --- a/drivers/infiniband/hw/mlx5/mr.c +++ b/drivers/infiniband/hw/mlx5/mr.c @@ -157,7 +157,7 @@ static void remove_keys(struct mlx5_ib_dev *dev, int c, int num) kfree(mr-pas); kfree(mr); } - }; + } } static ssize_t size_write(struct file *filp, const char __user *buf, @@ -435,7 +435,7 @@ static void clean_keys(struct mlx5_ib_dev *dev, int c) kfree(mr-pas); kfree(mr); } - }; + } } static int mlx5_mr_cache_debugfs_init(struct mlx5_ib_dev *dev) -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] iSER initiator updates for 3.11
Hi Roland, Indeed late, but these are some iser initiator updates for 3.11 We have another batch coming which changes the initiator driver such that it can use FRWRs (Fast-Reg-Work-Requests) when FMRs aren't available, e.g on ConnectX (mlx4) VF or Connect-IB (mlx5) - the other batch should be ready by next week. Or. Or Gerlitz (1): IB/iser: Use proper debug level value for info prints Shlomo Pongratz (2): IB/iser: Restructure allocation/deallocation of connection resources IB/iser: Accept session-cmds_max from user space drivers/infiniband/ulp/iser/iscsi_iser.c | 19 +++-- drivers/infiniband/ulp/iser/iscsi_iser.h | 25 -- drivers/infiniband/ulp/iser/iser_initiator.c | 115 --- drivers/infiniband/ulp/iser/iser_memory.c| 13 +-- drivers/infiniband/ulp/iser/iser_verbs.c | 134 -- 5 files changed, 202 insertions(+), 104 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] IB/iser: Accept session-cmds_max from user space
From: Shlomo Pongratz shlo...@mellanox.com
Use cmds_max passed from user space to be the number of PDUs to be
supported for the session instead of hard-coded ISCSI_DEF_XMIT_CMDS_MAX.
Specifically, this allows to control the max number of SCSI commands
for the seesion. Also don't ignore the qdepth passed from user space.
Derive from session-cmds_max the actual number of RX buffers
and FMR pool size to allocate during the connection bind phase.
Since the iser transport connection is established before the iscsi
session/connection are created and bounded, we still use one hard coded
quantity ISER_DEF_XMIT_CMDS_MAX to compute the maximum number of
work-requests to be supported by the RC QP used for the connection.
The above quantity is made to be a power of two between ISCSI_TOTAL_CMDS_MIN
(16) and ISER_DEF_XMIT_CMDS_MAX (512) inclusive.
Signed-off-by: Shlomo Pongratz shlo...@mellanox.com
Signed-off-by: Or Gerlitz ogerl...@mellanox.com
---
drivers/infiniband/ulp/iser/iscsi_iser.c | 19 ---
drivers/infiniband/ulp/iser/iscsi_iser.h | 21 +++--
drivers/infiniband/ulp/iser/iser_initiator.c | 25 +++--
drivers/infiniband/ulp/iser/iser_verbs.c |8
4 files changed, 46 insertions(+), 27 deletions(-)
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c
b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 2e84ef8..705de7b 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -347,6 +347,7 @@ iscsi_iser_conn_bind(struct iscsi_cls_session *cls_session,
{
struct iscsi_conn *conn = cls_conn-dd_data;
struct iscsi_iser_conn *iser_conn;
+ struct iscsi_session *session;
struct iser_conn *ib_conn;
struct iscsi_endpoint *ep;
int error;
@@ -365,7 +366,8 @@ iscsi_iser_conn_bind(struct iscsi_cls_session *cls_session,
}
ib_conn = ep-dd_data;
- if (iser_alloc_rx_descriptors(ib_conn))
+ session = conn-session;
+ if (iser_alloc_rx_descriptors(ib_conn, session))
return -ENOMEM;
/* binds the iSER connection retrieved from the previously
@@ -419,12 +421,13 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
struct iscsi_cls_session *cls_session;
struct iscsi_session *session;
struct Scsi_Host *shost;
- struct iser_conn *ib_conn;
+ struct iser_conn *ib_conn = NULL;
shost = iscsi_host_alloc(iscsi_iser_sht, 0, 0);
if (!shost)
return NULL;
shost-transportt = iscsi_iser_scsi_transport;
+ shost-cmd_per_lun = qdepth;
shost-max_lun = iscsi_max_lun;
shost-max_id = 0;
shost-max_channel = 0;
@@ -441,12 +444,14 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
ep ? ib_conn-device-ib_device-dma_device : NULL))
goto free_host;
- /*
-* we do not support setting can_queue cmd_per_lun from userspace yet
-* because we preallocate so many resources
-*/
+ if (cmds_max ISER_DEF_XMIT_CMDS_MAX) {
+ iser_info(cmds_max changed from %u to %u\n,
+ cmds_max, ISER_DEF_XMIT_CMDS_MAX);
+ cmds_max = ISER_DEF_XMIT_CMDS_MAX;
+ }
+
cls_session = iscsi_session_setup(iscsi_iser_transport, shost,
- ISCSI_DEF_XMIT_CMDS_MAX, 0,
+ cmds_max, 0,
sizeof(struct iscsi_iser_task),
initial_cmdsn, 0);
if (!cls_session)
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h
b/drivers/infiniband/ulp/iser/iscsi_iser.h
index fee8829..a6a3e27 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -102,7 +102,13 @@
/* support up to 512KB in one RDMA */
#define ISCSI_ISER_SG_TABLESIZE (0x8 SHIFT_4K)
-#define ISER_DEF_CMD_PER_LUN ISCSI_DEF_XMIT_CMDS_MAX
+#define ISER_DEF_XMIT_CMDS_DEFUALT 512
+#if ISCSI_DEF_XMIT_CMDS_MAX ISER_DEF_XMIT_CMDS_DEFUALT
+ #define ISER_DEF_XMIT_CMDS_MAX ISCSI_DEF_XMIT_CMDS_MAX
+#else
+ #define ISER_DEF_XMIT_CMDS_MAX ISER_DEF_XMIT_CMDS_DEFUALT
+#endif
+#define ISER_DEF_CMD_PER_LUN ISER_DEF_XMIT_CMDS_MAX
/* QP settings */
/* Maximal bounds on received asynchronous PDUs */
@@ -111,9 +117,9 @@
#define ISER_MAX_TX_MISC_PDUS 6 /* NOOP_OUT(2), TEXT(1), *
* SCSI_TMFUNC(2), LOGOUT(1) */
-#define ISER_QP_MAX_RECV_DTOS (ISCSI_DEF_XMIT_CMDS_MAX)
+#define ISER_QP_MAX_RECV_DTOS (ISER_DEF_XMIT_CMDS_MAX)
-#define ISER_MIN_POSTED_RX (ISCSI_DEF_XMIT_CMDS_MAX 2)
+#define ISER_MIN_POSTED_RX (ISER_DEF_XMIT_CMDS_MAX 2)
/* the max TX (send)
[PATCH 2/3] IB/iser: Restructure allocation/deallocation of connection resources
From: Shlomo Pongratz shlo...@mellanox.com
This is a preparation step to a patch that accepts the number of max SCSI
commands to be supported for the session from the user space iSCSI tools.
Move the allocation of the login buffer, FMR pool and its associated
page vector from iser_create_ib_conn_res() which is called prior to
a point in time where we actually know how many commands should be
supported to iser_alloc_rx_descriptors() which is called during the
iscsi connection bind step where this quantity is known.
Also do small refactoring around the deallocation to make that path
similar to the allocation one.
Signed-off-by: Shlomo Pongratz shlo...@mellanox.com
Signed-off-by: Or Gerlitz ogerl...@mellanox.com
---
drivers/infiniband/ulp/iser/iscsi_iser.h |2 +
drivers/infiniband/ulp/iser/iser_initiator.c | 92 ++-
drivers/infiniband/ulp/iser/iser_verbs.c | 128 --
3 files changed, 151 insertions(+), 71 deletions(-)
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h
b/drivers/infiniband/ulp/iser/iscsi_iser.h
index d694bcd..fee8829 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -395,4 +395,6 @@ void iser_dma_unmap_task_data(struct iscsi_iser_task
*iser_task);
int iser_initialize_task_headers(struct iscsi_task *task,
struct iser_tx_desc *tx_desc);
int iser_alloc_rx_descriptors(struct iser_conn *ib_conn);
+int iser_create_fmr_pool(struct iser_conn *ib_conn);
+void iser_free_fmr_pool(struct iser_conn *ib_conn);
#endif
diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c
b/drivers/infiniband/ulp/iser/iser_initiator.c
index b6d81a8..626d950 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -170,6 +170,76 @@ static void iser_create_send_desc(struct iser_conn
*ib_conn,
}
}
+static void iser_free_login_buf(struct iser_conn *ib_conn)
+{
+ if (!ib_conn-login_buf)
+ return;
+
+ if (ib_conn-login_req_dma)
+ ib_dma_unmap_single(ib_conn-device-ib_device,
+ ib_conn-login_req_dma,
+ ISCSI_DEF_MAX_RECV_SEG_LEN, DMA_TO_DEVICE);
+
+ if (ib_conn-login_resp_dma)
+ ib_dma_unmap_single(ib_conn-device-ib_device,
+ ib_conn-login_resp_dma,
+ ISER_RX_LOGIN_SIZE, DMA_FROM_DEVICE);
+
+ kfree(ib_conn-login_buf);
+
+ /* make sure we never redo any unmapping */
+ ib_conn-login_req_dma = 0;
+ ib_conn-login_resp_dma = 0;
+ ib_conn-login_buf = NULL;
+}
+
+static int iser_alloc_login_buf(struct iser_conn *ib_conn)
+{
+ struct iser_device *device;
+ int req_err, resp_err;
+
+ BUG_ON(ib_conn-device == NULL);
+
+ device = ib_conn-device;
+
+ ib_conn-login_buf = kmalloc(ISCSI_DEF_MAX_RECV_SEG_LEN +
+ISER_RX_LOGIN_SIZE, GFP_KERNEL);
+ if (!ib_conn-login_buf)
+ goto out_err;
+
+ ib_conn-login_req_buf = ib_conn-login_buf;
+ ib_conn-login_resp_buf = ib_conn-login_buf +
+ ISCSI_DEF_MAX_RECV_SEG_LEN;
+
+ ib_conn-login_req_dma = ib_dma_map_single(ib_conn-device-ib_device,
+ (void *)ib_conn-login_req_buf,
+ ISCSI_DEF_MAX_RECV_SEG_LEN, DMA_TO_DEVICE);
+
+ ib_conn-login_resp_dma = ib_dma_map_single(ib_conn-device-ib_device,
+ (void *)ib_conn-login_resp_buf,
+ ISER_RX_LOGIN_SIZE, DMA_FROM_DEVICE);
+
+ req_err = ib_dma_mapping_error(device-ib_device,
+ ib_conn-login_req_dma);
+ resp_err = ib_dma_mapping_error(device-ib_device,
+ ib_conn-login_resp_dma);
+
+ if (req_err || resp_err) {
+ if (req_err)
+ ib_conn-login_req_dma = 0;
+ if (resp_err)
+ ib_conn-login_resp_dma = 0;
+ goto free_login_buf;
+ }
+ return 0;
+
+free_login_buf:
+ iser_free_login_buf(ib_conn);
+
+out_err:
+ iser_err(unable to alloc or map login buf\n);
+ return -ENOMEM;
+}
int iser_alloc_rx_descriptors(struct iser_conn *ib_conn)
{
@@ -179,6 +249,12 @@ int iser_alloc_rx_descriptors(struct iser_conn *ib_conn)
struct ib_sge *rx_sg;
struct iser_device *device = ib_conn-device;
+ if (iser_create_fmr_pool(ib_conn))
+ goto create_fmr_pool_failed;
+
+ if (iser_alloc_login_buf(ib_conn))
+ goto alloc_login_buf_fail;
+
ib_conn-rx_descs = kmalloc(ISER_QP_MAX_RECV_DTOS *
sizeof(struct iser_rx_desc), GFP_KERNEL);
if (!ib_conn-rx_descs)
@@
[PATCH 1/3] IB/iser: Use proper debug level value for info prints
Commit 4f363882612 IB/iser: Move informational messages from error to info
level
was setting info prints to require lower value for the debug level vs warning
prints
which isn't the common convention, fix that. Also move the prints on unaligned
SG
from warning to debug level.
Signed-off-by: Or Gerlitz ogerl...@mellanox.com
---
drivers/infiniband/ulp/iser/iscsi_iser.h |4 ++--
drivers/infiniband/ulp/iser/iser_memory.c | 13 ++---
2 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h
b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 4f069c0..d694bcd 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -78,14 +78,14 @@
#define iser_warn(fmt, arg...) \
do {\
- if (iser_debug_level 1) \
+ if (iser_debug_level 0) \
pr_warn(PFX %s: fmt, \
__func__ , ## arg); \
} while (0)
#define iser_info(fmt, arg...) \
do {\
- if (iser_debug_level 0) \
+ if (iser_debug_level 1) \
pr_info(PFX %s: fmt, \
__func__ , ## arg); \
} while (0)
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c
b/drivers/infiniband/ulp/iser/iser_memory.c
index 7827baf..797e49f 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -267,11 +267,8 @@ static void iser_data_buf_dump(struct iser_data_buf *data,
struct scatterlist *sg;
int i;
- if (iser_debug_level == 0)
- return;
-
for_each_sg(sgl, sg, data-dma_nents, i)
- iser_warn(sg[%d] dma_addr:0x%lX page:0x%p
+ iser_dbg(sg[%d] dma_addr:0x%lX page:0x%p
off:0x%x sz:0x%x dma_len:0x%x\n,
i, (unsigned long)ib_sg_dma_address(ibdev, sg),
sg_page(sg), sg-offset,
@@ -373,9 +370,11 @@ int iser_reg_rdma_mem(struct iscsi_iser_task *iser_task,
if (aligned_len != mem-dma_nents ||
(!ib_conn-fmr_pool mem-dma_nents 1)) {
iscsi_conn-fmr_unalign_cnt++;
- iser_warn(rdma alignment violation (%d/%d aligned) or FMR not
supported\n,
- aligned_len, mem-size);
- iser_data_buf_dump(mem, ibdev);
+ iser_dbg(rdma alignment violation (%d/%d aligned) or FMR not
supported\n,
+aligned_len, mem-size);
+
+ if (iser_debug_level 0)
+ iser_data_buf_dump(mem, ibdev);
/* unmap the command data before accessing it */
iser_dma_unmap_task_data(iser_task);
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] IB/iser: Restructure allocation/deallocation of connection resources
On 07/08/13 15:19, Or Gerlitz wrote: + iser_err(FMR alloction failed, err %d\n, ret); I see alloction instead of allocation - this looks like an (unimportant) typo ? Bart. -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] IB/iser: Accept session-cmds_max from user space
On 07/08/13 15:19, Or Gerlitz wrote: +#define ISER_DEF_XMIT_CMDS_DEFUALT 512 +#if ISCSI_DEF_XMIT_CMDS_MAX ISER_DEF_XMIT_CMDS_DEFUALT This looks like another spelling issue - shouldn't DEFUALT be changed into DEFAULT ? Bart. -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] IB/iser: Accept session-cmds_max from user space
On 08/07/2013 17:00, Bart Van Assche wrote: On 07/08/13 15:19, Or Gerlitz wrote: +#define ISER_DEF_XMIT_CMDS_DEFUALT 512 +#if ISCSI_DEF_XMIT_CMDS_MAX ISER_DEF_XMIT_CMDS_DEFUALT This looks like another spelling issue - shouldn't DEFUALT be changed into DEFAULT ? Bart. thanks for the quick feedback, will fix and re-submit -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V1 1/3] IB/iser: Use proper debug level value for info prints
Commit 4f363882612 IB/iser: Move informational messages from error to info
level
was setting info prints to require lower value for the debug level vs warning
prints
which isn't the common convention, fix that. Also move the prints on unaligned
SG
from warning to debug level.
Signed-off-by: Or Gerlitz ogerl...@mellanox.com
---
drivers/infiniband/ulp/iser/iscsi_iser.h |4 ++--
drivers/infiniband/ulp/iser/iser_memory.c | 13 ++---
2 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h
b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 4f069c0..d694bcd 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -78,14 +78,14 @@
#define iser_warn(fmt, arg...) \
do {\
- if (iser_debug_level 1) \
+ if (iser_debug_level 0) \
pr_warn(PFX %s: fmt, \
__func__ , ## arg); \
} while (0)
#define iser_info(fmt, arg...) \
do {\
- if (iser_debug_level 0) \
+ if (iser_debug_level 1) \
pr_info(PFX %s: fmt, \
__func__ , ## arg); \
} while (0)
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c
b/drivers/infiniband/ulp/iser/iser_memory.c
index 7827baf..797e49f 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -267,11 +267,8 @@ static void iser_data_buf_dump(struct iser_data_buf *data,
struct scatterlist *sg;
int i;
- if (iser_debug_level == 0)
- return;
-
for_each_sg(sgl, sg, data-dma_nents, i)
- iser_warn(sg[%d] dma_addr:0x%lX page:0x%p
+ iser_dbg(sg[%d] dma_addr:0x%lX page:0x%p
off:0x%x sz:0x%x dma_len:0x%x\n,
i, (unsigned long)ib_sg_dma_address(ibdev, sg),
sg_page(sg), sg-offset,
@@ -373,9 +370,11 @@ int iser_reg_rdma_mem(struct iscsi_iser_task *iser_task,
if (aligned_len != mem-dma_nents ||
(!ib_conn-fmr_pool mem-dma_nents 1)) {
iscsi_conn-fmr_unalign_cnt++;
- iser_warn(rdma alignment violation (%d/%d aligned) or FMR not
supported\n,
- aligned_len, mem-size);
- iser_data_buf_dump(mem, ibdev);
+ iser_dbg(rdma alignment violation (%d/%d aligned) or FMR not
supported\n,
+aligned_len, mem-size);
+
+ if (iser_debug_level 0)
+ iser_data_buf_dump(mem, ibdev);
/* unmap the command data before accessing it */
iser_dma_unmap_task_data(iser_task);
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V1 0/3] iSER initiator updates for 3.11
Hi Roland, Indeed late, but these are some iser initiator updates for 3.11 We have another batch coming which changes the initiator driver such that it can use FRWRs (Fast-Reg-Work-Requests) when FMRs aren't available, e.g on ConnectX (mlx4) VF or Connect-IB (mlx5) - the other batch should be ready by next week. changes from V0: - fixed two typos pointed by Bart Van Assche (thanks!) Or. Or Gerlitz (1): IB/iser: Use proper debug level value for info prints Shlomo Pongratz (2): IB/iser: Restructure allocation/deallocation of connection resources IB/iser: Accept session-cmds_max from user space drivers/infiniband/ulp/iser/iscsi_iser.c | 19 +++-- drivers/infiniband/ulp/iser/iscsi_iser.h | 25 -- drivers/infiniband/ulp/iser/iser_initiator.c | 115 --- drivers/infiniband/ulp/iser/iser_memory.c| 13 +-- drivers/infiniband/ulp/iser/iser_verbs.c | 134 -- 5 files changed, 202 insertions(+), 104 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V1 2/3] IB/iser: Restructure allocation/deallocation of connection resources
From: Shlomo Pongratz shlo...@mellanox.com
This is a preparation step to a patch that accepts the number of max SCSI
commands to be supported for the session from the user space iSCSI tools.
Move the allocation of the login buffer, FMR pool and its associated
page vector from iser_create_ib_conn_res() which is called prior to
a point in time where we actually know how many commands should be
supported to iser_alloc_rx_descriptors() which is called during the
iscsi connection bind step where this quantity is known.
Also do small refactoring around the deallocation to make that path
similar to the allocation one.
Signed-off-by: Shlomo Pongratz shlo...@mellanox.com
Signed-off-by: Or Gerlitz ogerl...@mellanox.com
---
drivers/infiniband/ulp/iser/iscsi_iser.h |2 +
drivers/infiniband/ulp/iser/iser_initiator.c | 92 ++-
drivers/infiniband/ulp/iser/iser_verbs.c | 128 --
3 files changed, 151 insertions(+), 71 deletions(-)
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h
b/drivers/infiniband/ulp/iser/iscsi_iser.h
index d694bcd..fee8829 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -395,4 +395,6 @@ void iser_dma_unmap_task_data(struct iscsi_iser_task
*iser_task);
int iser_initialize_task_headers(struct iscsi_task *task,
struct iser_tx_desc *tx_desc);
int iser_alloc_rx_descriptors(struct iser_conn *ib_conn);
+int iser_create_fmr_pool(struct iser_conn *ib_conn);
+void iser_free_fmr_pool(struct iser_conn *ib_conn);
#endif
diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c
b/drivers/infiniband/ulp/iser/iser_initiator.c
index b6d81a8..626d950 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -170,6 +170,76 @@ static void iser_create_send_desc(struct iser_conn
*ib_conn,
}
}
+static void iser_free_login_buf(struct iser_conn *ib_conn)
+{
+ if (!ib_conn-login_buf)
+ return;
+
+ if (ib_conn-login_req_dma)
+ ib_dma_unmap_single(ib_conn-device-ib_device,
+ ib_conn-login_req_dma,
+ ISCSI_DEF_MAX_RECV_SEG_LEN, DMA_TO_DEVICE);
+
+ if (ib_conn-login_resp_dma)
+ ib_dma_unmap_single(ib_conn-device-ib_device,
+ ib_conn-login_resp_dma,
+ ISER_RX_LOGIN_SIZE, DMA_FROM_DEVICE);
+
+ kfree(ib_conn-login_buf);
+
+ /* make sure we never redo any unmapping */
+ ib_conn-login_req_dma = 0;
+ ib_conn-login_resp_dma = 0;
+ ib_conn-login_buf = NULL;
+}
+
+static int iser_alloc_login_buf(struct iser_conn *ib_conn)
+{
+ struct iser_device *device;
+ int req_err, resp_err;
+
+ BUG_ON(ib_conn-device == NULL);
+
+ device = ib_conn-device;
+
+ ib_conn-login_buf = kmalloc(ISCSI_DEF_MAX_RECV_SEG_LEN +
+ISER_RX_LOGIN_SIZE, GFP_KERNEL);
+ if (!ib_conn-login_buf)
+ goto out_err;
+
+ ib_conn-login_req_buf = ib_conn-login_buf;
+ ib_conn-login_resp_buf = ib_conn-login_buf +
+ ISCSI_DEF_MAX_RECV_SEG_LEN;
+
+ ib_conn-login_req_dma = ib_dma_map_single(ib_conn-device-ib_device,
+ (void *)ib_conn-login_req_buf,
+ ISCSI_DEF_MAX_RECV_SEG_LEN, DMA_TO_DEVICE);
+
+ ib_conn-login_resp_dma = ib_dma_map_single(ib_conn-device-ib_device,
+ (void *)ib_conn-login_resp_buf,
+ ISER_RX_LOGIN_SIZE, DMA_FROM_DEVICE);
+
+ req_err = ib_dma_mapping_error(device-ib_device,
+ ib_conn-login_req_dma);
+ resp_err = ib_dma_mapping_error(device-ib_device,
+ ib_conn-login_resp_dma);
+
+ if (req_err || resp_err) {
+ if (req_err)
+ ib_conn-login_req_dma = 0;
+ if (resp_err)
+ ib_conn-login_resp_dma = 0;
+ goto free_login_buf;
+ }
+ return 0;
+
+free_login_buf:
+ iser_free_login_buf(ib_conn);
+
+out_err:
+ iser_err(unable to alloc or map login buf\n);
+ return -ENOMEM;
+}
int iser_alloc_rx_descriptors(struct iser_conn *ib_conn)
{
@@ -179,6 +249,12 @@ int iser_alloc_rx_descriptors(struct iser_conn *ib_conn)
struct ib_sge *rx_sg;
struct iser_device *device = ib_conn-device;
+ if (iser_create_fmr_pool(ib_conn))
+ goto create_fmr_pool_failed;
+
+ if (iser_alloc_login_buf(ib_conn))
+ goto alloc_login_buf_fail;
+
ib_conn-rx_descs = kmalloc(ISER_QP_MAX_RECV_DTOS *
sizeof(struct iser_rx_desc), GFP_KERNEL);
if (!ib_conn-rx_descs)
@@
[PATCH V1 3/3] IB/iser: Accept session-cmds_max from user space
From: Shlomo Pongratz shlo...@mellanox.com
Use cmds_max passed from user space to be the number of PDUs to be
supported for the session instead of hard-coded ISCSI_DEF_XMIT_CMDS_MAX.
Specifically, this allows to control the max number of SCSI commands
for the seesion. Also don't ignore the qdepth passed from user space.
Derive from session-cmds_max the actual number of RX buffers
and FMR pool size to allocate during the connection bind phase.
Since the iser transport connection is established before the iscsi
session/connection are created and bounded, we still use one hard coded
quantity ISER_DEF_XMIT_CMDS_MAX to compute the maximum number of
work-requests to be supported by the RC QP used for the connection.
The above quantity is made to be a power of two between ISCSI_TOTAL_CMDS_MIN
(16) and ISER_DEF_XMIT_CMDS_MAX (512) inclusive.
Signed-off-by: Shlomo Pongratz shlo...@mellanox.com
Signed-off-by: Or Gerlitz ogerl...@mellanox.com
---
drivers/infiniband/ulp/iser/iscsi_iser.c | 19 ---
drivers/infiniband/ulp/iser/iscsi_iser.h | 21 +++--
drivers/infiniband/ulp/iser/iser_initiator.c | 25 +++--
drivers/infiniband/ulp/iser/iser_verbs.c |8
4 files changed, 46 insertions(+), 27 deletions(-)
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c
b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 2e84ef8..705de7b 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -347,6 +347,7 @@ iscsi_iser_conn_bind(struct iscsi_cls_session *cls_session,
{
struct iscsi_conn *conn = cls_conn-dd_data;
struct iscsi_iser_conn *iser_conn;
+ struct iscsi_session *session;
struct iser_conn *ib_conn;
struct iscsi_endpoint *ep;
int error;
@@ -365,7 +366,8 @@ iscsi_iser_conn_bind(struct iscsi_cls_session *cls_session,
}
ib_conn = ep-dd_data;
- if (iser_alloc_rx_descriptors(ib_conn))
+ session = conn-session;
+ if (iser_alloc_rx_descriptors(ib_conn, session))
return -ENOMEM;
/* binds the iSER connection retrieved from the previously
@@ -419,12 +421,13 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
struct iscsi_cls_session *cls_session;
struct iscsi_session *session;
struct Scsi_Host *shost;
- struct iser_conn *ib_conn;
+ struct iser_conn *ib_conn = NULL;
shost = iscsi_host_alloc(iscsi_iser_sht, 0, 0);
if (!shost)
return NULL;
shost-transportt = iscsi_iser_scsi_transport;
+ shost-cmd_per_lun = qdepth;
shost-max_lun = iscsi_max_lun;
shost-max_id = 0;
shost-max_channel = 0;
@@ -441,12 +444,14 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
ep ? ib_conn-device-ib_device-dma_device : NULL))
goto free_host;
- /*
-* we do not support setting can_queue cmd_per_lun from userspace yet
-* because we preallocate so many resources
-*/
+ if (cmds_max ISER_DEF_XMIT_CMDS_MAX) {
+ iser_info(cmds_max changed from %u to %u\n,
+ cmds_max, ISER_DEF_XMIT_CMDS_MAX);
+ cmds_max = ISER_DEF_XMIT_CMDS_MAX;
+ }
+
cls_session = iscsi_session_setup(iscsi_iser_transport, shost,
- ISCSI_DEF_XMIT_CMDS_MAX, 0,
+ cmds_max, 0,
sizeof(struct iscsi_iser_task),
initial_cmdsn, 0);
if (!cls_session)
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h
b/drivers/infiniband/ulp/iser/iscsi_iser.h
index fee8829..d2fc55a 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -102,7 +102,13 @@
/* support up to 512KB in one RDMA */
#define ISCSI_ISER_SG_TABLESIZE (0x8 SHIFT_4K)
-#define ISER_DEF_CMD_PER_LUN ISCSI_DEF_XMIT_CMDS_MAX
+#define ISER_DEF_XMIT_CMDS_DEFAULT 512
+#if ISCSI_DEF_XMIT_CMDS_MAX ISER_DEF_XMIT_CMDS_DEFAULT
+ #define ISER_DEF_XMIT_CMDS_MAX ISCSI_DEF_XMIT_CMDS_MAX
+#else
+ #define ISER_DEF_XMIT_CMDS_MAX ISER_DEF_XMIT_CMDS_DEFAULT
+#endif
+#define ISER_DEF_CMD_PER_LUN ISER_DEF_XMIT_CMDS_MAX
/* QP settings */
/* Maximal bounds on received asynchronous PDUs */
@@ -111,9 +117,9 @@
#define ISER_MAX_TX_MISC_PDUS 6 /* NOOP_OUT(2), TEXT(1), *
* SCSI_TMFUNC(2), LOGOUT(1) */
-#define ISER_QP_MAX_RECV_DTOS (ISCSI_DEF_XMIT_CMDS_MAX)
+#define ISER_QP_MAX_RECV_DTOS (ISER_DEF_XMIT_CMDS_MAX)
-#define ISER_MIN_POSTED_RX (ISCSI_DEF_XMIT_CMDS_MAX 2)
+#define ISER_MIN_POSTED_RX (ISER_DEF_XMIT_CMDS_MAX 2)
/* the max TX (send)
Re: [PATCH V2] libibverbs: Allow arbitrary int values for MTU
On Jul 5, 2013, at 3:11 PM, Roland Dreier rol...@purestorage.com wrote: So what happens if I have an old application binary, and I run against a new libibverbs without recompiling? Also it seems that I'm forced to change my source code to be able to compile against new libibverbs? I previously sent an ABI-preserving version of this patch, but it was hated by Doug Ledford and (eventually) Jason Gunthorpe. After long discussion (see thread starting here: http://www.spinics.net/lists/linux-rdma/msg15951.html), they decided that they wanted a clean break that forces both source code and ABI changes, which resulted in this patch. I personally don't care which way this goes; I just want the ability to have non-enum MTU values. -- Jeff Squyres jsquy...@cisco.com For corporate legal information go to: http://www.cisco.com/web/about/doing_business/legal/cri/ -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] libibverbs: Allow arbitrary int values for MTU
[resending to reply-all, sorry Jeff] On Mon, Jul 8, 2013 at 9:26 AM, Jeff Squyres (jsquyres) jsquy...@cisco.com wrote: So what happens if I have an old application binary, and I run against a new libibverbs without recompiling? Also it seems that I'm forced to change my source code to be able to compile against new libibverbs? I previously sent an ABI-preserving version of this patch, but it was hated by Doug Ledford and (eventually) Jason Gunthorpe. After long discussion (see thread starting here: http://www.spinics.net/lists/linux-rdma/msg15951.html), they decided that they wanted a clean break that forces both source code and ABI changes, which resulted in this patch. I personally don't care which way this goes; I just want the ability to have non-enum MTU values. So I guess I need to go back and read all of that thread carefully, but I don't think that silently breaking old binaries and also breaking sources is the right way to go. What about preserving the behavior of the existing API / ABI and then adding a new function to return the size of the maximum datagram for a device? - R. -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] libibverbs: Allow arbitrary int values for MTU
On Mon, Jul 08, 2013 at 10:19:33AM -0700, Roland Dreier wrote: [resending to reply-all, sorry Jeff] On Mon, Jul 8, 2013 at 9:26 AM, Jeff Squyres (jsquyres) jsquy...@cisco.com wrote: So what happens if I have an old application binary, and I run against a new libibverbs without recompiling? Also it seems that I'm forced to change my source code to be able to compile against new libibverbs? I previously sent an ABI-preserving version of this patch, but it was hated by Doug Ledford and (eventually) Jason Gunthorpe. After long discussion (see thread starting here: http://www.spinics.net/lists/linux-rdma/msg15951.html), they decided that they wanted a clean break that forces both source code and ABI changes, which resulted in this patch. I personally don't care which way this goes; I just want the ability to have non-enum MTU values. So I guess I need to go back and read all of that thread carefully, but I don't think that silently breaking old binaries and also breaking sources is the right way to go. What about preserving the behavior of the existing API / ABI and then adding a new function to return the size of the maximum datagram for a device? It is not just a device, but the QP attrs and so on, so there would be quite a few new core functions needed to extend the MTU, and that flows back into the kernel interface too... Jeff's patch doesn't break old binaries, old binaries, running with normal IB MTUs work fine. The structure layouts all stay the same, etc. Old sources will need an update to support non-IB MTUs no matter what, forcing an update via the compiler seems saner then letting them remain silently out of date.. Jason -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 07/13] scsi_transport_srp: Add transport layer error handling
Though, now that I've unpacked it -- I don't think it is OK for dev_loss_tmo to be off, but fast IO to be on? That drops another conditional. The combination of dev_loss_tmo off and reconnect_delay 0 worked fine in my tests. An I/O failure was detected shortly after the cable to the target was pulled. I/O resumed shortly after the cable to the target was reinserted. Perhaps I don't understand your answer -- I'm asking about dev_loss_tmo 0, and fast_io_fail_tmo = 0. The other transports do not allow this scenario, and I'm asking if it makes sense for SRP to allow it. But now that you mention reconnect_delay, what is the meaning of that when it is negative? That's not in the documentation. And should it be considered in srp_tmo_valid() -- are there values of reconnect_delay that cause problems? I'm starting to get a bit concerned about this patch -- can you, Vu, and Sebastian comment on the testing you have done? Hello Bart, After running cable pull test on two local IB links for several hrs, I/Os got stuck. Further commands multipath -ll or fdisk -l got stuck and never return Here are the stack dump for srp-x kernel threads. I'll run with #DEBUG to get more debug info on scsi host rport -vu srp_threads.txt.tgz Description: application/compressed
Re: [PATCH 03/13] infiniband: use get_unused_fd_flags(0) instead of get_unused_fd()
Thanks, I just applied a patch to convert to get_unused_fd_flags(O_CLOEXEC) in uverbs, since there isn't anything useful that can be done with uverbs fds across an exec. - R. -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 07/13] scsi_transport_srp: Add transport layer error handling
On 07/08/13 19:26, Vu Pham wrote:
After running cable pull test on two local IB links for several hrs,
I/Os got stuck.
Further commands multipath -ll or fdisk -l got stuck and never return
Here are the stack dump for srp-x kernel threads.
I'll run with #DEBUG to get more debug info on scsi host rport
Hello Vu,
I had a quick look at the stack dump that was attached to your e-mail.
It shows that scsi_execute_req() hangs in blk_execute_rq(). It would be
appreciated if you could continue your tests with the kernel patch below
applied on top of v3 of this patch series. This patch should avoid that
a transport layer error that occurs after device removal has started can
cause the SCSI device state to change into blocked. This patch also
causes such TL errors to fail I/O quickly (scsi_host_alloc() zero-
initializes the memory it allocates so no explicit initialization of the
deleted variable is necessary).
Thanks,
Bart.
diff --git a/drivers/scsi/scsi_transport_srp.c
b/drivers/scsi/scsi_transport_srp.c
index 1b9ebd5..1bb7c63 100644
--- a/drivers/scsi/scsi_transport_srp.c
+++ b/drivers/scsi/scsi_transport_srp.c
@@ -522,6 +522,12 @@ void srp_start_tl_fail_timers(struct srp_rport *rport)
int fast_io_fail_tmo, dev_loss_tmo, delay;
mutex_lock(rport-mutex);
+ if (rport-deleted) {
+ srp_rport_set_state(rport, SRP_RPORT_FAIL_FAST);
+ scsi_target_unblock(shost-shost_gendev,
+ SDEV_TRANSPORT_OFFLINE);
+ goto unlock;
+ }
delay = rport-reconnect_delay;
fast_io_fail_tmo = rport-fast_io_fail_tmo;
dev_loss_tmo = rport-dev_loss_tmo;
@@ -542,6 +548,7 @@ void srp_start_tl_fail_timers(struct srp_rport *rport)
if (dev_loss_tmo = 0)
queue_delayed_work(system_long_wq, rport-dev_loss_work,
1UL * dev_loss_tmo * HZ);
+unlock:
mutex_unlock(rport-mutex);
}
EXPORT_SYMBOL(srp_start_tl_fail_timers);
@@ -730,6 +737,7 @@ void srp_rport_del(struct srp_rport *rport)
mutex_lock(rport-mutex);
if (rport-state == SRP_RPORT_BLOCKED)
__rport_fast_io_fail_timedout(rport);
+ rport-deleted = true;
mutex_unlock(rport-mutex);
put_device(dev);
diff --git a/include/scsi/scsi_transport_srp.h
b/include/scsi/scsi_transport_srp.h
index fbcc985..a4addcf 100644
--- a/include/scsi/scsi_transport_srp.h
+++ b/include/scsi/scsi_transport_srp.h
@@ -54,6 +54,7 @@ struct srp_rport {
int dev_loss_tmo;
struct delayed_work fast_io_fail_work;
struct delayed_work dev_loss_work;
+ booldeleted;
};
struct srp_function_template {
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 07/13] scsi_transport_srp: Add transport layer error handling
On Thu, 2013-07-04 at 10:01 +0200, Bart Van Assche wrote: On 07/03/13 20:57, David Dillow wrote: And I'm getting the strong sense that the answer to my question about fast_io_fail_tmo = 0 when dev_loss_tmo is that we should not allow that combination, even if it doesn't break the kernel. If it doesn't make sense, there is no reason to create an opportunity for user confusion. Let's take a step back. I think we agree that the only combinations of timeout parameters that are useful are those combinations that guarantee that SCSI commands will be finished in a reasonable time and also that allow multipath to detect failed paths. The first requirement comes down to limiting the value fast_io_fail_tmo can be set to. The second requirement means that either reconnect_delay or fast_io_fail_tmo must be set (please note that a reconnect failure changes the state of all involved SCSI devices into SDEV_TRANSPORT_OFFLINE). So how about modifying srp_tmo_valid() as follows: * Add an argument called reconnect_delay. * Add the following code in that function: if (reconnect_delay 0 fast_io_fail_tmo 0 dev_loss_tmo 0) return -EINVAL; I think this sounds reasonable; I need to make sure I understand the new behaviors of the code to be sure. I'm also concerned about Vu's bug report at this late stage; I'll be watching for its resolution -- hopefully in time for inclusion. -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/13] infiniband: use get_unused_fd_flags(0) instead of get_unused_fd()
Le 08.07.2013 20:23, Roland Dreier a écrit : Thanks, I just applied a patch to convert to get_unused_fd_flags(O_CLOEXEC) in uverbs, since there isn't anything useful that can be done with uverbs fds across an exec. Thanks. In fact, InfiniBand was my main target and I kept this change (setting O_CLOEXEC) for another batch. It's following my patch on libibverbs [PATCH] open files with close on exec flag http://thread.gmane.org/gmane.linux.drivers.rdma/15727 http://marc.info/?l=linux-rdmam=136908991102575w=2 This patch was already quoting Dan Walsh, Excuse me son, but your code is leaking !!! http://danwalsh.livejournal.com/53603.html but I couldn't resist to post it again. BTW, I was working on the rationnal/commit message for setting flags to O_CLOEXEC on kernel side, please find the draft if revelant. 8-- InfiniBand verbs/RDMA: use O_CLOEXEC This subsystem is allocating new file descriptor through the InfiniBand verbs / RDMA API. Thoses file descriptors are created after a write() from userspace to a special device file. No read operation is needed to get the file descriptor: it is returned to userspace in a buffer whose address was stored as part of the buffer passed to write(). If the write() succeed, the response buffer is updated and the new file descriptor is available. But such file descriptors are mostly hidden to application developpers by libibverbs / librdma_cm libraries API. In fact, application developpers could use InfiniBand verbs / RDMA without using directly the file descriptors. Here's how are created the two file descriptors (using mlx4 as example): - ibv_context.async_fd: kernel ib_uverbs_get_context() : linux/drivers/infiniband/core/uverbs_cmd.c uverbs_cmd_table[IB_USER_VERBS_CMD_GET_CONTEXT]() : linux/drivers/infiniband/core/uverbs_main.c ib_uverbs_write() : linux/drivers/infiniband/core/uverbs_main.c uverbs_mmap_fops.write : linux/drivers/infiniband/core/uverbs_main.c userspace ibv_cmd_get_context() : libibverbs/src/cmd.c mlx4_alloc_context() : libmlx4/src/mlx4.c mlx4_dev_ops.alloc_context : libmlx4/src/mlx4.c __ibv_open_device() : libibverbs/src/device.c - ibv_comp_channel.fd: kernel ib_uverbs_create_comp_channel() : linux/drivers/infiniband/core/uverbs_cmd.c uverbs_cmd_table[IB_USER_VERBS_CMD_CREATE_COMP_CHANNEL]() : linux/drivers/infiniband/core/uverbs_main.c ib_uverbs_write() : linux/drivers/infiniband/core/uverbs_main.c uverbs_mmap_fops.write : linux/drivers/infiniband/core/uverbs_main.c userspace ibv_create_comp_channel() : libibverbs/src/verbs.c But those file descriptors are of no use for another program executed through exec(): - without the memory mappings for special memory pages, the file descriptor are of no use ... - the userland libraries/drivers are not ready to found the devices already opened. [ In fact, supporting fork() is already a challenge for thoses API. ] So those file descriptors can safely be opened with O_CLOEXEC without disturbing users of InfiniBand verbs /RDMA 8-- Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
