* [PATCH] xenbus: avoid zero returns from read()
@ 2010-09-08 22:10 Daniel De Graaf
2010-09-09 9:27 ` Jun Zhu (Intern)
0 siblings, 1 reply; 5+ messages in thread
From: Daniel De Graaf @ 2010-09-08 22:10 UTC (permalink / raw)
To: Jeremy Fitzhardinge; +Cc: Jun Zhu (Intern), xen-devel
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);
^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH] xenbus: avoid zero returns from read()
2010-09-08 22:10 [PATCH] xenbus: avoid zero returns from read() Daniel De Graaf
@ 2010-09-09 9:27 ` Jun Zhu (Intern)
2010-09-09 14:40 ` Daniel De Graaf
0 siblings, 1 reply; 5+ messages in thread
From: Jun Zhu (Intern) @ 2010-09-09 9:27 UTC (permalink / raw)
To: Daniel De Graaf, Jeremy Fitzhardinge; +Cc: xen-devel
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);
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] xenbus: avoid zero returns from read()
2010-09-09 9:27 ` Jun Zhu (Intern)
@ 2010-09-09 14:40 ` Daniel De Graaf
2010-09-09 15:25 ` Jeremy Fitzhardinge
0 siblings, 1 reply; 5+ messages in thread
From: Daniel De Graaf @ 2010-09-09 14:40 UTC (permalink / raw)
To: Jeremy Fitzhardinge; +Cc: Jun Zhu (Intern), xen-devel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Re: [PATCH] xenbus: avoid zero returns from read()
2010-09-09 14:40 ` Daniel De Graaf
@ 2010-09-09 15:25 ` Jeremy Fitzhardinge
2010-12-30 18:39 ` Daniel De Graaf
0 siblings, 1 reply; 5+ messages in thread
From: Jeremy Fitzhardinge @ 2010-09-09 15:25 UTC (permalink / raw)
To: Daniel De Graaf; +Cc: Jun Zhu (Intern), xen-devel
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);
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xenbus: avoid zero returns from read()
2010-09-09 15:25 ` Jeremy Fitzhardinge
@ 2010-12-30 18:39 ` Daniel De Graaf
0 siblings, 0 replies; 5+ messages in thread
From: Daniel De Graaf @ 2010-12-30 18:39 UTC (permalink / raw)
To: Jeremy Fitzhardinge; +Cc: Jun Zhu, xen-devel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-12-30 18:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-08 22:10 [PATCH] xenbus: avoid zero returns from read() Daniel De Graaf
2010-09-09 9:27 ` Jun Zhu (Intern)
2010-09-09 14:40 ` Daniel De Graaf
2010-09-09 15:25 ` Jeremy Fitzhardinge
2010-12-30 18:39 ` Daniel De Graaf
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.