From: David Laight <David.Laight@ACULAB.COM>
To: 'Vlastimil Babka' <vbabka@suse.cz>, Christoph Lameter <cl@gentwo.org>
Cc: Rustam Kovhaev <rkovhaev@gmail.com>,
"penberg@kernel.org" <penberg@kernel.org>,
"rientjes@google.com" <rientjes@google.com>,
"iamjoonsoo.kim@lge.com" <iamjoonsoo.kim@lge.com>,
"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
"corbet@lwn.net" <corbet@lwn.net>,
"djwong@kernel.org" <djwong@kernel.org>,
"david@fromorbit.com" <david@fromorbit.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
"viro@zeniv.linux.org.uk" <viro@zeniv.linux.org.uk>,
"dvyukov@google.com" <dvyukov@google.com>
Subject: RE: [PATCH v4] slob: add size header to all allocations
Date: Tue, 30 Nov 2021 15:21:47 +0000 [thread overview]
Message-ID: <e79cd6da011a412f8c68e132ba74dc5c@AcuMS.aculab.com> (raw)
In-Reply-To: <d0927ca6-1710-5b2b-3682-6a80eb4e48d1@suse.cz>
From: Vlastimil Babka
> Sent: 30 November 2021 14:56
>
> On 11/23/21 11:18, David Laight wrote:
> > From: Vlastimil Babka
> >> Sent: 22 November 2021 10:46
> >>
> >> On 11/22/21 11:36, Christoph Lameter wrote:
> >> > On Mon, 22 Nov 2021, Vlastimil Babka wrote:
> >> >
> >> >> But it seems there's no reason we couldn't do better? I.e. use the value of
> >> >> SLOB_HDR_SIZE only to align the beginning of actual object (and name the
> >> >> define different than SLOB_HDR_SIZE). But the size of the header, where we
> >> >> store the object lenght could be just a native word - 4 bytes on 32bit, 8 on
> >> >> 64bit. The address of the header shouldn't have a reason to be also aligned
> >> >> to ARCH_KMALLOC_MINALIGN / ARCH_SLAB_MINALIGN as only SLOB itself processes
> >> >> it and not the slab consumers which rely on those alignments?
> >> >
> >> > Well the best way would be to put it at the end of the object in order to
> >> > avoid the alignment problem. This is a particular issue with SLOB because
> >> > it allows multiple types of objects in a single page frame.
> >> >
> >> > If only one type of object would be allowed then the object size etc can
> >> > be stored in the page struct.
> >
> > Or just a single byte that is the index of the associated free list structure.
> > For 32bit and for the smaller kmalloc() area it may be reasonable to have
> > a separate array indexed by the page of the address.
> >
> >> > So I guess placement at the beginning cannot be avoided. That in turn runs
> >> > into trouble with the DMA requirements on some platforms where the
> >> > beginning of the object has to be cache line aligned.
> >>
> >> It's no problem to have the real beginning of the object aligned, and the
> >> prepended header not.
> >
> > I'm not sure that helps.
> > The header can't share a cache line with the previous item (because it
> > might be mapped for DMA) so will always take a full cache line.
>
> So if this is true, then I think we already have a problem with SLOB today
> (and AFAICS it's not even due to changes done by my 2019 commit 59bb47985c1d
> ("mm, sl[aou]b: guarantee natural alignment for kmalloc(power-of-two)" but
> older).
>
> Let's say we are on arm64 where (AFAICS):
> ARCH_KMALLOC_MINALIGN = ARCH_DMA_MINALIGN = 128
> ARCH_SLAB_MINALIGN = 64
Is that valid?
Isn't SLAB being used to implement kmalloc() so the architecture
defined alignment must apply?
> The point is that ARCH_SLAB_MINALIGN is smaller than ARCH_DMA_MINALIGN.
>
> Let's say we call kmalloc(64) and get a completely fresh page.
> In SLOB, alloc() or rather __do_kmalloc_node() will calculate minalign to
> max(ARCH_KMALLOC_MINALIGN, ARCH_SLAB_MINALIGN) thus 128.
> It will call slob_alloc() for size of size+minalign=64+128=192, align and
> align_offset = 128
> Thus the allocation will use 128 bytes for the header, 64 for the object.
> Both the header and object aligned to 128 bytes.
> But the remaining 64 bytes of the second 128 bytes will remain free, as the
> allocated size is 192 bytes:
>
> | 128B header, aligned | 64B object | 64B free | rest also free |
That is horribly wasteful on memory :-)
> If there's another kmalloc allocation, the 128 bytes aligment due to
> ARCH_KMALLOC_MINALIGN will avoid it from using these 64 bytes, so that's
> fine. But if there's a kmem_cache_alloc() from a cache serving <=64B
> objects, it will be aligned to ARCH_SLAB_MINALIGN and happily use those 64
> bytes that share the 128 block where the previous kmalloc allocation lies.
If the memory returned by kmem_cache_alloc() can be used for DMA then
ARCH_DMA_MINALIGN has to apply to the returned buffers.
So, maybe, that cache can't exist?
I'd expect that ARCH_DMA_MINALIGN forces allocations to be a multiple
of that size.
More particularly the rest of the area can't be allocated to anything else.
So it ought to be valid to return the 2nd half of a 128 byte cache line
provided the first half isn't written while the allocation is active.
But that ARCH_KMALLOC_MINALIGN only applies to 'large' items?
Small items only need aligning to the power of 2 below their size.
So 8 bytes items only need 8 byte alignment even though a larger
item might need (say) 64 byte alignment.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
next prev parent reply other threads:[~2021-11-30 15:23 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-29 21:23 [PATCH] xfs: use kmem_cache_free() for kmem_cache objects Rustam Kovhaev
2021-09-30 4:42 ` Dave Chinner
2021-09-30 8:13 ` Vlastimil Babka
2021-09-30 18:48 ` Rustam Kovhaev
2021-09-30 21:10 ` Vlastimil Babka
2021-10-01 0:32 ` Rustam Kovhaev
2021-10-04 1:07 ` David Rientjes
2021-10-12 20:43 ` Darrick J. Wong
2021-10-12 20:43 ` Darrick J. Wong
2021-10-12 21:32 ` Vlastimil Babka
2021-10-12 23:22 ` Darrick J. Wong
2021-10-13 7:38 ` Vlastimil Babka
2021-10-13 16:56 ` Rustam Kovhaev
2021-10-15 0:57 ` Darrick J. Wong
2021-10-18 3:38 ` [PATCH] slob: add size header to all allocations Rustam Kovhaev
2021-10-18 9:22 ` Vlastimil Babka
2021-10-19 1:22 ` Rustam Kovhaev
2021-10-20 11:46 ` Hyeonggon Yoo
2021-10-21 17:36 ` Vlastimil Babka
2021-10-23 6:41 ` [PATCH v2] " Rustam Kovhaev
2021-10-25 9:36 ` Vlastimil Babka
2021-10-25 21:49 ` Rustam Kovhaev
2021-10-29 3:05 ` [PATCH v3] " Rustam Kovhaev
2021-11-16 11:26 ` Vlastimil Babka
2021-11-16 23:19 ` Rustam Kovhaev
2021-11-22 1:30 ` [PATCH v4] " Rustam Kovhaev
2021-11-22 9:22 ` Christoph Lameter
2021-11-22 9:40 ` Vlastimil Babka
2021-11-22 10:36 ` Christoph Lameter
2021-11-22 10:45 ` Vlastimil Babka
2021-11-22 11:40 ` Christoph Lameter
2021-11-22 11:49 ` Vlastimil Babka
2021-11-23 10:18 ` David Laight
2021-11-30 7:00 ` Rustam Kovhaev
2021-11-30 9:23 ` David Laight
2021-11-30 9:41 ` Christoph Lameter
2021-11-30 14:55 ` Vlastimil Babka
2021-11-30 15:21 ` David Laight [this message]
2021-11-30 15:39 ` Vlastimil Babka
2021-11-30 15:26 ` Christoph Lameter
2021-10-24 10:43 ` [PATCH] " Hyeonggon Yoo
2021-10-25 8:19 ` Vlastimil Babka
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e79cd6da011a412f8c68e132ba74dc5c@AcuMS.aculab.com \
--to=david.laight@aculab.com \
--cc=akpm@linux-foundation.org \
--cc=cl@gentwo.org \
--cc=corbet@lwn.net \
--cc=david@fromorbit.com \
--cc=djwong@kernel.org \
--cc=dvyukov@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=iamjoonsoo.kim@lge.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=penberg@kernel.org \
--cc=rientjes@google.com \
--cc=rkovhaev@gmail.com \
--cc=vbabka@suse.cz \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).