From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754704AbdCaJZo (ORCPT ); Fri, 31 Mar 2017 05:25:44 -0400 Received: from mail-db5eur01on0136.outbound.protection.outlook.com ([104.47.2.136]:31232 "EHLO EUR01-DB5-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754181AbdCaJZk (ORCPT ); Fri, 31 Mar 2017 05:25:40 -0400 Authentication-Results: vger.kernel.org; dkim=none (message not signed) header.d=none;vger.kernel.org; dmarc=none action=none header.from=virtuozzo.com; Subject: Re: [PATCH 1/4] mm/vmalloc: allow to call vfree() in atomic context To: Andrew Morton References: <20170330102719.13119-1-aryabinin@virtuozzo.com> <20170330152229.f2108e718114ed77acae7405@linux-foundation.org> CC: , , , , , , , , , , , , , , From: Andrey Ryabinin Message-ID: Date: Fri, 31 Mar 2017 12:26:53 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <20170330152229.f2108e718114ed77acae7405@linux-foundation.org> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [195.214.232.6] X-ClientProxiedBy: HE1PR09CA0085.eurprd09.prod.outlook.com (10.174.50.157) To AM5PR0801MB2049.eurprd08.prod.outlook.com (10.168.158.139) X-MS-Office365-Filtering-Correlation-Id: f1e60960-5d9b-4929-fb09-08d47817e065 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001)(201703131423075)(201703031133081);SRVR:AM5PR0801MB2049; X-Microsoft-Exchange-Diagnostics: 1;AM5PR0801MB2049;3:vks68/evIBSRIjH/zQHIwTNBX7w2d7F/1yd4SXKyZ51hXVYwyQdcR5HZem0w4wpcTCvANTEi2gykeEnHS/rsbsc1EloU3GvFGS7tX219IVPuv7/9TBWXPi2xP/pEW4jFkhlEj5cXA1Dapq9zuysR8SplvMRkQRkb6sE3Ik//QQ8vZr8/z9mRo014bV1pIDDDKP7OkEfEUi4oIix4RnPmmDDIr4DIaVvuejpWmynIh3AC9zqzT1/lNrCFpyzqsYfgAUpRMKshbGRDLXyKOZff7n2FxdMmwrNMHcE9tMU8dnWKFh/IMcoom209CNBw4pf7JlnxIhx4paWWn4uvunsGbw==;25:PxQ80ZgiQNhOM/XOxwE2yDGWoEo6FD7NrRK6OtRaHpSIb9CCvJO7tOj3Q4frRoHizmLz40e5BQo+yLNMWiRx3pFKyvO9O/sZTEZCdO27TPBExftaNlm/sV+XBGVbu9Npkmmsav/eRXSnGxKOq7HG3EjCA4L1Ss9ZC2QkiyqbaKeUVo4dpD7tGtkKchh4hOr8gx2JECZDJwu0iUJGPOYuYkwktSvgWyPAC6REQ5qKKsyCHA7BOVynE//TOyWuNNhO6vVc+rf08N+T8OFdOSRaY1uZBnsrDDSLGMDG5OLtLeAQBv3/ZED1s9j3YgzdABgpy35C2ElQCeEu2q9HQ8wAaIWo/cvCPEDdijuJqo+E7SNK/+IfM0+T8NBdYRU67sVNNabOCxkPHQ+GpPH7X/eImfq/Co1iw3iQestrVf2NQrx/vtntTJh8CYgogwWaOHNfdo5eYR++3IiwlbqWE6TQ9g== X-Microsoft-Exchange-Diagnostics: 1;AM5PR0801MB2049;31:9Zqq8G9kCk16DWKD+9GBtsgZ9riPk1HAFtdW7GYspoJ4I80z4kfmfJS36MrnB5FXIasn5u5vzza8uA0Mo5soSnjuXSI+99bme/92Pmj5a3jaPGe+IvdDN8o3nstB5f3WFWI9NdYgfyqdmVQjFWZuezs5QgF9CXaGva2dQ6dYCrHUGB+Bgf7dNgX88cTspQsPTM3E8N17PZ76G6my7xSvPWgZwM2ABv+JGvHsoH8mhZOTJWPspE8Aj+VQpZP0Bxmny1j/ZslmBFUBMobnzB1kyQ==;20:2tZOLt4WHrAQnSDTQcZfMita2RBjakBUoKgkoyb9qScoBsGtpTKcjBawey7s+QVVVMWDr+FJxme0KgTOzlYi/ec/zud/++iSRgmvzB71UwLb0WEWH2tFYMnN2sfEFIgvYLKXMuKT1txF6vIC4mAMmSzJVygFHMCStWdAgk8ky0BEFM3g8TOVHeIX3ZbzMzrzyhtMybzus1zMvGvLj/NFZRknYH3FjJaDTvCXe0puWi9jN+RIlWyLk99k451SMi5HwqF9NEwDAVqsKV2Lpc6azLiME1fJ+w2Hzn1acHqtgZobSrBbRaBs1V85Xr1sJJmTwO11C9lCwAMaD9bIKTyDpCuGta5B0jGWA3zkRKL4rjIXPi/6bT2R+xj4Wz8m54XX79GKyXHzCVWL89PhfM7PkHAeeGy2BLlCi+9ir9b70Zk= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(61668805478150)(42068640409301); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040450)(601004)(2401047)(8121501046)(5005006)(3002001)(10201501046)(93006095)(93001095)(6041248)(20161123562025)(201703131423075)(201702281528075)(201703061421075)(20161123555025)(20161123564025)(20161123560025)(6072148);SRVR:AM5PR0801MB2049;BCL:0;PCL:0;RULEID:;SRVR:AM5PR0801MB2049; X-Microsoft-Exchange-Diagnostics: 1;AM5PR0801MB2049;4:ZlexoHFtRJ2FTO9MEfa4/eSS6mUg+1+jdQQQV7v1xwvFuQiNdUMieWnqaVY4gq+1filTSGnhhW0aJC/VRFYppDkcUwwmWviNtO70vuBvYEIgURBQJxl/oD21jh1hvhAHs04NYgUqu3WUUyWNMt3CTywJ7lGTN/Jjw0VFT4DHvtrTx6qYAV4ZxTrcWktCdg+TFe4hN7tEllYBdCxAsKF3kvyxjO2ggaIEn/Gt80fVfx2xt4Z6xY0zQPWxTk1imwHyh/arT3tl9H2JbGODTx6EbSEYODUMyBvslndxtLpG1sa2RN9XQJtkW522xF5dk17mJbsCpw332xDjXc40P/0p46Pdr3BijchkLSKRq4/ba4wtllBhaXpiSJPubmMpa/9K8bIJtmQ8lFMC8hlPJPvQd/qnTSE+cLYsEyCven+FIgOSpNNNbCcWxOUaWoW45s46bln/jPfP8vW/79QsDp10KDdiNLRJoYxmynpA+Jr0HxqRHKA/DobxFLqVIJsToshX/25OprwLwZTyBp/HlZlEIv/LbOWhGsiZ/6kIHDOB1k2uTRywiRl47p+1GNUMsvZxVg58CfxsWjkl2PFBu/utQdYAuccjLVjF2OQ+KSektjO9z5vsk4ieLXub9PuXsbio1qrzivOh6b4uvvyRqk9W00seIekQXoROv8kvAtDnZvAN3p58I6THroq2GerDqXlC8OF7QVFW+feC6LpXjfl6pf7vx3hkqA/AiMrR3OW6fxfn96iF3mLMpsTK0OM8CTCNVubjv7so15a6jB9N7X0ny/ao0SQGWngjh7jtu8nWrno= X-Forefront-PRVS: 02638D901B X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6049001)(6009001)(39830400002)(39400400002)(39410400002)(39450400003)(24454002)(377454003)(86362001)(50986999)(42186005)(7416002)(7736002)(2906002)(305945005)(81166006)(31686004)(33646002)(65956001)(66066001)(65806001)(575784001)(47776003)(31696002)(76176999)(36756003)(90366009)(8676002)(189998001)(64126003)(54906002)(53546009)(50466002)(54356999)(4326008)(77096006)(6486002)(6306002)(25786009)(966004)(6246003)(110136004)(4001350100001)(53936002)(230700001)(6916009)(229853002)(6666003)(65826007)(6116002)(3846002)(2950100002)(5660300001)(38730400002)(23746002)(83506001);DIR:OUT;SFP:1102;SCL:1;SRVR:AM5PR0801MB2049;H:[172.16.25.12];FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1;AM5PR0801MB2049;23:C6Qx/9kCoxQ5hHW1UBIzECHMHSkt22b7+CV?= =?Windows-1252?Q?lYZWVdYzDAmg5v60gOeRztWh+LEvEEKs1aLqlY2Subyo4k0RLqvpFu28?= =?Windows-1252?Q?OvqhtdretzizIa84MeJVKL6aMSTHcARwWiN1Aqz8cxRQNfblcnN2f4Eu?= =?Windows-1252?Q?/vD8Wvo2/rUlP73AVNTIK3o8J/UQbfGkHSyUZHPOzScZkdppTaSCcSLc?= =?Windows-1252?Q?5tYj1H4V5JbXbD6xwJm3nGWfRuoIHi2YJ4SetPz2+DuSb5wCMXIDtpAn?= =?Windows-1252?Q?c9wA3/p4iqwDtfxb1OwyPRtb9p4EKAPGvhrHvB/+bS0w7pCWsMrCMWv1?= =?Windows-1252?Q?bHTVqhblVd0yjfMgtD186FoNeqwJXZETwS662SEY7vcVH5eKDH9xD4pQ?= =?Windows-1252?Q?xgYgKgAYmYLpK1/MTvLHlFqaAREJGJEOF1PG3DGkPg1HrYQWzDL0eHlw?= =?Windows-1252?Q?w+6opsOLyiAJBTowQ8AlWHlooIF1cytdJvucTCifubEvKuBJo5lTS0L/?= =?Windows-1252?Q?Q1l4C5FPUeuWT0WEH+TGIKey69azJQZ/KezDAy0UoKXCXRzLQieyZQEs?= =?Windows-1252?Q?XJiOvwXxEYmMfgQEW0XyLhnywiimAkWcl90eaThjFxMRX2BF6l59I7Q6?= =?Windows-1252?Q?VX25kOesSafcALYVk8Dc/Z0pQhmQvb7ScedNw9QhITt8+AE3FCdTzuZH?= =?Windows-1252?Q?hGn1qYJ5yLy2OGZMH5R3UOOuPeAygfXQhYaDRuE5ZUj+2m5pAIpU7MWO?= =?Windows-1252?Q?X0/erAXGt8hozVfE+uw09405FglHUVVE2a2WMf6zxivAj/v+iu6oBQ0Q?= =?Windows-1252?Q?ioBFNYvpa7DRmyJQsLhMg25jZPEQC5gdjg2OHeWfoNoa/YMGWTvNqbuL?= =?Windows-1252?Q?vOseMPjs/jbLBh9Z3oLErpEnp42hjygvMrpdVHbAMjNMjPPCgbAOJaE3?= =?Windows-1252?Q?uav5AVa6Cua+hf+fc1YuFTWyb0WB24H9hLRra58nYxfnG05t4SDUucOJ?= =?Windows-1252?Q?+/TuuuY6huc/gI1TN3cf1Mh03Izc1UgTCOeIVUAm23/DbmC++m5n7932?= =?Windows-1252?Q?23uNMEw1p0tbVojcCT9UGGsUiKT3YFDj8xOyC8kUI8+n4xtGvuilAWQ0?= =?Windows-1252?Q?leBR2mgEHghRuGFDZJ5+qCH4/LRd1QDgzbvcPuVCvw5GgNHDDMV0/7Kb?= =?Windows-1252?Q?a7iXVPMab+kl1YFXmTgin1fleiPtkdTtkYpF4zhN3aJHZ2QD3SvJxgaM?= =?Windows-1252?Q?hatm+mmk6gUyZFdBnbuNkhCkILgTUlvj8GqsVG0kB/edeJ+jFAYBP78s?= =?Windows-1252?Q?0H/evYJAhdWD7OUi8bCpgzmjCFngrWWejWbWAY3Z6yBEV5P/Ar252uxo?= =?Windows-1252?Q?Bz5PMun4/f0gIBvKVws8tEFjieIeYArK89+nolfLYGJymhXl29Z/FNAO?= =?Windows-1252?Q?iZCExd5HQ+grWov+PKonDlC6q0ei88mKF/CR4H3m4QKMNINQMyXovpyT?= =?Windows-1252?Q?nhhTDof8=3D?= X-Microsoft-Exchange-Diagnostics: 1;AM5PR0801MB2049;6:rP4LfbynzJ+J1qjFz5ReM2RX0FVfdF2Igi+mRFv7oOAsRCNAZCXbhHVFXNUcpChUGkQRxi6a2mzWVWJq5vB84ZbWjYYFvUCwg6hdzNJlpzoNtl5wiCGxcl/w3HiUd0cwMtL7qvSDQzv/DTcHp24YarP1BWJwNd2KlOF2AEhVL8rxyJbi2F9AK4MaFb8oTgruO7f2dbiCKGiL1ZY8ODH5wFWXPs0MnGgMewlU3PvDmp17Gi8hSq1NYPY5ZUZZ1SoQditOUmtF7ECrZzKArsHCER/YnJu8Bu7rsHTqPFnU/G74K1dytH+R8e98qKsI0lMcqfECaajM8Y6y3DEviOJyAL5PlNZIz8qUDCNTZXiwFvYppoZ5WW19vEAzfRNy9jp8XBs/hEavG52tmGnnc5J+Jg==;5:pTW2VkSN33i6xjF8auwpwCXJorV2S5hYhqC1qYI12kIyUX8MTgHf9bRY9BDIV81QkTm9ZoyG6bm1VaegN3PJ3nK5C/mISz/QZL1qWaYifUg3ISI0vnyHW17R7TKn9cOAh9rU21zuGvGoCs0orj0RzA==;24:eEIaAURqR4rK85qZWMtfmMcOzl/ovnSmf196607uQtrkCkkEM6b5HDZ/3PVr/Aj1a7m7rJDNBtapZD+FWvzoPSsxmIL4WiWklZIGpm5PSH8= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;AM5PR0801MB2049;7:ZSLKuWJCri7E5f27TYSOVEp9Px1Yvnavof3kbdlNbdvrcaFxzl4x+EVAHa4Kff6iluUvJKgqWyIr5Wli/+mnyWAYwWlaU3n8hQ+CHaWQHpAkv9wG1Ye6uyXrtvBeO6mqcEwTHGrf5yOnQxuJTOKXnSZJ4RUZ5JbIkmY04hIG35qOofKNrmX9W9GZr7DQx82uv4CnuMfJ+eVo3J6RQvXfwj7EmqYR+DHlLhv6Qa+vFgQ+G6y+d59gK8nKa2tHkRhlJ5gTLJsoHTN86KDHnqWqSgTC0zVD6/PCbKuhM6xQHwVdmQjKqi11tQzK+qMAavkMViyfsNfVt8icKK2jZE2VVw==;20:bOhHd66FxoDP4ALi65z7EV3upzHUzHV9nydtZHoqQHmeZHUe8Q2vtG2GLeZ8DengN7w2nMSJk6bvvnB5vC8j0Fn1wDj/hV2opEoNVHjV/rBJque2nZuivIZpeOdfcoSNcTNqxgfdk6Z8HG5aAQnb2TNBCfRzuVH3gaFqYcvtxj8= X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Mar 2017 09:25:29.5903 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0801MB2049 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/31/2017 01:22 AM, Andrew Morton wrote: > On Thu, 30 Mar 2017 13:27:16 +0300 Andrey Ryabinin wrote: > >> Commit 5803ed292e63 ("mm: mark all calls into the vmalloc subsystem >> as potentially sleeping") added might_sleep() to remove_vm_area() from >> vfree(), and commit 763b218ddfaf ("mm: add preempt points into >> __purge_vmap_area_lazy()") actually made vfree() potentially sleeping. >> >> This broke vmwgfx driver which calls vfree() under spin_lock(). >> >> BUG: sleeping function called from invalid context at mm/vmalloc.c:1480 >> in_atomic(): 1, irqs_disabled(): 0, pid: 341, name: plymouthd >> 2 locks held by plymouthd/341: >> #0: (drm_global_mutex){+.+.+.}, at: [] drm_release+0x3b/0x3b0 [drm] >> #1: (&(&tfile->lock)->rlock){+.+...}, at: [] ttm_object_file_release+0x28/0x90 [ttm] >> >> Call Trace: >> dump_stack+0x86/0xc3 >> ___might_sleep+0x17d/0x250 >> __might_sleep+0x4a/0x80 >> remove_vm_area+0x22/0x90 >> __vunmap+0x2e/0x110 >> vfree+0x42/0x90 >> kvfree+0x2c/0x40 >> drm_ht_remove+0x1a/0x30 [drm] >> ttm_object_file_release+0x50/0x90 [ttm] >> vmw_postclose+0x47/0x60 [vmwgfx] >> drm_release+0x290/0x3b0 [drm] >> __fput+0xf8/0x210 >> ____fput+0xe/0x10 >> task_work_run+0x85/0xc0 >> exit_to_usermode_loop+0xb4/0xc0 >> do_syscall_64+0x185/0x1f0 >> entry_SYSCALL64_slow_path+0x25/0x25 >> >> This can be fixed in vmgfx, but it would be better to make vfree() >> non-sleeping again because we may have other bugs like this one. > > I tend to disagree: adding yet another schedule_work() introduces > additional overhead and adds some risk of ENOMEM errors which wouldn't > occur with a synchronous free. > >> __purge_vmap_area_lazy() is the only function in the vfree() path that >> wants to be able to sleep. So it make sense to schedule >> __purge_vmap_area_lazy() via schedule_work() so it runs only in sleepable >> context. > > vfree() already does > > if (unlikely(in_interrupt())) > __vfree_deferred(addr); > > so it seems silly to introduce another defer-to-kernel-thread thing > when we already have one. > >> This will have a minimal effect on the regular vfree() path. >> since __purge_vmap_area_lazy() is rarely called. > > hum, OK, so perhaps the overhead isn't too bad. > > Remind me: where does __purge_vmap_area_lazy() sleep? It's cond_resched_lock(&vmap_area_lock); > > Seems to me that a better fix would be to make vfree() atomic, if poss. > > Otherwise, to fix callers so they call vfree from sleepable context. > That will reduce kernel latencies as well. > Currently we know only two such callers: ttm_object_file_release() and ttm_object_device_release(). Both of them call vfree() under spin_lock() for no reason, Thomas said that he has patch to fix this: http://lkml.kernel.org/r/f1c0b9ec-c0c8-502c-c7f0-fe692c73ab04@vmware.com So this patch is more like an attempt to address other similar bugs possibly introduced by commit 5803ed292e63 ("mm: mark all calls into the vmalloc subsystem as potentially sleeping") It's quite possible that we overlooked something. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [PATCH 1/4] mm/vmalloc: allow to call vfree() in atomic context To: Andrew Morton References: <20170330102719.13119-1-aryabinin@virtuozzo.com> <20170330152229.f2108e718114ed77acae7405@linux-foundation.org> CC: , , , , , , , , , , , , , , From: Andrey Ryabinin Message-ID: Date: Fri, 31 Mar 2017 12:26:53 +0300 MIME-Version: 1.0 In-Reply-To: <20170330152229.f2108e718114ed77acae7405@linux-foundation.org> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: On 03/31/2017 01:22 AM, Andrew Morton wrote: > On Thu, 30 Mar 2017 13:27:16 +0300 Andrey Ryabinin wrote: > >> Commit 5803ed292e63 ("mm: mark all calls into the vmalloc subsystem >> as potentially sleeping") added might_sleep() to remove_vm_area() from >> vfree(), and commit 763b218ddfaf ("mm: add preempt points into >> __purge_vmap_area_lazy()") actually made vfree() potentially sleeping. >> >> This broke vmwgfx driver which calls vfree() under spin_lock(). >> >> BUG: sleeping function called from invalid context at mm/vmalloc.c:1480 >> in_atomic(): 1, irqs_disabled(): 0, pid: 341, name: plymouthd >> 2 locks held by plymouthd/341: >> #0: (drm_global_mutex){+.+.+.}, at: [] drm_release+0x3b/0x3b0 [drm] >> #1: (&(&tfile->lock)->rlock){+.+...}, at: [] ttm_object_file_release+0x28/0x90 [ttm] >> >> Call Trace: >> dump_stack+0x86/0xc3 >> ___might_sleep+0x17d/0x250 >> __might_sleep+0x4a/0x80 >> remove_vm_area+0x22/0x90 >> __vunmap+0x2e/0x110 >> vfree+0x42/0x90 >> kvfree+0x2c/0x40 >> drm_ht_remove+0x1a/0x30 [drm] >> ttm_object_file_release+0x50/0x90 [ttm] >> vmw_postclose+0x47/0x60 [vmwgfx] >> drm_release+0x290/0x3b0 [drm] >> __fput+0xf8/0x210 >> ____fput+0xe/0x10 >> task_work_run+0x85/0xc0 >> exit_to_usermode_loop+0xb4/0xc0 >> do_syscall_64+0x185/0x1f0 >> entry_SYSCALL64_slow_path+0x25/0x25 >> >> This can be fixed in vmgfx, but it would be better to make vfree() >> non-sleeping again because we may have other bugs like this one. > > I tend to disagree: adding yet another schedule_work() introduces > additional overhead and adds some risk of ENOMEM errors which wouldn't > occur with a synchronous free. > >> __purge_vmap_area_lazy() is the only function in the vfree() path that >> wants to be able to sleep. So it make sense to schedule >> __purge_vmap_area_lazy() via schedule_work() so it runs only in sleepable >> context. > > vfree() already does > > if (unlikely(in_interrupt())) > __vfree_deferred(addr); > > so it seems silly to introduce another defer-to-kernel-thread thing > when we already have one. > >> This will have a minimal effect on the regular vfree() path. >> since __purge_vmap_area_lazy() is rarely called. > > hum, OK, so perhaps the overhead isn't too bad. > > Remind me: where does __purge_vmap_area_lazy() sleep? It's cond_resched_lock(&vmap_area_lock); > > Seems to me that a better fix would be to make vfree() atomic, if poss. > > Otherwise, to fix callers so they call vfree from sleepable context. > That will reduce kernel latencies as well. > Currently we know only two such callers: ttm_object_file_release() and ttm_object_device_release(). Both of them call vfree() under spin_lock() for no reason, Thomas said that he has patch to fix this: http://lkml.kernel.org/r/f1c0b9ec-c0c8-502c-c7f0-fe692c73ab04@vmware.com So this patch is more like an attempt to address other similar bugs possibly introduced by commit 5803ed292e63 ("mm: mark all calls into the vmalloc subsystem as potentially sleeping") It's quite possible that we overlooked something. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-f198.google.com (mail-io0-f198.google.com [209.85.223.198]) by kanga.kvack.org (Postfix) with ESMTP id 15DCC6B0038 for ; Fri, 31 Mar 2017 05:25:36 -0400 (EDT) Received: by mail-io0-f198.google.com with SMTP id s63so23957730ioe.3 for ; Fri, 31 Mar 2017 02:25:36 -0700 (PDT) Received: from EUR01-VE1-obe.outbound.protection.outlook.com (mail-ve1eur01on0120.outbound.protection.outlook.com. [104.47.1.120]) by mx.google.com with ESMTPS id r204si1985477itc.34.2017.03.31.02.25.34 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 31 Mar 2017 02:25:34 -0700 (PDT) Subject: Re: [PATCH 1/4] mm/vmalloc: allow to call vfree() in atomic context References: <20170330102719.13119-1-aryabinin@virtuozzo.com> <20170330152229.f2108e718114ed77acae7405@linux-foundation.org> From: Andrey Ryabinin Message-ID: Date: Fri, 31 Mar 2017 12:26:53 +0300 MIME-Version: 1.0 In-Reply-To: <20170330152229.f2108e718114ed77acae7405@linux-foundation.org> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: Andrew Morton Cc: penguin-kernel@I-love.SAKURA.ne.jp, linux-kernel@vger.kernel.org, mhocko@kernel.org, linux-mm@kvack.org, hpa@zytor.com, chris@chris-wilson.co.uk, hch@lst.de, mingo@elte.hu, jszhang@marvell.com, joelaf@google.com, joaodias@google.com, willy@infradead.org, tglx@linutronix.de, thellstrom@vmware.com, stable@vger.kernel.org On 03/31/2017 01:22 AM, Andrew Morton wrote: > On Thu, 30 Mar 2017 13:27:16 +0300 Andrey Ryabinin wrote: > >> Commit 5803ed292e63 ("mm: mark all calls into the vmalloc subsystem >> as potentially sleeping") added might_sleep() to remove_vm_area() from >> vfree(), and commit 763b218ddfaf ("mm: add preempt points into >> __purge_vmap_area_lazy()") actually made vfree() potentially sleeping. >> >> This broke vmwgfx driver which calls vfree() under spin_lock(). >> >> BUG: sleeping function called from invalid context at mm/vmalloc.c:1480 >> in_atomic(): 1, irqs_disabled(): 0, pid: 341, name: plymouthd >> 2 locks held by plymouthd/341: >> #0: (drm_global_mutex){+.+.+.}, at: [] drm_release+0x3b/0x3b0 [drm] >> #1: (&(&tfile->lock)->rlock){+.+...}, at: [] ttm_object_file_release+0x28/0x90 [ttm] >> >> Call Trace: >> dump_stack+0x86/0xc3 >> ___might_sleep+0x17d/0x250 >> __might_sleep+0x4a/0x80 >> remove_vm_area+0x22/0x90 >> __vunmap+0x2e/0x110 >> vfree+0x42/0x90 >> kvfree+0x2c/0x40 >> drm_ht_remove+0x1a/0x30 [drm] >> ttm_object_file_release+0x50/0x90 [ttm] >> vmw_postclose+0x47/0x60 [vmwgfx] >> drm_release+0x290/0x3b0 [drm] >> __fput+0xf8/0x210 >> ____fput+0xe/0x10 >> task_work_run+0x85/0xc0 >> exit_to_usermode_loop+0xb4/0xc0 >> do_syscall_64+0x185/0x1f0 >> entry_SYSCALL64_slow_path+0x25/0x25 >> >> This can be fixed in vmgfx, but it would be better to make vfree() >> non-sleeping again because we may have other bugs like this one. > > I tend to disagree: adding yet another schedule_work() introduces > additional overhead and adds some risk of ENOMEM errors which wouldn't > occur with a synchronous free. > >> __purge_vmap_area_lazy() is the only function in the vfree() path that >> wants to be able to sleep. So it make sense to schedule >> __purge_vmap_area_lazy() via schedule_work() so it runs only in sleepable >> context. > > vfree() already does > > if (unlikely(in_interrupt())) > __vfree_deferred(addr); > > so it seems silly to introduce another defer-to-kernel-thread thing > when we already have one. > >> This will have a minimal effect on the regular vfree() path. >> since __purge_vmap_area_lazy() is rarely called. > > hum, OK, so perhaps the overhead isn't too bad. > > Remind me: where does __purge_vmap_area_lazy() sleep? It's cond_resched_lock(&vmap_area_lock); > > Seems to me that a better fix would be to make vfree() atomic, if poss. > > Otherwise, to fix callers so they call vfree from sleepable context. > That will reduce kernel latencies as well. > Currently we know only two such callers: ttm_object_file_release() and ttm_object_device_release(). Both of them call vfree() under spin_lock() for no reason, Thomas said that he has patch to fix this: http://lkml.kernel.org/r/f1c0b9ec-c0c8-502c-c7f0-fe692c73ab04@vmware.com So this patch is more like an attempt to address other similar bugs possibly introduced by commit 5803ed292e63 ("mm: mark all calls into the vmalloc subsystem as potentially sleeping") It's quite possible that we overlooked something. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org