* [PATCH 0/2] xen/scsiback: correct two issues @ 2016-02-05 13:15 Juergen Gross 2016-02-05 13:21 ` [PATCH 1/2] xen/scsiback: correct frontend counting Juergen Gross 0 siblings, 1 reply; 10+ messages in thread From: Juergen Gross @ 2016-02-05 13:15 UTC (permalink / raw) To: linux-kernel, xen-devel, konrad.wilk, david.vrabel, boris.ostrovsky Cc: Juergen Gross Correct two issues in the Xen pvscsi backend. Juergen Gross (2): xen/scsiback: correct frontend counting xen/scsiback: avoid warnings when adding multiple LUNs to a domain drivers/xen/xen-scsiback.c | 75 ++++++++++++++++++++++++++-------------------- 1 file changed, 42 insertions(+), 33 deletions(-) -- 2.6.2 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] xen/scsiback: correct frontend counting 2016-02-05 13:15 [PATCH 0/2] xen/scsiback: correct two issues Juergen Gross @ 2016-02-05 13:21 ` Juergen Gross 2016-02-05 13:21 ` [PATCH 2/2] xen/scsiback: avoid warnings when adding multiple LUNs to a domain Juergen Gross 2016-02-05 15:42 ` [PATCH 1/2] xen/scsiback: correct frontend counting Boris Ostrovsky 0 siblings, 2 replies; 10+ messages in thread From: Juergen Gross @ 2016-02-05 13:21 UTC (permalink / raw) To: linux-kernel, xen-devel, konrad.wilk, david.vrabel, boris.ostrovsky Cc: Juergen Gross, stable When adding a new frontend to xen-scsiback don't decrement the number of active frontends in case of no error. Not doing so results in a failure when trying to remove the xen-pvscsi nexus even if no domain is using it. Signed-off-by: Juergen Gross <jgross@suse.com> Cc: stable@vger.kernel.org --- drivers/xen/xen-scsiback.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c index ad4eb10..51387d7 100644 --- a/drivers/xen/xen-scsiback.c +++ b/drivers/xen/xen-scsiback.c @@ -939,12 +939,12 @@ out: spin_unlock_irqrestore(&info->v2p_lock, flags); out_free: - mutex_lock(&tpg->tv_tpg_mutex); - tpg->tv_tpg_fe_count--; - mutex_unlock(&tpg->tv_tpg_mutex); - - if (err) + if (err) { + mutex_lock(&tpg->tv_tpg_mutex); + tpg->tv_tpg_fe_count--; + mutex_unlock(&tpg->tv_tpg_mutex); kfree(new); + } return err; } -- 2.6.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] xen/scsiback: avoid warnings when adding multiple LUNs to a domain 2016-02-05 13:21 ` [PATCH 1/2] xen/scsiback: correct frontend counting Juergen Gross @ 2016-02-05 13:21 ` Juergen Gross 2016-02-05 15:50 ` Boris Ostrovsky 2016-02-05 15:42 ` [PATCH 1/2] xen/scsiback: correct frontend counting Boris Ostrovsky 1 sibling, 1 reply; 10+ messages in thread From: Juergen Gross @ 2016-02-05 13:21 UTC (permalink / raw) To: linux-kernel, xen-devel, konrad.wilk, david.vrabel, boris.ostrovsky Cc: Juergen Gross, stable When adding more than one LUN to a frontend a warning for a failed assignment is issued in dom0 for each already existing LUN. Avoid this warning. Signed-off-by: Juergen Gross <jgross@suse.com> Cc: stable@vger.kernel.org --- drivers/xen/xen-scsiback.c | 65 ++++++++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 28 deletions(-) diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c index 51387d7..69de879 100644 --- a/drivers/xen/xen-scsiback.c +++ b/drivers/xen/xen-scsiback.c @@ -849,15 +849,31 @@ static int scsiback_map(struct vscsibk_info *info) } /* + Check for a translation entry being present +*/ +static struct v2p_entry *scsiback_chk_translation_entry( + struct vscsibk_info *info, struct ids_tuple *v) +{ + struct list_head *head = &(info->v2p_entry_lists); + struct v2p_entry *entry; + + list_for_each_entry(entry, head, l) + if ((entry->v.chn == v->chn) && + (entry->v.tgt == v->tgt) && + (entry->v.lun == v->lun)) + return entry; + + return NULL; +} + +/* Add a new translation entry */ static int scsiback_add_translation_entry(struct vscsibk_info *info, char *phy, struct ids_tuple *v) { int err = 0; - struct v2p_entry *entry; struct v2p_entry *new; - struct list_head *head = &(info->v2p_entry_lists); unsigned long flags; char *lunp; unsigned long long unpacked_lun; @@ -917,15 +933,10 @@ static int scsiback_add_translation_entry(struct vscsibk_info *info, spin_lock_irqsave(&info->v2p_lock, flags); /* Check double assignment to identical virtual ID */ - list_for_each_entry(entry, head, l) { - if ((entry->v.chn == v->chn) && - (entry->v.tgt == v->tgt) && - (entry->v.lun == v->lun)) { - pr_warn("Virtual ID is already used. Assignment was not performed.\n"); - err = -EEXIST; - goto out; - } - + if (scsiback_chk_translation_entry(info, v)) { + pr_warn("Virtual ID is already used. Assignment was not performed.\n"); + err = -EEXIST; + goto out; } /* Create a new translation entry and add to the list */ @@ -933,7 +944,7 @@ static int scsiback_add_translation_entry(struct vscsibk_info *info, new->v = *v; new->tpg = tpg; new->lun = unpacked_lun; - list_add_tail(&new->l, head); + list_add_tail(&new->l, &info->v2p_entry_lists); out: spin_unlock_irqrestore(&info->v2p_lock, flags); @@ -962,33 +973,31 @@ static int scsiback_del_translation_entry(struct vscsibk_info *info, struct ids_tuple *v) { struct v2p_entry *entry; - struct list_head *head = &(info->v2p_entry_lists); unsigned long flags; spin_lock_irqsave(&info->v2p_lock, flags); /* Find out the translation entry specified */ - list_for_each_entry(entry, head, l) { - if ((entry->v.chn == v->chn) && - (entry->v.tgt == v->tgt) && - (entry->v.lun == v->lun)) { - goto found; - } - } - - spin_unlock_irqrestore(&info->v2p_lock, flags); - return 1; - -found: - /* Delete the translation entry specfied */ - __scsiback_del_translation_entry(entry); + entry = scsiback_chk_translation_entry(info, v); + if (entry) + __scsiback_del_translation_entry(entry); spin_unlock_irqrestore(&info->v2p_lock, flags); - return 0; + return entry == NULL; } static void scsiback_do_add_lun(struct vscsibk_info *info, const char *state, char *phy, struct ids_tuple *vir, int try) { + struct v2p_entry *entry; + unsigned long flags; + + if (try) { + spin_lock_irqsave(&info->v2p_lock, flags); + entry = scsiback_chk_translation_entry(info, vir); + spin_unlock_irqrestore(&info->v2p_lock, flags); + if (entry) + return; + } if (!scsiback_add_translation_entry(info, phy, vir)) { if (xenbus_printf(XBT_NIL, info->dev->nodename, state, "%d", XenbusStateInitialised)) { -- 2.6.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] xen/scsiback: avoid warnings when adding multiple LUNs to a domain 2016-02-05 13:21 ` [PATCH 2/2] xen/scsiback: avoid warnings when adding multiple LUNs to a domain Juergen Gross @ 2016-02-05 15:50 ` Boris Ostrovsky 2016-02-05 15:55 ` Boris Ostrovsky 2016-02-05 16:59 ` Juergen Gross 0 siblings, 2 replies; 10+ messages in thread From: Boris Ostrovsky @ 2016-02-05 15:50 UTC (permalink / raw) To: Juergen Gross, linux-kernel, xen-devel, konrad.wilk, david.vrabel; +Cc: stable On 02/05/2016 08:21 AM, Juergen Gross wrote: > When adding more than one LUN to a frontend a warning for a failed > assignment is issued in dom0 for each already existing LUN. Avoid this > warning. Aren't you just factoring out the check? The warning is still printed for each scsiback_add_translation_entry() invocation, no? > > Signed-off-by: Juergen Gross <jgross@suse.com> > Cc: stable@vger.kernel.org > --- > drivers/xen/xen-scsiback.c | 65 ++++++++++++++++++++++++++-------------------- > 1 file changed, 37 insertions(+), 28 deletions(-) > > diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c > index 51387d7..69de879 100644 > --- a/drivers/xen/xen-scsiback.c > +++ b/drivers/xen/xen-scsiback.c > @@ -849,15 +849,31 @@ static int scsiback_map(struct vscsibk_info *info) > } > > /* > + Check for a translation entry being present > +*/ > +static struct v2p_entry *scsiback_chk_translation_entry( > + struct vscsibk_info *info, struct ids_tuple *v) > +{ > + struct list_head *head = &(info->v2p_entry_lists); > + struct v2p_entry *entry; > + > + list_for_each_entry(entry, head, l) > + if ((entry->v.chn == v->chn) && > + (entry->v.tgt == v->tgt) && > + (entry->v.lun == v->lun)) > + return entry; > + > + return NULL; > +} > + > +/* > Add a new translation entry > */ > static int scsiback_add_translation_entry(struct vscsibk_info *info, > char *phy, struct ids_tuple *v) > { > int err = 0; > - struct v2p_entry *entry; > struct v2p_entry *new; > - struct list_head *head = &(info->v2p_entry_lists); > unsigned long flags; > char *lunp; > unsigned long long unpacked_lun; > @@ -917,15 +933,10 @@ static int scsiback_add_translation_entry(struct vscsibk_info *info, > spin_lock_irqsave(&info->v2p_lock, flags); > > /* Check double assignment to identical virtual ID */ > - list_for_each_entry(entry, head, l) { > - if ((entry->v.chn == v->chn) && > - (entry->v.tgt == v->tgt) && > - (entry->v.lun == v->lun)) { > - pr_warn("Virtual ID is already used. Assignment was not performed.\n"); > - err = -EEXIST; > - goto out; > - } > - > + if (scsiback_chk_translation_entry(info, v)) { > + pr_warn("Virtual ID is already used. Assignment was not performed.\n"); > + err = -EEXIST; > + goto out; > } > > /* Create a new translation entry and add to the list */ > @@ -933,7 +944,7 @@ static int scsiback_add_translation_entry(struct vscsibk_info *info, > new->v = *v; > new->tpg = tpg; > new->lun = unpacked_lun; > - list_add_tail(&new->l, head); > + list_add_tail(&new->l, &info->v2p_entry_lists); > > out: > spin_unlock_irqrestore(&info->v2p_lock, flags); > @@ -962,33 +973,31 @@ static int scsiback_del_translation_entry(struct vscsibk_info *info, > struct ids_tuple *v) > { > struct v2p_entry *entry; > - struct list_head *head = &(info->v2p_entry_lists); > unsigned long flags; > > spin_lock_irqsave(&info->v2p_lock, flags); > /* Find out the translation entry specified */ > - list_for_each_entry(entry, head, l) { > - if ((entry->v.chn == v->chn) && > - (entry->v.tgt == v->tgt) && > - (entry->v.lun == v->lun)) { > - goto found; > - } > - } > - > - spin_unlock_irqrestore(&info->v2p_lock, flags); > - return 1; > - > -found: > - /* Delete the translation entry specfied */ > - __scsiback_del_translation_entry(entry); > + entry = scsiback_chk_translation_entry(info, v); > + if (entry) > + __scsiback_del_translation_entry(entry); > > spin_unlock_irqrestore(&info->v2p_lock, flags); > - return 0; > + return entry == NULL; Might be better to return -ENOENT instead of 1 above and -EEXISTS if entry!=NULL, given that this returns an int. -boris > } > > static void scsiback_do_add_lun(struct vscsibk_info *info, const char *state, > char *phy, struct ids_tuple *vir, int try) > { > + struct v2p_entry *entry; > + unsigned long flags; > + > + if (try) { > + spin_lock_irqsave(&info->v2p_lock, flags); > + entry = scsiback_chk_translation_entry(info, vir); > + spin_unlock_irqrestore(&info->v2p_lock, flags); > + if (entry) > + return; > + } > if (!scsiback_add_translation_entry(info, phy, vir)) { > if (xenbus_printf(XBT_NIL, info->dev->nodename, state, > "%d", XenbusStateInitialised)) { ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] xen/scsiback: avoid warnings when adding multiple LUNs to a domain 2016-02-05 15:50 ` Boris Ostrovsky @ 2016-02-05 15:55 ` Boris Ostrovsky 2016-02-05 16:59 ` Juergen Gross 1 sibling, 0 replies; 10+ messages in thread From: Boris Ostrovsky @ 2016-02-05 15:55 UTC (permalink / raw) To: Juergen Gross, linux-kernel, xen-devel, konrad.wilk, david.vrabel; +Cc: stable On 02/05/2016 10:50 AM, Boris Ostrovsky wrote: > > >> @@ -962,33 +973,31 @@ static int >> scsiback_del_translation_entry(struct vscsibk_info *info, >> struct ids_tuple *v) >> { >> struct v2p_entry *entry; >> - struct list_head *head = &(info->v2p_entry_lists); >> unsigned long flags; >> spin_lock_irqsave(&info->v2p_lock, flags); >> /* Find out the translation entry specified */ >> - list_for_each_entry(entry, head, l) { >> - if ((entry->v.chn == v->chn) && >> - (entry->v.tgt == v->tgt) && >> - (entry->v.lun == v->lun)) { >> - goto found; >> - } >> - } >> - >> - spin_unlock_irqrestore(&info->v2p_lock, flags); >> - return 1; >> - >> -found: >> - /* Delete the translation entry specfied */ >> - __scsiback_del_translation_entry(entry); >> + entry = scsiback_chk_translation_entry(info, v); >> + if (entry) >> + __scsiback_del_translation_entry(entry); >> spin_unlock_irqrestore(&info->v2p_lock, flags); >> - return 0; >> + return entry == NULL; > > Might be better to return -ENOENT instead of 1 above and -EEXISTS if > entry!=NULL, given that this returns an int. Nevermind reference to 1, it's removed. But returning an error code instead of a bool is still better I think. -boris ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] xen/scsiback: avoid warnings when adding multiple LUNs to a domain 2016-02-05 15:50 ` Boris Ostrovsky 2016-02-05 15:55 ` Boris Ostrovsky @ 2016-02-05 16:59 ` Juergen Gross 2016-02-05 17:24 ` Boris Ostrovsky 1 sibling, 1 reply; 10+ messages in thread From: Juergen Gross @ 2016-02-05 16:59 UTC (permalink / raw) To: Boris Ostrovsky, linux-kernel, xen-devel, konrad.wilk, david.vrabel; +Cc: stable On 05/02/16 16:50, Boris Ostrovsky wrote: > > > On 02/05/2016 08:21 AM, Juergen Gross wrote: >> When adding more than one LUN to a frontend a warning for a failed >> assignment is issued in dom0 for each already existing LUN. Avoid this >> warning. > > Aren't you just factoring out the check? The warning is still printed > for each scsiback_add_translation_entry() invocation, no? I don't call scsiback_add_translation_entry() in the critical case. > > >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> Cc: stable@vger.kernel.org >> --- >> drivers/xen/xen-scsiback.c | 65 >> ++++++++++++++++++++++++++-------------------- >> 1 file changed, 37 insertions(+), 28 deletions(-) >> >> diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c >> index 51387d7..69de879 100644 >> --- a/drivers/xen/xen-scsiback.c >> +++ b/drivers/xen/xen-scsiback.c >> @@ -849,15 +849,31 @@ static int scsiback_map(struct vscsibk_info *info) >> } >> /* >> + Check for a translation entry being present >> +*/ >> +static struct v2p_entry *scsiback_chk_translation_entry( >> + struct vscsibk_info *info, struct ids_tuple *v) >> +{ >> + struct list_head *head = &(info->v2p_entry_lists); >> + struct v2p_entry *entry; >> + >> + list_for_each_entry(entry, head, l) >> + if ((entry->v.chn == v->chn) && >> + (entry->v.tgt == v->tgt) && >> + (entry->v.lun == v->lun)) >> + return entry; >> + >> + return NULL; >> +} >> + >> +/* >> Add a new translation entry >> */ >> static int scsiback_add_translation_entry(struct vscsibk_info *info, >> char *phy, struct ids_tuple *v) >> { >> int err = 0; >> - struct v2p_entry *entry; >> struct v2p_entry *new; >> - struct list_head *head = &(info->v2p_entry_lists); >> unsigned long flags; >> char *lunp; >> unsigned long long unpacked_lun; >> @@ -917,15 +933,10 @@ static int scsiback_add_translation_entry(struct >> vscsibk_info *info, >> spin_lock_irqsave(&info->v2p_lock, flags); >> /* Check double assignment to identical virtual ID */ >> - list_for_each_entry(entry, head, l) { >> - if ((entry->v.chn == v->chn) && >> - (entry->v.tgt == v->tgt) && >> - (entry->v.lun == v->lun)) { >> - pr_warn("Virtual ID is already used. Assignment was not >> performed.\n"); >> - err = -EEXIST; >> - goto out; >> - } >> - >> + if (scsiback_chk_translation_entry(info, v)) { >> + pr_warn("Virtual ID is already used. Assignment was not >> performed.\n"); >> + err = -EEXIST; >> + goto out; >> } >> /* Create a new translation entry and add to the list */ >> @@ -933,7 +944,7 @@ static int scsiback_add_translation_entry(struct >> vscsibk_info *info, >> new->v = *v; >> new->tpg = tpg; >> new->lun = unpacked_lun; >> - list_add_tail(&new->l, head); >> + list_add_tail(&new->l, &info->v2p_entry_lists); >> out: >> spin_unlock_irqrestore(&info->v2p_lock, flags); >> @@ -962,33 +973,31 @@ static int scsiback_del_translation_entry(struct >> vscsibk_info *info, >> struct ids_tuple *v) >> { >> struct v2p_entry *entry; >> - struct list_head *head = &(info->v2p_entry_lists); >> unsigned long flags; >> spin_lock_irqsave(&info->v2p_lock, flags); >> /* Find out the translation entry specified */ >> - list_for_each_entry(entry, head, l) { >> - if ((entry->v.chn == v->chn) && >> - (entry->v.tgt == v->tgt) && >> - (entry->v.lun == v->lun)) { >> - goto found; >> - } >> - } >> - >> - spin_unlock_irqrestore(&info->v2p_lock, flags); >> - return 1; >> - >> -found: >> - /* Delete the translation entry specfied */ >> - __scsiback_del_translation_entry(entry); >> + entry = scsiback_chk_translation_entry(info, v); >> + if (entry) >> + __scsiback_del_translation_entry(entry); >> spin_unlock_irqrestore(&info->v2p_lock, flags); >> - return 0; >> + return entry == NULL; > > Might be better to return -ENOENT instead of 1 above and -EEXISTS if > entry!=NULL, given that this returns an int. I just didn't want to change more than necessary. In case it is okay to do some cleanup as well I'd rather change the return type to "bool". Juergen > > -boris > >> } >> static void scsiback_do_add_lun(struct vscsibk_info *info, const >> char *state, >> char *phy, struct ids_tuple *vir, int try) >> { >> + struct v2p_entry *entry; >> + unsigned long flags; >> + >> + if (try) { >> + spin_lock_irqsave(&info->v2p_lock, flags); >> + entry = scsiback_chk_translation_entry(info, vir); >> + spin_unlock_irqrestore(&info->v2p_lock, flags); >> + if (entry) >> + return; >> + } >> if (!scsiback_add_translation_entry(info, phy, vir)) { >> if (xenbus_printf(XBT_NIL, info->dev->nodename, state, >> "%d", XenbusStateInitialised)) { > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] xen/scsiback: avoid warnings when adding multiple LUNs to a domain 2016-02-05 16:59 ` Juergen Gross @ 2016-02-05 17:24 ` Boris Ostrovsky 2016-02-08 12:28 ` Juergen Gross 0 siblings, 1 reply; 10+ messages in thread From: Boris Ostrovsky @ 2016-02-05 17:24 UTC (permalink / raw) To: Juergen Gross, linux-kernel, xen-devel, konrad.wilk, david.vrabel; +Cc: stable On 02/05/2016 11:59 AM, Juergen Gross wrote: > On 05/02/16 16:50, Boris Ostrovsky wrote: >> >> On 02/05/2016 08:21 AM, Juergen Gross wrote: >>> When adding more than one LUN to a frontend a warning for a failed >>> assignment is issued in dom0 for each already existing LUN. Avoid this >>> warning. >> Aren't you just factoring out the check? The warning is still printed >> for each scsiback_add_translation_entry() invocation, no? > I don't call scsiback_add_translation_entry() in the critical case. Which is scsiback_do_add_lun()? If yes then perhaps you could mention it in the commit message since there are few changes that this patch provides and it's not clear which is the one that prevents the warning. > > @@ -962,33 +973,31 @@ static int scsiback_del_translation_entry(struct > vscsibk_info *info, > struct ids_tuple *v) > { > struct v2p_entry *entry; > - struct list_head *head = &(info->v2p_entry_lists); > unsigned long flags; > spin_lock_irqsave(&info->v2p_lock, flags); > /* Find out the translation entry specified */ > - list_for_each_entry(entry, head, l) { > - if ((entry->v.chn == v->chn) && > - (entry->v.tgt == v->tgt) && > - (entry->v.lun == v->lun)) { > - goto found; > - } > - } > - > - spin_unlock_irqrestore(&info->v2p_lock, flags); > - return 1; > - > -found: > - /* Delete the translation entry specfied */ > - __scsiback_del_translation_entry(entry); > + entry = scsiback_chk_translation_entry(info, v); > + if (entry) > + __scsiback_del_translation_entry(entry); > spin_unlock_irqrestore(&info->v2p_lock, flags); > - return 0; > + return entry == NULL; >> Might be better to return -ENOENT instead of 1 above and -EEXISTS if >> entry!=NULL, given that this returns an int. > I just didn't want to change more than necessary. In case it is > okay to do some cleanup as well I'd rather change the return type > to "bool". I don't think using error code will require changing anything except the last line above (which is already a change anyway) But using a bool is OK too. -boris ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] xen/scsiback: avoid warnings when adding multiple LUNs to a domain 2016-02-05 17:24 ` Boris Ostrovsky @ 2016-02-08 12:28 ` Juergen Gross 0 siblings, 0 replies; 10+ messages in thread From: Juergen Gross @ 2016-02-08 12:28 UTC (permalink / raw) To: Boris Ostrovsky, linux-kernel, xen-devel, konrad.wilk, david.vrabel; +Cc: stable On 05/02/16 18:24, Boris Ostrovsky wrote: > > > On 02/05/2016 11:59 AM, Juergen Gross wrote: >> On 05/02/16 16:50, Boris Ostrovsky wrote: >>> >>> On 02/05/2016 08:21 AM, Juergen Gross wrote: >>>> When adding more than one LUN to a frontend a warning for a failed >>>> assignment is issued in dom0 for each already existing LUN. Avoid this >>>> warning. >>> Aren't you just factoring out the check? The warning is still printed >>> for each scsiback_add_translation_entry() invocation, no? >> I don't call scsiback_add_translation_entry() in the critical case. > > Which is scsiback_do_add_lun()? If yes then perhaps you could mention it > in the commit message since there are few changes that this patch > provides and it's not clear which is the one that prevents the warning. > >> >> @@ -962,33 +973,31 @@ static int scsiback_del_translation_entry(struct >> vscsibk_info *info, >> struct ids_tuple *v) >> { >> struct v2p_entry *entry; >> - struct list_head *head = &(info->v2p_entry_lists); >> unsigned long flags; >> spin_lock_irqsave(&info->v2p_lock, flags); >> /* Find out the translation entry specified */ >> - list_for_each_entry(entry, head, l) { >> - if ((entry->v.chn == v->chn) && >> - (entry->v.tgt == v->tgt) && >> - (entry->v.lun == v->lun)) { >> - goto found; >> - } >> - } >> - >> - spin_unlock_irqrestore(&info->v2p_lock, flags); >> - return 1; >> - >> -found: >> - /* Delete the translation entry specfied */ >> - __scsiback_del_translation_entry(entry); >> + entry = scsiback_chk_translation_entry(info, v); >> + if (entry) >> + __scsiback_del_translation_entry(entry); >> spin_unlock_irqrestore(&info->v2p_lock, flags); >> - return 0; >> + return entry == NULL; >>> Might be better to return -ENOENT instead of 1 above and -EEXISTS if >>> entry!=NULL, given that this returns an int. >> I just didn't want to change more than necessary. In case it is >> okay to do some cleanup as well I'd rather change the return type >> to "bool". > > I don't think using error code will require changing anything except the > last line above (which is already a change anyway) And returning -ENOENT or 0 will be even better, I guess. Juergen ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] xen/scsiback: correct frontend counting 2016-02-05 13:21 ` [PATCH 1/2] xen/scsiback: correct frontend counting Juergen Gross 2016-02-05 13:21 ` [PATCH 2/2] xen/scsiback: avoid warnings when adding multiple LUNs to a domain Juergen Gross @ 2016-02-05 15:42 ` Boris Ostrovsky 2016-02-05 15:44 ` Juergen Gross 1 sibling, 1 reply; 10+ messages in thread From: Boris Ostrovsky @ 2016-02-05 15:42 UTC (permalink / raw) To: Juergen Gross, linux-kernel, xen-devel, konrad.wilk, david.vrabel; +Cc: stable On 02/05/2016 08:21 AM, Juergen Gross wrote: > When adding a new frontend to xen-scsiback don't decrement the number > of active frontends in case of no error. Not doing so results in a I think you meant "Doing so". Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> > failure when trying to remove the xen-pvscsi nexus even if no domain > is using it. > > Signed-off-by: Juergen Gross <jgross@suse.com> > Cc: stable@vger.kernel.org > --- > drivers/xen/xen-scsiback.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c > index ad4eb10..51387d7 100644 > --- a/drivers/xen/xen-scsiback.c > +++ b/drivers/xen/xen-scsiback.c > @@ -939,12 +939,12 @@ out: > spin_unlock_irqrestore(&info->v2p_lock, flags); > > out_free: > - mutex_lock(&tpg->tv_tpg_mutex); > - tpg->tv_tpg_fe_count--; > - mutex_unlock(&tpg->tv_tpg_mutex); > - > - if (err) > + if (err) { > + mutex_lock(&tpg->tv_tpg_mutex); > + tpg->tv_tpg_fe_count--; > + mutex_unlock(&tpg->tv_tpg_mutex); > kfree(new); > + } > > return err; > } ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] xen/scsiback: correct frontend counting 2016-02-05 15:42 ` [PATCH 1/2] xen/scsiback: correct frontend counting Boris Ostrovsky @ 2016-02-05 15:44 ` Juergen Gross 0 siblings, 0 replies; 10+ messages in thread From: Juergen Gross @ 2016-02-05 15:44 UTC (permalink / raw) To: Boris Ostrovsky, linux-kernel, xen-devel, konrad.wilk, david.vrabel; +Cc: stable On 05/02/16 16:42, Boris Ostrovsky wrote: > > > On 02/05/2016 08:21 AM, Juergen Gross wrote: >> When adding a new frontend to xen-scsiback don't decrement the number >> of active frontends in case of no error. Not doing so results in a > > I think you meant "Doing so". I think so, too. > > Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> Thanks, Juergen > > >> failure when trying to remove the xen-pvscsi nexus even if no domain >> is using it. >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> Cc: stable@vger.kernel.org >> --- >> drivers/xen/xen-scsiback.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c >> index ad4eb10..51387d7 100644 >> --- a/drivers/xen/xen-scsiback.c >> +++ b/drivers/xen/xen-scsiback.c >> @@ -939,12 +939,12 @@ out: >> spin_unlock_irqrestore(&info->v2p_lock, flags); >> out_free: >> - mutex_lock(&tpg->tv_tpg_mutex); >> - tpg->tv_tpg_fe_count--; >> - mutex_unlock(&tpg->tv_tpg_mutex); >> - >> - if (err) >> + if (err) { >> + mutex_lock(&tpg->tv_tpg_mutex); >> + tpg->tv_tpg_fe_count--; >> + mutex_unlock(&tpg->tv_tpg_mutex); >> kfree(new); >> + } >> return err; >> } > > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-02-08 12:28 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-02-05 13:15 [PATCH 0/2] xen/scsiback: correct two issues Juergen Gross 2016-02-05 13:21 ` [PATCH 1/2] xen/scsiback: correct frontend counting Juergen Gross 2016-02-05 13:21 ` [PATCH 2/2] xen/scsiback: avoid warnings when adding multiple LUNs to a domain Juergen Gross 2016-02-05 15:50 ` Boris Ostrovsky 2016-02-05 15:55 ` Boris Ostrovsky 2016-02-05 16:59 ` Juergen Gross 2016-02-05 17:24 ` Boris Ostrovsky 2016-02-08 12:28 ` Juergen Gross 2016-02-05 15:42 ` [PATCH 1/2] xen/scsiback: correct frontend counting Boris Ostrovsky 2016-02-05 15:44 ` Juergen Gross
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).