sysfs: write returns ENOMEM?

sysfs: write returns ENOMEM?

[Dmitry Torokhov]

[Apologies if you see this message twice - I accidentially sent it in HTML
 format first time around and I am pretty sure LKML will eat it]

Hi,

According to the SuS write() can not return ENOMEM, only ENOBUFS is allowed
(surprisingly read() is allowed to use both ENOMEM and ENOBUFS):

http://www.opengroup.org/onlinepubs/000095399/functions/write.html

Should we adjust sysfs write to follow the standard?

-- 
Dmitry

===================================================================
sysfs: write should return ENOBUFS

According to SuS ENOMEM is not a valid return code for write(),
ENOBUFS should be returned.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 fs/sysfs/file.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

Index: work/fs/sysfs/file.c
===================================================================
--- work.orig/fs/sysfs/file.c
+++ work/fs/sysfs/file.c
@@ -180,7 +180,7 @@ fill_write_buffer(struct sysfs_buffer * 
 	if (!buffer->page)
 		buffer->page = (char *)get_zeroed_page(GFP_KERNEL);
 	if (!buffer->page)
-		return -ENOMEM;
+		return -ENOBUFS;
 
 	if (count >= PAGE_SIZE)
 		count = PAGE_SIZE;

Re: sysfs: write returns ENOMEM?

[Greg KH]

On Fri, Aug 19, 2005 at 12:55:25AM -0500, Dmitry Torokhov wrote:
> [Apologies if you see this message twice - I accidentially sent it in HTML
>  format first time around and I am pretty sure LKML will eat it]
> 
> Hi,
> 
> According to the SuS write() can not return ENOMEM, only ENOBUFS is allowed
> (surprisingly read() is allowed to use both ENOMEM and ENOBUFS):
> 
> http://www.opengroup.org/onlinepubs/000095399/functions/write.html
> 
> Should we adjust sysfs write to follow the standard?

Sure, I don't have a problem with this.  Anyone else object?

thanks,

greg k-h

Re: sysfs: write returns ENOMEM?

[Pekka Enberg]

Hi Dmitry,

On 8/19/05, Dmitry Torokhov <dtor_core@ameritech.net> wrote:
> According to the SuS write() can not return ENOMEM, only ENOBUFS is allowed
> (surprisingly read() is allowed to use both ENOMEM and ENOBUFS):
> 
> http://www.opengroup.org/onlinepubs/000095399/functions/write.html
> 
> Should we adjust sysfs write to follow the standard?

Please note that sysfs is not the only one to do this. A quick peek
reveals XFS and CIFS returing ENOMEM for write() and there are
probably others as well. Perhaps we should replace ENOMEM with ENOBUFS
in vfs_write() instead?

linvfs_write
	xfs_write
		xfs_zero_eof
			xfs_zero_last_block
				xfs_iozero
					-> Returns -ENOMEM

cifs_user_write
	cifs_reopen_file
		-> Returns -ENOMEM

                            Pekka

Re: sysfs: write returns ENOMEM?

[Nathan Scott]

> On 8/19/05, Dmitry Torokhov <dtor_core@ameritech.net> wrote:
> > According to the SuS write() can not return ENOMEM, only ENOBUFS is allowed
> > (surprisingly read() is allowed to use both ENOMEM and ENOBUFS):
> > 
> > http://www.opengroup.org/onlinepubs/000095399/functions/write.html
> > 
> > Should we adjust sysfs write to follow the standard?
> 
> Please note that sysfs is not the only one to do this. A quick peek
> reveals XFS and CIFS returing ENOMEM for write() and there are
> probably others as well. Perhaps we should replace ENOMEM with ENOBUFS

FWIW, all filesystems using the generic page cache routines are able
to return this - see mm/filemap.c -> generic_file_buffered_write...

	page = __grab_cache_page(mapping,index,&cached_page,&lru_pvec);
	if (!page) {
		status = -ENOMEM;
		break;
	}

which is a similar condition to the one under which the XFS code is
returning this error.  Let me know what the verdict is and I'll get
the XFS side of this merged if its really necessary.

cheers.

-- 
Nathan

Re: sysfs: write returns ENOMEM?

[Pekka Enberg]

[-- Attachment #1: Type: text/plain, Size: 1373 bytes --]

On 8/23/05, Nathan Scott <nathans@sgi.com> wrote:
> FWIW, all filesystems using the generic page cache routines are able
> to return this - see mm/filemap.c -> generic_file_buffered_write...

I don't think it makes much sense to fix this in individual
filesystems as many functions returning -NOMEM can be used in other
paths as well where they're ok.

Andrew, please consider picking this up for -mm. (I've included it as
an attachment as well as gmail will surely mess up the patch. Sorry.)

                               Pekka

[PATCH] VFS: return ENOBUFS instead of ENOMEM for vfs_write()

As noticed by Dmitry Torokhov, write() can not return ENOMEM:

http://www.opengroup.org/onlinepubs/000095399/functions/write.html

Currently almost all filesystems can return -ENOMEM due to
generic_file_buffered_write() in mm/filemap.c so filter out the invalid
error code in vfs_write().

Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---

 read_write.c |    2 ++
 1 files changed, 2 insertions(+)

Index: 2.6-mm/fs/read_write.c
===================================================================
--- 2.6-mm.orig/fs/read_write.c
+++ 2.6-mm/fs/read_write.c
@@ -310,6 +310,8 @@ ssize_t vfs_write(struct file *file, con
                }
        }

+       if (ret == -ENOMEM)
+               ret = -ENOBUFS;
        return ret;
 }

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vfs-vfs_write-return-ENOBUFS-instead-ENOMEM.patch --]
[-- Type: text/x-patch; name="vfs-vfs_write-return-ENOBUFS-instead-ENOMEM.patch", Size: 759 bytes --]

[PATCH] VFS: return ENOBUFS instead of ENOMEM for vfs_write()

As noticed by Dmitry Torokhov, write() can not return ENOMEM:

http://www.opengroup.org/onlinepubs/000095399/functions/write.html

Currently almost all filesystems can return -ENOMEM due to
generic_file_buffered_write() in mm/filemap.c so filter out the invalid
error code in vfs_write().

Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---

 read_write.c |    2 ++
 1 files changed, 2 insertions(+)

Index: 2.6-mm/fs/read_write.c
===================================================================
--- 2.6-mm.orig/fs/read_write.c
+++ 2.6-mm/fs/read_write.c
@@ -310,6 +310,8 @@ ssize_t vfs_write(struct file *file, con
 		}
 	}
 
+	if (ret == -ENOMEM)
+		ret = -ENOBUFS;
 	return ret;
 }
 

Re: sysfs: write returns ENOMEM?

[Andrew Morton]

Pekka Enberg <penberg@gmail.com> wrote:
>
> @@ -310,6 +310,8 @@ ssize_t vfs_write(struct file *file, con
>                  }
>          }
> 
>  +       if (ret == -ENOMEM)
>  +               ret = -ENOBUFS;
>          return ret;
>   }
> 

That's lame.  It'd be better to hunt down all the -ENOMEMs and fix them up.

Re: sysfs: write returns ENOMEM?

[Pekka J Enberg]

Andrew Morton writes:
> That's lame.  It'd be better to hunt down all the -ENOMEMs and fix them up.

So there's our verdict. Thanks, Andrew :-) 

       Pekka 

[PATCH] mm: return ENOBUFS instead of ENOMEM in generic_file_buffered_write

[Pekka J Enberg]

As noticed by Dmitry Torokhov, write() can not return ENOMEM:

http://www.opengroup.org/onlinepubs/000095399/functions/write.html

Therefore fixup generic_file_buffered_write() in mm/filemap.c (pointed out by
Nathan Scott).

Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---

 filemap.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

Index: 2.6-mm/mm/filemap.c
===================================================================
--- 2.6-mm.orig/mm/filemap.c
+++ 2.6-mm/mm/filemap.c
@@ -1942,7 +1942,7 @@ generic_file_buffered_write(struct kiocb
 
 		page = __grab_cache_page(mapping,index,&cached_page,&lru_pvec);
 		if (!page) {
-			status = -ENOMEM;
+			status = -ENOBUFS;
 			break;
 		}
 

Re: [PATCH] mm: return ENOBUFS instead of ENOMEM in generic_file_buffered_write

[Christoph Hellwig]

On Tue, Aug 23, 2005 at 11:46:33AM +0300, Pekka J Enberg wrote:
> As noticed by Dmitry Torokhov, write() can not return ENOMEM:
> 
> http://www.opengroup.org/onlinepubs/000095399/functions/write.html
> 
> Therefore fixup generic_file_buffered_write() in mm/filemap.c (pointed out by
> Nathan Scott).

We had this discussion before, for EACCESS then.  We've always been returning
more errnos than SuS mentioned and Linus declared it's fine.

Re: [PATCH] mm: return ENOBUFS instead of ENOMEM in generic_file_buffered_write

[Dmitry Torokhov]

On 8/23/05, Christoph Hellwig <hch@infradead.org> wrote:
> On Tue, Aug 23, 2005 at 11:46:33AM +0300, Pekka J Enberg wrote:
> > As noticed by Dmitry Torokhov, write() can not return ENOMEM:
> >
> > http://www.opengroup.org/onlinepubs/000095399/functions/write.html
> >
> > Therefore fixup generic_file_buffered_write() in mm/filemap.c (pointed out by
> > Nathan Scott).
> 
> We had this discussion before, for EACCESS then.  We've always been returning
> more errnos than SuS mentioned and Linus declared it's fine.
> 

So does that mean that any error code is allowed? I would love to be
able to return ENODEV from a sysfs attribute if its device happens to
be removed in process. Is there a list of valid errnos for Linux that
supercedes SuS?

-- 
Dmitry