| Message ID | 20200130022441.95488-1-gshan@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | mm: Rework {enabled,defrag}_{show,store} | expand |
On Thu 30-01-20 13:24:38, Gavin Shan wrote: > The inappropriate settings can be accepted by "enabled" and "defrag" > interface, as the following example shows. This series reworks the > related functions and fixes the issue by the way. Do we really care? This is a root only interface and we do expect root to know how to use the interface. > # cat /sys/kernel/mm/transparent_hugepage/enabled > always [madvise] never > # echo neveroff > /sys/kernel/mm/transparent_hugepage/enabled > # cat /sys/kernel/mm/transparent_hugepage/enabled > always madvise [never] > > Gavin Shan (3): > mm: Rework {enabled,defrag}_store() > mm: Rework {enabled,defrag}_show() > mm: Rework setup_transparent_hugepage() > > mm/huge_memory.c | 216 +++++++++++++++++++++++++++-------------------- > 1 file changed, 125 insertions(+), 91 deletions(-) > > -- > 2.23.0 >
On 1/30/20 8:49 AM, Michal Hocko wrote: > On Thu 30-01-20 13:24:38, Gavin Shan wrote: >> The inappropriate settings can be accepted by "enabled" and "defrag" >> interface, as the following example shows. This series reworks the >> related functions and fixes the issue by the way. > > Do we really care? This is a root only interface and we do expect root > to know how to use the interface. Also I think David already solved even this case with sysfs_streq() that Andrew suggested: https://lore.kernel.org/linux-mm/alpine.DEB.2.21.2001171411020.56385@chino.kir.corp.google.com/ >> # cat /sys/kernel/mm/transparent_hugepage/enabled >> always [madvise] never >> # echo neveroff > /sys/kernel/mm/transparent_hugepage/enabled >> # cat /sys/kernel/mm/transparent_hugepage/enabled >> always madvise [never] >> >> Gavin Shan (3): >> mm: Rework {enabled,defrag}_store() >> mm: Rework {enabled,defrag}_show() >> mm: Rework setup_transparent_hugepage() >> >> mm/huge_memory.c | 216 +++++++++++++++++++++++++++-------------------- >> 1 file changed, 125 insertions(+), 91 deletions(-) >> >> -- >> 2.23.0 >> >
On 1/30/20 6:49 PM, Michal Hocko wrote: > On Thu 30-01-20 13:24:38, Gavin Shan wrote: >> The inappropriate settings can be accepted by "enabled" and "defrag" >> interface, as the following example shows. This series reworks the >> related functions and fixes the issue by the way. > > Do we really care? This is a root only interface and we do expect root > to know how to use the interface. > I'm not sure actually, but the behavior (suffix matching) isn't well documented in admin-guide/mm/transhuge.rst at least. I think it's not bad to enforce a full matching here because even root user can input something wrong :) Thanks, Gavin. >> # cat /sys/kernel/mm/transparent_hugepage/enabled >> always [madvise] never >> # echo neveroff > /sys/kernel/mm/transparent_hugepage/enabled >> # cat /sys/kernel/mm/transparent_hugepage/enabled >> always madvise [never] >> >> Gavin Shan (3): >> mm: Rework {enabled,defrag}_store() >> mm: Rework {enabled,defrag}_show() >> mm: Rework setup_transparent_hugepage() >> >> mm/huge_memory.c | 216 +++++++++++++++++++++++++++-------------------- >> 1 file changed, 125 insertions(+), 91 deletions(-) >> >> -- >> 2.23.0 >> >
On 1/30/20 8:46 PM, Vlastimil Babka wrote: > On 1/30/20 8:49 AM, Michal Hocko wrote: >> On Thu 30-01-20 13:24:38, Gavin Shan wrote: >>> The inappropriate settings can be accepted by "enabled" and "defrag" >>> interface, as the following example shows. This series reworks the >>> related functions and fixes the issue by the way. >> >> Do we really care? This is a root only interface and we do expect root >> to know how to use the interface. > > Also I think David already solved even this case with sysfs_streq() that > Andrew suggested: > > https://lore.kernel.org/linux-mm/alpine.DEB.2.21.2001171411020.56385@chino.kir.corp.google.com/ > Thanks for the linker. I didn't notice there already have a patch to address the issue. Also, I perhaps post patch to improve the extendibility on top of David's patch if you agree. Otherwise, it can be ignored. By the way, it's nice to know sysfs_streq(). Thanks, Gavin >>> # cat /sys/kernel/mm/transparent_hugepage/enabled >>> always [madvise] never >>> # echo neveroff > /sys/kernel/mm/transparent_hugepage/enabled >>> # cat /sys/kernel/mm/transparent_hugepage/enabled >>> always madvise [never] >>> >>> Gavin Shan (3): >>> mm: Rework {enabled,defrag}_store() >>> mm: Rework {enabled,defrag}_show() >>> mm: Rework setup_transparent_hugepage() >>> >>> mm/huge_memory.c | 216 +++++++++++++++++++++++++++-------------------- >>> 1 file changed, 125 insertions(+), 91 deletions(-) >>> >>> -- >>> 2.23.0 >>> >> >
The inappropriate settings can be accepted by "enabled" and "defrag" interface, as the following example shows. This series reworks the related functions and fixes the issue by the way. # cat /sys/kernel/mm/transparent_hugepage/enabled always [madvise] never # echo neveroff > /sys/kernel/mm/transparent_hugepage/enabled # cat /sys/kernel/mm/transparent_hugepage/enabled always madvise [never] Gavin Shan (3): mm: Rework {enabled,defrag}_store() mm: Rework {enabled,defrag}_show() mm: Rework setup_transparent_hugepage() mm/huge_memory.c | 216 +++++++++++++++++++++++++++-------------------- 1 file changed, 125 insertions(+), 91 deletions(-)