thr3ads.net - Xen devel - [Xen-devel] [PATCH] xenbus: avoid zero returns from read() [Sep 2010]

If this information is useful, please help other people find it:
Share via:

Daniel De Graaf

2010-Sep-08 22:10 UTC

[Xen-devel] [PATCH] xenbus: avoid zero returns from read()

It is possible to get a zero return from read() in instances where the
queue is not empty but has no elements with data to deliver to the user.
Since a zero return from read is an error indicator, resume waiting or
return -EAGAIN (for a nonblocking fd) in this case.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

---
diff --git a/drivers/xen/xenfs/xenbus.c b/drivers/xen/xenfs/xenbus.c
index 88c87c9..0ddef43 100644
--- a/drivers/xen/xenfs/xenbus.c
+++ b/drivers/xen/xenfs/xenbus.c
@@ -121,6 +121,7 @@ static ssize_t xenbus_file_read(struct file *filp,
 	int ret;
 
 	mutex_lock(&u->reply_mutex);
+again:
 	while (list_empty(&u->read_buffers)) {
 		mutex_unlock(&u->reply_mutex);
 		if (filp->f_flags & O_NONBLOCK)
@@ -159,6 +160,8 @@ static ssize_t xenbus_file_read(struct file *filp,
 					struct read_buffer, list);
 		}
 	}
+	if (i == 0)
+		goto again;
 
 out:
 	mutex_unlock(&u->reply_mutex);

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jun Zhu (Intern)

2010-Sep-09 09:27 UTC

head link

[Xen-devel] RE: [PATCH] xenbus: avoid zero returns from read()

Is it related to the following patch? The following patch fixes the problem of
queue deletion.

diff --git a/drivers/xen/xenfs/xenbus.c b/drivers/xen/xenfs/xenbus.c
index 64b3be4..763e90d 100644
--- a/drivers/xen/xenfs/xenbus.c
+++ b/drivers/xen/xenfs/xenbus.c
@@ -143,7 +143,7 @@ static ssize_t xenbus_file_read(struct file *filp,
                i += sz - ret;
                rb->cons += sz - ret;
 
-               if (ret != sz) {
+               if (ret != 0) {
                        if (i == 0)
                                i = -EFAULT;
                        goto out;

Jun Zhu
Citrix Systems UK
________________________________________
From: Daniel De Graaf [dgdegra@tycho.nsa.gov]
Sent: 08 September 2010 18:10
To: Jeremy Fitzhardinge
Cc: xen-devel; Jun Zhu (Intern)
Subject: [PATCH] xenbus: avoid zero returns from read()

It is possible to get a zero return from read() in instances where the
queue is not empty but has no elements with data to deliver to the user.
Since a zero return from read is an error indicator, resume waiting or
return -EAGAIN (for a nonblocking fd) in this case.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

---
diff --git a/drivers/xen/xenfs/xenbus.c b/drivers/xen/xenfs/xenbus.c
index 88c87c9..0ddef43 100644
--- a/drivers/xen/xenfs/xenbus.c
+++ b/drivers/xen/xenfs/xenbus.c
@@ -121,6 +121,7 @@ static ssize_t xenbus_file_read(struct file *filp,
        int ret;

        mutex_lock(&u->reply_mutex);
+again:
        while (list_empty(&u->read_buffers)) {
                mutex_unlock(&u->reply_mutex);
                if (filp->f_flags & O_NONBLOCK)
@@ -159,6 +160,8 @@ static ssize_t xenbus_file_read(struct file *filp,
                                        struct read_buffer, list);
                }
        }
+       if (i == 0)
+               goto again;

 out:
        mutex_unlock(&u->reply_mutex);

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Daniel De Graaf

2010-Sep-09 14:40 UTC

head link

[Xen-devel] Re: [PATCH] xenbus: avoid zero returns from read()

On 09/09/2010 05:27 AM, Jun Zhu (Intern) wrote:> Is it related to the following patch? The following patch fixes the problem
of queue deletion.
Yes, this would have caused a zero return due to an empty item being left
in the queue. It''s likely my patch is not needed with this one applied;
we
are already careful to avoid adding zero-length elements to the queue,
which was my original idea for the cause.
> diff --git a/drivers/xen/xenfs/xenbus.c b/drivers/xen/xenfs/xenbus.c
> index 64b3be4..763e90d 100644
> --- a/drivers/xen/xenfs/xenbus.c
> +++ b/drivers/xen/xenfs/xenbus.c
> @@ -143,7 +143,7 @@ static ssize_t xenbus_file_read(struct file *filp,
>                 i += sz - ret;
>                 rb->cons += sz - ret;
>  
> -               if (ret != sz) {
> +               if (ret != 0) {
>                         if (i == 0)
>                                 i = -EFAULT;
>                         goto out;
> 
> Jun Zhu
> Citrix Systems UK
> ________________________________________
> From: Daniel De Graaf [dgdegra@tycho.nsa.gov]
> Sent: 08 September 2010 18:10
> To: Jeremy Fitzhardinge
> Cc: xen-devel; Jun Zhu (Intern)
> Subject: [PATCH] xenbus: avoid zero returns from read()
> 
> It is possible to get a zero return from read() in instances where the
> queue is not empty but has no elements with data to deliver to the user.
> Since a zero return from read is an error indicator, resume waiting or
> return -EAGAIN (for a nonblocking fd) in this case.
> 
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> 
> ---
> diff --git a/drivers/xen/xenfs/xenbus.c b/drivers/xen/xenfs/xenbus.c
> index 88c87c9..0ddef43 100644
> --- a/drivers/xen/xenfs/xenbus.c
> +++ b/drivers/xen/xenfs/xenbus.c
> @@ -121,6 +121,7 @@ static ssize_t xenbus_file_read(struct file *filp,
>         int ret;
> 
>         mutex_lock(&u->reply_mutex);
> +again:
>         while (list_empty(&u->read_buffers)) {
>                 mutex_unlock(&u->reply_mutex);
>                 if (filp->f_flags & O_NONBLOCK)
> @@ -159,6 +160,8 @@ static ssize_t xenbus_file_read(struct file *filp,
>                                         struct read_buffer, list);
>                 }
>         }
> +       if (i == 0)
> +               goto again;
> 
>  out:
>         mutex_unlock(&u->reply_mutex);

-- 
Daniel De Graaf
National Security Agency

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jeremy Fitzhardinge

2010-Sep-09 15:25 UTC

head link

Re: [Xen-devel] Re: [PATCH] xenbus: avoid zero returns from read()

On 09/10/2010 12:40 AM, Daniel De Graaf wrote:> On 09/09/2010 05:27 AM, Jun Zhu (Intern) wrote:
>> Is it related to the following patch? The following patch fixes the
problem of queue deletion.
> Yes, this would have caused a zero return due to an empty item being left
> in the queue. It''s likely my patch is not needed with this one
applied; we
> are already careful to avoid adding zero-length elements to the queue,
> which was my original idea for the cause.
No, that''s a separate issue.  This patch fixes the case where the
usermode buffer is near a page boundary so the copy to usermode is
truncated.

Anyway, I''ve applied both.

    J
>> diff --git a/drivers/xen/xenfs/xenbus.c b/drivers/xen/xenfs/xenbus.c
>> index 64b3be4..763e90d 100644
>> --- a/drivers/xen/xenfs/xenbus.c
>> +++ b/drivers/xen/xenfs/xenbus.c
>> @@ -143,7 +143,7 @@ static ssize_t xenbus_file_read(struct file *filp,
>>                 i += sz - ret;
>>                 rb->cons += sz - ret;
>>  
>> -               if (ret != sz) {
>> +               if (ret != 0) {
>>                         if (i == 0)
>>                                 i = -EFAULT;
>>                         goto out;
>>
>> Jun Zhu
>> Citrix Systems UK
>> ________________________________________
>> From: Daniel De Graaf [dgdegra@tycho.nsa.gov]
>> Sent: 08 September 2010 18:10
>> To: Jeremy Fitzhardinge
>> Cc: xen-devel; Jun Zhu (Intern)
>> Subject: [PATCH] xenbus: avoid zero returns from read()
>>
>> It is possible to get a zero return from read() in instances where the
>> queue is not empty but has no elements with data to deliver to the
user.
>> Since a zero return from read is an error indicator, resume waiting or
>> return -EAGAIN (for a nonblocking fd) in this case.
>>
>> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>>
>> ---
>> diff --git a/drivers/xen/xenfs/xenbus.c b/drivers/xen/xenfs/xenbus.c
>> index 88c87c9..0ddef43 100644
>> --- a/drivers/xen/xenfs/xenbus.c
>> +++ b/drivers/xen/xenfs/xenbus.c
>> @@ -121,6 +121,7 @@ static ssize_t xenbus_file_read(struct file *filp,
>>         int ret;
>>
>>         mutex_lock(&u->reply_mutex);
>> +again:
>>         while (list_empty(&u->read_buffers)) {
>>                 mutex_unlock(&u->reply_mutex);
>>                 if (filp->f_flags & O_NONBLOCK)
>> @@ -159,6 +160,8 @@ static ssize_t xenbus_file_read(struct file *filp,
>>                                         struct read_buffer, list);
>>                 }
>>         }
>> +       if (i == 0)
>> +               goto again;
>>
>>  out:
>>         mutex_unlock(&u->reply_mutex);
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Daniel De Graaf

2010-Dec-30 18:39 UTC

head link

[Xen-devel] Re: [PATCH] xenbus: avoid zero returns from read()

On 09/09/2010 11:25 AM, Jeremy Fitzhardinge wrote:>  On 09/10/2010 12:40 AM, Daniel De Graaf wrote:
>> On 09/09/2010 05:27 AM, Jun Zhu (Intern) wrote:
>>> Is it related to the following patch? The following patch fixes the
problem of queue deletion.
>> Yes, this would have caused a zero return due to an empty item being
left
>> in the queue. It''s likely my patch is not needed with this one
applied; we
>> are already careful to avoid adding zero-length elements to the queue,
>> which was my original idea for the cause.
> 
> No, that''s a separate issue.  This patch fixes the case where the
> usermode buffer is near a page boundary so the copy to usermode is
> truncated.
> 
> Anyway, I''ve applied both.
> 
>     J
One or both of these patches are needed in the 2.6.37 kernel; I have
observed a zero return from a read() on xenbus on 2.6.37-rc8 that is
fixed by this patch.
>>> diff --git a/drivers/xen/xenfs/xenbus.c
b/drivers/xen/xenfs/xenbus.c
>>> index 64b3be4..763e90d 100644
>>> --- a/drivers/xen/xenfs/xenbus.c
>>> +++ b/drivers/xen/xenfs/xenbus.c
>>> @@ -143,7 +143,7 @@ static ssize_t xenbus_file_read(struct file
*filp,
>>>                 i += sz - ret;
>>>                 rb->cons += sz - ret;
>>>  
>>> -               if (ret != sz) {
>>> +               if (ret != 0) {
>>>                         if (i == 0)
>>>                                 i = -EFAULT;
>>>                         goto out;
>>>
>>> Jun Zhu
>>> Citrix Systems UK
>>> ________________________________________
>>> From: Daniel De Graaf [dgdegra@tycho.nsa.gov]
>>> Sent: 08 September 2010 18:10
>>> To: Jeremy Fitzhardinge
>>> Cc: xen-devel; Jun Zhu (Intern)
>>> Subject: [PATCH] xenbus: avoid zero returns from read()
>>>
>>> It is possible to get a zero return from read() in instances where
the
>>> queue is not empty but has no elements with data to deliver to the
user.
>>> Since a zero return from read is an error indicator, resume waiting
or
>>> return -EAGAIN (for a nonblocking fd) in this case.
>>>
>>> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>>>
>>> ---
>>> diff --git a/drivers/xen/xenfs/xenbus.c
b/drivers/xen/xenfs/xenbus.c
>>> index 88c87c9..0ddef43 100644
>>> --- a/drivers/xen/xenfs/xenbus.c
>>> +++ b/drivers/xen/xenfs/xenbus.c
>>> @@ -121,6 +121,7 @@ static ssize_t xenbus_file_read(struct file
*filp,
>>>         int ret;
>>>
>>>         mutex_lock(&u->reply_mutex);
>>> +again:
>>>         while (list_empty(&u->read_buffers)) {
>>>                 mutex_unlock(&u->reply_mutex);
>>>                 if (filp->f_flags & O_NONBLOCK)
>>> @@ -159,6 +160,8 @@ static ssize_t xenbus_file_read(struct file
*filp,
>>>                                         struct read_buffer, list);
>>>                 }
>>>         }
>>> +       if (i == 0)
>>> +               goto again;
>>>
>>>  out:
>>>         mutex_unlock(&u->reply_mutex);
>>
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

-- 
Daniel De Graaf
National Security Agency

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Xen devel - Sep 2010 - [PATCH] xenbus: avoid zero returns from read()

[Xen-devel] [PATCH] xenbus: avoid zero returns from read()

[Xen-devel] RE: [PATCH] xenbus: avoid zero returns from read()

[Xen-devel] Re: [PATCH] xenbus: avoid zero returns from read()

Re: [Xen-devel] Re: [PATCH] xenbus: avoid zero returns from read()

[Xen-devel] Re: [PATCH] xenbus: avoid zero returns from read()