| Message ID | 20200602141519.7099-6-vbabka@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | replace runtime slub_debug toggling with more capable boot parameter | expand |
On Tue, Jun 02, 2020 at 04:15:19PM +0200, Vlastimil Babka wrote: > The attribute reflects the SLAB_RECLAIM_ACCOUNT cache flag. It's not clear why > this attribute was writable in the first place, as it's tied to how the cache > is used by its creator, it's not a user tunable. Furthermore: > > - it affects slab merging, but that's not being checked while toggled > - if affects whether __GFP_RECLAIMABLE flag is used to allocate page, but > the runtime toggle doesn't update allocflags > - it affects cache_vmstat_idx() so runtime toggle might lead to incosistency > of NR_SLAB_RECLAIMABLE and NR_SLAB_UNRECLAIMABLE > > Thus make it read-only. > > Signed-off-by: Vlastimil Babka <vbabka@suse.cz> Reviewed-by: Kees Cook <keescook@chromium.org>
On Tue, Jun 02, 2020 at 04:15:19PM +0200, Vlastimil Babka wrote: > The attribute reflects the SLAB_RECLAIM_ACCOUNT cache flag. It's not clear why > this attribute was writable in the first place, as it's tied to how the cache > is used by its creator, it's not a user tunable. Furthermore: > > - it affects slab merging, but that's not being checked while toggled > - if affects whether __GFP_RECLAIMABLE flag is used to allocate page, but > the runtime toggle doesn't update allocflags > - it affects cache_vmstat_idx() so runtime toggle might lead to incosistency > of NR_SLAB_RECLAIMABLE and NR_SLAB_UNRECLAIMABLE > > Thus make it read-only. > > Signed-off-by: Vlastimil Babka <vbabka@suse.cz> Acked-by: Roman Gushchin <guro@fb.com> > --- > mm/slub.c | 11 +---------- > 1 file changed, 1 insertion(+), 10 deletions(-) > > diff --git a/mm/slub.c b/mm/slub.c > index 38dd6f3ebb04..d4a9a097da50 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -5223,16 +5223,7 @@ static ssize_t reclaim_account_show(struct kmem_cache *s, char *buf) > { > return sprintf(buf, "%d\n", !!(s->flags & SLAB_RECLAIM_ACCOUNT)); > } > - > -static ssize_t reclaim_account_store(struct kmem_cache *s, > - const char *buf, size_t length) > -{ > - s->flags &= ~SLAB_RECLAIM_ACCOUNT; > - if (buf[0] == '1') > - s->flags |= SLAB_RECLAIM_ACCOUNT; > - return length; > -} > -SLAB_ATTR(reclaim_account); > +SLAB_ATTR_RO(reclaim_account); > > static ssize_t hwcache_align_show(struct kmem_cache *s, char *buf) > { > -- > 2.26.2 > >
diff --git a/mm/slub.c b/mm/slub.c index 38dd6f3ebb04..d4a9a097da50 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -5223,16 +5223,7 @@ static ssize_t reclaim_account_show(struct kmem_cache *s, char *buf) { return sprintf(buf, "%d\n", !!(s->flags & SLAB_RECLAIM_ACCOUNT)); } - -static ssize_t reclaim_account_store(struct kmem_cache *s, - const char *buf, size_t length) -{ - s->flags &= ~SLAB_RECLAIM_ACCOUNT; - if (buf[0] == '1') - s->flags |= SLAB_RECLAIM_ACCOUNT; - return length; -} -SLAB_ATTR(reclaim_account); +SLAB_ATTR_RO(reclaim_account); static ssize_t hwcache_align_show(struct kmem_cache *s, char *buf) {
The attribute reflects the SLAB_RECLAIM_ACCOUNT cache flag. It's not clear why this attribute was writable in the first place, as it's tied to how the cache is used by its creator, it's not a user tunable. Furthermore: - it affects slab merging, but that's not being checked while toggled - if affects whether __GFP_RECLAIMABLE flag is used to allocate page, but the runtime toggle doesn't update allocflags - it affects cache_vmstat_idx() so runtime toggle might lead to incosistency of NR_SLAB_RECLAIMABLE and NR_SLAB_UNRECLAIMABLE Thus make it read-only. Signed-off-by: Vlastimil Babka <vbabka@suse.cz> --- mm/slub.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-)