Hi,
Jean Delvare has noticed that if a driver happens to declare its
attribute as RW but doesn't provide store() method attempt to write
into such attribute will cause spinning process as most of the
attribute implementations return 0 in case of missing store causing
endless retries. In some cases missing show/store will return -EPERM,
-EACCESS or -EINVAL.
I think we should unify implementations and have them all return -ENOSYS
(function not implemented) when corresponding method (show/store) is
missing.
--
Dmitry
sysfs: fix the rest of the kernel so if an attribute doesn't
implement show or store method read/write will return
-ENOSYS instead of 0 or -EINVAL or -EPERM.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/acpi/scan.c | 4 ++--
drivers/cpufreq/cpufreq.c | 4 ++--
drivers/firmware/edd.c | 2 +-
drivers/firmware/efivars.c | 4 ++--
drivers/infiniband/core/sysfs.c | 2 +-
kernel/params.c | 4 ++--
security/seclvl.c | 4 ++--
7 files changed, 12 insertions(+), 12 deletions(-)
Index: dtor/drivers/cpufreq/cpufreq.c
===================================================================
--- dtor.orig/drivers/cpufreq/cpufreq.c
+++ dtor/drivers/cpufreq/cpufreq.c
@@ -521,7 +521,7 @@ static ssize_t show(struct kobject * kob
policy = cpufreq_cpu_get(policy->cpu);
if (!policy)
return -EINVAL;
- ret = fattr->show ? fattr->show(policy,buf) : 0;
+ ret = fattr->show ? fattr->show(policy,buf) : -ENOSYS;
cpufreq_cpu_put(policy);
return ret;
}
@@ -535,7 +535,7 @@ static ssize_t store(struct kobject * ko
policy = cpufreq_cpu_get(policy->cpu);
if (!policy)
return -EINVAL;
- ret = fattr->store ? fattr->store(policy,buf,count) : 0;
+ ret = fattr->store ? fattr->store(policy,buf,count) : -ENOSYS;
cpufreq_cpu_put(policy);
return ret;
}
Index: dtor/drivers/firmware/efivars.c
===================================================================
--- dtor.orig/drivers/firmware/efivars.c
+++ dtor/drivers/firmware/efivars.c
@@ -352,7 +352,7 @@ static ssize_t efivar_attr_show(struct k
{
struct efivar_entry *var = to_efivar_entry(kobj);
struct efivar_attribute *efivar_attr = to_efivar_attr(attr);
- ssize_t ret = 0;
+ ssize_t ret = -ENOSYS;
if (!capable(CAP_SYS_ADMIN))
return -EACCES;
@@ -368,7 +368,7 @@ static ssize_t efivar_attr_store(struct
{
struct efivar_entry *var = to_efivar_entry(kobj);
struct efivar_attribute *efivar_attr = to_efivar_attr(attr);
- ssize_t ret = 0;
+ ssize_t ret = -ENOSYS;
if (!capable(CAP_SYS_ADMIN))
return -EACCES;
Index: dtor/kernel/params.c
===================================================================
--- dtor.orig/kernel/params.c
+++ dtor/kernel/params.c
@@ -629,7 +629,7 @@ static ssize_t module_attr_show(struct k
mk = to_module_kobject(kobj);
if (!attribute->show)
- return -EPERM;
+ return -ENOSYS;
if (!try_module_get(mk->mod))
return -ENODEV;
@@ -653,7 +653,7 @@ static ssize_t module_attr_store(struct
mk = to_module_kobject(kobj);
if (!attribute->store)
- return -EPERM;
+ return -ENOSYS;
if (!try_module_get(mk->mod))
return -ENODEV;
Index: dtor/drivers/infiniband/core/sysfs.c
===================================================================
--- dtor.orig/drivers/infiniband/core/sysfs.c
+++ dtor/drivers/infiniband/core/sysfs.c
@@ -71,7 +71,7 @@ static ssize_t port_attr_show(struct kob
struct ib_port *p = container_of(kobj, struct ib_port, kobj);
if (!port_attr->show)
- return 0;
+ return -ENOSYS;
return port_attr->show(p, port_attr, buf);
}
Index: dtor/security/seclvl.c
===================================================================
--- dtor.orig/security/seclvl.c
+++ dtor/security/seclvl.c
@@ -155,7 +155,7 @@ seclvl_attr_store(struct kobject *kobj,
struct seclvl_obj *obj = container_of(kobj, struct seclvl_obj, kobj);
struct seclvl_attribute *attribute =
container_of(attr, struct seclvl_attribute, attr);
- return (attribute->store ? attribute->store(obj, buf, len) : 0);
+ return attribute->store ? attribute->store(obj, buf, len) : -ENOSYS;
}
static ssize_t
@@ -164,7 +164,7 @@ seclvl_attr_show(struct kobject *kobj, s
struct seclvl_obj *obj = container_of(kobj, struct seclvl_obj, kobj);
struct seclvl_attribute *attribute =
container_of(attr, struct seclvl_attribute, attr);
- return (attribute->show ? attribute->show(obj, buf) : 0);
+ return attribute->show ? attribute->show(obj, buf) : -ENOSYS;
}
/**
Index: dtor/drivers/acpi/scan.c
===================================================================
--- dtor.orig/drivers/acpi/scan.c
+++ dtor/drivers/acpi/scan.c
@@ -65,14 +65,14 @@ static ssize_t acpi_device_attr_show(str
{
struct acpi_device *device = to_acpi_device(kobj);
struct acpi_device_attribute *attribute = to_handle_attr(attr);
- return attribute->show ? attribute->show(device, buf) : 0;
+ return attribute->show ? attribute->show(device, buf) : -ENOSYS;
}
static ssize_t acpi_device_attr_store(struct kobject *kobj,
struct attribute *attr, const char *buf, size_t len)
{
struct acpi_device *device = to_acpi_device(kobj);
struct acpi_device_attribute *attribute = to_handle_attr(attr);
- return attribute->store ? attribute->store(device, buf, len) : len;
+ return attribute->store ? attribute->store(device, buf, len) : -ENOSYS;
}
static struct sysfs_ops acpi_device_sysfs_ops = {
Index: dtor/drivers/firmware/edd.c
===================================================================
--- dtor.orig/drivers/firmware/edd.c
+++ dtor/drivers/firmware/edd.c
@@ -115,7 +115,7 @@ edd_attr_show(struct kobject * kobj, str
{
struct edd_device *dev = to_edd_device(kobj);
struct edd_attribute *edd_attr = to_edd_attr(attr);
- ssize_t ret = 0;
+ ssize_t ret = -ENOSYS;
if (edd_attr->show)
ret = edd_attr->show(dev, buf);
sysfs: fix drivers/block so if an attribute doesn't implement
show or store method read/write will return -ENOSYS
instead of 0 or -EINVAL.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
as-iosched.c | 4 ++--
cfq-iosched.c | 4 ++--
deadline-iosched.c | 4 ++--
genhd.c | 2 +-
ll_rw_blk.c | 4 ++--
5 files changed, 9 insertions(+), 9 deletions(-)
Index: dtor/drivers/block/as-iosched.c
===================================================================
--- dtor.orig/drivers/block/as-iosched.c
+++ dtor/drivers/block/as-iosched.c
@@ -2044,7 +2044,7 @@ as_attr_show(struct kobject *kobj, struc
struct as_fs_entry *entry = to_as(attr);
if (!entry->show)
- return 0;
+ return -ENOSYS;
return entry->show(e->elevator_data, page);
}
@@ -2057,7 +2057,7 @@ as_attr_store(struct kobject *kobj, stru
struct as_fs_entry *entry = to_as(attr);
if (!entry->store)
- return -EINVAL;
+ return -ENOSYS;
return entry->store(e->elevator_data, page, length);
}
Index: dtor/drivers/block/cfq-iosched.c
===================================================================
--- dtor.orig/drivers/block/cfq-iosched.c
+++ dtor/drivers/block/cfq-iosched.c
@@ -1772,7 +1772,7 @@ cfq_attr_show(struct kobject *kobj, stru
struct cfq_fs_entry *entry = to_cfq(attr);
if (!entry->show)
- return 0;
+ return -ENOSYS;
return entry->show(e->elevator_data, page);
}
@@ -1785,7 +1785,7 @@ cfq_attr_store(struct kobject *kobj, str
struct cfq_fs_entry *entry = to_cfq(attr);
if (!entry->store)
- return -EINVAL;
+ return -ENOSYS;
return entry->store(e->elevator_data, page, length);
}
Index: dtor/drivers/block/genhd.c
===================================================================
--- dtor.orig/drivers/block/genhd.c
+++ dtor/drivers/block/genhd.c
@@ -321,7 +321,7 @@ static ssize_t disk_attr_show(struct kob
struct gendisk *disk = to_disk(kobj);
struct disk_attribute *disk_attr =
container_of(attr,struct disk_attribute,attr);
- ssize_t ret = 0;
+ ssize_t ret = -ENOSYS;
if (disk_attr->show)
ret = disk_attr->show(disk,page);
Index: dtor/drivers/block/ll_rw_blk.c
===================================================================
--- dtor.orig/drivers/block/ll_rw_blk.c
+++ dtor/drivers/block/ll_rw_blk.c
@@ -3582,7 +3582,7 @@ queue_attr_show(struct kobject *kobj, st
q = container_of(kobj, struct request_queue, kobj);
if (!entry->show)
- return 0;
+ return -ENOSYS;
return entry->show(q, page);
}
@@ -3596,7 +3596,7 @@ queue_attr_store(struct kobject *kobj, s
q = container_of(kobj, struct request_queue, kobj);
if (!entry->store)
- return -EINVAL;
+ return -ENOSYS;
return entry->store(q, page, length);
}
Index: dtor/drivers/block/deadline-iosched.c
===================================================================
--- dtor.orig/drivers/block/deadline-iosched.c
+++ dtor/drivers/block/deadline-iosched.c
@@ -886,7 +886,7 @@ deadline_attr_show(struct kobject *kobj,
struct deadline_fs_entry *entry = to_deadline(attr);
if (!entry->show)
- return 0;
+ return -ENOSYS;
return entry->show(e->elevator_data, page);
}
@@ -899,7 +899,7 @@ deadline_attr_store(struct kobject *kobj
struct deadline_fs_entry *entry = to_deadline(attr);
if (!entry->store)
- return -EINVAL;
+ return -ENOSYS;
return entry->store(e->elevator_data, page, length);
}
sysfs: if attribute does not implement show or store method
read/write should return -ENOSYS instead of 0, EACCESS
or EINVAL.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
bin.c | 4 ++--
file.c | 19 ++++++++++++-------
2 files changed, 14 insertions(+), 9 deletions(-)
Index: dtor/fs/sysfs/file.c
===================================================================
--- dtor.orig/fs/sysfs/file.c
+++ dtor/fs/sysfs/file.c
@@ -23,7 +23,7 @@ subsys_attr_show(struct kobject * kobj,
{
struct subsystem * s = to_subsys(kobj);
struct subsys_attribute * sattr = to_sattr(attr);
- ssize_t ret = 0;
+ ssize_t ret = -ENOSYS;
if (sattr->show)
ret = sattr->show(s,page);
@@ -36,7 +36,7 @@ subsys_attr_store(struct kobject * kobj,
{
struct subsystem * s = to_subsys(kobj);
struct subsys_attribute * sattr = to_sattr(attr);
- ssize_t ret = 0;
+ ssize_t ret = -ENOSYS;
if (sattr->store)
ret = sattr->store(s,page,count);
@@ -274,17 +274,17 @@ static int check_perm(struct inode * ino
* or the subsystem have no operations.
*/
if (!ops)
- goto Eaccess;
+ goto Enosys;
/* File needs write support.
* The inode's perms must say it's ok,
* and we must have a store method.
*/
if (file->f_mode & FMODE_WRITE) {
-
- if (!(inode->i_mode & S_IWUGO) || !ops->store)
+ if (!(inode->i_mode & S_IWUGO))
goto Eaccess;
-
+ if (!ops->store)
+ goto Enosys;
}
/* File needs read support.
@@ -292,8 +292,10 @@ static int check_perm(struct inode * ino
* must be a show method for it.
*/
if (file->f_mode & FMODE_READ) {
- if (!(inode->i_mode & S_IRUGO) || !ops->show)
+ if (!(inode->i_mode & S_IRUGO))
goto Eaccess;
+ if (!ops->show)
+ goto Enosys;
}
/* No error? Great, allocate a buffer for the file, and store it
@@ -313,6 +315,9 @@ static int check_perm(struct inode * ino
Einval:
error = -EINVAL;
goto Done;
+ Enosys:
+ error = -ENOSYS;
+ goto Done;
Eaccess:
error = -EACCES;
module_put(attr->owner);
Index: dtor/fs/sysfs/bin.c
===================================================================
--- dtor.orig/fs/sysfs/bin.c
+++ dtor/fs/sysfs/bin.c
@@ -25,7 +25,7 @@ fill_read(struct dentry *dentry, char *b
struct kobject * kobj = to_kobj(dentry->d_parent);
if (!attr->read)
- return -EINVAL;
+ return -ENOSYS;
return attr->read(kobj, buffer, off, count);
}
@@ -71,7 +71,7 @@ flush_write(struct dentry *dentry, char
struct kobject *kobj = to_kobj(dentry->d_parent);
if (!attr->write)
- return -EINVAL;
+ return -ENOSYS;
return attr->write(kobj, buffer, offset, count);
}
sysfs: fix drivers/base so if an attribute doesn't implement
show or store method read/write will return -ENOSYS
instead of 0.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
bus.c | 4 ++--
class.c | 4 ++--
core.c | 4 ++--
sys.c | 4 ++--
4 files changed, 8 insertions(+), 8 deletions(-)
Index: dtor/drivers/base/class.c
===================================================================
--- dtor.orig/drivers/base/class.c
+++ dtor/drivers/base/class.c
@@ -26,7 +26,7 @@ class_attr_show(struct kobject * kobj, s
{
struct class_attribute * class_attr = to_class_attr(attr);
struct class * dc = to_class(kobj);
- ssize_t ret = 0;
+ ssize_t ret = -ENOSYS;
if (class_attr->show)
ret = class_attr->show(dc, buf);
@@ -39,7 +39,7 @@ class_attr_store(struct kobject * kobj,
{
struct class_attribute * class_attr = to_class_attr(attr);
struct class * dc = to_class(kobj);
- ssize_t ret = 0;
+ ssize_t ret = -ENOSYS;
if (class_attr->store)
ret = class_attr->store(dc, buf, count);
Index: dtor/drivers/base/sys.c
===================================================================
--- dtor.orig/drivers/base/sys.c
+++ dtor/drivers/base/sys.c
@@ -37,7 +37,7 @@ sysdev_show(struct kobject * kobj, struc
if (sysdev_attr->show)
return sysdev_attr->show(sysdev, buffer);
- return 0;
+ return -ENOSYS;
}
@@ -50,7 +50,7 @@ sysdev_store(struct kobject * kobj, stru
if (sysdev_attr->store)
return sysdev_attr->store(sysdev, buffer, count);
- return 0;
+ return -ENOSYS;
}
static struct sysfs_ops sysfs_ops = {
Index: dtor/drivers/base/bus.c
===================================================================
--- dtor.orig/drivers/base/bus.c
+++ dtor/drivers/base/bus.c
@@ -36,7 +36,7 @@ drv_attr_show(struct kobject * kobj, str
{
struct driver_attribute * drv_attr = to_drv_attr(attr);
struct device_driver * drv = to_driver(kobj);
- ssize_t ret = 0;
+ ssize_t ret = -ENOSYS;
if (drv_attr->show)
ret = drv_attr->show(drv, buf);
@@ -49,7 +49,7 @@ drv_attr_store(struct kobject * kobj, st
{
struct driver_attribute * drv_attr = to_drv_attr(attr);
struct device_driver * drv = to_driver(kobj);
- ssize_t ret = 0;
+ ssize_t ret = -ENOSYS;
if (drv_attr->store)
ret = drv_attr->store(drv, buf, count);
Index: dtor/drivers/base/core.c
===================================================================
--- dtor.orig/drivers/base/core.c
+++ dtor/drivers/base/core.c
@@ -38,7 +38,7 @@ dev_attr_show(struct kobject * kobj, str
{
struct device_attribute * dev_attr = to_dev_attr(attr);
struct device * dev = to_dev(kobj);
- ssize_t ret = 0;
+ ssize_t ret = -ENOSYS;
if (dev_attr->show)
ret = dev_attr->show(dev, buf);
@@ -51,7 +51,7 @@ dev_attr_store(struct kobject * kobj, st
{
struct device_attribute * dev_attr = to_dev_attr(attr);
struct device * dev = to_dev(kobj);
- ssize_t ret = 0;
+ ssize_t ret = -ENOSYS;
if (dev_attr->store)
ret = dev_attr->store(dev, buf, count);
sysfs: fix drivers/pci so if an attribute does not implement
show or store method read/write will return -ENOSYS
instead of 0.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
hotplug/pci_hotplug_core.c | 4 ++--
hotplug/rpadlpar_sysfs.c | 2 +-
pci-driver.c | 26 ++++++++++++++------------
3 files changed, 17 insertions(+), 15 deletions(-)
Index: dtor/drivers/pci/hotplug/rpadlpar_sysfs.c
===================================================================
--- dtor.orig/drivers/pci/hotplug/rpadlpar_sysfs.c
+++ dtor/drivers/pci/hotplug/rpadlpar_sysfs.c
@@ -48,7 +48,7 @@ dlpar_attr_store(struct kobject * kobj,
struct dlpar_io_attr *dlpar_attr = container_of(attr,
struct dlpar_io_attr, attr);
return dlpar_attr->store ?
- dlpar_attr->store(dlpar_attr, buf, nbytes) : 0;
+ dlpar_attr->store(dlpar_attr, buf, nbytes) : -ENOSYS;
}
static struct sysfs_ops dlpar_attr_sysfs_ops = {
Index: dtor/drivers/pci/pci-driver.c
===================================================================
--- dtor.orig/drivers/pci/pci-driver.c
+++ dtor/drivers/pci/pci-driver.c
@@ -327,13 +327,14 @@ pci_driver_attr_show(struct kobject * ko
{
struct device_driver *driver = kobj_to_pci_driver(kobj);
struct driver_attribute *dattr = attr_to_driver_attribute(attr);
- ssize_t ret = 0;
+ ssize_t ret;
- if (get_driver(driver)) {
- if (dattr->show)
- ret = dattr->show(driver, buf);
- put_driver(driver);
- }
+ if (!get_driver(driver))
+ return -ENODEV;
+
+ ret = dattr->show ? dattr->show(driver, buf) : -ENOSYS;
+
+ put_driver(driver);
return ret;
}
@@ -343,13 +344,14 @@ pci_driver_attr_store(struct kobject * k
{
struct device_driver *driver = kobj_to_pci_driver(kobj);
struct driver_attribute *dattr = attr_to_driver_attribute(attr);
- ssize_t ret = 0;
+ ssize_t ret;
- if (get_driver(driver)) {
- if (dattr->store)
- ret = dattr->store(driver, buf, count);
- put_driver(driver);
- }
+ if (!get_driver(driver))
+ return -ENODEV;
+
+ ret = dattr->store ? dattr->store(driver, buf, count) : -ENOSYS;
+
+ put_driver(driver);
return ret;
}
Index: dtor/drivers/pci/hotplug/pci_hotplug_core.c
===================================================================
--- dtor.orig/drivers/pci/hotplug/pci_hotplug_core.c
+++ dtor/drivers/pci/hotplug/pci_hotplug_core.c
@@ -73,7 +73,7 @@ static ssize_t hotplug_slot_attr_show(st
{
struct hotplug_slot *slot = to_hotplug_slot(kobj);
struct hotplug_slot_attribute *attribute = to_hotplug_attr(attr);
- return attribute->show ? attribute->show(slot, buf) : 0;
+ return attribute->show ? attribute->show(slot, buf) : -ENOSYS;
}
static ssize_t hotplug_slot_attr_store(struct kobject *kobj,
@@ -81,7 +81,7 @@ static ssize_t hotplug_slot_attr_store(s
{
struct hotplug_slot *slot = to_hotplug_slot(kobj);
struct hotplug_slot_attribute *attribute = to_hotplug_attr(attr);
- return attribute->store ? attribute->store(slot, buf, len) : 0;
+ return attribute->store ? attribute->store(slot, buf, len) : -ENOSYS;
}
static struct sysfs_ops hotplug_slot_sysfs_ops = {
On Thu, Apr 28, 2005 at 12:30:09AM -0500, Dmitry Torokhov wrote:
> Hi,
>
> Jean Delvare has noticed that if a driver happens to declare its
> attribute as RW but doesn't provide store() method attempt to write
> into such attribute will cause spinning process as most of the
> attribute implementations return 0 in case of missing store causing
> endless retries. In some cases missing show/store will return -EPERM,
> -EACCESS or -EINVAL.
>
> I think we should unify implementations and have them all return -ENOSYS
> (function not implemented) when corresponding method (show/store) is
> missing.
What is the POSIX standard for this? ENOSYS or EACCESS?
Or anyone have a link that I can look this up at?
thanks,
greg k-h
* Greg KH ([email protected]) wrote:
> On Thu, Apr 28, 2005 at 12:30:09AM -0500, Dmitry Torokhov wrote:
> > Hi,
> >
> > Jean Delvare has noticed that if a driver happens to declare its
> > attribute as RW but doesn't provide store() method attempt to write
> > into such attribute will cause spinning process as most of the
> > attribute implementations return 0 in case of missing store causing
> > endless retries. In some cases missing show/store will return -EPERM,
> > -EACCESS or -EINVAL.
> >
> > I think we should unify implementations and have them all return -ENOSYS
> > (function not implemented) when corresponding method (show/store) is
> > missing.
>
> What is the POSIX standard for this? ENOSYS or EACCESS?
SuSv3 suggests EBADF, however we already do EINVAL at VFS for no write
op. Although, returning 0 (i.e. wrote zero bytes) is still meaningful
too.
> Or anyone have a link that I can look this up at?
http://www.opengroup.org/onlinepubs/000095399/functions/write.html
thanks,
-chris
On Thu, 2005-04-28 at 10:26 -0700, Greg KH wrote:
> What is the POSIX standard for this? ENOSYS or EACCESS?
>
> Or anyone have a link that I can look this up at?
ENOSYS isn't defined by SUSv3 (and thus not POSIX) for write(2).
EACCESS is defined only for socket errors, but you could of course
hijack it. I think it is silly, though, since the open(2) succeeded.
Ideally, the open for writing should fail with EACCESS.
EIO actually means that an internal or physical I/O error occurred, PLUS
it is reserved for implementation-defined errors. So that makes sense.
The main thing is to _not_ return zero. That would cause stdio to
resubmit indefinitely.
Robert Love
On Thu, 2005-04-28 at 10:37 -0700, Chris Wright wrote:
> SuSv3 suggests EBADF, however we already do EINVAL at VFS for no write
> op. Although, returning 0 (i.e. wrote zero bytes) is still meaningful
> too.
I think EBADF implies that you opened the thing not for writing, and now
you are trying to write to it.
But my understanding of this problem is that the open is succeeding for
writes, but when the actual write is performed, the store fails (or does
not exist).
EIO makes sense for that. EACCESS less so, but still some.
Robert Love
On 4/28/05, Chris Wright <[email protected]> wrote:
> * Greg KH ([email protected]) wrote:
> > On Thu, Apr 28, 2005 at 12:30:09AM -0500, Dmitry Torokhov wrote:
> > > Hi,
> > >
> > > Jean Delvare has noticed that if a driver happens to declare its
> > > attribute as RW but doesn't provide store() method attempt to write
> > > into such attribute will cause spinning process as most of the
> > > attribute implementations return 0 in case of missing store causing
> > > endless retries. In some cases missing show/store will return -EPERM,
> > > -EACCESS or -EINVAL.
> > >
> > > I think we should unify implementations and have them all return -ENOSYS
> > > (function not implemented) when corresponding method (show/store) is
> > > missing.
> >
> > What is the POSIX standard for this? ENOSYS or EACCESS?
>
> SuSv3 suggests EBADF, however we already do EINVAL at VFS for no write
> op. Although, returning 0 (i.e. wrote zero bytes) is still meaningful
> too.
>
Returning 0 causes caller to immediately repeat operation causing the
loop and 100% CPU utiluization as was reported. If ENOSYS is not
acceptable (after all the problem is sysfs-specific, other fs-es
either support write or they don't for entire volume) I'd say (looking
at the descripptions) we should use ENXIO instead of EBADF:
[ENXIO]
A request was made of a nonexistent device, or the request was outside
the capabilities of the device.
[EBADF]
The fildes argument is not a valid file descriptor open for writing.
We have a valid FD and it was opened for wrinting, so EBADF is not
really applicable. But then I may be talking complete gibberish...
--
Dmitry
On Thu, 28 Apr 2005, Dmitry Torokhov wrote:
> On 4/28/05, Chris Wright <[email protected]> wrote:
>> * Greg KH ([email protected]) wrote:
>>> On Thu, Apr 28, 2005 at 12:30:09AM -0500, Dmitry Torokhov wrote:
>>>> Hi,
>>>>
>>>> Jean Delvare has noticed that if a driver happens to declare its
>>>> attribute as RW but doesn't provide store() method attempt to write
>>>> into such attribute will cause spinning process as most of the
>>>> attribute implementations return 0 in case of missing store causing
>>>> endless retries. In some cases missing show/store will return -EPERM,
>>>> -EACCESS or -EINVAL.
>>>>
>>>> I think we should unify implementations and have them all return -ENOSYS
>>>> (function not implemented) when corresponding method (show/store) is
>>>> missing.
>>>
>>> What is the POSIX standard for this? ENOSYS or EACCESS?
>>
>> SuSv3 suggests EBADF, however we already do EINVAL at VFS for no write
>> op. Although, returning 0 (i.e. wrote zero bytes) is still meaningful
>> too.
>>
>
> Returning 0 causes caller to immediately repeat operation causing the
> loop and 100% CPU utiluization as was reported. If ENOSYS is not
> acceptable (after all the problem is sysfs-specific, other fs-es
> either support write or they don't for entire volume) I'd say (looking
> at the descripptions) we should use ENXIO instead of EBADF:
>
> [ENXIO]
> A request was made of a nonexistent device, or the request was outside
> the capabilities of the device.
>
> [EBADF]
> The fildes argument is not a valid file descriptor open for writing.
>
> We have a valid FD and it was opened for wrinting, so EBADF is not
> really applicable. But then I may be talking complete gibberish...
>
> --
> Dmitry
> -
Perhaps...
ENOSPC, i.e., the device is full. This is the usual return value
for a CD/ROM and `df` always shows it's full even if it has
one small file.
The only thing that really makes sense is EPERM but it is usually
associated with permissions rather than if an operation is permitted.
Cheers,
Dick Johnson
Penguin : Linux version 2.6.11 on an i686 machine (5537.79 BogoMips).
Notice : All mail here is now cached for review by Dictator Bush.
98.36% of all statistics are fiction.
On Thu, 2005-04-28 at 15:56 -0400, Richard B. Johnson wrote:
> Perhaps...
> ENOSPC, i.e., the device is full. This is the usual return value
> for a CD/ROM and `df` always shows it's full even if it has
> one small file.
But there is space on the backing device. Given two files on the same
file system, one should not be returning ENOSPC on write and the other
succeeding.
> The only thing that really makes sense is EPERM but it is usually
> associated with permissions rather than if an operation is permitted.
EPERM isn't even a valid SUSv3 return for write(2). Permissions don't
apply to writes, only opens (and, thus, this is a valid return for
open(2)).
The answer has already been suggested: EACCESS or EIO.
Robert Love
sysfs: if attribute does not implement show or store method
read/write should return -EIO instead of 0 or -EINVAL.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
bin.c | 4 ++--
file.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
Index: dtor/fs/sysfs/file.c
===================================================================
--- dtor.orig/fs/sysfs/file.c
+++ dtor/fs/sysfs/file.c
@@ -23,7 +23,7 @@ subsys_attr_show(struct kobject * kobj,
{
struct subsystem * s = to_subsys(kobj);
struct subsys_attribute * sattr = to_sattr(attr);
- ssize_t ret = 0;
+ ssize_t ret = -EIO;
if (sattr->show)
ret = sattr->show(s,page);
@@ -36,7 +36,7 @@ subsys_attr_store(struct kobject * kobj,
{
struct subsystem * s = to_subsys(kobj);
struct subsys_attribute * sattr = to_sattr(attr);
- ssize_t ret = 0;
+ ssize_t ret = -EIO;
if (sattr->store)
ret = sattr->store(s,page,count);
Index: dtor/fs/sysfs/bin.c
===================================================================
--- dtor.orig/fs/sysfs/bin.c
+++ dtor/fs/sysfs/bin.c
@@ -25,7 +25,7 @@ fill_read(struct dentry *dentry, char *b
struct kobject * kobj = to_kobj(dentry->d_parent);
if (!attr->read)
- return -EINVAL;
+ return -EIO;
return attr->read(kobj, buffer, off, count);
}
@@ -71,7 +71,7 @@ flush_write(struct dentry *dentry, char
struct kobject *kobj = to_kobj(dentry->d_parent);
if (!attr->write)
- return -EINVAL;
+ return -EIO;
return attr->write(kobj, buffer, offset, count);
}
sysfs: fix drivers/base so if an attribute doesn't implement
show or store method read/write will return -EIO
instead of 0.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
bus.c | 4 ++--
class.c | 4 ++--
core.c | 4 ++--
sys.c | 4 ++--
4 files changed, 8 insertions(+), 8 deletions(-)
Index: dtor/drivers/base/class.c
===================================================================
--- dtor.orig/drivers/base/class.c
+++ dtor/drivers/base/class.c
@@ -26,7 +26,7 @@ class_attr_show(struct kobject * kobj, s
{
struct class_attribute * class_attr = to_class_attr(attr);
struct class * dc = to_class(kobj);
- ssize_t ret = 0;
+ ssize_t ret = -EIO;
if (class_attr->show)
ret = class_attr->show(dc, buf);
@@ -39,7 +39,7 @@ class_attr_store(struct kobject * kobj,
{
struct class_attribute * class_attr = to_class_attr(attr);
struct class * dc = to_class(kobj);
- ssize_t ret = 0;
+ ssize_t ret = -EIO;
if (class_attr->store)
ret = class_attr->store(dc, buf, count);
Index: dtor/drivers/base/sys.c
===================================================================
--- dtor.orig/drivers/base/sys.c
+++ dtor/drivers/base/sys.c
@@ -37,7 +37,7 @@ sysdev_show(struct kobject * kobj, struc
if (sysdev_attr->show)
return sysdev_attr->show(sysdev, buffer);
- return 0;
+ return -EIO;
}
@@ -50,7 +50,7 @@ sysdev_store(struct kobject * kobj, stru
if (sysdev_attr->store)
return sysdev_attr->store(sysdev, buffer, count);
- return 0;
+ return -EIO;
}
static struct sysfs_ops sysfs_ops = {
Index: dtor/drivers/base/bus.c
===================================================================
--- dtor.orig/drivers/base/bus.c
+++ dtor/drivers/base/bus.c
@@ -36,7 +36,7 @@ drv_attr_show(struct kobject * kobj, str
{
struct driver_attribute * drv_attr = to_drv_attr(attr);
struct device_driver * drv = to_driver(kobj);
- ssize_t ret = 0;
+ ssize_t ret = -EIO;
if (drv_attr->show)
ret = drv_attr->show(drv, buf);
@@ -49,7 +49,7 @@ drv_attr_store(struct kobject * kobj, st
{
struct driver_attribute * drv_attr = to_drv_attr(attr);
struct device_driver * drv = to_driver(kobj);
- ssize_t ret = 0;
+ ssize_t ret = -EIO;
if (drv_attr->store)
ret = drv_attr->store(drv, buf, count);
Index: dtor/drivers/base/core.c
===================================================================
--- dtor.orig/drivers/base/core.c
+++ dtor/drivers/base/core.c
@@ -38,7 +38,7 @@ dev_attr_show(struct kobject * kobj, str
{
struct device_attribute * dev_attr = to_dev_attr(attr);
struct device * dev = to_dev(kobj);
- ssize_t ret = 0;
+ ssize_t ret = -EIO;
if (dev_attr->show)
ret = dev_attr->show(dev, buf);
@@ -51,7 +51,7 @@ dev_attr_store(struct kobject * kobj, st
{
struct device_attribute * dev_attr = to_dev_attr(attr);
struct device * dev = to_dev(kobj);
- ssize_t ret = 0;
+ ssize_t ret = -EIO;
if (dev_attr->store)
ret = dev_attr->store(dev, buf, count);
sysfs: fix drivers/pci so if an attribute does not implement
show or store method read/write will return -EIO
instead of 0.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
hotplug/pci_hotplug_core.c | 4 ++--
hotplug/rpadlpar_sysfs.c | 2 +-
pci-driver.c | 26 ++++++++++++++------------
3 files changed, 17 insertions(+), 15 deletions(-)
Index: dtor/drivers/pci/hotplug/rpadlpar_sysfs.c
===================================================================
--- dtor.orig/drivers/pci/hotplug/rpadlpar_sysfs.c
+++ dtor/drivers/pci/hotplug/rpadlpar_sysfs.c
@@ -48,7 +48,7 @@ dlpar_attr_store(struct kobject * kobj,
struct dlpar_io_attr *dlpar_attr = container_of(attr,
struct dlpar_io_attr, attr);
return dlpar_attr->store ?
- dlpar_attr->store(dlpar_attr, buf, nbytes) : 0;
+ dlpar_attr->store(dlpar_attr, buf, nbytes) : -EIO;
}
static struct sysfs_ops dlpar_attr_sysfs_ops = {
Index: dtor/drivers/pci/pci-driver.c
===================================================================
--- dtor.orig/drivers/pci/pci-driver.c
+++ dtor/drivers/pci/pci-driver.c
@@ -327,13 +327,14 @@ pci_driver_attr_show(struct kobject * ko
{
struct device_driver *driver = kobj_to_pci_driver(kobj);
struct driver_attribute *dattr = attr_to_driver_attribute(attr);
- ssize_t ret = 0;
+ ssize_t ret;
- if (get_driver(driver)) {
- if (dattr->show)
- ret = dattr->show(driver, buf);
- put_driver(driver);
- }
+ if (!get_driver(driver))
+ return -ENODEV;
+
+ ret = dattr->show ? dattr->show(driver, buf) : -EIO;
+
+ put_driver(driver);
return ret;
}
@@ -343,13 +344,14 @@ pci_driver_attr_store(struct kobject * k
{
struct device_driver *driver = kobj_to_pci_driver(kobj);
struct driver_attribute *dattr = attr_to_driver_attribute(attr);
- ssize_t ret = 0;
+ ssize_t ret;
- if (get_driver(driver)) {
- if (dattr->store)
- ret = dattr->store(driver, buf, count);
- put_driver(driver);
- }
+ if (!get_driver(driver))
+ return -ENODEV;
+
+ ret = dattr->store ? dattr->store(driver, buf, count) : -EIO;
+
+ put_driver(driver);
return ret;
}
Index: dtor/drivers/pci/hotplug/pci_hotplug_core.c
===================================================================
--- dtor.orig/drivers/pci/hotplug/pci_hotplug_core.c
+++ dtor/drivers/pci/hotplug/pci_hotplug_core.c
@@ -73,7 +73,7 @@ static ssize_t hotplug_slot_attr_show(st
{
struct hotplug_slot *slot = to_hotplug_slot(kobj);
struct hotplug_slot_attribute *attribute = to_hotplug_attr(attr);
- return attribute->show ? attribute->show(slot, buf) : 0;
+ return attribute->show ? attribute->show(slot, buf) : -EIO;
}
static ssize_t hotplug_slot_attr_store(struct kobject *kobj,
@@ -81,7 +81,7 @@ static ssize_t hotplug_slot_attr_store(s
{
struct hotplug_slot *slot = to_hotplug_slot(kobj);
struct hotplug_slot_attribute *attribute = to_hotplug_attr(attr);
- return attribute->store ? attribute->store(slot, buf, len) : 0;
+ return attribute->store ? attribute->store(slot, buf, len) : -EIO;
}
static struct sysfs_ops hotplug_slot_sysfs_ops = {
sysfs: fix drivers/block so if an attribute doesn't implement
show or store method read/write will return -EIO
instead of 0 or -EINVAL.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
as-iosched.c | 4 ++--
cfq-iosched.c | 4 ++--
deadline-iosched.c | 4 ++--
genhd.c | 2 +-
ll_rw_blk.c | 4 ++--
5 files changed, 9 insertions(+), 9 deletions(-)
Index: dtor/drivers/block/as-iosched.c
===================================================================
--- dtor.orig/drivers/block/as-iosched.c
+++ dtor/drivers/block/as-iosched.c
@@ -2044,7 +2044,7 @@ as_attr_show(struct kobject *kobj, struc
struct as_fs_entry *entry = to_as(attr);
if (!entry->show)
- return 0;
+ return -EIO;
return entry->show(e->elevator_data, page);
}
@@ -2057,7 +2057,7 @@ as_attr_store(struct kobject *kobj, stru
struct as_fs_entry *entry = to_as(attr);
if (!entry->store)
- return -EINVAL;
+ return -EIO;
return entry->store(e->elevator_data, page, length);
}
Index: dtor/drivers/block/cfq-iosched.c
===================================================================
--- dtor.orig/drivers/block/cfq-iosched.c
+++ dtor/drivers/block/cfq-iosched.c
@@ -1772,7 +1772,7 @@ cfq_attr_show(struct kobject *kobj, stru
struct cfq_fs_entry *entry = to_cfq(attr);
if (!entry->show)
- return 0;
+ return -EIO;
return entry->show(e->elevator_data, page);
}
@@ -1785,7 +1785,7 @@ cfq_attr_store(struct kobject *kobj, str
struct cfq_fs_entry *entry = to_cfq(attr);
if (!entry->store)
- return -EINVAL;
+ return -EIO;
return entry->store(e->elevator_data, page, length);
}
Index: dtor/drivers/block/genhd.c
===================================================================
--- dtor.orig/drivers/block/genhd.c
+++ dtor/drivers/block/genhd.c
@@ -321,7 +321,7 @@ static ssize_t disk_attr_show(struct kob
struct gendisk *disk = to_disk(kobj);
struct disk_attribute *disk_attr =
container_of(attr,struct disk_attribute,attr);
- ssize_t ret = 0;
+ ssize_t ret = -EIO;
if (disk_attr->show)
ret = disk_attr->show(disk,page);
Index: dtor/drivers/block/ll_rw_blk.c
===================================================================
--- dtor.orig/drivers/block/ll_rw_blk.c
+++ dtor/drivers/block/ll_rw_blk.c
@@ -3582,7 +3582,7 @@ queue_attr_show(struct kobject *kobj, st
q = container_of(kobj, struct request_queue, kobj);
if (!entry->show)
- return 0;
+ return -EIO;
return entry->show(q, page);
}
@@ -3596,7 +3596,7 @@ queue_attr_store(struct kobject *kobj, s
q = container_of(kobj, struct request_queue, kobj);
if (!entry->store)
- return -EINVAL;
+ return -EIO;
return entry->store(q, page, length);
}
Index: dtor/drivers/block/deadline-iosched.c
===================================================================
--- dtor.orig/drivers/block/deadline-iosched.c
+++ dtor/drivers/block/deadline-iosched.c
@@ -886,7 +886,7 @@ deadline_attr_show(struct kobject *kobj,
struct deadline_fs_entry *entry = to_deadline(attr);
if (!entry->show)
- return 0;
+ return -EIO;
return entry->show(e->elevator_data, page);
}
@@ -899,7 +899,7 @@ deadline_attr_store(struct kobject *kobj
struct deadline_fs_entry *entry = to_deadline(attr);
if (!entry->store)
- return -EINVAL;
+ return -EIO;
return entry->store(e->elevator_data, page, length);
}
sysfs: fix the rest of the kernel so if an attribute doesn't
implement show or store method read/write will return
-EIO instead of 0 or -EINVAL or -EPERM.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/acpi/scan.c | 4 ++--
drivers/cpufreq/cpufreq.c | 4 ++--
drivers/firmware/edd.c | 2 +-
drivers/firmware/efivars.c | 4 ++--
drivers/infiniband/core/sysfs.c | 2 +-
kernel/params.c | 4 ++--
security/seclvl.c | 4 ++--
7 files changed, 12 insertions(+), 12 deletions(-)
Index: dtor/drivers/cpufreq/cpufreq.c
===================================================================
--- dtor.orig/drivers/cpufreq/cpufreq.c
+++ dtor/drivers/cpufreq/cpufreq.c
@@ -521,7 +521,7 @@ static ssize_t show(struct kobject * kob
policy = cpufreq_cpu_get(policy->cpu);
if (!policy)
return -EINVAL;
- ret = fattr->show ? fattr->show(policy,buf) : 0;
+ ret = fattr->show ? fattr->show(policy,buf) : -EIO;
cpufreq_cpu_put(policy);
return ret;
}
@@ -535,7 +535,7 @@ static ssize_t store(struct kobject * ko
policy = cpufreq_cpu_get(policy->cpu);
if (!policy)
return -EINVAL;
- ret = fattr->store ? fattr->store(policy,buf,count) : 0;
+ ret = fattr->store ? fattr->store(policy,buf,count) : -EIO;
cpufreq_cpu_put(policy);
return ret;
}
Index: dtor/drivers/firmware/efivars.c
===================================================================
--- dtor.orig/drivers/firmware/efivars.c
+++ dtor/drivers/firmware/efivars.c
@@ -352,7 +352,7 @@ static ssize_t efivar_attr_show(struct k
{
struct efivar_entry *var = to_efivar_entry(kobj);
struct efivar_attribute *efivar_attr = to_efivar_attr(attr);
- ssize_t ret = 0;
+ ssize_t ret = -EIO;
if (!capable(CAP_SYS_ADMIN))
return -EACCES;
@@ -368,7 +368,7 @@ static ssize_t efivar_attr_store(struct
{
struct efivar_entry *var = to_efivar_entry(kobj);
struct efivar_attribute *efivar_attr = to_efivar_attr(attr);
- ssize_t ret = 0;
+ ssize_t ret = -EIO;
if (!capable(CAP_SYS_ADMIN))
return -EACCES;
Index: dtor/kernel/params.c
===================================================================
--- dtor.orig/kernel/params.c
+++ dtor/kernel/params.c
@@ -629,7 +629,7 @@ static ssize_t module_attr_show(struct k
mk = to_module_kobject(kobj);
if (!attribute->show)
- return -EPERM;
+ return -EIO;
if (!try_module_get(mk->mod))
return -ENODEV;
@@ -653,7 +653,7 @@ static ssize_t module_attr_store(struct
mk = to_module_kobject(kobj);
if (!attribute->store)
- return -EPERM;
+ return -EIO;
if (!try_module_get(mk->mod))
return -ENODEV;
Index: dtor/drivers/infiniband/core/sysfs.c
===================================================================
--- dtor.orig/drivers/infiniband/core/sysfs.c
+++ dtor/drivers/infiniband/core/sysfs.c
@@ -71,7 +71,7 @@ static ssize_t port_attr_show(struct kob
struct ib_port *p = container_of(kobj, struct ib_port, kobj);
if (!port_attr->show)
- return 0;
+ return -EIO;
return port_attr->show(p, port_attr, buf);
}
Index: dtor/security/seclvl.c
===================================================================
--- dtor.orig/security/seclvl.c
+++ dtor/security/seclvl.c
@@ -155,7 +155,7 @@ seclvl_attr_store(struct kobject *kobj,
struct seclvl_obj *obj = container_of(kobj, struct seclvl_obj, kobj);
struct seclvl_attribute *attribute =
container_of(attr, struct seclvl_attribute, attr);
- return (attribute->store ? attribute->store(obj, buf, len) : 0);
+ return attribute->store ? attribute->store(obj, buf, len) : -EIO;
}
static ssize_t
@@ -164,7 +164,7 @@ seclvl_attr_show(struct kobject *kobj, s
struct seclvl_obj *obj = container_of(kobj, struct seclvl_obj, kobj);
struct seclvl_attribute *attribute =
container_of(attr, struct seclvl_attribute, attr);
- return (attribute->show ? attribute->show(obj, buf) : 0);
+ return attribute->show ? attribute->show(obj, buf) : -EIO;
}
/**
Index: dtor/drivers/acpi/scan.c
===================================================================
--- dtor.orig/drivers/acpi/scan.c
+++ dtor/drivers/acpi/scan.c
@@ -65,14 +65,14 @@ static ssize_t acpi_device_attr_show(str
{
struct acpi_device *device = to_acpi_device(kobj);
struct acpi_device_attribute *attribute = to_handle_attr(attr);
- return attribute->show ? attribute->show(device, buf) : 0;
+ return attribute->show ? attribute->show(device, buf) : -EIO;
}
static ssize_t acpi_device_attr_store(struct kobject *kobj,
struct attribute *attr, const char *buf, size_t len)
{
struct acpi_device *device = to_acpi_device(kobj);
struct acpi_device_attribute *attribute = to_handle_attr(attr);
- return attribute->store ? attribute->store(device, buf, len) : len;
+ return attribute->store ? attribute->store(device, buf, len) : -EIO;
}
static struct sysfs_ops acpi_device_sysfs_ops = {
Index: dtor/drivers/firmware/edd.c
===================================================================
--- dtor.orig/drivers/firmware/edd.c
+++ dtor/drivers/firmware/edd.c
@@ -115,7 +115,7 @@ edd_attr_show(struct kobject * kobj, str
{
struct edd_device *dev = to_edd_device(kobj);
struct edd_attribute *edd_attr = to_edd_attr(attr);
- ssize_t ret = 0;
+ ssize_t ret = -EIO;
if (edd_attr->show)
ret = edd_attr->show(dev, buf);