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
===================================================================
--- linux-2.6.16.51.orig/fs/sysfs/dir.c
+++ linux-2.6.16.51/fs/sysfs/dir.c
@@ -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
===================================================================
--- linux-2.6.16.51.orig/fs/sysfs/inode.c
+++ linux-2.6.16.51/fs/sysfs/inode.c
@@ -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
===================================================================
--- linux-2.6.16.51.orig/fs/sysfs/mount.c
+++ linux-2.6.16.51/fs/sysfs/mount.c
@@ -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
===================================================================
--- linux-2.6.16.51.orig/include/linux/sysfs.h
+++ linux-2.6.16.51/include/linux/sysfs.h
@@ -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;
};
[Please Cc: Adrian Bunk (added) for 2.6.16-stable material]
* 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.
>
> Comments?
>
> Thanks,
>
> -Eric
>
> Signed-off-by: Eric Sandeen <[email protected]>
>
> Index: linux-2.6.16.51/fs/sysfs/dir.c
> ===================================================================
> --- linux-2.6.16.51.orig/fs/sysfs/dir.c
> +++ linux-2.6.16.51/fs/sysfs/dir.c
> @@ -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
> ===================================================================
> --- linux-2.6.16.51.orig/fs/sysfs/inode.c
> +++ linux-2.6.16.51/fs/sysfs/inode.c
> @@ -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
> ===================================================================
> --- linux-2.6.16.51.orig/fs/sysfs/mount.c
> +++ linux-2.6.16.51/fs/sysfs/mount.c
> @@ -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
> ===================================================================
> --- linux-2.6.16.51.orig/include/linux/sysfs.h
> +++ linux-2.6.16.51/include/linux/sysfs.h
> @@ -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;
> };
Eric Sandeen 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.
>
> Comments?
>
> Thanks,
>
> -Eric
Here's one that works w/2.6.21... the global should probably be an atomic
or lock-protected, I suppose, but again, if we don't guarantee uniqueness
anyway...?
Signed-off-by: Eric Sandeen <[email protected]>
Index: linux-2.6.21/fs/sysfs/dir.c
===================================================================
--- linux-2.6.21.orig/fs/sysfs/dir.c
+++ linux-2.6.21/fs/sysfs/dir.c
@@ -32,6 +32,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
*/
@@ -511,7 +519,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++;
@@ -540,10 +548,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.21/fs/sysfs/inode.c
===================================================================
--- linux-2.6.21.orig/fs/sysfs/inode.c
+++ linux-2.6.21/fs/sysfs/inode.c
@@ -140,6 +140,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;
lockdep_set_class(&inode->i_mutex, &sysfs_inode_imutex_key);
if (sd->s_iattr) {
Index: linux-2.6.21/fs/sysfs/mount.c
===================================================================
--- linux-2.6.21.orig/fs/sysfs/mount.c
+++ linux-2.6.21/fs/sysfs/mount.c
@@ -33,6 +33,7 @@ static struct sysfs_dirent sysfs_root =
.s_element = NULL,
.s_type = SYSFS_ROOT,
.s_iattr = NULL,
+ .s_ino = 1,
};
static void sysfs_clear_inode(struct inode *inode)
Index: linux-2.6.21/fs/sysfs/sysfs.h
===================================================================
--- linux-2.6.21.orig/fs/sysfs/sysfs.h
+++ linux-2.6.21/fs/sysfs/sysfs.h
@@ -5,6 +5,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;
atomic_t s_event;
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.
Andrew Morton wrote:
> 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.
>
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.
-Eric
-Eric
On Mon, 21 May 2007 19:18:55 -0500 Eric Sandeen <[email protected]> wrote:
> Andrew Morton wrote:
> > 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.
> >
>
> 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.
Andrew Morton wrote:
> 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.
The oops part is explained in #2. sysfs_dirent->s_dentry can go away
anytime and the original code accesses it without any synchronization,
so it can end up dereferencing NULL or access already freed memory.
And, yeah, this is another place where reclaim-related oops occurs.
--
tejun
(2nd try, better(?) changelog, quilt refreshed(!) patch)
--
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
For regular files in sysfs, sysfs_readdir wants to traverse
sysfs_dirent->s_dentry->d_inode->i_ino to get to the inode number.
But, the dentry can be reclaimed under memory pressure, and there
is no synchronization with readdir. This patch follows Tejun's
scheme of allocating and storing an inode number in the new s_ino
member of a sysfs_dirent, when dirents are created, and retrieving
it from there for readdir, so that the pointer chain doesn't have
to be traversed.
Tejun's upstream patch uses a new-ish "ida" allocator which brings along
some extra complexity; this -stable patch has a brain-dead incrementing
counter which does not guarantee uniqueness, but because sysfs doesn't
hash inodes as iunique expects, uniqueness wasn't guaranteed today anyway.
Signed-off-by: Eric Sandeen <[email protected]>
Index: linux-2.6.21/fs/sysfs/dir.c
===================================================================
--- linux-2.6.21.orig/fs/sysfs/dir.c
+++ linux-2.6.21/fs/sysfs/dir.c
@@ -30,6 +30,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
*/
@@ -41,6 +49,7 @@ static struct sysfs_dirent * __sysfs_new
if (!sd)
return NULL;
+ sd->s_ino = sysfs_get_inum();
atomic_set(&sd->s_count, 1);
atomic_set(&sd->s_event, 1);
INIT_LIST_HEAD(&sd->s_children);
@@ -509,7 +518,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++;
@@ -538,10 +547,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.21/fs/sysfs/inode.c
===================================================================
--- linux-2.6.21.orig/fs/sysfs/inode.c
+++ linux-2.6.21/fs/sysfs/inode.c
@@ -140,6 +140,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;
lockdep_set_class(&inode->i_mutex, &sysfs_inode_imutex_key);
if (sd->s_iattr) {
Index: linux-2.6.21/fs/sysfs/mount.c
===================================================================
--- linux-2.6.21.orig/fs/sysfs/mount.c
+++ linux-2.6.21/fs/sysfs/mount.c
@@ -33,6 +33,7 @@ static struct sysfs_dirent sysfs_root =
.s_element = NULL,
.s_type = SYSFS_ROOT,
.s_iattr = NULL,
+ .s_ino = 1,
};
static void sysfs_clear_inode(struct inode *inode)
Index: linux-2.6.21/fs/sysfs/sysfs.h
===================================================================
--- linux-2.6.21.orig/fs/sysfs/sysfs.h
+++ linux-2.6.21/fs/sysfs/sysfs.h
@@ -5,6 +5,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;
atomic_t s_event;
On Mon, May 21, 2007 at 01:11:21PM -0500, Eric Sandeen 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.
>
> Comments?
First of all, thanks for any contributions to 2.6.16.
My biggest problem with this patch is:
It's not yet fixed in Linus' tree - and if it isn't important enough for
being fixed in 2.6.22, it can't be important enough for 2.6.16.
And no matter whether this might include adding ida to 2.6.16, I'd
prefer to apply something as near as possible to whatever gets
into 2.6.22.
> Thanks,
>
> -Eric
>...
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
This is a note to let you know that we have just queued up the patch titled
Subject: sysfs: store sysfs inode nrs in s_ino to avoid readdir oopses
to the 2.6.21-stable tree. Its filename is
sysfs-store-sysfs-inode-nrs-in-s_ino-to-avoid-readdir-oopses.patch
A git repo of this tree can be found at
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
>From [email protected] Mon May 21 19:34:01 2007
From: Eric Sandeen <[email protected]>
Date: Mon, 21 May 2007 21:32:40 -0500
Subject: sysfs: store sysfs inode nrs in s_ino to avoid readdir oopses
To: Eric Sandeen <[email protected]>
Cc: [email protected], Andrew Morton <[email protected]>, Tejun Heo <[email protected]>, Linux Kernel Mailing List <[email protected]>, Maneesh Soni <[email protected]>
Message-ID: <[email protected]>
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
For regular files in sysfs, sysfs_readdir wants to traverse
sysfs_dirent->s_dentry->d_inode->i_ino to get to the inode number.
But, the dentry can be reclaimed under memory pressure, and there
is no synchronization with readdir. This patch follows Tejun's
scheme of allocating and storing an inode number in the new s_ino
member of a sysfs_dirent, when dirents are created, and retrieving
it from there for readdir, so that the pointer chain doesn't have
to be traversed.
Tejun's upstream patch uses a new-ish "ida" allocator which brings along
some extra complexity; this -stable patch has a brain-dead incrementing
counter which does not guarantee uniqueness, but because sysfs doesn't
hash inodes as iunique expects, uniqueness wasn't guaranteed today anyway.
Signed-off-by: Eric Sandeen <[email protected]>
Cc: Tejun Heo <[email protected]>
Cc: Maneesh Soni <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
--- linux-2.6.21.orig/fs/sysfs/dir.c
+++ linux-2.6.21/fs/sysfs/dir.c
@@ -30,6 +30,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
*/
@@ -41,6 +49,7 @@ static struct sysfs_dirent * __sysfs_new
if (!sd)
return NULL;
+ sd->s_ino = sysfs_get_inum();
atomic_set(&sd->s_count, 1);
atomic_set(&sd->s_event, 1);
INIT_LIST_HEAD(&sd->s_children);
@@ -509,7 +518,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++;
@@ -538,10 +547,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.21/fs/sysfs/inode.c
===================================================================
--- linux-2.6.21.orig/fs/sysfs/inode.c
+++ linux-2.6.21/fs/sysfs/inode.c
@@ -140,6 +140,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;
lockdep_set_class(&inode->i_mutex, &sysfs_inode_imutex_key);
if (sd->s_iattr) {
Index: linux-2.6.21/fs/sysfs/mount.c
===================================================================
--- linux-2.6.21.orig/fs/sysfs/mount.c
+++ linux-2.6.21/fs/sysfs/mount.c
@@ -33,6 +33,7 @@ static struct sysfs_dirent sysfs_root =
.s_element = NULL,
.s_type = SYSFS_ROOT,
.s_iattr = NULL,
+ .s_ino = 1,
};
static void sysfs_clear_inode(struct inode *inode)
Index: linux-2.6.21/fs/sysfs/sysfs.h
===================================================================
--- linux-2.6.21.orig/fs/sysfs/sysfs.h
+++ linux-2.6.21/fs/sysfs/sysfs.h
@@ -5,6 +5,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;
atomic_t s_event;
_______________________________________________
stable mailing list
[email protected]
http://linux.kernel.org/mailman/listinfo/stable
Patches currently in stable-queue which might be from [email protected] are
queue-2.6.21/sysfs-store-sysfs-inode-nrs-in-s_ino-to-avoid-readdir-oopses.patch
* [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
Chris Wright wrote:
> * [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...)
*shrug*
-Eric
> thanks,
> -chris
>
* Eric Sandeen ([email protected]) wrote:
> 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...)
Right, my point is the source of the backport is in -mm for maybe as
long as 6 weeks but not in Linus' tree. I s'pose it's chicken and egg
for the ida code. I'd prefer to see your patch upstream, and Tejun's
code becomes the ida code plus conversion of the simple counter to ida
at some later point in time.
On Wed, Jun 06, 2007 at 04:36:22PM -0500, Eric Sandeen wrote:
> Chris Wright wrote:
> > * [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...)
Ok, I'll pull it out as the number of people really seeing this in the
wild is so small that it's not worth it.
thanks,
greg k-h