Re: [PATCH v2 1/4] mm: correct mask size for slub page->objects
On Thu, Sep 12, 2019 at 03:11:14PM -0600, Yu Zhao wrote:
> On Thu, Sep 12, 2019 at 12:40:35PM +0300, Kirill A. Shutemov wrote:
> > On Wed, Sep 11, 2019 at 08:31:08PM -0600, Yu Zhao wrote:
> > > Mask of slub objects per page shouldn't be larger than what
> > > page->objects can hold.
> > >
> > > It requires more than 2^15 objects to hit the problem, and I don't
> > > think anybody would. It'd be nice to have the mask fixed, but not
> > > really worth cc'ing the stable.
> > >
> > > Fixes: 50d5c41cd151 ("slub: Do not use frozen page flag but a bit in the
> > > page counters")
> > > Signed-off-by: Yu Zhao
> >
> > I don't think the patch fixes anything.
>
> Technically it does. It makes no sense for a mask to have more bits
> than the variable that holds the masked value. I had to look up the
> commit history to find out why and go through the code to make sure
> it doesn't actually cause any problem.
>
> My hope is that nobody else would have to go through the same trouble.
Just put some comments then.
--
Kirill A. Shutemov
Re: [PATCH v2 1/4] mm: correct mask size for slub page->objects
On Thu, Sep 12, 2019 at 12:40:35PM +0300, Kirill A. Shutemov wrote:
> On Wed, Sep 11, 2019 at 08:31:08PM -0600, Yu Zhao wrote:
> > Mask of slub objects per page shouldn't be larger than what
> > page->objects can hold.
> >
> > It requires more than 2^15 objects to hit the problem, and I don't
> > think anybody would. It'd be nice to have the mask fixed, but not
> > really worth cc'ing the stable.
> >
> > Fixes: 50d5c41cd151 ("slub: Do not use frozen page flag but a bit in the
> > page counters")
> > Signed-off-by: Yu Zhao
>
> I don't think the patch fixes anything.
Technically it does. It makes no sense for a mask to have more bits
than the variable that holds the masked value. I had to look up the
commit history to find out why and go through the code to make sure
it doesn't actually cause any problem.
My hope is that nobody else would have to go through the same trouble.
> Yes, we have one spare bit between order and number of object that is not
> used and always zero. So what?
>
> I can imagine for some microarchitecures accessing higher 16 bits of int
> is cheaper than shifting by 15.
Well, I highly doubt the inconsistency is intended for such
optimization, even it existed.
Re: [PATCH v2 1/4] mm: correct mask size for slub page->objects
On Wed, Sep 11, 2019 at 08:31:08PM -0600, Yu Zhao wrote:
> Mask of slub objects per page shouldn't be larger than what
> page->objects can hold.
>
> It requires more than 2^15 objects to hit the problem, and I don't
> think anybody would. It'd be nice to have the mask fixed, but not
> really worth cc'ing the stable.
>
> Fixes: 50d5c41cd151 ("slub: Do not use frozen page flag but a bit in the page
> counters")
> Signed-off-by: Yu Zhao
I don't think the patch fixes anything.
Yes, we have one spare bit between order and number of object that is not
used and always zero. So what?
I can imagine for some microarchitecures accessing higher 16 bits of int
is cheaper than shifting by 15.
--
Kirill A. Shutemov
[PATCH v2 1/4] mm: correct mask size for slub page->objects
Mask of slub objects per page shouldn't be larger than what
page->objects can hold.
It requires more than 2^15 objects to hit the problem, and I don't
think anybody would. It'd be nice to have the mask fixed, but not
really worth cc'ing the stable.
Fixes: 50d5c41cd151 ("slub: Do not use frozen page flag but a bit in the page
counters")
Signed-off-by: Yu Zhao
---
mm/slub.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/mm/slub.c b/mm/slub.c
index 8834563cdb4b..62053ceb4464 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -187,7 +187,7 @@ static inline bool kmem_cache_has_cpu_partial(struct
kmem_cache *s)
*/
#define DEBUG_METADATA_FLAGS (SLAB_RED_ZONE | SLAB_POISON | SLAB_STORE_USER)
-#define OO_SHIFT 16
+#define OO_SHIFT 15
#define OO_MASK((1 << OO_SHIFT) - 1)
#define MAX_OBJS_PER_PAGE 32767 /* since page.objects is u15 */
@@ -343,6 +343,8 @@ static inline unsigned int oo_order(struct
kmem_cache_order_objects x)
static inline unsigned int oo_objects(struct kmem_cache_order_objects x)
{
+ BUILD_BUG_ON(OO_MASK > MAX_OBJS_PER_PAGE);
+
return x.x & OO_MASK;
}
--
2.23.0.162.g0b9fbb3734-goog
