* [PATCH Linux-2.6.18] scsifront: avoid aquiring same lock twice if ring is full
@ 2015-01-13 16:22 Juergen Gross
2015-01-13 18:53 ` Pasi Kärkkäinen
2015-01-14 9:40 ` Jan Beulich
0 siblings, 2 replies; 4+ messages in thread
From: Juergen Gross @ 2015-01-13 16:22 UTC (permalink / raw)
To: xen-devel, jbeulich; +Cc: 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 <jgross@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);
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH Linux-2.6.18] scsifront: avoid aquiring same lock twice if ring is full
2015-01-13 16:22 [PATCH Linux-2.6.18] scsifront: avoid aquiring same lock twice if ring is full Juergen Gross
@ 2015-01-13 18:53 ` Pasi Kärkkäinen
2015-01-14 4:27 ` Juergen Gross
2015-01-14 9:40 ` Jan Beulich
1 sibling, 1 reply; 4+ messages in thread
From: Pasi Kärkkäinen @ 2015-01-13 18:53 UTC (permalink / raw)
To: Juergen Gross; +Cc: jbeulich, xen-devel
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 <jgross@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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH Linux-2.6.18] scsifront: avoid aquiring same lock twice if ring is full
2015-01-13 18:53 ` Pasi Kärkkäinen
@ 2015-01-14 4:27 ` Juergen Gross
0 siblings, 0 replies; 4+ messages in thread
From: Juergen Gross @ 2015-01-14 4:27 UTC (permalink / raw)
To: Pasi Kärkkäinen; +Cc: jbeulich, xen-devel
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 <jgross@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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH Linux-2.6.18] scsifront: avoid aquiring same lock twice if ring is full
2015-01-13 16:22 [PATCH Linux-2.6.18] scsifront: avoid aquiring same lock twice if ring is full Juergen Gross
2015-01-13 18:53 ` Pasi Kärkkäinen
@ 2015-01-14 9:40 ` Jan Beulich
1 sibling, 0 replies; 4+ messages in thread
From: Jan Beulich @ 2015-01-14 9:40 UTC (permalink / raw)
To: Juergen Gross; +Cc: xen-devel
>>> On 13.01.15 at 17:22, <"jgross@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 <jgross@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);
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-01-14 9:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-13 16:22 [PATCH Linux-2.6.18] scsifront: avoid aquiring same lock twice if ring is full Juergen Gross
2015-01-13 18:53 ` Pasi Kärkkäinen
2015-01-14 4:27 ` Juergen Gross
2015-01-14 9:40 ` Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.