Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <d841c1ab-c0d1-5130-11fc-c8ea04cc9511@amd.com>
Date:   Thu, 9 Jun 2022 11:19:23 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Subject: Re: [PATCH 03/13] mm: shmem: provide oom badness for shmem files
Content-Language: en-US
To:     Michal Hocko <mhocko@suse.com>,
        =?UTF-8?Q?Christian_K=c3=b6nig?= <ckoenig.leichtzumerken@gmail.com>
Cc:     =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>,
        linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
        intel-gfx@lists.freedesktop.org, amd-gfx@lists.freedesktop.org,
        nouveau@lists.freedesktop.org, linux-tegra@vger.kernel.org,
        linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
        alexander.deucher@amd.com, daniel@ffwll.ch,
        viro@zeniv.linux.org.uk, akpm@linux-foundation.org,
        hughd@google.com, andrey.grodzovsky@amd.com
References: <20220531100007.174649-1-christian.koenig@amd.com>
 <20220531100007.174649-4-christian.koenig@amd.com>
 <YqG67sox6L64E6wV@dhcp22.suse.cz>
 <77b99722-fc13-e5c5-c9be-7d4f3830859c@amd.com>
 <YqHuH5brYFQUfW8l@dhcp22.suse.cz>
 <26d3e1c7-d73c-cc95-54ef-58b2c9055f0c@gmail.com>
 <YqIB0bavUeU8Abwl@dhcp22.suse.cz>
From:   Felix Kuehling <felix.kuehling@amd.com>
In-Reply-To: <YqIB0bavUeU8Abwl@dhcp22.suse.cz>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RE1rMFZKajZHNFcyeFpCMnF6VGNkMnUxMnJ0Ymcwdmlqb3JJaCtlamJHZUFx?=
 =?utf-8?B?eVR6a3dScDJ2cmpNNHlhMmlDUkdsWHFkZXpJS0V0aWd3bmNVZXAvODdRL0ZS?=
 =?utf-8?B?M0xvdmFHZ3kzaVdhZDFDVHdzd1ArcHVQWk10a3JUR2VBRFJqTWZ5Qit6Nlhv?=
 =?utf-8?B?NEJlTUR0cURZWllaM2Nkc1VaNm5TTVR5YVBFN1V4dnRhS0pvZE96YklUOHAx?=
 =?utf-8?B?YmhQUE1wZWhMTlBuODlmY092Y3dhN1J1dTU3azF6bUlNL3RXdFkwRzhlam96?=
 =?utf-8?B?VEUzRGhvWHB4d2ZJS0JuRHQ4WWhEWXFkTnphbVRNSHZORjdocWZ5bCs1RXVI?=
 =?utf-8?B?dGgzSmExZjVmVGlnT1pTemNZei8rY2JJWWFtd1JTT2YrZitlRDZRdjM1aDJn?=
 =?utf-8?B?TUcxUWVLbGI3d1ZFRU5QSnNFQkp6S0hqaE84aHFIWHVXakdHa3lWMXVFSHJN?=
 =?utf-8?B?UXdrV1pIVEJFcGpHYTB2MmlESk9FVDB5Lzd2S0tWZ3BuSXVxcVpRSXdsZzRm?=
 =?utf-8?B?ZDM2QXVzMkhmQ3FNNDM4Q2FISEQ0V1VSVi9wVDNHSERkVVAxcHJMN2t3UHdz?=
 =?utf-8?B?V2J6WElCVnY4SGZ2M0pSK3kvUnM4cFkwcUp3c013VU1CZmVRUnJsd0cxKzJO?=
 =?utf-8?B?Z1k5Q1ZmN3Q2dEFQd29VWnBYMTZnZENObjMzcjVJUThJSUhPeEYrRnFiM1JD?=
 =?utf-8?B?VWJ2MEFsVFlkWlpOQUJBZnFOREY4RitMcjVIK2pGSURJSnJQUnhLTzh3N2Ns?=
 =?utf-8?B?bkhIYzk4UEg5VFEwYnhrNE13MFUwNHNGM09Qemp5Ty9TZTd0ZzBRR2hkRGlz?=
 =?utf-8?B?QkdsQkxPWkFGY091bFFGdUdKNHFVL2k1dld0elJWR25oM0hNWU5pbkZpaGdL?=
 =?utf-8?B?OEhmWHV5ekFYdkEzRkd0aVBvMEgrVDViMTJuY05LZkZsZ3RJRzN0N2JObVNs?=
 =?utf-8?B?QU9vSGNkaGFWWDVvcUtPUHBrUnIwYVRUTGQ5amxEanhiZEFkWUgzOGxnbkxS?=
 =?utf-8?B?MFEyZkJPa28ycHVqUjhWUkYrNUlnQkNJdGU3WHVDSHZ6RElFZnRTaWZtc0Va?=
 =?utf-8?B?RWp3K3lXTytQekZMUVlPcERpbElnR2dVMHBsaFdQcWVJNUJKSHo0Z1RqQkRR?=
 =?utf-8?B?TDVQRXNtRVptbVZRa1ZHdDlqeStXbXlVSmpQSFkwZDcwcU43VEx5VUZQbmo3?=
 =?utf-8?B?R0RnV0cveG1oNXNmbXk5NjZ1cVFxcFNQczBKL1EzZWpEelNaOEZrUDFwb2ZX?=
 =?utf-8?B?cmxEVkRsaW9PaVpGVHBERDlzR3JMOHhDdi9XVkNKM2lBWDZrYkhjWWJ2NUZz?=
 =?utf-8?B?Z2lsTU1FUktEak1YSmRwalRxWWhoZGc2WmJqWlZ0NERGcWFaSjJuMTlnQWhJ?=
 =?utf-8?B?TXZqWnZxeEszMHY4aktNSkFSa21na2I4M0VDYTVEUXZmRVFaWnZhQ3QwTFRC?=
 =?utf-8?B?VHdxWG9JLzQ0U2hld0NjMngzVnI5MlFVcXc3dlFtMjh0VGUyQXpIZk1menZI?=
 =?utf-8?B?RVpiWEpNTHEyb1N6UDZoYmxKbTNObS9KeFZjWmFxRk9xQVRJWnFOdTRKUXF1?=
 =?utf-8?B?Ymx5ZnR2d2hta3RpdWlRWjVhUGxPY3BuNEU3RkswdTFtbFEvYzkrRTdrejBG?=
 =?utf-8?B?azFSTklGUGJlZUNvUitxclJ0R21Hc0xxRWlZWGNSdmdxeTFyV1NpZkhqWVhW?=
 =?utf-8?B?c2k2Z0xRRDE5L2NKclRCK3g3YWNESzVqR1RmUStSS2FQbG5BejNjbC9QMFVU?=
 =?utf-8?B?YlN5Nk1rdHRsLzdaWEd2ZnFUb0c0VVBiVlRCZDhXblJ5Nzl1WHpldWFwb2FS?=
 =?utf-8?B?bW9ydkc0UnB3QUxFUmFOZnFFY1g5dHNWbkZrQTZhZHFlUFMwV2VzRXdQL2ow?=
 =?utf-8?B?K2ZZM0R6TlliTVB5OW4vZHVzL0dlTUVGa0JlRUpWZ0dTNzAzbTYwNVUrRTND?=
 =?utf-8?B?L0ppVEV1ZEFETFdlY2FLRTBkY0Zsb2xJS1F0NGtPNHVYTjB3UGJHVVdzK1hq?=
 =?utf-8?B?R0llcFlCYzVLZDVFT3VGdE5BNGJuQnNUTWJiSW5QbG0xSzI4S28xdWU0eFg0?=
 =?utf-8?B?cmFJSG5NQ2IwdTdyU0NSdC9vT25ueHJ6ektlUGdmT2RFcTlEZ1U2TFdjT01N?=
 =?utf-8?B?enNKV0pkd2E1a0xKWS9nZEp3bGNuMWFwaG9QcDI1Z0ZxNmUzMVNBSDN0Y3Bs?=
 =?utf-8?B?VjNVRExpS1hCZ0JmanY2d0pIVnZnZU1QMVh1SW1iTzJoZ09xTHJJT3lIWE1D?=
 =?utf-8?B?c3lDMlNIeGRvOFpPQUFSYWZkREZDQ2Vqc2k2d281SlQ1NWhlcFltdlcrQ0xl?=
 =?utf-8?B?RmlpK3lpbWRuNnFBY2FNbFdVNk9ZYjJlbWt4WEFpQnU4ZlZVc1RpQT09?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 664f4fe9-e0ab-490f-fe1e-08da4a2b705b
X-MS-Exchange-CrossTenant-AuthSource: BN9PR12MB5115.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Jun 2022 15:19:25.7191
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: nlKJqEwT1t1Wfgh/cT4XGkspICWpXoldJF3BowHYX91SjVPODGue8LvgF6Exz2fmRzKWf6kRPYSxSlIiMIPZjw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB5757
X-Spam-Status: No, score=-3.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE,
        URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


Am 2022-06-09 um 10:21 schrieb Michal Hocko:
> On Thu 09-06-22 16:10:33, Christian König wrote:
>> Am 09.06.22 um 14:57 schrieb Michal Hocko:
>>> On Thu 09-06-22 14:16:56, Christian König wrote:
>>>> Am 09.06.22 um 11:18 schrieb Michal Hocko:
>>>>> On Tue 31-05-22 11:59:57, Christian König wrote:
>>>>>> This gives the OOM killer an additional hint which processes are
>>>>>> referencing shmem files with potentially no other accounting for them.
>>>>>>
>>>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>>>> ---
>>>>>>     mm/shmem.c | 6 ++++++
>>>>>>     1 file changed, 6 insertions(+)
>>>>>>
>>>>>> diff --git a/mm/shmem.c b/mm/shmem.c
>>>>>> index 4b2fea33158e..a4ad92a16968 100644
>>>>>> --- a/mm/shmem.c
>>>>>> +++ b/mm/shmem.c
>>>>>> @@ -2179,6 +2179,11 @@ unsigned long shmem_get_unmapped_area(struct file *file,
>>>>>>     	return inflated_addr;
>>>>>>     }
>>>>>> +static long shmem_oom_badness(struct file *file)
>>>>>> +{
>>>>>> +	return i_size_read(file_inode(file)) >> PAGE_SHIFT;
>>>>>> +}
>>>>> This doesn't really represent the in memory size of the file, does it?
>>>> Well the file could be partially or fully swapped out as anonymous memory or
>>>> the address space only sparse populated, but even then just using the file
>>>> size as OOM badness sounded like the most straightforward approach to me.
>>> It covers hole as well, right?
>> Yes, exactly.
> So let's say I have a huge sparse shmem file. I will get killed because
> the oom_badness of such a file would be large as well...

Would killing processes free shmem files, though? Aren't those 
persistent anyway? In that case, shmem files should not contribute to 
oom_badness at all.

I guess a special case would be files that were removed from the 
filesystem but are still open in some processes.

Regards,
   Felix


>
>>>> What could happen is that the file is also mmaped and we double account.
>>>>
>>>>> Also the memcg oom handling could be considerably skewed if the file was
>>>>> shared between more memcgs.
>>>> Yes, and that's one of the reasons why I didn't touched the memcg by this
>>>> and only affected the classic OOM killer.
>>> oom_badness is for all oom handlers, including memcg. Maybe I have
>>> misread an earlier patch but I do not see anything specific to global
>>> oom handling.
>> As far as I can see the oom_badness() function is only used in
>> oom_kill.c and in procfs to return the oom score. Did I missed
>> something?
> oom_kill.c implements most of the oom killer functionality. Memcg oom
> killing is a part of that. Have a look at select_bad_process.
>