- fix oops in sysfs_readdir

This is a non-ida backport of Tejun's patch in -mm at:
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc1/2.6.22-rc1-mm1/broken-out/gregkh-driver-sysfs-allocate-inode-number-using-ida.patch for the 2.6.16 -stable tree - it follows the same scheme of using s_ino to safely
store & retrieve the inode number of sysfs entries for use in sysfs_readdir,
but uses a brain-dead-simple inode nr allocator rather than ida, which would
bring along a lot of newer, more complex code.

No, this doesn't guarantee uniqueness of sysfs inode numbers, but then the code in -stable today doesn't either - and with this change, at least it shouldn't oops.

Comments?

Thanks,

-Eric

Signed-off-by Eric Sandeen <[email protected]>

Index: linux-2.6.16.51/fs/sysfs/dir.c
===================================================================

fs/sysfs/dir.c +10 -5

@@ -29,6 +29,14 @@ static struct dentry_operations sysfs_de
 	.d_iput		= sysfs_d_iput,
 };
 
+static unsigned int sysfs_inode_counter;
+ino_t sysfs_get_inum(void)
+{
+	if (unlikely(sysfs_inode_counter < 3))
+		sysfs_inode_counter = 3;
+	return sysfs_inode_counter++;
+}
+
 /*
  * Allocates a new sysfs_dirent and links it to the parent sysfs_dirent
  */

@@ -385,7 +393,7 @@ static int sysfs_readdir(struct file * f
 
 	switch (i) {
 		case 0:
-			ino = dentry->d_inode->i_ino;
+			ino = parent_sd->s_ino;
 			if (filldir(dirent, ".", 1, i, ino, DT_DIR) < 0)
 				break;
 			filp->f_pos++;

@@ -415,10 +423,7 @@ static int sysfs_readdir(struct file * f
 
 				name = sysfs_get_name(next);
 				len = strlen(name);
-				if (next->s_dentry)
-					ino = next->s_dentry->d_inode->i_ino;
-				else
-					ino = iunique(sysfs_sb, 2);
+				ino = next->s_ino;
 
 				if (filldir(dirent, name, len, filp->f_pos, ino,
 						 dt_type(next)) < 0)

Index: linux-2.6.16.51/fs/sysfs/inode.c
===================================================================

fs/sysfs/inode.c +1 -0

@@ -119,6 +119,7 @@ struct inode * sysfs_new_inode(mode_t mo
 		inode->i_mapping->a_ops = &sysfs_aops;
 		inode->i_mapping->backing_dev_info = &sysfs_backing_dev_info;
 		inode->i_op = &sysfs_inode_operations;
+		inode->i_ino = sd->s_ino;
 
 		if (sd->s_iattr) {
 			/* sysfs_dirent has non-default attributes

Index: linux-2.6.16.51/fs/sysfs/mount.c
===================================================================

fs/sysfs/mount.c +1 -0

@@ -29,6 +29,7 @@ static struct sysfs_dirent sysfs_root = 
 	.s_element	= NULL,
 	.s_type		= SYSFS_ROOT,
 	.s_iattr	= NULL,
+	.s_ino		= 1,
 };
 
 static int sysfs_fill_super(struct super_block *sb, void *data, int silent)

Index: linux-2.6.16.51/include/linux/sysfs.h
===================================================================

include/linux/sysfs.h +1 -0

@@ -72,6 +72,7 @@ struct sysfs_dirent {
 	void 			* s_element;
 	int			s_type;
 	umode_t			s_mode;
+	ino_t			s_ino;
 	struct dentry		* s_dentry;
 	struct iattr		* s_iattr;
 };

This is a non-ida backport of Tejun's patch in -mm at:
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc1/2.6.22-rc1-mm1/broken-out/gregkh-driver-sysfs-allocate-inode-number-using-ida.patch for the 2.6.16 -stable tree - it follows the same scheme of using s_ino to safely
store & retrieve the inode number of sysfs entries for use in sysfs_readdir,
but uses a brain-dead-simple inode nr allocator rather than ida, which would
bring along a lot of newer, more complex code.

No, this doesn't guarantee uniqueness of sysfs inode numbers, but then the code in -stable today doesn't either - and with this change, at least it shouldn't oops.

Comments?

Thanks,

-Eric

This is a non-ida backport of Tejun's patch in -mm at:
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc1/2.6.22-rc1-mm1/broken-out/gregkh-driver-sysfs-allocate-inode-number-using-ida.patch for the 2.6.16 -stable tree - it follows the same scheme of using s_ino to safely
store & retrieve the inode number of sysfs entries for use in sysfs_readdir,
but uses a brain-dead-simple inode nr allocator rather than ida, which would
bring along a lot of newer, more complex code.

No, this doesn't guarantee uniqueness of sysfs inode numbers, but then the code in -stable today doesn't either - and with this change, at least it shouldn't oops.

Quoted 10 lines · On Mon, 21 May 2007 13:11:21 -0500 Eric Sandeen <[email protected]> wrote:

This is a non-ida backport of Tejun's patch in -mm at:
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc1/2.6.22-rc1-mm1/broken-out/gregkh-driver-sysfs-allocate-inode-number-using-ida.patch for the 2.6.16 -stable tree - it follows the same scheme of using s_ino to safely
store & retrieve the inode number of sysfs entries for use in sysfs_readdir,
but uses a brain-dead-simple inode nr allocator rather than ida, which would
bring along a lot of newer, more complex code.

No, this doesn't guarantee uniqueness of sysfs inode numbers, but then the code in -stable today doesn't either - and with this change, at least it shouldn't oops.

So I'm sitting here whether to commend this patch to google kernel maintainers for 2.6.18 backport, but I realise I don't know what it does. And I don't know if it fixes the reclaim-time oopses they were intermittently seeing, or if it fixes something else and if so what that is.

Sigh. Better changelogs, please.

Quoted 19 lines · Andrew Morton

Quoted 10 lines · On Mon, 21 May 2007 13:11:21 -0500 Eric Sandeen <[email protected]> wrote:

This is a non-ida backport of Tejun's patch in -mm at:
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc1/2.6.22-rc1-mm1/broken-out/gregkh-driver-sysfs-allocate-inode-number-using-ida.patch for the 2.6.16 -stable tree - it follows the same scheme of using s_ino to safely
store & retrieve the inode number of sysfs entries for use in sysfs_readdir,
but uses a brain-dead-simple inode nr allocator rather than ida, which would
bring along a lot of newer, more complex code.

No, this doesn't guarantee uniqueness of sysfs inode numbers, but then the code in -stable today doesn't either - and with this change, at least it shouldn't oops.

So I'm sitting here whether to commend this patch to google kernel maintainers for 2.6.18 backport, but I realise I don't know what it does. And I don't know if it fixes the reclaim-time oopses they were intermittently seeing, or if it fixes something else and if so what that is.

Quoted 2 lines

Sigh. Better changelogs, please.

Sorry Andrew. I referenced Tejun's upstream patch in -mm which has a nice changelog etc, and this is a backport of that, and does the same thing in the same way and solves the same problem - but that doesn't help if you just want to toss this message into your patch stack. Will fix up & resend.

Actually, someone (eg distros) looking at Tejun's changelog would still be struggling to answer the question "do I need this". The one thing it claims to fix is "duplicate inode numbers". But why is that a problem? What are the user-visible consequences of not merging the patch? Unobvious.

This is a non-ida backport of Tejun's patch in -mm at:
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc1/2.6.22-rc1-mm1/broken-out/gregkh-driver-sysfs-allocate-inode-number-using-ida.patch for the 2.6.16 -stable tree - it follows the same scheme of using s_ino to safely
store & retrieve the inode number of sysfs entries for use in sysfs_readdir,
but uses a brain-dead-simple inode nr allocator rather than ida, which would
bring along a lot of newer, more complex code.

No, this doesn't guarantee uniqueness of sysfs inode numbers, but then the code in -stable today doesn't either - and with this change, at least it shouldn't oops.

Comments?

Thanks,

-Eric
...

From [email protected] Mon May 21 19:34:01 2007

From: Eric Sandeen <[email protected]>
Subject: sysfs: store sysfs inode nrs in s_ino to avoid readdir oopses

Backport of ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc1/2.6.22-rc1-mm1/broken-out/gregkh-driver-sysfs-allocate-inode-number-using-ida.patch

Quoted 6 lines · * [email protected] ([email protected]) wrote:

From: Eric Sandeen <[email protected]>
Subject: sysfs: store sysfs inode nrs in s_ino to avoid readdir oopses

Backport of ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc1/2.6.22-rc1-mm1/broken-out/gregkh-driver-sysfs-allocate-inode-number-using-ida.patch

I didn't put this into -stable queue because the ida version is not upstream yet, so I don't think it's appropriate to backport it at this point in time.

thanks,
-chris

Well, my backport of Tejun's patch explicitly doesn't use ida for just that reason...

It uses a simple counter instead (which may give dup inode numbers, but I think we have that today, and at least this shouldn't oops...)

Quoted 11 lines · Chris Wright

Quoted 6 lines · * [email protected] ([email protected]) wrote:

From: Eric Sandeen <[email protected]>
Subject: sysfs: store sysfs inode nrs in s_ino to avoid readdir oopses

Backport of ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc1/2.6.22-rc1-mm1/broken-out/gregkh-driver-sysfs-allocate-inode-number-using-ida.patch

I didn't put this into -stable queue because the ida version is not upstream yet, so I don't think it's appropriate to backport it at this point in time.

Well, my backport of Tejun's patch explicitly doesn't use ida for just that reason...

It uses a simple counter instead (which may give dup inode numbers, but I think we have that today, and at least this shouldn't oops...)