| Message ID | 20190912023111.219636-1-yuzhao@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/4] mm: correct mask size for slub page->objects | expand |
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 <yuzhao@google.com> 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.
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 <yuzhao@google.com> > > 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.
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 <yuzhao@google.com> > > > > 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.
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; }
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 <yuzhao@google.com> --- mm/slub.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)