Skip to contentSkip to conversation

sysfs variable size files, lost dget, ...

PATCH In review Manuel Estrada Sainz 1 participant 2 messages May 21, 2003 – May 22, 2003 Driver core

lore
Share & tools
lore

Manuel Estrada Sainz

author
#0 lore raw

PATCH 1/1 sysfs variable size files, lost dget, ...

Hi,

While working on request_firmware() interface, I found some trouble with sysfs and Greg said that you are the one to send this patches:

- sysfs-bin-flexible-size.diff:
Make dynamically sized files possible. And return the right value on successful write.

- sysfs-bin-lost-dget.diff:
I was having trouble when calling request_firmware() from a work queue, and after a little investigations it seams that this dget got lost along the way. Adding it back fixed the issue. Or am I causing a dentry leak now?

- sysfs-bin-header.diff:
I guess that I am the first user of sysfs's binary interface :-)

Thanks

Manuel

Signature · 6 lines
--- Manuel Estrada Sainz <[email protected]>
                         <[email protected]>
			 <[email protected]>
------------------------ <[email protected]> -------------------
Let us have the serenity to accept the things we cannot change, courage to
change the things we can, and wisdom to know the difference.
fs/sysfs/bin.c +19 -10
diff --exclude=CVS -urN linux-2.5.orig/fs/sysfs/bin.c linux-2.5.mine/fs/sysfs/bin.c
@@ -30,10 +30,15 @@
 	loff_t offs = *off;
 	int ret;
 
-	if (offs > size)
-		return 0;
-	if (offs + count > size)
-		count = size - offs;
+	if (count > PAGE_SIZE)
+		count = PAGE_SIZE;
+
+	if(size){
+		if (offs > size)
+			return 0;
+		if (offs + count > size)
+			count = size - offs;
+	}
 
 	ret = fill_read(dentry, buffer, offs, count);
 	if (ret < 0) 

@@ -69,10 +74,14 @@
 	loff_t offs = *off;
 	int ret;
 
-	if (offs > size)
-		return 0;
-	if (offs + count > size)
-		count = size - offs;
+	if (count > PAGE_SIZE)
+		count = PAGE_SIZE;
+	if (size) {
+		if (offs > size)
+			return 0;
+		if (offs + count > size)
+			count = size - offs;
+	}
 
 	ret = -EFAULT;
 	if (copy_from_user(buffer + offs, userbuf, count))

@@ -81,7 +90,7 @@
 	count = flush_write(dentry, buffer, offs, count);
 	if (count > 0)
 		*off = offs + count;
-	ret = 0;
+	ret = count;
  Done:
 	return ret;
 }

@@ -102,7 +111,7 @@
 		goto Done;
 
 	error = -ENOMEM;
-	file->private_data = kmalloc(attr->size, GFP_KERNEL);
+	file->private_data = kmalloc(PAGE_SIZE, GFP_KERNEL);
 	if (!file->private_data)
 		goto Done;

Index: include/linux/sysfs.h
===================================================================
RCS file: /home/cvs/linux-2.5/include/linux/sysfs.h,v retrieving revision 1.9

linux/sysfs.h +3 -0
diff -u -r1.9 sysfs.h
@@ -23,6 +23,9 @@
 	ssize_t (*write)(struct kobject *, char *, loff_t, size_t);
 };
 
+int sysfs_create_bin_file(struct kobject * kobj, struct bin_attribute * attr);
+int sysfs_remove_bin_file(struct kobject * kobj, struct bin_attribute * attr);
+
 struct sysfs_ops {
 	ssize_t	(*show)(struct kobject *, struct attribute *,char *);
 	ssize_t	(*store)(struct kobject *,struct attribute *,const char *, size_t);
fs/sysfs/inode.c +3 -2
diff --exclude=CVS -urN linux-2.5.orig/fs/sysfs/inode.c linux-2.5.mine/fs/sysfs/inode.c
@@ -60,9 +60,10 @@
  Proceed:
 	if (init)
 		error = init(inode);
-	if (!error)
+	if (!error){
 		d_instantiate(dentry, inode);
-	else
+		dget(dentry); /* Extra count - pin the dentry in core */
+	} else
 		iput(inode);
  Done:
 	return error;
author
#1 lore raw
Quoted 15 lines · Manuel Estrada Sainz

Hi,

While working on request_firmware() interface, I found some trouble with sysfs and Greg said that you are the one to send this patches:

- sysfs-bin-flexible-size.diff:
Make dynamically sized files possible. And return the right value on successful write.

- sysfs-bin-lost-dget.diff:
I was having trouble when calling request_firmware() from a work queue, and after a little investigations it seams that this dget got lost along the way. Adding it back fixed the issue. Or am I causing a dentry leak now?

Quoted 6 lines

- sysfs-bin-header.diff:
I guess that I am the first user of sysfs's binary interface :-)

Thanks

Manuel

You can ignore those patches, the dynamic size stuff would fail badly when handling more than PAGE_SIZE data :-(

Updated patches attached.

Thanks

Manuel

Signature · 6 lines
--- Manuel Estrada Sainz <[email protected]>
                         <[email protected]>
			 <[email protected]>
------------------------ <[email protected]> -------------------
Let us have the serenity to accept the things we cannot change, courage to
change the things we can, and wisdom to know the difference.
fs/sysfs/bin.c +21 -12
diff --exclude=CVS -urN linux-2.5.orig/fs/sysfs/bin.c linux-2.5.mine/fs/sysfs/bin.c
@@ -30,10 +30,15 @@
 	loff_t offs = *off;
 	int ret;
 
-	if (offs > size)
-		return 0;
-	if (offs + count > size)
-		count = size - offs;
+	if (count > PAGE_SIZE)
+		count = PAGE_SIZE;
+
+	if (size) {
+		if (offs > size)
+			return 0;
+		if (offs + count > size)
+			count = size - offs;
+	}
 
 	ret = fill_read(dentry, buffer, offs, count);
 	if (ret < 0) 

@@ -41,7 +46,7 @@
 	count = ret;
 
 	ret = -EFAULT;
-	if (copy_to_user(userbuf, buffer + offs, count) != 0)
+	if (copy_to_user(userbuf, buffer, count) != 0)
 		goto Done;
 
 	*off = offs + count;

@@ -69,19 +74,23 @@
 	loff_t offs = *off;
 	int ret;
 
-	if (offs > size)
-		return 0;
-	if (offs + count > size)
-		count = size - offs;
+	if (count > PAGE_SIZE)
+		count = PAGE_SIZE;
+	if (size) {
+		if (offs > size)
+			return 0;
+		if (offs + count > size)
+			count = size - offs;
+	}
 
 	ret = -EFAULT;
-	if (copy_from_user(buffer + offs, userbuf, count))
+	if (copy_from_user(buffer, userbuf, count))
 		goto Done;
 
 	count = flush_write(dentry, buffer, offs, count);
 	if (count > 0)
 		*off = offs + count;
-	ret = 0;
+	ret = count;
  Done:
 	return ret;
 }

@@ -102,7 +111,7 @@
 		goto Done;
 
 	error = -ENOMEM;
-	file->private_data = kmalloc(attr->size, GFP_KERNEL);
+	file->private_data = kmalloc(PAGE_SIZE, GFP_KERNEL);
 	if (!file->private_data)
 		goto Done;
include/linux/sysfs.h +3 -0
diff --exclude=CVS -urN linux-2.5.orig/include/linux/sysfs.h linux-2.5.mine/include/linux/sysfs.h
@@ -23,6 +23,9 @@
 	ssize_t (*write)(struct kobject *, char *, loff_t, size_t);
 };
 
+int sysfs_create_bin_file(struct kobject * kobj, struct bin_attribute * attr);
+int sysfs_remove_bin_file(struct kobject * kobj, struct bin_attribute * attr);
+
 struct sysfs_ops {
 	ssize_t	(*show)(struct kobject *, struct attribute *,char *);
 	ssize_t	(*store)(struct kobject *,struct attribute *,const char *, size_t);
fs/sysfs/inode.c +3 -2
diff --exclude=CVS -urN linux-2.5.orig/fs/sysfs/inode.c linux-2.5.mine/fs/sysfs/inode.c
@@ -60,9 +60,10 @@
  Proceed:
 	if (init)
 		error = init(inode);
-	if (!error)
+	if (!error) {
 		d_instantiate(dentry, inode);
-	else
+		dget(dentry); /* Extra count - pin the dentry in core */
+	} else
 		iput(inode);
  Done:
 	return error;