Re: [Xen-devel] [PATCH][linux 2.6.18] remove pointless error handling in scsiback

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 01.07.14 at 17:27, <"jgross@xxxxxxxx".non-mime.internet> wrote:
> From: Juergen Gross <jgross@xxxxxxxx>
> 
> It makes no sense to try repeating traversing the lun list in case of a
> possible mismatch between the original counting of luns and the real 
> processing.

Perhaps the retry logic isn't correct, but with the lock being dropped
between the two loops I'm not sure the retry makes no sense. Can
you explain?

Jan

> To make things worse, the retry itself was wrong as the number of detected
> luns was not resetted for the retry leading to the next failing retry at 
> once.
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> 
> 
> diff -r 0251893a7d5a drivers/xen/scsiback/emulate.c
> --- a/drivers/xen/scsiback/emulate.c  Mon Jun 16 16:11:33 2014 +0200
> +++ b/drivers/xen/scsiback/emulate.c  Tue Jul 01 17:19:27 2014 +0200
> @@ -228,7 +228,6 @@ static void __report_luns(pending_req_t 
>       unsigned int alloc_luns = 0;
>       unsigned int req_bufflen = 0;
>       unsigned int actual_len = 0;
> -     unsigned int retry_cnt = 0;
>       int select_report = (int)cmd[2];
>       int i, lun_cnt = 0, lun, upper, err = 0;
>       
> @@ -245,7 +244,7 @@ static void __report_luns(pending_req_t 
>       alloc_luns = __nr_luns_under_host(info);
>       alloc_len  = sizeof(struct scsi_lun) * alloc_luns
>                               + VSCSI_REPORT_LUNS_HEADER;
> -retry:
> +
>       if ((buff = kzalloc(alloc_len, GFP_KERNEL)) == NULL) {
>               printk(KERN_ERR "scsiback:%s kmalloc err\n", __FUNCTION__);
>               goto fail;
> @@ -261,14 +260,6 @@ retry:
>                       if (lun_cnt >= alloc_luns) {
>                               spin_unlock_irqrestore(&info->v2p_lock,
>                                                       flags);
> -
> -                             if (retry_cnt < VSCSI_REPORT_LUNS_RETRY) {
> -                                     retry_cnt++;
> -                                     if (buff)
> -                                             kfree(buff);
> -                                     goto retry;
> -                             }
> -
>                               goto fail;
>                       }
>  




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel