From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1761690AbdKQUvj (ORCPT ); Fri, 17 Nov 2017 15:51:39 -0500 Received: from mail-he1eur01on0121.outbound.protection.outlook.com ([104.47.0.121]:26896 "EHLO EUR01-HE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1161270AbdKQURG (ORCPT ); Fri, 17 Nov 2017 15:17:06 -0500 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=ktkhai@virtuozzo.com; Subject: Re: [PATCH] net: Convert net_mutex into rw_semaphore and down read it on net->init/->exit To: "Eric W. Biederman" Cc: Cong Wang , David Miller , vyasevic@redhat.com, kstewart@linuxfoundation.org, pombredanne@nexb.com, Vladislav Yasevich , mark.rutland@arm.com, Greg KH , Alexey Dobriyan , Florian Westphal , Nicolas Dichtel , roman.kapl@sysgo.com, Paul Moore , David Ahern , Daniel Borkmann , lucien xin , Matthias Schiffer , rshearma@brocade.com, LKML , Linux Kernel Network Developers , avagin@virtuozzo.com, gorcunov@virtuozzo.com References: <151066759055.14465.9783879083192000862.stgit@localhost.localdomain> <88152c11-a5b5-90f8-be46-99ed6c722064@virtuozzo.com> <87shdg8bzd.fsf@xmission.com> <8c808278-1925-37c0-619c-87bd1802790a@virtuozzo.com> <06b1d740-d443-ac23-a7b0-675e7b6ff6f9@virtuozzo.com> <87r2sz33n4.fsf@xmission.com> <24d2557d-5790-2a69-3758-d155e2b940be@virtuozzo.com> <87mv3kwxel.fsf@xmission.com> From: Kirill Tkhai Message-ID: <9879eb2d-a8eb-cfc0-45b4-3cb28e97e156@virtuozzo.com> Date: Fri, 17 Nov 2017 23:16:53 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <87mv3kwxel.fsf@xmission.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [128.69.179.206] X-ClientProxiedBy: AM5PR0602CA0006.eurprd06.prod.outlook.com (2603:10a6:203:a3::16) To HE1PR0801MB1338.eurprd08.prod.outlook.com (2603:10a6:3:39::28) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 4857a367-26c5-4495-74b4-08d52df82830 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001)(4534020)(4602075)(7168020)(4627115)(201703031133081)(201702281549075)(2017052603199);SRVR:HE1PR0801MB1338; X-Microsoft-Exchange-Diagnostics: 1;HE1PR0801MB1338;3:kiy0hSePJbZbkIM8sTYUKXsV43lRGX2LypX3qZVntyiumwpSWTWOEPSlOb9o1okvYcmkQy6r4nS50bg7YYYHd8iDcJeo9RzQ/2XMAUfS9UZgbAA/WDrdS8NAnCsjrP+mFaLu8GZ/4GFvCkG+73pHvRDGRXwwT8F/QEcs0OYzZcSVSAph+murOeEdCqadWzelhPCIkdLNatZLcM2atZBTeR+uwe99ZAWIwtKSsJANGwqT0EXLbKJNqH3X11cgewHg;25:8pTmAbhtz+jlVTdEGt4eU+Rjkx9eKf0XN7YMSONrTkYrcHCYvVBrC+g8sxyL7MDWF7JkKuQWuDW+O2XhudSumg2NnS+M4SmHqGj+mrI4j9tHpzr2eN+IJE77CW9NA0eZZkTBF/a4Im7SOgDfMvPrJxKKQpCqwvb8Hkw/u819ekL546DfulzSZIHygW4qKlLB0yxzTxTe52DuEOi1w5B7nmlWkik3jjOehrMGs4whHLWyLzZfGNUZ605kwT70z3FDRtFIptSfK/UpUgAQbVb7Z4fmSAwUyHo+zY4PRudT6UbLNAD9FR0B3CS5mZgAk2QL1/3Olw2HFNoJj7j58nOZCQ==;31:baa/qBUWGefOfxiowcHyoTe/KHHcjLuPxPJKLXIopYC/Ej754JC5fzPmr+oMqX5GZtoSaZ/1HoJw+8tTvSizdeOabKxJ7ILOcc38C8no/CUj1QIZ5RqZP7t/P0GeY3HI7fNiw65S1qIyNCJ3HCcAZO509MvPmNIvySDDZ8kwrhUAfDCDhIxjCu6TgyDLR3omRh7WAckBm7/EUoO54+v3A9JRvX/H4oDP2qZotLZW+2c= X-MS-TrafficTypeDiagnostic: HE1PR0801MB1338: X-Microsoft-Exchange-Diagnostics: 1;HE1PR0801MB1338;20:kn0JytcObv9DNAPw0BxcbJA215/Uv63gZd7UP2axOupEWTb3+A+dZ+HwlF6A60yTGIFz6YkSxulqZIMOPLrm/dCWm4JSt3ajTZ8nOqnudNv3gxK0xQwiaaWp04qp+N3GzIpx10BESHnOvveVikuk6IpAV1MxrDhmJk/siT2+Nwk6S2SuyMjvJ+jmeefjHrSudtYlW6DnKZ6hHcUv7eRnUP5IU3xP02gLoq7qS8+VajpV7lylbvL6079n5Dv3iypgqb5fkUlIcwCEMewZ+Otnw6eFMfAB7HwrGu1nikOMl+BVcsTY4yx92cS0EfyJf5rs0C0Vwd6m90qltD3m3cYiXLpT7HJDKV+BZuRbQs8V9uszUlsdQx1Xk8pV80DeGuX4NOWyoMdf6TcNl1yNd7R81wuKeEwCKGEeW3FjYcIKgK4=;4:DxoESBU0jUpX0gL/DYqMWqXXRxMKddWyBMvZZ6+vZXa2qx7A+6K25oLmmpinxZNryrEnUZdbSHkT1ftPGQOQERNrebusEVYmxggU4mmiE+Fo00uwEH63cac/GnFZyXnpIYe8LqjEQMka5Qfh/gYAm3yF+5mvSET+GdP0piVXEwWTOBLQgvdV2+G5MO0qDFLdKxAUBsdbNPLztwFWImKpeaxzOVXEBN2dF7AhvnXpP1+cv0HKFDRQMVH4xmTm4oYbZ93y5wDr7nMRmGx8vJTaoQ== X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(5005006)(8121501046)(10201501046)(3231022)(93006095)(93001095)(100000703101)(100105400095)(3002001)(6041248)(20161123564025)(20161123562025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123560025)(20161123558100)(20161123555025)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);SRVR:HE1PR0801MB1338;BCL:0;PCL:0;RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);SRVR:HE1PR0801MB1338; X-Forefront-PRVS: 049486C505 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(6069001)(6009001)(376002)(39830400002)(346002)(199003)(24454002)(189002)(58126008)(97736004)(6666003)(31696002)(7736002)(68736007)(8676002)(83506002)(65956001)(47776003)(65806001)(7416002)(50986999)(66066001)(5660300001)(76176999)(54356999)(305945005)(53546010)(6116002)(65826007)(3846002)(2950100002)(33646002)(23676003)(107886003)(81156014)(101416001)(6916009)(25786009)(230700001)(478600001)(64126003)(81166006)(105586002)(106356001)(6512007)(8936002)(31686004)(2906002)(36756003)(53936002)(229853002)(86362001)(316002)(4326008)(39060400002)(6246003)(189998001)(50466002)(6506006)(93886005)(6486002)(16526018)(55236003)(54906003);DIR:OUT;SFP:1102;SCL:1;SRVR:HE1PR0801MB1338;H:localhost.localdomain;FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtIRTFQUjA4MDFNQjEzMzg7MjM6bmIyR2w0N1VmODNIVGs0bEJzYmZXRUxa?= =?utf-8?B?Qys1KzMweDM0ZUdjaTRzRjZ2QmY3Q1VNNjRFM1NvNXZVRFRGMkdhQ2h6RWNE?= =?utf-8?B?RTJheG15NVFjRFJGOElYR2ZOZ3c1QUR6blVtaHFjMDNWdzZjaTB4QmtJYUIr?= =?utf-8?B?VDVUd0pPQVJYWVEzTmY4cXRjczBXRnB0aXlaaG1UQUJiR1gyY0VzK2N2dUN4?= =?utf-8?B?YTd0cUdZYjdQZnFwWEhRaHJmR0tydG82dDlJanlTUzhvQlBZZkZ4YTl1Sm9G?= =?utf-8?B?V1R5VnJSVVo2S2xNc1h0V1pKb3BUT2VmVk1lbW1QYUJyTGwzOFRhdVh6SGgw?= =?utf-8?B?Zit0a0I5STZkd2ZBZGRiSzUwbldhR2c3MFFzWEwwYTc3VTNvTTNOdDVVNUI4?= =?utf-8?B?Qjg1dmNwTFpSbW95b0lVU2I4blVTMmFGcHZZbEhEaTZ2bFZlT041OVBYTW9m?= =?utf-8?B?bG5sdEl1cUJnUnpmVkl5MGFRZnZBRmRhdE5PV2I3UkhZZW1vUW5hRHZZYnh4?= =?utf-8?B?cjhnTGVzcXJLUUN6L2Y3azFzMmxiSDdrUTdrTUZmV0FHVmxoeWNOZG1uNDdW?= =?utf-8?B?cW1scWZxUlJodFVRNGlmUVpzQy9LZjFORFRZaWM3NkxsNHk0Zk9nNktROWxw?= =?utf-8?B?ZFpSa3drdDd3bVZrOVhpNyswTEVIenBCcHVXWEtiRzBqYS9DcUhwTkxEN1cv?= =?utf-8?B?U0JnTGUrZnE0TkZBaHllNnFPemJMMnpCLzRDclQxUnlkRHFuOW1VbThkSUVR?= =?utf-8?B?dHFnR0dRNVdsd1NmdUl3a3FubEZsdkw1OHdsdDkrQVpVcHRjZE0yc1ZNNnp3?= =?utf-8?B?UzFMYU8rUlNpYzUzWUd0RndFNTJpeVhFQXNpTEpjKzdYWUZoTkJpdDNGd2Va?= =?utf-8?B?b2w2VGxnVlBRcWVKeHljS3M5cTE1b04zYW9VYU1YT0g0d0RvdU5KV3dFaGxT?= =?utf-8?B?VHpUeWptcHUzS1o4QU9Ydk9Xc2FjOFZpNlVQYUJMWjljMFNMV0lpVzJJVm4z?= =?utf-8?B?UW9ETmJQYWdLSkxTcnZZNXNnOEh6dWc2dUtSdm9aaG1qOE5SVEo2bjlsRjBC?= =?utf-8?B?T1kwMmtkQkV3ekV5L2hjVHNJNVd6bUY3dDcvYlZLeCthS0dkVzAvWENMcXBV?= =?utf-8?B?US83cVIwalE2Y2I0OFl1czNUYjBrSFVHQ1lBNm1rVVBUY2p1WTZ3NTBqMnZ0?= =?utf-8?B?UUgzYVUrMUxpSTZQRlE0dUdMWHhpeDJMY2xOdFRQdGM4TncyY1g2aGhlZXFp?= =?utf-8?B?QW44MnNoamRtQmRSUTdQVHUvb1lPQjRrZlBoT1Q5ZWdFRjhXL3VtK3VOZDBh?= =?utf-8?B?YnJ4NVhRajlUWFVRaHpVU2JRM3ZuUElnOW9JbUgyblhGTzdiWUlNcFRmakkv?= =?utf-8?B?T0M2STNwZ1IwY25MOVdxTXNzV1RqOE5CVm9GajFGZzdzcm9ON2YwZDZyd09o?= =?utf-8?B?blNveldZaFVDZjE5bFhFWjZYNG5lR0FzUzBnUjA4V2RxRFVsQ1BuMlo1ZS9k?= =?utf-8?B?azZwYmF4amN0Mnc3bzlwZW9TdjMrUjQvekJDaDRheU42VXlmU3ZDY1RXU0ZX?= =?utf-8?B?SnA3MWRCTlNTSHdTZWVOTTJnS09FelJCK1llSlFPa0ZadFZ0dE1xdjFzZVhL?= =?utf-8?B?QXU3Z1h4aUtld1BUaFFDVUUxeXZLRnNINmloeFZ4ZEM1R0JHL0h1eVUrOGtW?= =?utf-8?B?L2E2NmFDVUQ0cDBVejcvZGsvVWdBSFlUZDhqUTZzOTM0TXpyR25icUpIUHV0?= =?utf-8?B?RkM3bmlnQVltaGVDK2RNbzU5UFRPcVRJQXVSMmQxTFVkV0tLS1FRSUxTc1E3?= =?utf-8?B?RzI2Y1IrQ0FvemFpYUc1TktGZEU4T2N5Tmw2dmlzOUI5ZDU0L29mNkd5aE1S?= =?utf-8?B?YUZEN3BGVWd2SVhuZEZ5Z1pUZ0d0emZxQ0NvNXZkWGxSQUZXbkVDMDMyRWdD?= =?utf-8?B?bnZBQmxkaTFyOUltYmozUmhYbmtZZ3h4RzY0UGpQOUNpVWRFU3haYmNwenJk?= =?utf-8?Q?uAV9VX13?= X-Microsoft-Exchange-Diagnostics: 1;HE1PR0801MB1338;6:FKaZNMujXte7VLVjU6ozSGEhKecxNk351ycWWmxKrwuxPNtiAQZ0Z0ZMz1pl6WonYQv/exfMyHdcoQbE/jylAkVuBLgnI6HFySVaDCh5+Ma0okBtGbU5ZL5g53MovLHp6WcQnvkoP6YB4LcGKQTLXg8c4p+spcLputpPBVnMngXecJJVC/uVsMDF9MpiyUFyjv40t2+3akv9urwFl5YHvHaFeOGQlgLcw9tvf2RYxbTVs4Yg75YK6WeX1vK3diMqOAtR4IZoM91P4Uh5QBE1t3x935fxu2rLImpnVgiKEhoThYCYG0Bt9gS7CE79e5c5ce0OzJhHb13hb2EoOY5YW8wOPFrPgjVa1+GPslF8O7s=;5:MIJB9K0GAJT64mrIOwG/V3PgtnwZEp/JEGMGMZt0O56JY4jkwXp5k6JEMGDhw9wqIRg1BQPgS0rE1QkAJHxA0LiJwza7DEZggY4e2iKrJJrIEOw5RPoTfL03Md940Qf9dkPXD4m3XclXBll7s9Gz+3Wl2lxb4rAzJKiMCpaYR2A=;24:alnng1g9uFdTuSOwMO+cBX2NEwnh4JsMc/9gc7t8OVnoZOf7d3kzlea/sC5CDmvSeodvUmu5QldYkr0WbJRQtreTYH4QJnbLz6BLYMzae1c=;7:51pwT1NEiLZVyYhvBlGuOmtwvOmTDxgfYytZJzoN25cf+spywy2pTbh2nE8h6tXaiM16cblEtFKg9M98FZCyEgf1mgxTDBHbgQnQfOrAcgQvfB5AuCL6zI9yBcxDS/UjF/bjRI6D14k30+G0svNhzvOChAQdTPGXl/Nqu6Fbk02HQbvOYheJGFobCI4rt19ORmxR0dYtzU9p1SMaWOE6REAtNBEYuGXkkf0xRmUcQKly12zKfu0sCf0bd9RNxq4s SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;HE1PR0801MB1338;20:RuM+R4vkSJ29hOMxTJ7gr8MCl/bN2U1/8mQ14QtTJuSX6xdXlxCO2nxb3ETlRfbYxDewCkpqHMjoTBn1d7QqHeIIUXET1W3WFxEZPBJUjltUkRTUc2+0ozmaTvYc5ohs5xGkKy92H1maJhLK8yZ+kqJC+yQaKaC2mzGfPGY5Klg= X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Nov 2017 20:16:56.3601 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 4857a367-26c5-4495-74b4-08d52df82830 X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 0bc7f26d-0264-416e-a6fc-8352af79c58f X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0801MB1338 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 17.11.2017 21:52, Eric W. Biederman wrote: > Kirill Tkhai writes: > >> On 15.11.2017 19:31, Eric W. Biederman wrote: >>> Kirill Tkhai writes: >>> >>>> On 15.11.2017 12:51, Kirill Tkhai wrote: >>>>> On 15.11.2017 06:19, Eric W. Biederman wrote: >>>>>> Kirill Tkhai writes: >>>>>> >>>>>>> On 14.11.2017 21:39, Cong Wang wrote: >>>>>>>> On Tue, Nov 14, 2017 at 5:53 AM, Kirill Tkhai wrote: >>>>>>>>> @@ -406,7 +406,7 @@ struct net *copy_net_ns(unsigned long flags, >>>>>>>>> >>>>>>>>> get_user_ns(user_ns); >>>>>>>>> >>>>>>>>> - rv = mutex_lock_killable(&net_mutex); >>>>>>>>> + rv = down_read_killable(&net_sem); >>>>>>>>> if (rv < 0) { >>>>>>>>> net_free(net); >>>>>>>>> dec_net_namespaces(ucounts); >>>>>>>>> @@ -421,7 +421,7 @@ struct net *copy_net_ns(unsigned long flags, >>>>>>>>> list_add_tail_rcu(&net->list, &net_namespace_list); >>>>>>>>> rtnl_unlock(); >>>>>>>>> } >>>>>>>>> - mutex_unlock(&net_mutex); >>>>>>>>> + up_read(&net_sem); >>>>>>>>> if (rv < 0) { >>>>>>>>> dec_net_namespaces(ucounts); >>>>>>>>> put_user_ns(user_ns); >>>>>>>>> @@ -446,7 +446,7 @@ static void cleanup_net(struct work_struct *work) >>>>>>>>> list_replace_init(&cleanup_list, &net_kill_list); >>>>>>>>> spin_unlock_irq(&cleanup_list_lock); >>>>>>>>> >>>>>>>>> - mutex_lock(&net_mutex); >>>>>>>>> + down_read(&net_sem); >>>>>>>>> >>>>>>>>> /* Don't let anyone else find us. */ >>>>>>>>> rtnl_lock(); >>>>>>>>> @@ -486,7 +486,7 @@ static void cleanup_net(struct work_struct *work) >>>>>>>>> list_for_each_entry_reverse(ops, &pernet_list, list) >>>>>>>>> ops_free_list(ops, &net_exit_list); >>>>>>>>> >>>>>>>>> - mutex_unlock(&net_mutex); >>>>>>>>> + up_read(&net_sem); >>>>>>>> >>>>>>>> After your patch setup_net() could run concurrently with cleanup_net(), >>>>>>>> given that ops_exit_list() is called on error path of setup_net() too, >>>>>>>> it means ops->exit() now could run concurrently if it doesn't have its >>>>>>>> own lock. Not sure if this breaks any existing user. >>>>>>> >>>>>>> Yes, there will be possible concurrent ops->init() for a net namespace, >>>>>>> and ops->exit() for another one. I hadn't found pernet operations, which >>>>>>> have a problem with that. If they exist, they are hidden and not clear seen. >>>>>>> The pernet operations in general do not touch someone else's memory. >>>>>>> If suddenly there is one, KASAN should show it after a while. >>>>>> >>>>>> Certainly the use of hash tables shared between multiple network >>>>>> namespaces would count. I don't rembmer how many of these we have but >>>>>> there used to be quite a few. >>>>> >>>>> Could you please provide an example of hash tables, you mean? >>>> >>>> Ah, I see, it's dccp_hashinfo etc. >> >> JFI, I've checked dccp_hashinfo, and it seems to be safe. >> >>> >>> The big one used to be the route cache. With resizable hash tables >>> things may be getting better in that regard. >> >> I've checked some fib-related things, and wasn't able to find that. >> Excuse me, could you please clarify, if it's an assumption, or >> there is exactly a problem hash table, you know? Could you please >> point it me more exactly, if it's so. > > Two things. > 1) Hash tables are one case I know where we access data from multiple > network namespaces. As such it can not be asserted that is no > possibility for problems. > > 2) The responsible way to handle this is one patch for each set of > methods explaining why those methods are safe to run in parallel. > > That ensures there is opportunity for review and people are going > slowly enough that they actually look at these issues. > > The reason I want to see this broken up is that at 200ish sets of > methods it is too much to review all at once. Ok, it's possible to split the changes in 400 patches, but there is a problem with three-state (no compile, module, built-in) drivers. Git bisect won't work anyway. Please see the description of the problem in cover message "[PATCH RFC 00/25] Replacing net_mutex with rw_semaphore" I sent today. > I completely agree that odds are that this can be made safe and that it > is mostly likely already safe in practically every instance. My guess > would be that if there are problems that need to be addressed they > happen in one or two places and we need to find them. If possible I > don't want to find them after the code has shipped in a stable release. Kirill