Re: [Xen-devel] [PATCH Linux-2.6.18] scsifront: avoid aquiring same lock twice if ring is full

2015-01-14 Thread Jan Beulich 
 On 13.01.15 at 17:22, jgr...@suse.com.non-mime.internet wrote:
 The locking in scsifront_dev_reset_handler() is obviously wrong. In
 case of a full ring the host lock is aquired twice.
 
 Fixing this issue enables to get rid of the endless fo loop with an
 explicit break statement.

Perhaps worth noting that it was my c/s 1209:4d1471ca031e that
screwed this up.

Jan

 Signed-off-by: Juergen Gross jgr...@suse.com
 ---
 
 diff -r 078f1bb69ea5 drivers/xen/scsifront/scsifront.c
 --- a/drivers/xen/scsifront/scsifront.c   Wed Dec 10 10:22:39 2014 +0100
 +++ b/drivers/xen/scsifront/scsifront.c   Tue Jan 13 14:32:33 2015 +0100
 @@ -447,12 +447,10 @@ static int scsifront_dev_reset_handler(s
   uint16_t rqid;
   int err = 0;
  
 - for (;;) {
  #if LINUX_VERSION_CODE = KERNEL_VERSION(2,6,12)
 - spin_lock_irq(host-host_lock);
 + spin_lock_irq(host-host_lock);
  #endif
 - if (!RING_FULL(info-ring))
 - break;
 + while (RING_FULL(info-ring)) {
   if (err) {
  #if LINUX_VERSION_CODE = KERNEL_VERSION(2,6,12)
   spin_unlock_irq(host-host_lock);




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

Re: [Xen-devel] [PATCH Linux-2.6.18] scsifront: avoid aquiring same lock twice if ring is full

2015-01-13 Thread Juergen Gross 

On 01/13/2015 07:53 PM, Pasi Kärkkäinen wrote:

Hi,

On Tue, Jan 13, 2015 at 05:22:58PM +0100, Juergen Gross wrote:

The locking in scsifront_dev_reset_handler() is obviously wrong. In
case of a full ring the host lock is aquired twice.

Fixing this issue enables to get rid of the endless fo loop with an
explicit break statement.



Is this patch needed in upstream Linux kernel aswell, now that Xen PVSCSI 
drivers are in upstream Linux ?


No, especially this part of the code was reorganized and doesn't
have that issue.

Juergen




Thanks,

-- Pasi



Signed-off-by: Juergen Gross jgr...@suse.com
---

diff -r 078f1bb69ea5 drivers/xen/scsifront/scsifront.c
--- a/drivers/xen/scsifront/scsifront.c Wed Dec 10 10:22:39 2014 +0100
+++ b/drivers/xen/scsifront/scsifront.c Tue Jan 13 14:32:33 2015 +0100
@@ -447,12 +447,10 @@ static int scsifront_dev_reset_handler(s
uint16_t rqid;
int err = 0;

-   for (;;) {
  #if LINUX_VERSION_CODE = KERNEL_VERSION(2,6,12)
-   spin_lock_irq(host-host_lock);
+   spin_lock_irq(host-host_lock);
  #endif
-   if (!RING_FULL(info-ring))
-   break;
+   while (RING_FULL(info-ring)) {
if (err) {
  #if LINUX_VERSION_CODE = KERNEL_VERSION(2,6,12)
spin_unlock_irq(host-host_lock);

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





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

Re: [Xen-devel] [PATCH Linux-2.6.18] scsifront: avoid aquiring same lock twice if ring is full

2015-01-13 Thread Pasi Kärkkäinen 
Hi,

On Tue, Jan 13, 2015 at 05:22:58PM +0100, Juergen Gross wrote:
 The locking in scsifront_dev_reset_handler() is obviously wrong. In
 case of a full ring the host lock is aquired twice.
 
 Fixing this issue enables to get rid of the endless fo loop with an
 explicit break statement.
 

Is this patch needed in upstream Linux kernel aswell, now that Xen PVSCSI 
drivers are in upstream Linux ?


Thanks,

-- Pasi


 Signed-off-by: Juergen Gross jgr...@suse.com
 ---
 
 diff -r 078f1bb69ea5 drivers/xen/scsifront/scsifront.c
 --- a/drivers/xen/scsifront/scsifront.c   Wed Dec 10 10:22:39 2014 +0100
 +++ b/drivers/xen/scsifront/scsifront.c   Tue Jan 13 14:32:33 2015 +0100
 @@ -447,12 +447,10 @@ static int scsifront_dev_reset_handler(s
   uint16_t rqid;
   int err = 0;
  
 - for (;;) {
  #if LINUX_VERSION_CODE = KERNEL_VERSION(2,6,12)
 - spin_lock_irq(host-host_lock);
 + spin_lock_irq(host-host_lock);
  #endif
 - if (!RING_FULL(info-ring))
 - break;
 + while (RING_FULL(info-ring)) {
   if (err) {
  #if LINUX_VERSION_CODE = KERNEL_VERSION(2,6,12)
   spin_unlock_irq(host-host_lock);
 
 ___
 Xen-devel mailing list
 Xen-devel@lists.xen.org
 http://lists.xen.org/xen-devel

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

[Xen-devel] [PATCH Linux-2.6.18] scsifront: avoid aquiring same lock twice if ring is full

2015-01-13 Thread Juergen Gross 
The locking in scsifront_dev_reset_handler() is obviously wrong. In
case of a full ring the host lock is aquired twice.

Fixing this issue enables to get rid of the endless fo loop with an
explicit break statement.

Signed-off-by: Juergen Gross jgr...@suse.com
---

diff -r 078f1bb69ea5 drivers/xen/scsifront/scsifront.c
--- a/drivers/xen/scsifront/scsifront.c Wed Dec 10 10:22:39 2014 +0100
+++ b/drivers/xen/scsifront/scsifront.c Tue Jan 13 14:32:33 2015 +0100
@@ -447,12 +447,10 @@ static int scsifront_dev_reset_handler(s
uint16_t rqid;
int err = 0;
 
-   for (;;) {
 #if LINUX_VERSION_CODE = KERNEL_VERSION(2,6,12)
-   spin_lock_irq(host-host_lock);
+   spin_lock_irq(host-host_lock);
 #endif
-   if (!RING_FULL(info-ring))
-   break;
+   while (RING_FULL(info-ring)) {
if (err) {
 #if LINUX_VERSION_CODE = KERNEL_VERSION(2,6,12)
spin_unlock_irq(host-host_lock);

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