[PATCH] FILESYSTEMS: Delete unused "int dummy[5]" from inodes_stat_t.

[PATCH] FILESYSTEMS: Delete unused "int dummy[5]" from inodes_stat_t.

[Robert P. J. Day]

Delete the apparently unused array "int dummy[5]" from struct
inodes_stat_t.

Signed-off-by: Robert P. J. Day <rpjday@mindspring.com>

---

  no idea what that array is for, but no one seems to care about it.
removal compile-tested on x86 with "make allyesconfig" and nobody
misses it (unless it's used for padding of some kind).


diff --git a/include/linux/fs.h b/include/linux/fs.h
index 7cf0c54..dec83dd 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -44,7 +44,6 @@ extern int get_max_files(void);
 struct inodes_stat_t {
 	int nr_inodes;
 	int nr_unused;
-	int dummy[5];
 };
 extern struct inodes_stat_t inodes_stat;

-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================

Re: [PATCH] FILESYSTEMS: Delete unused "int dummy[5]" from inodes_stat_t.

[Andrew Morton]

On Tue, 29 May 2007 13:11:07 -0400 (EDT)
"Robert P. J. Day" <rpjday@mindspring.com> wrote:

> 
> Delete the apparently unused array "int dummy[5]" from struct
> inodes_stat_t.
> 
> Signed-off-by: Robert P. J. Day <rpjday@mindspring.com>
> 
> ---
> 
>   no idea what that array is for, but no one seems to care about it.
> removal compile-tested on x86 with "make allyesconfig" and nobody
> misses it (unless it's used for padding of some kind).
> 
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 7cf0c54..dec83dd 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -44,7 +44,6 @@ extern int get_max_files(void);
>  struct inodes_stat_t {
>  	int nr_inodes;
>  	int nr_unused;
> -	int dummy[5];
>  };
>  extern struct inodes_stat_t inodes_stat;
> 

kernel/sysctl.c:

	{
		.ctl_name	= FS_STATINODE,
		.procname	= "inode-state",
		.data		= &inodes_stat,
		.maxlen		= 7*sizeof(int),
		.mode		= 0444,
		.proc_handler	= &proc_dointvec,
	},

akpm:/home/akpm> cat /proc/sys/fs/inode-state 
608039  178454  0       0       0       0       0

So it _is_ used: to present those five zeroes.  I think this is for
back-compatibility with some cretaceous-era kernel.

Re: [PATCH] FILESYSTEMS: Delete unused "int dummy[5]" from inodes_stat_t.

[Robert P. J. Day]

On Tue, 29 May 2007, Andrew Morton wrote:

> On Tue, 29 May 2007 13:11:07 -0400 (EDT)
> "Robert P. J. Day" <rpjday@mindspring.com> wrote:
>
> >
> > Delete the apparently unused array "int dummy[5]" from struct
> > inodes_stat_t.
> >
> > Signed-off-by: Robert P. J. Day <rpjday@mindspring.com>
> >
> > ---
> >
> >   no idea what that array is for, but no one seems to care about it.
> > removal compile-tested on x86 with "make allyesconfig" and nobody
> > misses it (unless it's used for padding of some kind).
> >
> >
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 7cf0c54..dec83dd 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -44,7 +44,6 @@ extern int get_max_files(void);
> >  struct inodes_stat_t {
> >  	int nr_inodes;
> >  	int nr_unused;
> > -	int dummy[5];
> >  };
> >  extern struct inodes_stat_t inodes_stat;
> >
>
> kernel/sysctl.c:
>
> 	{
> 		.ctl_name	= FS_STATINODE,
> 		.procname	= "inode-state",
> 		.data		= &inodes_stat,
> 		.maxlen		= 7*sizeof(int),
> 		.mode		= 0444,
> 		.proc_handler	= &proc_dointvec,
> 	},
>
> akpm:/home/akpm> cat /proc/sys/fs/inode-state
> 608039  178454  0       0       0       0       0
>
> So it _is_ used: to present those five zeroes.  I think this is for
> back-compatibility with some cretaceous-era kernel.

ah, gotcha.  well, i'll leave this up to someone else to do anything
with if they are so inclined.

rday
-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================

Re: [PATCH] FILESYSTEMS: Delete unused "int dummy[5]" from inodes_stat_t.

[Randy Dunlap]

On Tue, 29 May 2007 14:07:01 -0400 (EDT) Robert P. J. Day wrote:

> On Tue, 29 May 2007, Andrew Morton wrote:
> 
> > On Tue, 29 May 2007 13:11:07 -0400 (EDT)
> > "Robert P. J. Day" <rpjday@mindspring.com> wrote:
> >
> > >
> > > Delete the apparently unused array "int dummy[5]" from struct
> > > inodes_stat_t.
> > >
> > > Signed-off-by: Robert P. J. Day <rpjday@mindspring.com>
> > >
> > > ---
> > >
> > >   no idea what that array is for, but no one seems to care about it.
> > > removal compile-tested on x86 with "make allyesconfig" and nobody
> > > misses it (unless it's used for padding of some kind).
> > >
> > >
> > > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > > index 7cf0c54..dec83dd 100644
> > > --- a/include/linux/fs.h
> > > +++ b/include/linux/fs.h
> > > @@ -44,7 +44,6 @@ extern int get_max_files(void);
> > >  struct inodes_stat_t {
> > >  	int nr_inodes;
> > >  	int nr_unused;
> > > -	int dummy[5];
> > >  };
> > >  extern struct inodes_stat_t inodes_stat;
> > >
> >
> > kernel/sysctl.c:
> >
> > 	{
> > 		.ctl_name	= FS_STATINODE,
> > 		.procname	= "inode-state",
> > 		.data		= &inodes_stat,
> > 		.maxlen		= 7*sizeof(int),
> > 		.mode		= 0444,
> > 		.proc_handler	= &proc_dointvec,
> > 	},
> >
> > akpm:/home/akpm> cat /proc/sys/fs/inode-state
> > 608039  178454  0       0       0       0       0
> >
> > So it _is_ used: to present those five zeroes.  I think this is for
> > back-compatibility with some cretaceous-era kernel.
> 
> ah, gotcha.  well, i'll leave this up to someone else to do anything
> with if they are so inclined.

There's little to be done, except possibly put a /* comment */
on the struct's dummy line so that we don't go thru this again
in N years.


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH] FILESYSTEMS: Delete unused "int dummy[5]" from inodes_stat_t.

[Robert P. J. Day]

On Tue, 29 May 2007, Randy Dunlap wrote:

> On Tue, 29 May 2007 14:07:01 -0400 (EDT) Robert P. J. Day wrote:
>
> > On Tue, 29 May 2007, Andrew Morton wrote:
> >
> > > kernel/sysctl.c:
> > >
> > > 	{
> > > 		.ctl_name	= FS_STATINODE,
> > > 		.procname	= "inode-state",
> > > 		.data		= &inodes_stat,
> > > 		.maxlen		= 7*sizeof(int),    <-----
> > > 		.mode		= 0444,
> > > 		.proc_handler	= &proc_dointvec,
> > > 	},
> > >
> > > akpm:/home/akpm> cat /proc/sys/fs/inode-state
> > > 608039  178454  0       0       0       0       0
> > >
> > > So it _is_ used: to present those five zeroes.  I think this is
> > > for back-compatibility with some cretaceous-era kernel.
> >
> > ah, gotcha.  well, i'll leave this up to someone else to do
> > anything with if they are so inclined.
>
> There's little to be done, except possibly put a /* comment */ on
> the struct's dummy line so that we don't go thru this again in N
> years.

  so, just to clarify, what *is* the value of those trailing five
zeroes?  andrew suggests it's to be backward-compatible with an old
kernel, which doesn't make much sense to me.  it would make more sense
to say that that's backward-compatible with some old userspace app
that always wants to see seven values and just ignores the last five.

  in any event, from Documentation/filesystems/proc.txt:

"inode-state contains two actual numbers and five dummy values. The
numbers are nr_inodes and nr_free_inodes (in order of appearance)."

if even the documentation calls them dummy values, do they really have
any residual value at this point?  and on that note, i'll stop harping
on this and move on.

rday
-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================

Re: [PATCH] FILESYSTEMS: Delete unused "int dummy[5]" from inodes_stat_t.

[Randy Dunlap]

Robert P. J. Day wrote:
> On Tue, 29 May 2007, Randy Dunlap wrote:
> 
>> On Tue, 29 May 2007 14:07:01 -0400 (EDT) Robert P. J. Day wrote:
>>
>>> On Tue, 29 May 2007, Andrew Morton wrote:
>>>
>>>> kernel/sysctl.c:
>>>>
>>>> 	{
>>>> 		.ctl_name	= FS_STATINODE,
>>>> 		.procname	= "inode-state",
>>>> 		.data		= &inodes_stat,
>>>> 		.maxlen		= 7*sizeof(int),    <-----
>>>> 		.mode		= 0444,
>>>> 		.proc_handler	= &proc_dointvec,
>>>> 	},
>>>>
>>>> akpm:/home/akpm> cat /proc/sys/fs/inode-state
>>>> 608039  178454  0       0       0       0       0
>>>>
>>>> So it _is_ used: to present those five zeroes.  I think this is
>>>> for back-compatibility with some cretaceous-era kernel.
>>> ah, gotcha.  well, i'll leave this up to someone else to do
>>> anything with if they are so inclined.
>> There's little to be done, except possibly put a /* comment */ on
>> the struct's dummy line so that we don't go thru this again in N
>> years.
> 
>   so, just to clarify, what *is* the value of those trailing five
> zeroes?  andrew suggests it's to be backward-compatible with an old
> kernel, which doesn't make much sense to me.  it would make more sense
> to say that that's backward-compatible with some old userspace app
> that always wants to see seven values and just ignores the last five.

Agreed, it's for compat with some (unknown) userspace app that reads
/proc/sys/fs/inode-state and scans for 7 (or more than 2) numbers there.
The mantra is "don't break userspace," so we leave the numbers there...

>   in any event, from Documentation/filesystems/proc.txt:
> 
> "inode-state contains two actual numbers and five dummy values. The
> numbers are nr_inodes and nr_free_inodes (in order of appearance)."
> 
> if even the documentation calls them dummy values, do they really have
> any residual value at this point?  and on that note, i'll stop harping
> on this and move on.
> 
> rday


-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH] FILESYSTEMS: Delete unused "int dummy[5]" from inodes_stat_t.

[Josh Boyer]

On 5/29/07, Randy Dunlap <randy.dunlap@oracle.com> wrote:
> Robert P. J. Day wrote:
> > On Tue, 29 May 2007, Randy Dunlap wrote:
> >
> >> On Tue, 29 May 2007 14:07:01 -0400 (EDT) Robert P. J. Day wrote:
> >>
> >>> On Tue, 29 May 2007, Andrew Morton wrote:
> >>>
> >>>> kernel/sysctl.c:
> >>>>
> >>>>    {
> >>>>            .ctl_name       = FS_STATINODE,
> >>>>            .procname       = "inode-state",
> >>>>            .data           = &inodes_stat,
> >>>>            .maxlen         = 7*sizeof(int),    <-----
> >>>>            .mode           = 0444,
> >>>>            .proc_handler   = &proc_dointvec,
> >>>>    },
> >>>>
> >>>> akpm:/home/akpm> cat /proc/sys/fs/inode-state
> >>>> 608039  178454  0       0       0       0       0
> >>>>
> >>>> So it _is_ used: to present those five zeroes.  I think this is
> >>>> for back-compatibility with some cretaceous-era kernel.
> >>> ah, gotcha.  well, i'll leave this up to someone else to do
> >>> anything with if they are so inclined.
> >> There's little to be done, except possibly put a /* comment */ on
> >> the struct's dummy line so that we don't go thru this again in N
> >> years.
> >
> >   so, just to clarify, what *is* the value of those trailing five
> > zeroes?  andrew suggests it's to be backward-compatible with an old
> > kernel, which doesn't make much sense to me.  it would make more sense
> > to say that that's backward-compatible with some old userspace app
> > that always wants to see seven values and just ignores the last five.
>
> Agreed, it's for compat with some (unknown) userspace app that reads
> /proc/sys/fs/inode-state and scans for 7 (or more than 2) numbers there.
> The mantra is "don't break userspace," so we leave the numbers there...

Couldn't you remove the dummy member and just have the proc entry
print out 5 dummy zeros?

josh

Re: [PATCH] FILESYSTEMS: Delete unused "int dummy[5]" from inodes_stat_t.

[Randy Dunlap]

On Tue, 29 May 2007 13:44:42 -0500 Josh Boyer wrote:

> On 5/29/07, Randy Dunlap <randy.dunlap@oracle.com> wrote:
> > Robert P. J. Day wrote:
> > > On Tue, 29 May 2007, Randy Dunlap wrote:
> > >
> > >> On Tue, 29 May 2007 14:07:01 -0400 (EDT) Robert P. J. Day wrote:
> > >>
> > >>> On Tue, 29 May 2007, Andrew Morton wrote:
> > >>>
> > >>>> kernel/sysctl.c:
> > >>>>
> > >>>>    {
> > >>>>            .ctl_name       = FS_STATINODE,
> > >>>>            .procname       = "inode-state",
> > >>>>            .data           = &inodes_stat,
> > >>>>            .maxlen         = 7*sizeof(int),    <-----
> > >>>>            .mode           = 0444,
> > >>>>            .proc_handler   = &proc_dointvec,
> > >>>>    },
> > >>>>
> > >>>> akpm:/home/akpm> cat /proc/sys/fs/inode-state
> > >>>> 608039  178454  0       0       0       0       0
> > >>>>
> > >>>> So it _is_ used: to present those five zeroes.  I think this is
> > >>>> for back-compatibility with some cretaceous-era kernel.
> > >>> ah, gotcha.  well, i'll leave this up to someone else to do
> > >>> anything with if they are so inclined.
> > >> There's little to be done, except possibly put a /* comment */ on
> > >> the struct's dummy line so that we don't go thru this again in N
> > >> years.
> > >
> > >   so, just to clarify, what *is* the value of those trailing five
> > > zeroes?  andrew suggests it's to be backward-compatible with an old
> > > kernel, which doesn't make much sense to me.  it would make more sense
> > > to say that that's backward-compatible with some old userspace app
> > > that always wants to see seven values and just ignores the last five.
> >
> > Agreed, it's for compat with some (unknown) userspace app that reads
> > /proc/sys/fs/inode-state and scans for 7 (or more than 2) numbers there.
> > The mantra is "don't break userspace," so we leave the numbers there...
> 
> Couldn't you remove the dummy member and just have the proc entry
> print out 5 dummy zeros?

In theory someone could do that, but it would (a) require a comment
so that someone else didn't try to remove it 5 years from now and
(b) require more proc/sysctl code just for that one sysctl entry
since the sysctl entries are all table-driven right now and printing
5 zeros would require a new /proc/sys handler for just this one
sysctl.


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH] FILESYSTEMS: Delete unused "int dummy[5]" from inodes_stat_t.

[Stefan Richter]

From: Stefan Richter <stefanr@s5r6.in-berlin.de>
Subject: fs: clarify "dummy" member in struct inodes_stat_t

Found by Robert P. J. Day:  The role of inodes_stat_t.dummy wasn't clear
and one might be tempted to remove it.  Give it a better name and add a
comment.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
Only quickly compile-tested.

Index: linux/include/linux/fs.h
===================================================================
--- linux.orig/include/linux/fs.h
+++ linux/include/linux/fs.h
@@ -44,7 +44,7 @@ extern int get_max_files(void);
 struct inodes_stat_t {
 	int nr_inodes;
 	int nr_unused;
-	int dummy[5];
+	int reserved[5];	/* for sysctl and procfs ABI compatibility */
 };
 extern struct inodes_stat_t inodes_stat;
 

-- 
Stefan Richter
-=====-=-=== -=-= ====-
http://arcgraph.de/sr/

Re: [PATCH] FILESYSTEMS: Delete unused "int dummy[5]" from inodes_stat_t.

[Adrian Bunk]

On Wed, May 30, 2007 at 08:41:56AM +0200, Stefan Richter wrote:
> From: Stefan Richter <stefanr@s5r6.in-berlin.de>
> Subject: fs: clarify "dummy" member in struct inodes_stat_t
> 
> Found by Robert P. J. Day:  The role of inodes_stat_t.dummy wasn't clear
> and one might be tempted to remove it.  Give it a better name and add a
> comment.
> 
> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
> ---
> Only quickly compile-tested.
> 
> Index: linux/include/linux/fs.h
> ===================================================================
> --- linux.orig/include/linux/fs.h
> +++ linux/include/linux/fs.h
> @@ -44,7 +44,7 @@ extern int get_max_files(void);
>  struct inodes_stat_t {
>  	int nr_inodes;
>  	int nr_unused;
> -	int dummy[5];
> +	int reserved[5];	/* for sysctl and procfs ABI compatibility */
>  };
>  extern struct inodes_stat_t inodes_stat;

Considering that we export this struct to usespace, I don't think 
renaming a member without a good reason is a good idea (but adding a 
comment makes sense).

> Stefan Richter

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

Re: [PATCH] FILESYSTEMS: Delete unused "int dummy[5]" from inodes_stat_t.

[Robert P. J. Day]

On Wed, 30 May 2007, Adrian Bunk wrote:

> On Wed, May 30, 2007 at 08:41:56AM +0200, Stefan Richter wrote:
> > From: Stefan Richter <stefanr@s5r6.in-berlin.de>
> > Subject: fs: clarify "dummy" member in struct inodes_stat_t
> >
> > Found by Robert P. J. Day:  The role of inodes_stat_t.dummy wasn't clear
> > and one might be tempted to remove it.  Give it a better name and add a
> > comment.
> >
> > Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
> > ---
> > Only quickly compile-tested.
> >
> > Index: linux/include/linux/fs.h
> > ===================================================================
> > --- linux.orig/include/linux/fs.h
> > +++ linux/include/linux/fs.h
> > @@ -44,7 +44,7 @@ extern int get_max_files(void);
> >  struct inodes_stat_t {
> >  	int nr_inodes;
> >  	int nr_unused;
> > -	int dummy[5];
> > +	int reserved[5];	/* for sysctl and procfs ABI compatibility */
> >  };
> >  extern struct inodes_stat_t inodes_stat;
>
> Considering that we export this struct to usespace, I don't think
> renaming a member without a good reason is a good idea (but adding a
> comment makes sense).

that makes sense but, given that that array is guaranteed to always
contain zeroes and therefore no useful information, it might be
amusing to rename it just to see what userspace apps break because
they're referring to it by name for what should be no good reason.

no, no, just kidding ... that would be cruel.  :-)

rday

p.s.  i apologize for this thread having gotten this involved -- i
thought i was submitting a fairly innocuous patch.  live and learn.

-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================

[trivial PATCH, updated] fs: clarify "dummy" member in struct inodes_stat_t

[Stefan Richter]

On 30 May, Robert P. J. Day wrote:
> On Wed, 30 May 2007, Adrian Bunk wrote:
>> Considering that we export this struct to usespace, I don't think
>> renaming a member without a good reason is a good idea (but adding a
>> comment makes sense).
[...]
> i thought i was submitting a fairly innocuous patch.  live and learn.

I've could have seen that it is defined outside #ifdef __KERNEL__.


From: Stefan Richter <stefanr@s5r6.in-berlin.de>
Subject: fs: clarify "dummy" member in struct inodes_stat_t

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
--- linux.orig/include/linux/fs.h
+++ linux/include/linux/fs.h
@@ -44,7 +44,7 @@ extern int get_max_files(void);
 struct inodes_stat_t {
 	int nr_inodes;
 	int nr_unused;
-	int dummy[5];
+	int dummy[5];		/* padding for sysctl ABI compatibility */
 };
 extern struct inodes_stat_t inodes_stat;
 

-- 
Stefan Richter
-=====-=-=== -=-= ====-
http://arcgraph.de/sr/

Re: [trivial PATCH, updated] fs: clarify "dummy" member in struct inodes_stat_t

[Randy Dunlap]

Stefan Richter wrote:
> On 30 May, Robert P. J. Day wrote:
>> On Wed, 30 May 2007, Adrian Bunk wrote:
>>> Considering that we export this struct to usespace, I don't think
>>> renaming a member without a good reason is a good idea (but adding a
>>> comment makes sense).
> [...]
>> i thought i was submitting a fairly innocuous patch.  live and learn.
> 
> I've could have seen that it is defined outside #ifdef __KERNEL__.
> 
> 
> From: Stefan Richter <stefanr@s5r6.in-berlin.de>
> Subject: fs: clarify "dummy" member in struct inodes_stat_t

Acked-by: Randy Dunlap <randy.dunlap@oracle.com>

> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
> --- linux.orig/include/linux/fs.h
> +++ linux/include/linux/fs.h
> @@ -44,7 +44,7 @@ extern int get_max_files(void);
>  struct inodes_stat_t {
>  	int nr_inodes;
>  	int nr_unused;
> -	int dummy[5];
> +	int dummy[5];		/* padding for sysctl ABI compatibility */
>  };
>  extern struct inodes_stat_t inodes_stat;
>  
> 


-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH] FILESYSTEMS: Delete unused "int dummy[5]" from inodes_stat_t.

[Alexey Dobriyan]

On Tue, May 29, 2007 at 01:11:07PM -0400, Robert P. J. Day wrote:
> Delete the apparently unused array "int dummy[5]" from struct
> inodes_stat_t.

>   no idea what that array is for, but no one seems to care about it.
> removal compile-tested on x86 with "make allyesconfig" and nobody
> misses it (unless it's used for padding of some kind).

> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -44,7 +44,6 @@ extern int get_max_files(void);
>  struct inodes_stat_t {
>  	int nr_inodes;
>  	int nr_unused;
> -	int dummy[5];
>  };
>  extern struct inodes_stat_t inodes_stat;

Please...

static ctl_table fs_table[] = {
	{
		.ctl_name	= FS_NRINODE,
		.procname	= "inode-nr",
		.data		= &inodes_stat,
		.maxlen		= 2*sizeof(int),
		.mode		= 0444,
		.proc_handler	= &proc_dointvec,
	},
	{
		.ctl_name	= FS_STATINODE,
		.procname	= "inode-state",
		.data		= &inodes_stat,
		.maxlen		= 7*sizeof(int),
				 ^^^