Behaviour change of /dev/fb0?

Behaviour change of /dev/fb0?

[Richard Purdie]

Ignoring whether this is a good idea or not, under 2.6.15 you could run

dd if=/dev/zero of=/dev/fb0

which would clear the framebuffer. It would end up saying "dd: /dev/fb0:
No space left on device".

Under 2.6.16 (and a recent git kernel), the same command clears the
screen but then hangs. Was the change in behaviour intentional? 

I've noticed this on a couple of ARM based Zaurus handhelds under both
w100fb and pxafb.

Richard

Re: [Linux-fbdev-devel] Behaviour change of /dev/fb0?

[Antonino A. Daplas]

Richard Purdie wrote:
> Ignoring whether this is a good idea or not, under 2.6.15 you could run
> 
> dd if=/dev/zero of=/dev/fb0
> 
> which would clear the framebuffer. It would end up saying "dd: /dev/fb0:
> No space left on device".
> 
> Under 2.6.16 (and a recent git kernel), the same command clears the
> screen but then hangs. Was the change in behaviour intentional? 
> 
> I've noticed this on a couple of ARM based Zaurus handhelds under both
> w100fb and pxafb.
> 

There is a change in behavior of fb_read and fb_write committed Jan 2006.
They return the number of bytes read or written if the requested size
is bigger than the remaining space.  Previously, they returned -ENOSPC.

But I haven't experienced hangs...

Tony

Re: [Linux-fbdev-devel] Behaviour change of /dev/fb0?

[Richard Purdie]

On Sat, 2006-04-15 at 07:29 +0800, Antonino A. Daplas wrote:
> Richard Purdie wrote:
> > Ignoring whether this is a good idea or not, under 2.6.15 you could run
> > 
> > dd if=/dev/zero of=/dev/fb0
> > 
> > which would clear the framebuffer. It would end up saying "dd: /dev/fb0:
> > No space left on device".
> > 
> > Under 2.6.16 (and a recent git kernel), the same command clears the
> > screen but then hangs. Was the change in behaviour intentional? 
> > 
> > I've noticed this on a couple of ARM based Zaurus handhelds under both
> > w100fb and pxafb.
> > 
> 
> There is a change in behavior of fb_read and fb_write committed Jan 2006.
> They return the number of bytes read or written if the requested size
> is bigger than the remaining space.  Previously, they returned -ENOSPC.
> 
> But I haven't experienced hangs...

The change in question is:

@@ -661,19 +664,19 @@ fb_write(struct file *file, const char _
 		return info->fbops->fb_write(file, buf, count, ppos);
 	
 	total_size = info->screen_size;
+
 	if (total_size == 0)
 		total_size = info->fix.smem_len;
 
 	if (p > total_size)
-	    return -ENOSPC;
+		return 0;
+
 	if (count >= total_size)
-	    count = total_size;
-	err = 0;
-	if (count + p > total_size) {
-	    count = total_size - p;
-	    err = -ENOSPC;
-	}
-	cnt = 0;
+		count = total_size;
+
+	if (count + p > total_size)
+		count = total_size - p;
+
 	buffer = kmalloc((count > PAGE_SIZE) ? PAGE_SIZE : count,
 			 GFP_KERNEL);
 	if (!buffer)

I agree with the latter part of this but not the change for the "if (p >
total_size)" case. If we return zero here, the writer will just keep
retrying to write to the device indefinitely and therefore hang. Would
it make sense to revert that part of the change?:



If we reach the end of the framebuffer, we should return an out of space
error, not zero. Returning zero implies we might be able to write
further bytes at some future time which isn't true.

Signed-off-by: Richard Purdie <rpurdie@rpsys.net>

Index: git/drivers/video/fbmem.c
===================================================================
--- git.orig/drivers/video/fbmem.c	2006-04-13 00:22:28.000000000 +0100
+++ git/drivers/video/fbmem.c	2006-04-15 01:04:37.000000000 +0100
@@ -674,7 +674,7 @@
 		total_size = info->fix.smem_len;
 
 	if (p > total_size)
-		return 0;
+		return -ENOSPC;
 
 	if (count >= total_size)
 		count = total_size;

Re: [Linux-fbdev-devel] Behaviour change of /dev/fb0?

[Antonino A. Daplas]

Richard Purdie wrote:
> Ignoring whether this is a good idea or not, under 2.6.15 you could run
> 
> dd if=/dev/zero of=/dev/fb0
> 
> which would clear the framebuffer. It would end up saying "dd: /dev/fb0:
> No space left on device".
> 
> Under 2.6.16 (and a recent git kernel), the same command clears the
> screen but then hangs. Was the change in behaviour intentional? 
> 
> I've noticed this on a couple of ARM based Zaurus handhelds under both
> w100fb and pxafb.
> 

After reading 'man 2 read' more thoroughly, I've adjusted fb_write()'s
return codes  appropriately.  Can you try this patch and let me know if it
fixes your problem.

Tony

fbdev: Fix return error of fb_write()

- return -EFBIG if file offset is past the maximum allowable offset
- return -EFBIG and write to end of framebuffer if size is bigger than the
  framebuffer length
- return -ENOSPC and write to end of framebuffer if size is bigger than the
  framebuffer length - file offset

Signed-off-by: Antonino Daplas <adaplas@pol.net>
---

 drivers/video/fbmem.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 944855b..8a643bf 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -669,13 +669,19 @@ fb_write(struct file *file, const char _
 		total_size = info->fix.smem_len;
 
 	if (p > total_size)
-		return 0;
+		return -EFBIG;
 
-	if (count >= total_size)
+	if (count >= total_size) {
+		err = -EFBIG;
 		count = total_size;
+	}
+
+	if (count + p > total_size) {
+		if (!err)
+			err = -ENOSPC;
 
-	if (count + p > total_size)
 		count = total_size - p;
+	}
 
 	buffer = kmalloc((count > PAGE_SIZE) ? PAGE_SIZE : count,
 			 GFP_KERNEL);

Re: [Linux-fbdev-devel] Behaviour change of /dev/fb0?

[Andrew Morton]

"Antonino A. Daplas" <adaplas@gmail.com> wrote:
>
> Richard Purdie wrote:
> > Ignoring whether this is a good idea or not, under 2.6.15 you could run
> > 
> > dd if=/dev/zero of=/dev/fb0
> > 
> > which would clear the framebuffer. It would end up saying "dd: /dev/fb0:
> > No space left on device".
> > 
> > Under 2.6.16 (and a recent git kernel), the same command clears the
> > screen but then hangs. Was the change in behaviour intentional? 
> > 
> > I've noticed this on a couple of ARM based Zaurus handhelds under both
> > w100fb and pxafb.
> > 
> 
> After reading 'man 2 read' more thoroughly, I've adjusted fb_write()'s
> return codes  appropriately.  Can you try this patch and let me know if it
> fixes your problem.
> 
> Tony
> 
> fbdev: Fix return error of fb_write()
> 
> - return -EFBIG if file offset is past the maximum allowable offset

OK.

> - return -EFBIG and write to end of framebuffer if size is bigger than the
>   framebuffer length

We should return the number of bytes written in this case.

> - return -ENOSPC and write to end of framebuffer if size is bigger than the
>   framebuffer length - file offset

Also here.


If we can transfer _any_ bytes, we should do so, then return the number of
bytes transferred.  If no bytes were transferrable then we should return
-Ewhatever.

[PATCH] fbdev: Fix return error of fb_write

[Antonino A. Daplas]

Fix return code of fb_write():

If at least 1 byte was transferred to the device, return number of bytes,
otherwise:

    - return -EFBIG - if file offset is past the maximum allowable offset or
      size is greater than framebuffer length
    - return -ENOSPC - if size is greater than framebuffer length - offset

Signed-off-by: Antonino Daplas <adaplas@pol.net>
---

Andrew Morton wrote:
> "Antonino A. Daplas" <adaplas@gmail.com> wrote:
>> Richard Purdie wrote:
>>
>> - return -EFBIG if file offset is past the maximum allowable offset
> 
> OK.
> 
>> - return -EFBIG and write to end of framebuffer if size is bigger than the
>>   framebuffer length
> 
> We should return the number of bytes written in this case.
> 
>> - return -ENOSPC and write to end of framebuffer if size is bigger than the
>>   framebuffer length - file offset
> 
> Also here.
> 
> 
> If we can transfer _any_ bytes, we should do so, then return the number of
> bytes transferred.  If no bytes were transferrable then we should return
> -Ewhatever.
> 
> 

Okay, here's try #2:

 drivers/video/fbmem.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 944855b..a4b6776 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -669,13 +669,19 @@ fb_write(struct file *file, const char _
 		total_size = info->fix.smem_len;
 
 	if (p > total_size)
-		return 0;
+		return -EFBIG;
 
-	if (count >= total_size)
+	if (count > total_size) {
+		err = -EFBIG;
 		count = total_size;
+	}
+
+	if (count + p > total_size) {
+		if (!err)
+			err = -ENOSPC;
 
-	if (count + p > total_size)
 		count = total_size - p;
+	}
 
 	buffer = kmalloc((count > PAGE_SIZE) ? PAGE_SIZE : count,
 			 GFP_KERNEL);
@@ -717,7 +723,7 @@ fb_write(struct file *file, const char _
 
 	kfree(buffer);
 
-	return (err) ? err : cnt;
+	return (cnt) ? cnt : err;
 }
 
 #ifdef CONFIG_KMOD

Re: [PATCH] fbdev: Fix return error of fb_write

[Richard Purdie]

On Sat, 2006-04-15 at 13:38 +0800, Antonino A. Daplas wrote:
> Fix return code of fb_write():
> 
> If at least 1 byte was transferred to the device, return number of bytes,
> otherwise:
> 
>     - return -EFBIG - if file offset is past the maximum allowable offset or
>       size is greater than framebuffer length
>     - return -ENOSPC - if size is greater than framebuffer length - offset
> 
> Signed-off-by: Antonino Daplas <adaplas@pol.net>

I can confirm this fixes the problem I reported, thanks!

Richard