Received: by 2002:ac0:a5a6:0:0:0:0:0 with SMTP id m35-v6csp2862994imm; Fri, 24 Aug 2018 06:42:04 -0700 (PDT) X-Google-Smtp-Source: ANB0VdaYDVA7UudaZgWcuh/vxu+i117Y1o5DloiEEQYo6TJH5Gk4T6EF/QbXHUPdRqeyux/86MWq X-Received: by 2002:a17:902:d211:: with SMTP id t17-v6mr1804013ply.258.1535118123935; Fri, 24 Aug 2018 06:42:03 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1535118123; cv=none; d=google.com; s=arc-20160816; b=KV1H6RQyGbK9TGMv7u86GFqGUbKU7nfTo+ak7dH4Fb7UjGrXzAPLpoB3e0yxjcZB7f c2SIoz2eeWOZllVIHs4tscjO+34K3/N6Sb/az8xwMengXkIJfqNjKJL14sce1q9KxWyp NFS6gPanlw8k6CiYyxyZpUOrfUB+e+24tDbaC6ZGUShpu7q9JDzMDxEE1KNEz64UAknm szCDyOmkQbmvxpT5fqWSgDXSk8emeFI8WE1lf3vmEVE5qTK9meBzrzNTC99msIICX8EX VBBzXrMcTLC77uPec7LGkg+JNi1wyf9PQQkJojEIi3OxQVNdoFc4l5b11EGtxf844zVn CVIw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-transfer-encoding:content-disposition:mime-version :references:message-id:subject:cc:to:from:date :arc-authentication-results; bh=tZdLIBlJQ8BVinKaDBVyEvml/SX8YGwv2NcEYomacN8=; b=pJnZ7bnJch6bgAfbl+hdNYZ1QGJoFikxewSxGpMUynGj2hppItgY55q9no0hX3XCkA TRr0YFA3zmf0mgFzWq9RrpxtzdiIJlPlqEtDdhL2qBVYRd/IyTH1QzOrjjkvaVrDn64C YnmtIDtJejGDu7J+WgqS2C+3yF0y16Fzi1GzRzckM/ScTgxuJA7arSMeDqWFAdOr7vtN YEZ4mdrUamrB71xplYslOJAB+XSgo8AabVPVc0NwwqyQV6tyLs8DIinJ+hz2TKSz6zPK pq5iCvNdw2M/CRxy6PURcsaxhMbCDh6oEwne/4Kv8DHimTKGfwZ6EJUj79HwtAtPPpQH 1m+w== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id 1-v6si6935480plm.34.2018.08.24.06.41.47; Fri, 24 Aug 2018 06:42:03 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727720AbeHXRO7 (ORCPT + 99 others); Fri, 24 Aug 2018 13:14:59 -0400 Received: from mx2.suse.de ([195.135.220.15]:54986 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726969AbeHXRO6 (ORCPT ); Fri, 24 Aug 2018 13:14:58 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id E3956AE06; Fri, 24 Aug 2018 13:40:12 +0000 (UTC) Date: Fri, 24 Aug 2018 15:40:09 +0200 From: Michal Hocko To: christian.koenig@amd.com Cc: kvm@vger.kernel.org, Radim =?utf-8?B?S3LEjW3DocWZ?= , Tetsuo Handa , Sudeep Dutt , dri-devel@lists.freedesktop.org, linux-mm@kvack.org, Andrea Arcangeli , Dimitri Sivanich , Jason Gunthorpe , linux-rdma@vger.kernel.org, amd-gfx@lists.freedesktop.org, David Airlie , Doug Ledford , David Rientjes , xen-devel@lists.xenproject.org, intel-gfx@lists.freedesktop.org, Leon Romanovsky , =?iso-8859-1?B?Suly9G1l?= Glisse , Rodrigo Vivi , Boris Ostrovsky , Juergen Gross , Mike Marciniszyn , Dennis Dalessandro , LKML , Ashutosh Dixit , Alex Deucher , Paolo Bonzini , Andrew Morton , Felix Kuehling Subject: Re: [PATCH] mm, oom: distinguish blockable mode for mmu notifiers Message-ID: <20180824134009.GS29735@dhcp22.suse.cz> References: <20180824115226.GK29735@dhcp22.suse.cz> <20180824120339.GL29735@dhcp22.suse.cz> <20180824123341.GN29735@dhcp22.suse.cz> <20180824130132.GP29735@dhcp22.suse.cz> <23d071d2-82e4-9b78-1000-be44db5f6523@gmail.com> <20180824132442.GQ29735@dhcp22.suse.cz> <86bd94d5-0ce8-c67f-07a5-ca9ebf399cdd@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <86bd94d5-0ce8-c67f-07a5-ca9ebf399cdd@gmail.com> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri 24-08-18 15:28:33, Christian K?nig wrote: > Am 24.08.2018 um 15:24 schrieb Michal Hocko: > > On Fri 24-08-18 15:10:08, Christian K?nig wrote: > > > Am 24.08.2018 um 15:01 schrieb Michal Hocko: > > > > On Fri 24-08-18 14:52:26, Christian K?nig wrote: > > > > > Am 24.08.2018 um 14:33 schrieb Michal Hocko: > > > > [...] > > > > > > Thiking about it some more, I can imagine that a notifier callback which > > > > > > performs an allocation might trigger a memory reclaim and that in turn > > > > > > might trigger a notifier to be invoked and recurse. But notifier > > > > > > shouldn't really allocate memory. They are called from deep MM code > > > > > > paths and this would be extremely deadlock prone. Maybe Jerome can come > > > > > > up some more realistic scenario. If not then I would propose to simplify > > > > > > the locking here. We have lockdep to catch self deadlocks and it is > > > > > > always better to handle a specific issue rather than having a code > > > > > > without a clear indication how it can recurse. > > > > > Well I agree that we should probably fix that, but I have some concerns to > > > > > remove the existing workaround. > > > > > > > > > > See we added that to get rid of a real problem in a customer environment and > > > > > I don't want to that to show up again. > > > > It would really help to know more about that case and fix it properly > > > > rather than workaround it like this. Anyway, let me think how to handle > > > > the non-blocking notifier invocation then. I was not able to come up > > > > with anything remotely sane yet. > > > With avoiding allocating memory in the write lock path I don't see an issue > > > any more with that. > > > > > > All what the write lock path does now is adding items to a linked lists, > > > arrays etc.... > > Can we change it to non-sleepable lock then? > > No, the write side doesn't sleep any more, but the read side does. > > See amdgpu_mn_invalidate_node() and that is where you actually need to > handle the non-blocking flag correctly. Ohh, right you are. We already handle that by bailing out before calling amdgpu_mn_invalidate_node in !blockable mode. So does this looks good to you? diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c index e55508b39496..48fa152231be 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c @@ -180,11 +180,15 @@ void amdgpu_mn_unlock(struct amdgpu_mn *mn) */ static int amdgpu_mn_read_lock(struct amdgpu_mn *amn, bool blockable) { - if (blockable) - mutex_lock(&amn->read_lock); - else if (!mutex_trylock(&amn->read_lock)) - return -EAGAIN; - + /* + * We can take sleepable lock even on !blockable mode because + * read_lock is only ever take from this path and the notifier + * lock never really sleeps. In fact the only reason why the + * later is sleepable is because the notifier itself might sleep + * in amdgpu_mn_invalidate_node but blockable mode is handled + * before calling into that path. + */ + mutex_lock(&amn->read_lock); if (atomic_inc_return(&amn->recursion) == 1) down_read_non_owner(&amn->lock); mutex_unlock(&amn->read_lock); -- Michal Hocko SUSE Labs