| Message ID | 158894059427.200862.341530589978120554.stgit@buzz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dcache: increase poison resistance | expand |
On 5/8/20 8:23 AM, Konstantin Khlebnikov wrote: > Count of buckets is required for estimating average length of hash chains. > Size of hash table depends on memory size and printed once at boot. > > Let's expose nr_buckets as sixth number in sysctl fs.dentry-state The hash bucket count is a constant determined at boot time. Is there a need to use up one dentry_stat entry for that? Besides one can get it by looking up the kernel dmesg log like: [ 0.055212] Dentry cache hash table entries: 8388608 (order: 14, 67108864 bytes) Cheers, Longman
On 08/05/2020 17.49, Waiman Long wrote: > On 5/8/20 8:23 AM, Konstantin Khlebnikov wrote: >> Count of buckets is required for estimating average length of hash chains. >> Size of hash table depends on memory size and printed once at boot. >> >> Let's expose nr_buckets as sixth number in sysctl fs.dentry-state > > The hash bucket count is a constant determined at boot time. Is there a need to use up one dentry_stat entry for that? Besides one can get > it by looking up the kernel dmesg log like: > > [ 0.055212] Dentry cache hash table entries: 8388608 (order: 14, 67108864 bytes) Grepping logs since boot time is a worst API ever. dentry-state shows count of dentries in various states. It's very convenient to show count of buckets next to it, because this number defines overall scale. > > Cheers, > Longman >
On 5/8/20 12:16 PM, Konstantin Khlebnikov wrote: > On 08/05/2020 17.49, Waiman Long wrote: >> On 5/8/20 8:23 AM, Konstantin Khlebnikov wrote: >>> Count of buckets is required for estimating average length of hash >>> chains. >>> Size of hash table depends on memory size and printed once at boot. >>> >>> Let's expose nr_buckets as sixth number in sysctl fs.dentry-state >> >> The hash bucket count is a constant determined at boot time. Is there >> a need to use up one dentry_stat entry for that? Besides one can get >> it by looking up the kernel dmesg log like: >> >> [ 0.055212] Dentry cache hash table entries: 8388608 (order: 14, >> 67108864 bytes) > > Grepping logs since boot time is a worst API ever. > > dentry-state shows count of dentries in various states. > It's very convenient to show count of buckets next to it, > because this number defines overall scale. I am not against using the last free entry for that. My only concern is when we want to expose another internal dcache data point via dentry-state, we will have to add one more number to the array which can cause all sort of compatibility problem. So do we want to use the last free slot for a constant that can be retrieved from somewhere else? Cheers, Longman
On 08/05/2020 22.05, Waiman Long wrote: > On 5/8/20 12:16 PM, Konstantin Khlebnikov wrote: >> On 08/05/2020 17.49, Waiman Long wrote: >>> On 5/8/20 8:23 AM, Konstantin Khlebnikov wrote: >>>> Count of buckets is required for estimating average length of hash chains. >>>> Size of hash table depends on memory size and printed once at boot. >>>> >>>> Let's expose nr_buckets as sixth number in sysctl fs.dentry-state >>> >>> The hash bucket count is a constant determined at boot time. Is there a need to use up one dentry_stat entry for that? Besides one can >>> get it by looking up the kernel dmesg log like: >>> >>> [ 0.055212] Dentry cache hash table entries: 8388608 (order: 14, 67108864 bytes) >> >> Grepping logs since boot time is a worst API ever. >> >> dentry-state shows count of dentries in various states. >> It's very convenient to show count of buckets next to it, >> because this number defines overall scale. > > I am not against using the last free entry for that. My only concern is when we want to expose another internal dcache data point via > dentry-state, we will have to add one more number to the array which can cause all sort of compatibility problem. So do we want to use the > last free slot for a constant that can be retrieved from somewhere else? I see no problem in adding more numbers into sysctl. Especially into such rarely used. This interface is designed for that. Also fields 'age_limit' and 'want_pages' are unused since kernel 2.2.0 > > Cheers, > Longman >
On 5/8/20 3:38 PM, Konstantin Khlebnikov wrote: > > > On 08/05/2020 22.05, Waiman Long wrote: >> On 5/8/20 12:16 PM, Konstantin Khlebnikov wrote: >>> On 08/05/2020 17.49, Waiman Long wrote: >>>> On 5/8/20 8:23 AM, Konstantin Khlebnikov wrote: >>>>> Count of buckets is required for estimating average length of hash >>>>> chains. >>>>> Size of hash table depends on memory size and printed once at boot. >>>>> >>>>> Let's expose nr_buckets as sixth number in sysctl fs.dentry-state >>>> >>>> The hash bucket count is a constant determined at boot time. Is >>>> there a need to use up one dentry_stat entry for that? Besides one >>>> can get it by looking up the kernel dmesg log like: >>>> >>>> [ 0.055212] Dentry cache hash table entries: 8388608 (order: 14, >>>> 67108864 bytes) >>> >>> Grepping logs since boot time is a worst API ever. >>> >>> dentry-state shows count of dentries in various states. >>> It's very convenient to show count of buckets next to it, >>> because this number defines overall scale. >> >> I am not against using the last free entry for that. My only concern >> is when we want to expose another internal dcache data point via >> dentry-state, we will have to add one more number to the array which >> can cause all sort of compatibility problem. So do we want to use the >> last free slot for a constant that can be retrieved from somewhere else? > > I see no problem in adding more numbers into sysctl. > Especially into such rarely used. > This interface is designed for that. > > Also fields 'age_limit' and 'want_pages' are unused since kernel 2.2.0 Well, I got rebuke the last time I want to reuse one of age_limit/want_pages entry for negative dentry count because of the potential of breaking some really old applications or tools. Changing dentry-state to output one more number can potentially break compatibility too. That is why I am questioning if it is a good idea to use up the last free slot. Cheers, Longman
On Fri, May 08, 2020 at 04:00:08PM -0400, Waiman Long wrote: > On 5/8/20 3:38 PM, Konstantin Khlebnikov wrote: > > On 08/05/2020 22.05, Waiman Long wrote: > > > On 5/8/20 12:16 PM, Konstantin Khlebnikov wrote: > > > > On 08/05/2020 17.49, Waiman Long wrote: > > > > > On 5/8/20 8:23 AM, Konstantin Khlebnikov wrote: > > > > > > Count of buckets is required for estimating average > > > > > > length of hash chains. > > > > > > Size of hash table depends on memory size and printed once at boot. > > > > > > > > > > > > Let's expose nr_buckets as sixth number in sysctl fs.dentry-state > > > > > > > > > > The hash bucket count is a constant determined at boot time. > > > > > Is there a need to use up one dentry_stat entry for that? > > > > > Besides one can get it by looking up the kernel dmesg log > > > > > like: > > > > > > > > > > [ 0.055212] Dentry cache hash table entries: 8388608 > > > > > (order: 14, 67108864 bytes) > > > > > > > > Grepping logs since boot time is a worst API ever. > > > > > > > > dentry-state shows count of dentries in various states. > > > > It's very convenient to show count of buckets next to it, > > > > because this number defines overall scale. > > > > > > I am not against using the last free entry for that. My only concern > > > is when we want to expose another internal dcache data point via > > > dentry-state, we will have to add one more number to the array which > > > can cause all sort of compatibility problem. So do we want to use > > > the last free slot for a constant that can be retrieved from > > > somewhere else? > > > > I see no problem in adding more numbers into sysctl. > > Especially into such rarely used. > > This interface is designed for that. > > > > Also fields 'age_limit' and 'want_pages' are unused since kernel 2.2.0 > > Well, I got rebuke the last time I want to reuse one of age_limit/want_pages > entry for negative dentry count because of the potential of breaking some > really old applications or tools. Changing dentry-state to output one more > number can potentially break compatibility too. That is why I am questioning > if it is a good idea to use up the last free slot. I'd rather see the nr_buckets exposed some other way, leaving this file for dynamic state.
diff --git a/Documentation/admin-guide/sysctl/fs.rst b/Documentation/admin-guide/sysctl/fs.rst index 2a45119e3331..b74df4714ddd 100644 --- a/Documentation/admin-guide/sysctl/fs.rst +++ b/Documentation/admin-guide/sysctl/fs.rst @@ -66,12 +66,12 @@ dentry-state From linux/include/linux/dcache.h:: struct dentry_stat_t dentry_stat { - int nr_dentry; - int nr_unused; - int age_limit; /* age in seconds */ - int want_pages; /* pages requested by system */ - int nr_negative; /* # of unused negative dentries */ - int dummy; /* Reserved for future use */ + long nr_dentry; + long nr_unused; + long age_limit; /* age in seconds */ + long want_pages; /* pages requested by system */ + long nr_negative; /* # of unused negative dentries */ + long nr_buckets; /* count of dcache hash buckets */ }; Dentries are dynamically allocated and deallocated. diff --git a/fs/dcache.c b/fs/dcache.c index b280e07e162b..386f97eaf2ff 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -3139,6 +3139,14 @@ static int __init set_dhash_entries(char *str) } __setup("dhash_entries=", set_dhash_entries); +static void __init dcache_init_hash(void) +{ + dentry_stat.nr_buckets = 1l << d_hash_shift; + + /* shift to use higher bits of 32 bit hash value */ + d_hash_shift = 32 - d_hash_shift; +} + static void __init dcache_init_early(void) { /* If hashes are distributed across NUMA nodes, defer @@ -3157,7 +3165,7 @@ static void __init dcache_init_early(void) NULL, 0, 0); - d_hash_shift = 32 - d_hash_shift; + dcache_init_hash(); } static void __init dcache_init(void) @@ -3185,7 +3193,7 @@ static void __init dcache_init(void) NULL, 0, 0); - d_hash_shift = 32 - d_hash_shift; + dcache_init_hash(); } /* SLAB cache for __getname() consumers */ diff --git a/include/linux/dcache.h b/include/linux/dcache.h index c1488cc84fd9..082b55068e4d 100644 --- a/include/linux/dcache.h +++ b/include/linux/dcache.h @@ -65,7 +65,7 @@ struct dentry_stat_t { long age_limit; /* age in seconds */ long want_pages; /* pages requested by system */ long nr_negative; /* # of unused negative dentries */ - long dummy; /* Reserved for future use */ + long nr_buckets; /* count of dcache hash buckets */ }; extern struct dentry_stat_t dentry_stat;
Count of buckets is required for estimating average length of hash chains. Size of hash table depends on memory size and printed once at boot. Let's expose nr_buckets as sixth number in sysctl fs.dentry-state Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> --- Documentation/admin-guide/sysctl/fs.rst | 12 ++++++------ fs/dcache.c | 12 ++++++++++-- include/linux/dcache.h | 2 +- 3 files changed, 17 insertions(+), 9 deletions(-)