Hi Hugh,
On 02/21/2013 04:26 AM, Hugh Dickins wrote:
> On Tue, 19 Feb 2013, Greg Thelen wrote:
>
>> This patch fixes several mempolicy leaks in the tmpfs mount logic.
>> These leaks are slow - on the order of one object leaked per mount
>> attempt.
>>
>> Leak 1 (umount doesn't free mpol allocated in mount):
>> while true; do
>> mount -t tmpfs -o mpol=interleave,size=100M nodev /mnt
>> umount /mnt
>> done
>>
>> Leak 2 (errors parsing remount options will leak mpol):
>> mount -t tmpfs -o size=100M nodev /mnt
>> while true; do
>> mount -o remount,mpol=interleave,size=x /mnt 2> /dev/null
>> done
>> umount /mnt
>>
>> Leak 3 (multiple mpol per mount leak mpol):
>> while true; do
>> mount -t tmpfs -o mpol=interleave,mpol=interleave,size=100M nodev /mnt
>> umount /mnt
>> done
>>
>> This patch fixes all of the above. I could have broken the patch into
>> three pieces but is seemed easier to review as one.
> Yes, I agree, and nicely fixed - but one doubt below. If you resolve
> that, please add my Acked-by: Hugh Dickins <[email protected]>
Could you explain me why shmem has more relationship with mempolicy? It
seems that there are many codes in shmem handle mempolicy, but other
components in mm subsystem just have little.
>
>> Signed-off-by: Greg Thelen <[email protected]>
>> ---
>> mm/shmem.c | 12 +++++++++---
>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/shmem.c b/mm/shmem.c
>> index efd0b3a..ed2cb26 100644
>> --- a/mm/shmem.c
>> +++ b/mm/shmem.c
>> @@ -2386,6 +2386,7 @@ static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
>> bool remount)
>> {
>> char *this_char, *value, *rest;
>> + struct mempolicy *mpol = NULL;
>> uid_t uid;
>> gid_t gid;
>>
>> @@ -2414,7 +2415,7 @@ static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
>> printk(KERN_ERR
>> "tmpfs: No value for mount option '%s'\n",
>> this_char);
>> - return 1;
>> + goto error;
>> }
>>
>> if (!strcmp(this_char,"size")) {
>> @@ -2463,19 +2464,23 @@ static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
>> if (!gid_valid(sbinfo->gid))
>> goto bad_val;
>> } else if (!strcmp(this_char,"mpol")) {
>> - if (mpol_parse_str(value, &sbinfo->mpol))
>> + mpol_put(mpol);
> I haven't tested to check, but don't we need
> mpol = NULL;
> here, in case the new option turns out to be bad?
>
>> + if (mpol_parse_str(value, &mpol))
>> goto bad_val;
>> } else {
>> printk(KERN_ERR "tmpfs: Bad mount option %s\n",
>> this_char);
>> - return 1;
>> + goto error;
>> }
>> }
>> + sbinfo->mpol = mpol;
>> return 0;
>>
>> bad_val:
>> printk(KERN_ERR "tmpfs: Bad value '%s' for mount option '%s'\n",
>> value, this_char);
>> +error:
>> + mpol_put(mpol);
>> return 1;
>>
>> }
>> @@ -2551,6 +2556,7 @@ static void shmem_put_super(struct super_block *sb)
>> struct shmem_sb_info *sbinfo = SHMEM_SB(sb);
>>
>> percpu_counter_destroy(&sbinfo->used_blocks);
>> + mpol_put(sbinfo->mpol);
>> kfree(sbinfo);
>> sb->s_fs_info = NULL;
>> }
>> --
>> 1.8.1.3
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to [email protected]. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
On Mon, 4 Mar 2013, Will Huck wrote:
>
> Could you explain me why shmem has more relationship with mempolicy? It seems
> that there are many codes in shmem handle mempolicy, but other components in
> mm subsystem just have little.
NUMA mempolicy is mostly handled in mm/mempolicy.c, which services the
mbind, migrate_pages, set_mempolicy, get_mempolicy system calls: which
govern how process memory is distributed across NUMA nodes.
mm/shmem.c is affected because it was also found useful to specify
mempolicy on the shared memory objects which may back process memory:
that includes SysV SHM and POSIX shared memory and tmpfs. mm/hugetlb.c
contains some mempolicy handling for hugetlbfs; fs/ramfs is kept minimal,
so nothing in there.
Those are the memory-based filesystems, where NUMA mempolicy is most
natural. The regular filesystems could support shared mempolicy too,
but that would raise more awkward design questions.
Hugh
Hi Hugh,
On 03/06/2013 03:40 AM, Hugh Dickins wrote:
> On Mon, 4 Mar 2013, Will Huck wrote:
>> Could you explain me why shmem has more relationship with mempolicy? It seems
>> that there are many codes in shmem handle mempolicy, but other components in
>> mm subsystem just have little.
> NUMA mempolicy is mostly handled in mm/mempolicy.c, which services the
> mbind, migrate_pages, set_mempolicy, get_mempolicy system calls: which
> govern how process memory is distributed across NUMA nodes.
>
> mm/shmem.c is affected because it was also found useful to specify
> mempolicy on the shared memory objects which may back process memory:
> that includes SysV SHM and POSIX shared memory and tmpfs. mm/hugetlb.c
> contains some mempolicy handling for hugetlbfs; fs/ramfs is kept minimal,
> so nothing in there.
>
> Those are the memory-based filesystems, where NUMA mempolicy is most
> natural. The regular filesystems could support shared mempolicy too,
> but that would raise more awkward design questions.
I found that if mbind several processes to one node and almost exhaust
memory, processes will just stuck and no processes make progress or be
killed. Is it normal?
>
> Hugh