Online Dating and friend finder

2009-05-19 Thread deepthi

Online Dating and friend finder

http://friendfinder.com/go/g1100916

http://friendfinder.com/go/g1100916
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---



Add better detection of if a command is not going to complete

2009-05-19 Thread Mike Christie
Hey Hannes,

This will not fix any hangs after the scsi eh or iscsi eh has fired, but 
I think this patch will help prevent the scsi eh from firing when we do 
not need it to like you have seen in some bugzillas. The patch was made 
over the my iscsi tree. It should also apply to scsi-rc-fixes with the 
patches I sent the other day.

I modified our command timedout handler so if a command has made some 
progress since the last timeout or if it is just getting started (it has 
been put on the wire but we have not yet got anything for it), then we 
will ask for some more time to run it.

This is helping here for these problems:
1. sending more IO than the disk/target can handle
2. using a shorter scsi cmd timeout with a slower link

I am going to combine this with those change_queue_depth patches if they 
are ok upstream, and in the end also add lpfc/qla2xxx's rampup/rampdown 
code to scsi-ml. So basically if we determine if we are sending too many 
IOs, then we can call some helper rampdown code to drop the queue depth 
for the user. If however it was a transient problem, the common ramp up 
code will detect it and increase it again.

I think with the combo rampup/rampdown and the modified 
iscsi_eh_cmd_timed_out in this patch, it should fix a lot of problems we 
see where the scsi-eh runs when it should not.

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---

>From 23acb13e1290eb0e0c08287fea1e396a17cd2167 Mon Sep 17 00:00:00 2001
From: Mike Christie 
Date: Mon, 18 May 2009 19:59:17 -0500
Subject: [PATCH 4/5] libiscsi: reset command timer if iscsi task is making progress

This patch has the iscsi eh cmd time out handler ask for more
time if the command has completed a pdu within the command timer.
It also makes sure that we check the transport and that the
transport checks do not accidentally reset the command timer
if the command really does need to be unjammed via the scsi eh.

Signed-off-by: Mike Christie 
---
 drivers/scsi/libiscsi.c |   52 +-
 drivers/scsi/libiscsi_tcp.c |6 +++-
 include/scsi/libiscsi.h |4 +++
 3 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 59908ae..4cc3184 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -1361,6 +1361,9 @@ static inline struct iscsi_task *iscsi_alloc_task(struct iscsi_conn *conn,
 	task->state = ISCSI_TASK_PENDING;
 	task->conn = conn;
 	task->sc = sc;
+	task->have_checked_conn = 0;
+	task->last_timeout = jiffies;
+	task->last_recv = jiffies;
 	INIT_LIST_HEAD(&task->running);
 	return task;
 }
@@ -1716,17 +1719,18 @@ static int iscsi_has_ping_timed_out(struct iscsi_conn *conn)
 		return 0;
 }
 
-static enum blk_eh_timer_return iscsi_eh_cmd_timed_out(struct scsi_cmnd *scmd)
+static enum blk_eh_timer_return iscsi_eh_cmd_timed_out(struct scsi_cmnd *sc)
 {
+	enum blk_eh_timer_return rc = BLK_EH_NOT_HANDLED;
+	struct iscsi_task *task = NULL;
 	struct iscsi_cls_session *cls_session;
 	struct iscsi_session *session;
 	struct iscsi_conn *conn;
-	enum blk_eh_timer_return rc = BLK_EH_NOT_HANDLED;
 
-	cls_session = starget_to_session(scsi_target(scmd->device));
+	cls_session = starget_to_session(scsi_target(sc->device));
 	session = cls_session->dd_data;
 
-	ISCSI_DBG_SESSION(session, "scsi cmd %p timedout\n", scmd);
+	ISCSI_DBG_SESSION(session, "scsi cmd %p timedout\n", sc);
 
 	spin_lock(&session->lock);
 	if (session->state != ISCSI_STATE_LOGGED_IN) {
@@ -1745,6 +1749,23 @@ static enum blk_eh_timer_return iscsi_eh_cmd_timed_out(struct scsi_cmnd *scmd)
 		goto done;
 	}
 
+	task = (struct iscsi_task *)sc->SCp.ptr;
+	if (!task)
+		goto done;
+	/*
+	 * If we have processed a PDU for the command since the last
+	 * timeout then ask for more time.
+	 */
+	if (time_after_eq(task->last_recv, task->last_timeout)) {
+		ISCSI_DBG_CONN(conn, "Command making progress. Asking "
+			   "scsi-ml for more time to complete. "
+			   "Last data recv at %lu. Last timeout was at "
+			   "%lu\n.", task->last_recv, task->last_timeout);
+		task->have_checked_conn = 0;
+		rc = BLK_EH_RESET_TIMER;
+		goto done;
+	}
+
 	if (!conn->recv_timeout && !conn->ping_timeout)
 		goto done;
 	/*
@@ -1755,20 +1776,29 @@ static enum blk_eh_timer_return iscsi_eh_cmd_timed_out(struct scsi_cmnd *scmd)
 		rc = BLK_EH_RESET_TIMER;
 		goto done;
 	}
+
+	/* Assumes nop timeout is shorter than scsi cmd timeout */
+	if (task->have_checked_conn)
+		goto done;
+
 	/*
-	 * if we are about to check the transport then give the command
-	 * more time
+	 * Checking the transport already or 

Re: Add better detection of if a command is not going to complete

2009-05-19 Thread Mike Christie

Oh yeah, Erez,

This will fix the issue we saw yesterday. Did you want me to port this 
for you to test? I think we will end up with the same result, because I 
am still making sure the transport is good before letting the scsi eh 
run. So if the nop still times out, we end up dropping the session and 
it command gets cleaned up that way.



Mike Christie wrote:
> Hey Hannes,
> 
> This will not fix any hangs after the scsi eh or iscsi eh has fired, but 
> I think this patch will help prevent the scsi eh from firing when we do 
> not need it to like you have seen in some bugzillas. The patch was made 
> over the my iscsi tree. It should also apply to scsi-rc-fixes with the 
> patches I sent the other day.
> 
> I modified our command timedout handler so if a command has made some 
> progress since the last timeout or if it is just getting started (it has 
> been put on the wire but we have not yet got anything for it), then we 
> will ask for some more time to run it.
> 
> This is helping here for these problems:
> 1. sending more IO than the disk/target can handle
> 2. using a shorter scsi cmd timeout with a slower link
> 
> I am going to combine this with those change_queue_depth patches if they 
> are ok upstream, and in the end also add lpfc/qla2xxx's rampup/rampdown 
> code to scsi-ml. So basically if we determine if we are sending too many 
> IOs, then we can call some helper rampdown code to drop the queue depth 
> for the user. If however it was a transient problem, the common ramp up 
> code will detect it and increase it again.
> 
> I think with the combo rampup/rampdown and the modified 
> iscsi_eh_cmd_timed_out in this patch, it should fix a lot of problems we 
> see where the scsi-eh runs when it should not.
> 


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---



Re: [PATCH 4/4] bnx2i: Add bnx2i iSCSI driver.

2009-05-19 Thread Mike Christie

Michael Chan wrote:
> On Thu, 2009-05-07 at 14:01 -0700, Mike Christie wrote:
>> Michael Chan wrote:
>>> On Wed, 2009-05-06 at 09:48 -0700, Mike Christie wrote:
 I think cxgb3i is one day going to want to support the same features 
 bnx2i does. If that is right, then should we just make the NX2_UIO 
 events common iscsi events, and hook cxb3i in? It would not use the 
 iscsi set param interface at all and would work just like bnx2i. Is that 
 possible? What about future drivers? Are done making iscsi cards and 
 drivers. If so, thank goodness :)  If not then maybe we want to consider 
 some future driver using the #2 module and possibly using this.

 If cxgb3i is really only going to support static ip setup and we think 
 that bnx2i is going to be unique on how it sets up the network then I 
 NX2_UIO private events are fine. Or is this a case of we are thinking 
 that iscsi hardware people are creating crazy interfaces so there is no 
 why to predict what they are going to do so there is no point in trying 
 to design for them.
>>> If there is any possibility that cxgb3i will use something similar to
>>> bnx2i, I think we can change the message to a standard one and make the
>>> message structure somewhat more generic.  We'll probably still need a
>>> private area in the message for hardware or vendor specific information.
>>>
>> Ok sounds good to me.
>>
> 
> Here are the more generic NETLINK_ISCSI messages and the iscsi transport
> code to support them, please review.
> 
> diff --git a/drivers/scsi/scsi_transport_iscsi.c 
> b/drivers/scsi/scsi_transport_iscsi.c
> index d69a53a..60cb6cb 100644
> --- a/drivers/scsi/scsi_transport_iscsi.c
> +++ b/drivers/scsi/scsi_transport_iscsi.c
> @@ -995,6 +995,39 @@ int iscsi_recv_pdu(struct iscsi_cls_conn *conn, struct 
> iscsi_hdr *hdr,
>  }
>  EXPORT_SYMBOL_GPL(iscsi_recv_pdu);
>  
> +int iscsi_offload_mesg(struct Scsi_Host *shost,
> +struct iscsi_transport *transport, uint32_t type,
> +char *data, uint16_t data_size)
> +{
> + struct nlmsghdr *nlh;
> + struct sk_buff *skb;
> + struct iscsi_uevent *ev;
> + int len = NLMSG_SPACE(sizeof(*ev) + data_size);
> +
> + skb = alloc_skb(len, GFP_ATOMIC);
> + if (!skb) {
> + printk(KERN_ERR "can not deliver iscsi offload message:OOM\n");
> + return -ENOMEM;
> + }
> +
> + nlh = __nlmsg_put(skb, 0, 0, 0, (len - sizeof(*nlh)), 0);
> + ev = NLMSG_DATA(nlh);
> + memset(ev, 0, sizeof(*ev));
> + ev->type = type;
> + ev->transport_handle = iscsi_handle(transport);
> + switch (type) {
> + case ISCSI_KEVENT_PATH_REQ:
> + ev->r.req_path.host_no = shost->host_no;
> + case ISCSI_KEVENT_IF_DOWN:
> + ev->r.notify_if_down.host_no = shost->host_no;
> + }
> +
> + memcpy((char*)ev + sizeof(*ev), data, data_size);
> +
> + return iscsi_broadcast_skb(skb, GFP_KERNEL);


You can sync up what the gfp flag used here and for the alloc_skb call 
above. If you have process context, you probably want to use GFP_NOIO, 
because this could be called for reconnect for a disk in use.

If you do not have process context then you would need to use GFP_ATOMIC.


> +}
> +EXPORT_SYMBOL_GPL(iscsi_offload_mesg);
> +
>  void iscsi_conn_error_event(struct iscsi_cls_conn *conn, enum iscsi_err 
> error)
>  {
>   struct nlmsghdr *nlh;
> @@ -1393,6 +1426,30 @@ iscsi_set_host_param(struct iscsi_transport *transport,
>  }
>  
>  static int
> +iscsi_set_path(struct iscsi_transport *transport, struct iscsi_uevent *ev)
> +{
> + struct Scsi_Host *shost;
> + struct iscsi_path *params;
> + int err;
> +
> + if (!transport->set_path)
> + return -ENOSYS;
> +
> + shost = scsi_host_lookup(ev->u.set_path.host_no);
> + if (!shost) {
> + printk(KERN_ERR "set path could not find host no %u\n",
> +ev->u.set_path.host_no);
> + return -ENODEV;
> + }
> +
> + params = (struct iscsi_path *)((char*)ev + sizeof(*ev));
> + err = transport->set_path(shost, params);
> +   
> + scsi_host_put(shost);
> + return err;
> +}
> +
> +static int
>  iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>  {
>   int err = 0;
> @@ -1411,7 +1468,8 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr 
> *nlh)
>   if (!try_module_get(transport->owner))
>   return -EINVAL;
>  
> - priv->daemon_pid = NETLINK_CREDS(skb)->pid;
> + if (nlh->nlmsg_type != ISCSI_UEVENT_PATH_UPDATE)
> + priv->daemon_pid = NETLINK_CREDS(skb)->pid;
>  

Instead of using broadcast above and in some other places and then doing 
this check, could we just use multicast groups or something else? The 
events from iscsid could be in one group and then events for uip would 
be in another?

Or is it more common to do it like this or will it break compat w

Re: Add better detection of if a command is not going to complete

2009-05-19 Thread Mike Christie

Mike Christie wrote:
> Hey Hannes,
> 
> This will not fix any hangs after the scsi eh or iscsi eh has fired, but 
> I think this patch will help prevent the scsi eh from firing when we do 
> not need it to like you have seen in some bugzillas. The patch was made 
> over the my iscsi tree. It should also apply to scsi-rc-fixes with the 
> patches I sent the other day.
> 
> I modified our command timedout handler so if a command has made some 
> progress since the last timeout or if it is just getting started (it has 
> been put on the wire but we have not yet got anything for it), then we 
> will ask for some more time to run it.
> 
> This is helping here for these problems:
> 1. sending more IO than the disk/target can handle
> 2. using a shorter scsi cmd timeout with a slower link
> 
> I am going to combine this with those change_queue_depth patches if they 
> are ok upstream, and in the end also add lpfc/qla2xxx's rampup/rampdown 
> code to scsi-ml. So basically if we determine if we are sending too many 
> IOs, then we can call some helper rampdown code to drop the queue depth 
> for the user. If however it was a transient problem, the common ramp up 
> code will detect it and increase it again.
> 
> I think with the combo rampup/rampdown and the modified 
> iscsi_eh_cmd_timed_out in this patch, it should fix a lot of problems we 
> see where the scsi-eh runs when it should not.
> 
> > 
> 


@@ -1361,6 +1361,9 @@ static inline struct iscsi_task 
*iscsi_alloc_task(struct iscsi_conn *conn,
task->state = ISCSI_TASK_PENDING;
task->conn = conn;
task->sc = sc;
+   task->have_checked_conn = 0;
+   task->last_timeout = jiffies;
+   task->last_recv = jiffies;
INIT_LIST_HEAD(&task->running);
return task;
  }


+* If we have processed a PDU for the command since the last
+* timeout then ask for more time.
+*/
+   if (time_after_eq(task->last_recv, task->last_timeout)) {


Oh yeah, for the case where the command has been sent but we have not 
got anything for it yet, we just give it one more cmd timeouts worth of 
time. That is where the eq part of the test above is commonly hit (when 
the task is allocate they are set to the same value). If the command 
times out again and we still have not got any data then we will let the 
scsi eh have it. In the future I was thinking that when we first detect 
this (before we have scsi ml reset the timer), we should decrease the 
queue_depth using the rampdown code I want to add to scsi-ml (the code 
posted yesterday only ramps down the depth when seeing a QUEUE_FULL so I 
would add a helper for use to call).


+   ISCSI_DBG_CONN(conn, "Command making progress. Asking "
+  "scsi-ml for more time to complete. "
+  "Last data recv at %lu. Last timeout was at "
+  "%lu\n.", task->last_recv,



--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---



Re: iscsid : mgmt_ipc_write_rsp: rsp to fd 5

2009-05-19 Thread Philippe

UP!
Philippe

On 13 mai, 16:21, Philippe  wrote:

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---



Re: Add better detection of if a command is not going to complete

2009-05-19 Thread Mike Christie
Mike Christie wrote:
> Hey Hannes,
> 
> This will not fix any hangs after the scsi eh or iscsi eh has fired, but 
> I think this patch will help prevent the scsi eh from firing when we do 
> not need it to like you have seen in some bugzillas. The patch was made 
> over the my iscsi tree. It should also apply to scsi-rc-fixes with the 
> patches I sent the other day.
> 
> I modified our command timedout handler so if a command has made some 
> progress since the last timeout or if it is just getting started (it has 
> been put on the wire but we have not yet got anything for it), then we 
> will ask for some more time to run it.
> 
> This is helping here for these problems:
> 1. sending more IO than the disk/target can handle
> 2. using a shorter scsi cmd timeout with a slower link
> 

Attached is a updated patch that should better handle larger writes. If 
we have successfully sent IO to the network layer or LLD in cxgb3i's 
case, and the command times out then we will give the response or r2t 
more time to reach us. If on the next timeout we still have not got 
anything then we will let the scsi eh run.

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---

>From 23acb13e1290eb0e0c08287fea1e396a17cd2167 Mon Sep 17 00:00:00 2001
From: Mike Christie 
Date: Mon, 18 May 2009 19:59:17 -0500
Subject: [PATCH 4/5] libiscsi: reset command timer if iscsi task is making progress

This patch has the iscsi eh cmd time out handler ask for more
time if the command has completed a pdu within the command timer.
It also makes sure that we check the transport and that the
transport checks do not accidentally reset the command timer
if the command really does need to be unjammed via the scsi eh.

Signed-off-by: Mike Christie 
---
 drivers/scsi/libiscsi.c |   52 +-
 drivers/scsi/libiscsi_tcp.c |6 +++-
 include/scsi/libiscsi.h |4 +++
 3 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 59908ae..4cc3184 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -1361,6 +1361,9 @@ static inline struct iscsi_task *iscsi_alloc_task(struct iscsi_conn *conn,
 	task->state = ISCSI_TASK_PENDING;
 	task->conn = conn;
 	task->sc = sc;
+	task->have_checked_conn = 0;
+	task->last_timeout = jiffies;
+	task->last_recv = jiffies;
 	INIT_LIST_HEAD(&task->running);
 	return task;
 }
@@ -1716,17 +1719,18 @@ static int iscsi_has_ping_timed_out(struct iscsi_conn *conn)
 		return 0;
 }
 
-static enum blk_eh_timer_return iscsi_eh_cmd_timed_out(struct scsi_cmnd *scmd)
+static enum blk_eh_timer_return iscsi_eh_cmd_timed_out(struct scsi_cmnd *sc)
 {
+	enum blk_eh_timer_return rc = BLK_EH_NOT_HANDLED;
+	struct iscsi_task *task = NULL;
 	struct iscsi_cls_session *cls_session;
 	struct iscsi_session *session;
 	struct iscsi_conn *conn;
-	enum blk_eh_timer_return rc = BLK_EH_NOT_HANDLED;
 
-	cls_session = starget_to_session(scsi_target(scmd->device));
+	cls_session = starget_to_session(scsi_target(sc->device));
 	session = cls_session->dd_data;
 
-	ISCSI_DBG_SESSION(session, "scsi cmd %p timedout\n", scmd);
+	ISCSI_DBG_SESSION(session, "scsi cmd %p timedout\n", sc);
 
 	spin_lock(&session->lock);
 	if (session->state != ISCSI_STATE_LOGGED_IN) {
@@ -1745,6 +1749,23 @@ static enum blk_eh_timer_return iscsi_eh_cmd_timed_out(struct scsi_cmnd *scmd)
 		goto done;
 	}
 
+	task = (struct iscsi_task *)sc->SCp.ptr;
+	if (!task)
+		goto done;
+	/*
+	 * If we have processed a PDU for the command since the last
+	 * timeout then ask for more time.
+	 */
+	if (time_after_eq(task->last_recv, task->last_timeout)) {
+		ISCSI_DBG_CONN(conn, "Command making progress. Asking "
+			   "scsi-ml for more time to complete. "
+			   "Last data recv at %lu. Last timeout was at "
+			   "%lu\n.", task->last_recv, task->last_timeout);
+		task->have_checked_conn = 0;
+		rc = BLK_EH_RESET_TIMER;
+		goto done;
+	}
+
 	if (!conn->recv_timeout && !conn->ping_timeout)
 		goto done;
 	/*
@@ -1755,20 +1776,29 @@ static enum blk_eh_timer_return iscsi_eh_cmd_timed_out(struct scsi_cmnd *scmd)
 		rc = BLK_EH_RESET_TIMER;
 		goto done;
 	}
+
+	/* Assumes nop timeout is shorter than scsi cmd timeout */
+	if (task->have_checked_conn)
+		goto done;
+
 	/*
-	 * if we are about to check the transport then give the command
-	 * more time
+	 * Checking the transport already or nop from a cmd timeout still
+	 * running
 	 */
-	if (time_before_eq(conn->last_recv + (conn->recv_timeout * HZ),
-			   jiffies)) {
+	if (conn->ping_task) {
+		task->have_checked_conn = 1;
 		rc = BLK_EH_RE

Re: [PATCH 4/4] bnx2i: Add bnx2i iSCSI driver.

2009-05-19 Thread Michael Chan


On Tue, 2009-05-19 at 07:22 -0700, Mike Christie wrote:
> Michael Chan wrote:
> > 
> > Here are the more generic NETLINK_ISCSI messages and the iscsi transport
> > code to support them, please review.
> > 
> > diff --git a/drivers/scsi/scsi_transport_iscsi.c 
> > b/drivers/scsi/scsi_transport_iscsi.c
> > index d69a53a..60cb6cb 100644
> > --- a/drivers/scsi/scsi_transport_iscsi.c
> > +++ b/drivers/scsi/scsi_transport_iscsi.c
> > @@ -995,6 +995,39 @@ int iscsi_recv_pdu(struct iscsi_cls_conn *conn, struct 
> > iscsi_hdr *hdr,
> >  }
> >  EXPORT_SYMBOL_GPL(iscsi_recv_pdu);
> >  
> > +int iscsi_offload_mesg(struct Scsi_Host *shost,
> > +  struct iscsi_transport *transport, uint32_t type,
> > +  char *data, uint16_t data_size)
> > +{
> > +   struct nlmsghdr *nlh;
> > +   struct sk_buff *skb;
> > +   struct iscsi_uevent *ev;
> > +   int len = NLMSG_SPACE(sizeof(*ev) + data_size);
> > +
> > +   skb = alloc_skb(len, GFP_ATOMIC);
> > +   if (!skb) {
> > +   printk(KERN_ERR "can not deliver iscsi offload message:OOM\n");
> > +   return -ENOMEM;
> > +   }
> > +
> > +   nlh = __nlmsg_put(skb, 0, 0, 0, (len - sizeof(*nlh)), 0);
> > +   ev = NLMSG_DATA(nlh);
> > +   memset(ev, 0, sizeof(*ev));
> > +   ev->type = type;
> > +   ev->transport_handle = iscsi_handle(transport);
> > +   switch (type) {
> > +   case ISCSI_KEVENT_PATH_REQ:
> > +   ev->r.req_path.host_no = shost->host_no;
> > +   case ISCSI_KEVENT_IF_DOWN:
> > +   ev->r.notify_if_down.host_no = shost->host_no;
> > +   }
> > +
> > +   memcpy((char*)ev + sizeof(*ev), data, data_size);
> > +
> > +   return iscsi_broadcast_skb(skb, GFP_KERNEL);
> 
> 
> You can sync up what the gfp flag used here and for the alloc_skb call 
> above. If you have process context, you probably want to use GFP_NOIO, 
> because this could be called for reconnect for a disk in use.
> 
> If you do not have process context then you would need to use GFP_ATOMIC.
> 
> 

We have process context, but I think we should make it more general for
other future drivers and use GFP_ATOMIC.

> > +}
> > +EXPORT_SYMBOL_GPL(iscsi_offload_mesg);
> > +
> >  void iscsi_conn_error_event(struct iscsi_cls_conn *conn, enum iscsi_err 
> > error)
> >  {
> > struct nlmsghdr *nlh;
> > @@ -1393,6 +1426,30 @@ iscsi_set_host_param(struct iscsi_transport 
> > *transport,
> >  }
> >  
> >  static int
> > +iscsi_set_path(struct iscsi_transport *transport, struct iscsi_uevent *ev)
> > +{
> > +   struct Scsi_Host *shost;
> > +   struct iscsi_path *params;
> > +   int err;
> > +
> > +   if (!transport->set_path)
> > +   return -ENOSYS;
> > +
> > +   shost = scsi_host_lookup(ev->u.set_path.host_no);
> > +   if (!shost) {
> > +   printk(KERN_ERR "set path could not find host no %u\n",
> > +  ev->u.set_path.host_no);
> > +   return -ENODEV;
> > +   }
> > +
> > +   params = (struct iscsi_path *)((char*)ev + sizeof(*ev));
> > +   err = transport->set_path(shost, params);
> > + 
> > +   scsi_host_put(shost);
> > +   return err;
> > +}
> > +
> > +static int
> >  iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
> >  {
> > int err = 0;
> > @@ -1411,7 +1468,8 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct 
> > nlmsghdr *nlh)
> > if (!try_module_get(transport->owner))
> > return -EINVAL;
> >  
> > -   priv->daemon_pid = NETLINK_CREDS(skb)->pid;
> > +   if (nlh->nlmsg_type != ISCSI_UEVENT_PATH_UPDATE)
> > +   priv->daemon_pid = NETLINK_CREDS(skb)->pid;
> >  
> 
> Instead of using broadcast above and in some other places and then doing 
> this check, could we just use multicast groups or something else? The 
> events from iscsid could be in one group and then events for uip would 
> be in another?

We need to do this check because we don't want the daemon_pid to be
overwritten with a pid that is not iscsid's.  If it was overwritten,
unicast NETLINK_ISCSI messages will not reach iscsid.

We can use multicast group 2 for the new messages if you prefer.  This
way, I think iscsid will not receive the new messages since it is only
listening on group 1.  The pid check will still be needed though.

> 
> Or is it more common to do it like this or will it break compat with 
> other tools if we change it?
> 



--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---



Re: [PATCH 4/4] bnx2i: Add bnx2i iSCSI driver.

2009-05-19 Thread Mike Christie

Michael Chan wrote:
> 
> On Tue, 2009-05-19 at 07:22 -0700, Mike Christie wrote:
>> Michael Chan wrote:
>>> Here are the more generic NETLINK_ISCSI messages and the iscsi transport
>>> code to support them, please review.
>>>
>>> diff --git a/drivers/scsi/scsi_transport_iscsi.c 
>>> b/drivers/scsi/scsi_transport_iscsi.c
>>> index d69a53a..60cb6cb 100644
>>> --- a/drivers/scsi/scsi_transport_iscsi.c
>>> +++ b/drivers/scsi/scsi_transport_iscsi.c
>>> @@ -995,6 +995,39 @@ int iscsi_recv_pdu(struct iscsi_cls_conn *conn, struct 
>>> iscsi_hdr *hdr,
>>>  }
>>>  EXPORT_SYMBOL_GPL(iscsi_recv_pdu);
>>>  
>>> +int iscsi_offload_mesg(struct Scsi_Host *shost,
>>> +  struct iscsi_transport *transport, uint32_t type,
>>> +  char *data, uint16_t data_size)
>>> +{
>>> +   struct nlmsghdr *nlh;
>>> +   struct sk_buff *skb;
>>> +   struct iscsi_uevent *ev;
>>> +   int len = NLMSG_SPACE(sizeof(*ev) + data_size);
>>> +
>>> +   skb = alloc_skb(len, GFP_ATOMIC);
>>> +   if (!skb) {
>>> +   printk(KERN_ERR "can not deliver iscsi offload message:OOM\n");
>>> +   return -ENOMEM;
>>> +   }
>>> +
>>> +   nlh = __nlmsg_put(skb, 0, 0, 0, (len - sizeof(*nlh)), 0);
>>> +   ev = NLMSG_DATA(nlh);
>>> +   memset(ev, 0, sizeof(*ev));
>>> +   ev->type = type;
>>> +   ev->transport_handle = iscsi_handle(transport);
>>> +   switch (type) {
>>> +   case ISCSI_KEVENT_PATH_REQ:
>>> +   ev->r.req_path.host_no = shost->host_no;
>>> +   case ISCSI_KEVENT_IF_DOWN:
>>> +   ev->r.notify_if_down.host_no = shost->host_no;
>>> +   }
>>> +
>>> +   memcpy((char*)ev + sizeof(*ev), data, data_size);
>>> +
>>> +   return iscsi_broadcast_skb(skb, GFP_KERNEL);
>>
>> You can sync up what the gfp flag used here and for the alloc_skb call 
>> above. If you have process context, you probably want to use GFP_NOIO, 
>> because this could be called for reconnect for a disk in use.
>>
>> If you do not have process context then you would need to use GFP_ATOMIC.
>>
>>
> 
> We have process context, but I think we should make it more general for
> other future drivers and use GFP_ATOMIC.


If you have process context just use GFP_NOIO. We can change it later 
when/if a driver needs it. I think we like to avoid GFP_ATOMIC if we 
can. Or just add a gfp_t argument to the function so the caller can do 
what is right for them if you want to make it general.


> 
>>> +}
>>> +EXPORT_SYMBOL_GPL(iscsi_offload_mesg);
>>> +
>>>  void iscsi_conn_error_event(struct iscsi_cls_conn *conn, enum iscsi_err 
>>> error)
>>>  {
>>> struct nlmsghdr *nlh;
>>> @@ -1393,6 +1426,30 @@ iscsi_set_host_param(struct iscsi_transport 
>>> *transport,
>>>  }
>>>  
>>>  static int
>>> +iscsi_set_path(struct iscsi_transport *transport, struct iscsi_uevent *ev)
>>> +{
>>> +   struct Scsi_Host *shost;
>>> +   struct iscsi_path *params;
>>> +   int err;
>>> +
>>> +   if (!transport->set_path)
>>> +   return -ENOSYS;
>>> +
>>> +   shost = scsi_host_lookup(ev->u.set_path.host_no);
>>> +   if (!shost) {
>>> +   printk(KERN_ERR "set path could not find host no %u\n",
>>> +  ev->u.set_path.host_no);
>>> +   return -ENODEV;
>>> +   }
>>> +
>>> +   params = (struct iscsi_path *)((char*)ev + sizeof(*ev));
>>> +   err = transport->set_path(shost, params);
>>> + 
>>> +   scsi_host_put(shost);
>>> +   return err;
>>> +}
>>> +
>>> +static int
>>>  iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>>>  {
>>> int err = 0;
>>> @@ -1411,7 +1468,8 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct 
>>> nlmsghdr *nlh)
>>> if (!try_module_get(transport->owner))
>>> return -EINVAL;
>>>  
>>> -   priv->daemon_pid = NETLINK_CREDS(skb)->pid;
>>> +   if (nlh->nlmsg_type != ISCSI_UEVENT_PATH_UPDATE)
>>> +   priv->daemon_pid = NETLINK_CREDS(skb)->pid;
>>>  
>> Instead of using broadcast above and in some other places and then doing 
>> this check, could we just use multicast groups or something else? The 
>> events from iscsid could be in one group and then events for uip would 
>> be in another?
> 
> We need to do this check because we don't want the daemon_pid to be
> overwritten with a pid that is not iscsid's.  If it was overwritten,
> unicast NETLINK_ISCSI messages will not reach iscsid.
> 
> We can use multicast group 2 for the new messages if you prefer.  This
> way, I think iscsid will not receive the new messages since it is only
> listening on group 1.  The pid check will still be needed though.
> 

ok.

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---