| Message ID | 20190910012652.3723-5-lpf.vector@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm, slab: Make kmalloc_info[] contain all types of names | expand |
On 9/10/19 3:26 AM, Pengfei Li wrote: > KMALLOC_DMA will be initialized only if KMALLOC_NORMAL with > the same index exists. > > And kmalloc_caches[KMALLOC_NORMAL][0] is always NULL. > > Therefore, the loop that initializes KMALLOC_DMA should start > at 1 instead of 0, which will reduce 1 meaningless attempt. IMHO the saving of one iteration isn't worth making the code more subtle. KMALLOC_SHIFT_LOW would be nice, but that would skip 1 + 2 which are special. Since you're doing these cleanups, have you considered reordering kmalloc_info, size_index, kmalloc_index() etc so that sizes 96 and 192 are ordered naturally between 64, 128 and 256? That should remove various special casing such as in create_kmalloc_caches(). I can't guarantee it will be possible without breaking e.g. constant folding optimizations etc., but seems to me it should be feasible. (There are definitely more places to change than those I listed.) > Signed-off-by: Pengfei Li <lpf.vector@gmail.com> > --- > mm/slab_common.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/slab_common.c b/mm/slab_common.c > index af45b5278fdc..c81fc7dc2946 100644 > --- a/mm/slab_common.c > +++ b/mm/slab_common.c > @@ -1236,7 +1236,7 @@ void __init create_kmalloc_caches(slab_flags_t flags) > slab_state = UP; > > #ifdef CONFIG_ZONE_DMA > - for (i = 0; i <= KMALLOC_SHIFT_HIGH; i++) { > + for (i = 1; i <= KMALLOC_SHIFT_HIGH; i++) { > struct kmem_cache *s = kmalloc_caches[KMALLOC_NORMAL][i]; > > if (s) { >
On Tue, Sep 10, 2019 at 6:26 PM Vlastimil Babka <vbabka@suse.cz> wrote: > > On 9/10/19 3:26 AM, Pengfei Li wrote: > > KMALLOC_DMA will be initialized only if KMALLOC_NORMAL with > > the same index exists. > > > > And kmalloc_caches[KMALLOC_NORMAL][0] is always NULL. > > > > Therefore, the loop that initializes KMALLOC_DMA should start > > at 1 instead of 0, which will reduce 1 meaningless attempt. > > IMHO the saving of one iteration isn't worth making the code more > subtle. KMALLOC_SHIFT_LOW would be nice, but that would skip 1 + 2 which > are special. > Yes, I agree with you. This really makes the code more subtle. > Since you're doing these cleanups, have you considered reordering > kmalloc_info, size_index, kmalloc_index() etc so that sizes 96 and 192 > are ordered naturally between 64, 128 and 256? That should remove > various special casing such as in create_kmalloc_caches(). I can't > guarantee it will be possible without breaking e.g. constant folding > optimizations etc., but seems to me it should be feasible. (There are > definitely more places to change than those I listed.) > In the past two days, I am working on what you suggested. So far, I have completed the coding work, but I need some time to make sure there are no bugs and verify the impact on performance. I will send v4 soon. Thank you for your review and suggestions. -- Pengfei > > Signed-off-by: Pengfei Li <lpf.vector@gmail.com> > > --- > > mm/slab_common.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/mm/slab_common.c b/mm/slab_common.c > > index af45b5278fdc..c81fc7dc2946 100644 > > --- a/mm/slab_common.c > > +++ b/mm/slab_common.c > > @@ -1236,7 +1236,7 @@ void __init create_kmalloc_caches(slab_flags_t flags) > > slab_state = UP; > > > > #ifdef CONFIG_ZONE_DMA > > - for (i = 0; i <= KMALLOC_SHIFT_HIGH; i++) { > > + for (i = 1; i <= KMALLOC_SHIFT_HIGH; i++) { > > struct kmem_cache *s = kmalloc_caches[KMALLOC_NORMAL][i]; > > > > if (s) { > > >
On 9/11/19 4:33 PM, Pengfei Li wrote: > In the past two days, I am working on what you suggested. Great! > So far, I have completed the coding work, but I need some time to make > sure there are no bugs and verify the impact on performance. It would probably be hard to measure with sufficient confidence in terms of runtime performance, but you could use e.g. ./scripts/bloat-o-meter to look for unexpected code increase due to compile-time optimizations becoming runtime.
diff --git a/mm/slab_common.c b/mm/slab_common.c index af45b5278fdc..c81fc7dc2946 100644 --- a/mm/slab_common.c +++ b/mm/slab_common.c @@ -1236,7 +1236,7 @@ void __init create_kmalloc_caches(slab_flags_t flags) slab_state = UP; #ifdef CONFIG_ZONE_DMA - for (i = 0; i <= KMALLOC_SHIFT_HIGH; i++) { + for (i = 1; i <= KMALLOC_SHIFT_HIGH; i++) { struct kmem_cache *s = kmalloc_caches[KMALLOC_NORMAL][i]; if (s) {
KMALLOC_DMA will be initialized only if KMALLOC_NORMAL with the same index exists. And kmalloc_caches[KMALLOC_NORMAL][0] is always NULL. Therefore, the loop that initializes KMALLOC_DMA should start at 1 instead of 0, which will reduce 1 meaningless attempt. Signed-off-by: Pengfei Li <lpf.vector@gmail.com> --- mm/slab_common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)