Online Dating and friend finder
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
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
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.
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
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
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
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.
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.
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 -~--~~~~--~~--~--~---