| Message ID | 20190912023111.219636-2-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:09PM -0600, Yu Zhao wrote: > The function doesn't need to return any value, and the check can be > done in one pass. > > There is a behavior change: before the patch, we stop at the first > invalid free object; after the patch, we stop at the first invalid > object, free or in use. This shouldn't matter because the original > behavior isn't intended anyway. > > Signed-off-by: Yu Zhao <yuzhao@google.com> > --- > mm/slub.c | 21 ++++++++------------- > 1 file changed, 8 insertions(+), 13 deletions(-) > > diff --git a/mm/slub.c b/mm/slub.c > index 62053ceb4464..7b7e1ee264ef 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -4386,31 +4386,26 @@ static int count_total(struct page *page) > #endif > > #ifdef CONFIG_SLUB_DEBUG > -static int validate_slab(struct kmem_cache *s, struct page *page, > +static void validate_slab(struct kmem_cache *s, struct page *page, > unsigned long *map) > { > void *p; > void *addr = page_address(page); > > - if (!check_slab(s, page) || > - !on_freelist(s, page, NULL)) > - return 0; > + if (!check_slab(s, page) || !on_freelist(s, page, NULL)) > + return; > > /* Now we know that a valid freelist exists */ > bitmap_zero(map, page->objects); > > get_map(s, page, map); > for_each_object(p, s, addr, page->objects) { > - if (test_bit(slab_index(p, s, addr), map)) > - if (!check_object(s, page, p, SLUB_RED_INACTIVE)) > - return 0; > - } > + u8 val = test_bit(slab_index(p, s, addr), map) ? > + SLUB_RED_INACTIVE : SLUB_RED_ACTIVE; Proper 'if' would be more readable. Other than that look fine to me. > > - for_each_object(p, s, addr, page->objects) > - if (!test_bit(slab_index(p, s, addr), map)) > - if (!check_object(s, page, p, SLUB_RED_ACTIVE)) > - return 0; > - return 1; > + if (!check_object(s, page, p, val)) > + break; > + } > } > > static void validate_slab_slab(struct kmem_cache *s, struct page *page, > -- > 2.23.0.162.g0b9fbb3734-goog >
diff --git a/mm/slub.c b/mm/slub.c index 62053ceb4464..7b7e1ee264ef 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -4386,31 +4386,26 @@ static int count_total(struct page *page) #endif #ifdef CONFIG_SLUB_DEBUG -static int validate_slab(struct kmem_cache *s, struct page *page, +static void validate_slab(struct kmem_cache *s, struct page *page, unsigned long *map) { void *p; void *addr = page_address(page); - if (!check_slab(s, page) || - !on_freelist(s, page, NULL)) - return 0; + if (!check_slab(s, page) || !on_freelist(s, page, NULL)) + return; /* Now we know that a valid freelist exists */ bitmap_zero(map, page->objects); get_map(s, page, map); for_each_object(p, s, addr, page->objects) { - if (test_bit(slab_index(p, s, addr), map)) - if (!check_object(s, page, p, SLUB_RED_INACTIVE)) - return 0; - } + u8 val = test_bit(slab_index(p, s, addr), map) ? + SLUB_RED_INACTIVE : SLUB_RED_ACTIVE; - for_each_object(p, s, addr, page->objects) - if (!test_bit(slab_index(p, s, addr), map)) - if (!check_object(s, page, p, SLUB_RED_ACTIVE)) - return 0; - return 1; + if (!check_object(s, page, p, val)) + break; + } } static void validate_slab_slab(struct kmem_cache *s, struct page *page,
The function doesn't need to return any value, and the check can be done in one pass. There is a behavior change: before the patch, we stop at the first invalid free object; after the patch, we stop at the first invalid object, free or in use. This shouldn't matter because the original behavior isn't intended anyway. Signed-off-by: Yu Zhao <yuzhao@google.com> --- mm/slub.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-)