It's time to hack on dentry_stat?

It's time to hack on dentry_stat?

[rae l]

I have noticed that struct dentry_stat_t dentry_stat is a struct with
six fields,
but only 2 of them are used, other 4 have never been used since v2.6.11,

from include/linux/dcache.h:

struct dentry_stat_t {
        int nr_dentry;
        int nr_unused;
        int age_limit;          /* age in seconds */
        int want_pages;         /* pages requested by system */
        int dummy[2];
};
extern struct dentry_stat_t dentry_stat;

(I don't know things before v2.6.11, my local linux-2.6.git repository
can only grep to v2.6.11)

$ PAGER= git grep -w -e dentry_stat
Documentation/sysctl/fs.txt:} dentry_stat = {0, 0, 45, 0,};
fs/dcache.c:struct dentry_stat_t dentry_stat = {
fs/dcache.c: * no dcache_lock, please.  The caller must decrement
dentry_stat.nr_dentry
fs/dcache.c:            dentry_stat.nr_unused--;
fs/dcache.c:    dentry_stat.nr_dentry--;        /* For d_free, below */
fs/dcache.c:            dentry_stat.nr_unused++;
fs/dcache.c:            dentry_stat.nr_unused--;
fs/dcache.c:                    dentry_stat.nr_unused++;
fs/dcache.c:            dentry_stat.nr_unused++;
fs/dcache.c:            dentry_stat.nr_unused--;
fs/dcache.c:    dentry_stat.nr_dentry -= detached;
fs/dcache.c:                    dentry_stat.nr_unused++;
fs/dcache.c:    return (dentry_stat.nr_unused / 100) *
sysctl_vfs_cache_pressure;
fs/dcache.c:    dentry_stat.nr_dentry++;
include/linux/dcache.h:extern struct dentry_stat_t dentry_stat;
kernel/sysctl.c:                .data           = &dentry_stat,

it's the same as that of v2.6.11:

$ PAGER= git grep -w -e dentry_stat v2.6.11 --
v2.6.11:Documentation/sysctl/fs.txt:} dentry_stat = {0, 0, 45, 0,};
v2.6.11:fs/dcache.c:struct dentry_stat_t dentry_stat = {
v2.6.11:fs/dcache.c: * no dcache_lock, please.  The caller must
decrement dentry_stat.nr_dentry
v2.6.11:fs/dcache.c:            dentry_stat.nr_unused++;
v2.6.11:fs/dcache.c:                    dentry_stat.nr_unused--;
v2.6.11:fs/dcache.c:            dentry_stat.nr_dentry--;        /* For
d_free, below */
v2.6.11:fs/dcache.c:            dentry_stat.nr_unused--;
v2.6.11:fs/dcache.c:    dentry_stat.nr_dentry--;        /* For d_free, below */
v2.6.11:fs/dcache.c:            dentry_stat.nr_unused--;
v2.6.11:fs/dcache.c:                    dentry_stat.nr_unused++;
v2.6.11:fs/dcache.c:            dentry_stat.nr_unused--;
v2.6.11:fs/dcache.c:                    dentry_stat.nr_unused--;
v2.6.11:fs/dcache.c:                    dentry_stat.nr_unused++;
v2.6.11:fs/dcache.c:                            dentry_stat.nr_unused--;
v2.6.11:fs/dcache.c:                            dentry_stat.nr_unused++;
v2.6.11:fs/dcache.c:    return (dentry_stat.nr_unused / 100) *
sysctl_vfs_cache_pressure;
v2.6.11:fs/dcache.c:    dentry_stat.nr_dentry++;
v2.6.11:include/linux/dcache.h:extern struct dentry_stat_t dentry_stat;
v2.6.11:kernel/sysctl.c:                .data           = &dentry_stat,

So I think that struct is outdated and can be simplified.

--
Cheng

Re: It's time to hack on dentry_stat?

[Matthew Wilcox]

On Wed, May 07, 2008 at 10:26:58PM +0800, rae l wrote:
> I have noticed that struct dentry_stat_t dentry_stat is a struct with
> six fields,
> but only 2 of them are used, other 4 have never been used since v2.6.11,
> 
> from include/linux/dcache.h:
> 
> struct dentry_stat_t {
>         int nr_dentry;
>         int nr_unused;
>         int age_limit;          /* age in seconds */
>         int want_pages;         /* pages requested by system */
>         int dummy[2];
> };

You have to not break:

$ cat /proc/sys/fs/dentry-state 
122104  115846  45      0       0       0

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: It's time to hack on dentry_stat?

[rae l]

On Wed, May 7, 2008 at 10:51 PM, Matthew Wilcox <matthew@wil.cx> wrote:
>  You have to not break:
>
>  $ cat /proc/sys/fs/dentry-state
>  122104  115846  45      0       0       0
But why? for some applicatioins?

The struct dentry_stat can be modified to have only two useful fields,
and let the other four printed with 0,
I have greped age_limit, that's also not used anymore, why we should
let it keep 45? Or Is there some historical reason?

Or just consider /proc/sys/fs/dentry-state as a binary application interface?

Re: It's time to hack on dentry_stat?

[Ray Lee]

On Wed, May 7, 2008 at 7:51 AM, Matthew Wilcox <matthew@wil.cx> wrote:
> On Wed, May 07, 2008 at 10:26:58PM +0800, rae l wrote:
>  > I have noticed that struct dentry_stat_t dentry_stat is a struct with
>  > six fields,
>  > but only 2 of them are used, other 4 have never been used since v2.6.11,
>  >
>  > from include/linux/dcache.h:
>  >
>  > struct dentry_stat_t {
>  >         int nr_dentry;
>  >         int nr_unused;
>  >         int age_limit;          /* age in seconds */
>  >         int want_pages;         /* pages requested by system */
>  >         int dummy[2];
>  > };
>
>  You have to not break:
>
>  $ cat /proc/sys/fs/dentry-state
>  122104  115846  45      0       0       0

One could hardcode the output of the last three (four?) fields, yes?
Slightly grotty, but if it comes with a decent memory savings...

Re: It's time to hack on dentry_stat?

[Matthew Wilcox]

On Wed, May 07, 2008 at 08:08:09AM -0700, Ray Lee wrote:
> One could hardcode the output of the last three (four?) fields, yes?
> Slightly grotty, but if it comes with a decent memory savings...

How can it be a 'decent memory saving'?  There is exactly _one_ struct
dentry_stat_t in the entire kernel.  So you're talking about saving 4
ints, or 16 bytes.  Bet you it'd be more bytes to hardcode printing zero.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: It's time to hack on dentry_stat?

[Ray Lee]

On Wed, May 7, 2008 at 8:25 AM, Matthew Wilcox <matthew@wil.cx> wrote:
> On Wed, May 07, 2008 at 08:08:09AM -0700, Ray Lee wrote:
>  > One could hardcode the output of the last three (four?) fields, yes?
>  > Slightly grotty, but if it comes with a decent memory savings...
>
>  How can it be a 'decent memory saving'?  There is exactly _one_ struct
>  dentry_stat_t in the entire kernel.  So you're talking about saving 4
>  ints, or 16 bytes.  Bet you it'd be more bytes to hardcode printing zero.

<shrug> Well, if there's one, then obviously there isn't. I'd assumed
it was a per-dentry overhead.

Re: It's time to hack on dentry_stat?

[rae l]

On Thu, May 8, 2008 at 12:10 AM, Ray Lee <ray-lk@madrabbit.org> wrote:
>  <shrug> Well, if there's one, then obviously there isn't. I'd assumed
>  it was a per-dentry overhead.

Good. If the binary application interface must be kept,
>  $ cat /proc/sys/fs/dentry-state
>  122104  115846  45      0       0       0
and to shrink dentry_stat can only save 16 bytes, there's no need to do this,

but the Documentation/ as this should be fixed?

Documentation/filesystems/proc.txt:

dentry-state
------------

Status of  the  directory  cache.  Since  directory  entries  are  dynamically
allocated and  deallocated,  this  file indicates the current status. It holds
six values, in which the last two are not used and are always zero. The others
are listed in table 2-1.

Table 2-1: Status files of the directory cache
..............................................................................
 File       Content
 nr_dentry  Almost always zero
 nr_unused  Number of unused cache entries
 age_limit
            in seconds after the entry may be reclaimed, when memory is short
 want_pages internally
..............................................................................

the age_limit and want_pages should be marked outdated and not used anymore,

But there's still one question:
1. Is age_limit always 45? Or the binary application interface would
expect it always 45?
   Can we change dentry_stat with 6 int values all initialized with
zero? Then the docs can be
   fixed with last four values not used and are always zero.

--
Cheng