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

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

[Steve French]

 > As noticed by Dmitry Torokhov, write() can not return ENOMEM
It turns out that Linux is ok here returning ENOMEM (even from a strict 
POSIX perspective) so the patch is not needed.

I consulted our longstanding POSIX workgroup representative to see what 
he could find out about this topic, and this particular one has some 
history (and it turns out ENOMEM is ok).   Also note that you can return 
more return codes as long as they do not conflict with meanings 
assignmed to others, and ENOBUFS was added not to exclude ENOMEM but to 
match some out of network buffer cases coming back from the 
corresponding socket case.   That the listed return codes for the read 
case and write cases were not symmetric (in listing return codes) was 
noticed as something needing fixing even by the guy who added ENOBUFS in 
the first place and is something that should be fixed up in future POSIX 
specs.

 > We've always been returning more errnos than SuS mentioned and Linus 
declared it's fine.
Christoph (see above line) is correct not just from a Linus perspective 
- it can be legal from a posix perspective to return other error codes 
(there are some exceptions e.g. when the case the new return code covers 
is the same as a listed return code creating obvious duplication)

See below:
          -------------------------------------------
<via Mark Brown, member of the POSIX 1003.1/1003.2 WG and its
Interpretions list>

First off, just because a specific errno is not listed in the ERRORS section
of a given API, doesn't mean that that errno can not be returned by an
implementation (1003.1-2001 Base Definitions Sec 2.3). The ERRORS section
describes errnos that must be used for a given condition, but other
conditions not explicitly listed may be reported. There are some APIs that
disallow reporting of additional error conditions, but they explicitly say
so in their entry.

Sec 2.3 does state that one cannot return a different errno than the one 
that
is listed for a given condition - You can't return EACCES when you mean 
ENOENT
and ENOENT is on the API's list. Does this mean that you can't return
ENOMEM (when getting space for a datastruct) if ENOBUFS is present?

My answer is that ENOMEM is conforming behavior. ENOBUFS has a different
meaning than ENOMEM. The complete descriptions of ENOBUFS and ENOMEM, taken
from the same Section 2.3:

ENOBUFS
No buffer space available. Insufficient buffer resources were available in
the system to perform the socket operation.

ENOMEM
Not enough space. The new process image requires more memory than is allowed
by the hardware or system-imposed memory management constraints.

Also, the history these errnos in both read() and write() shows that 
they were
not present in versions of 1003.1 before the 2001 version. These errnos were
added to the spec for the 2001 version in an attempt to rationalize their
behavior with recv() and send(), which can operate like read() or write()
under certain circumstances. recv() had both ENOBUFS and ENOMEM, so they
went into read(), send() only had ENOBUFS. However, it was conforming 
behavior
to return ENOMEM before the 2001 specification, and no specific intent
was offered to break existing conforming implementations by this change.


-------------------
Mark Brown/Austin/IBM
STSM, UNIX/Linux OS Standards