From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id A43F6C3DA78 for ; Tue, 17 Jan 2023 18:21:43 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 19FE76B0075; Tue, 17 Jan 2023 13:21:43 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 129276B0078; Tue, 17 Jan 2023 13:21:43 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EE5C06B007B; Tue, 17 Jan 2023 13:21:42 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id DEAD96B0075 for ; Tue, 17 Jan 2023 13:21:42 -0500 (EST) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 9E4441203A3 for ; Tue, 17 Jan 2023 18:21:42 +0000 (UTC) X-FDA: 80365109244.12.8AF28CE Received: from mail-yw1-f171.google.com (mail-yw1-f171.google.com [209.85.128.171]) by imf25.hostedemail.com (Postfix) with ESMTP id 171B0A001A for ; Tue, 17 Jan 2023 18:21:40 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=l09YM7Dg; spf=pass (imf25.hostedemail.com: domain of surenb@google.com designates 209.85.128.171 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1673979701; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=UPkI1/XPnXEYuO+vM82XNHCrnZexpEfdFeQbNCsgLlg=; b=5aMwm0o96sQZKFfVNnj+OtIf2WVdSPFTC6fwpp/umB1F4UYlQWmx6JlTFqiA3Z+hYb9Z+8 /IdVaJr6xiB/TN6AhJ7lcaD7vf1ZC0vQuSHaLfr9EmBB3F8uThHxmFmcvHAJ/iJdqlVYdE j4+eDXqPqQ03r3fP6lolYnAqj+zHCd4= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=l09YM7Dg; spf=pass (imf25.hostedemail.com: domain of surenb@google.com designates 209.85.128.171 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1673979701; a=rsa-sha256; cv=none; b=8mgaHIFowEpE/JRh39PhERX+aczCGombdTybQSpa+ENEqCUta0QyV834JUfwJppXlqbFA4 y8ug+UYu+EKpxhi7aTDHV0AxDa1mfe1o6M4POPku/CN9Ee0n3q4MGwGbqVmzruvJoZm1L7 V6fGYJ///3rFNWGQL8fM1jU5XKDpDYM= Received: by mail-yw1-f171.google.com with SMTP id 00721157ae682-4d0f843c417so320817027b3.7 for ; Tue, 17 Jan 2023 10:21:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=UPkI1/XPnXEYuO+vM82XNHCrnZexpEfdFeQbNCsgLlg=; b=l09YM7DgpJrYenLsXlM+va6cEssEkvL2Rig3ks1d+kPEMg43Yd/9i46bxaNGqXrNzU WjF1S9YWMBF02gJj8qcCis6AocP2nZVkRUVdnFKhy5DP9owLAGDlI8QpovJ1pUrlvsJf 5iE/N6rpZ0ZKIvHmwfer+HDs0VWYGFwkXi0U71ClRBTuOTTLuK579j4M/dCnSuSWepSf /nDjdkvzy41hV9c56R2hqZYfBpRF84Pgy/RtZpQb5JT0WJre7DoMqU8nMFrAk+BNJ8NI b0cHVD+ArloI/nXrbKgbrs41kZCzrBvh/ogxlblgEZoYQr3EkdK2FfU1iubwu4QJVZwy uA4g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=UPkI1/XPnXEYuO+vM82XNHCrnZexpEfdFeQbNCsgLlg=; b=BO7YRdVDGHoJz6brHJ02CieYWZlPlgtN0zwFFYzPaXCnHRio5/PG2I1HyeRkLYr51e +73zXgFNt1WWHut8imT12gwTsWm+E+sjulv1D03Dgd8tTBUqL0cyeOoWFjJLcNTloxxI pc6r0oGQMoAoA4eVupheBjuqKiE7rJ9PB6ZXBQA69zyPK85AAnDxLheJygS8NtMAJz2S 7mcAZ8XWpYYgGDmMKcumuwaz16U49z0PyrCZ7P8ouaoiWuugUXjVhpfNbnQtVr2yBtPr 9Kmx2TtqPGDjj+hXnqjLETot9DX1Zo1GE7+ZDsXoMwD8WTN2bOffYw0o2KfGVHO5qqLu 4dXg== X-Gm-Message-State: AFqh2krxBmYVlsF0UMQND0lzK7SZeaBAfQYOYFx17+qniev2Hq/W+1AV GF5cPZq9TRtCbO6ZojMGVZ4QMblEgOrfOXFAFM5ezQ== X-Google-Smtp-Source: AMrXdXtR/XHP9ovCnWbD161pkcn3Hnd8M02agqfKeuyh3ZxoQTVKeW2ejwTZly26Cbevr74QXooOy2KdhiuW5tZJObA= X-Received: by 2002:a0d:d484:0:b0:4dc:4113:f224 with SMTP id w126-20020a0dd484000000b004dc4113f224mr593908ywd.455.1673979699991; Tue, 17 Jan 2023 10:21:39 -0800 (PST) MIME-Version: 1.0 References: <20230109205336.3665937-42-surenb@google.com> <20230116140649.2012-1-hdanton@sina.com> <20230117031632.2321-1-hdanton@sina.com> <20230117083355.2374-1-hdanton@sina.com> In-Reply-To: <20230117083355.2374-1-hdanton@sina.com> From: Suren Baghdasaryan Date: Tue, 17 Jan 2023 10:21:28 -0800 Message-ID: Subject: Re: [PATCH 41/41] mm: replace rw_semaphore with atomic_t in vma_lock To: Hillf Danton Cc: vbabka@suse.cz, hannes@cmpxchg.org, mgorman@techsingularity.net, peterz@infradead.org, hughd@google.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 171B0A001A X-Rspam-User: X-Stat-Signature: e66pynta33o8aebja5m5x9kn3dc4rcop X-HE-Tag: 1673979700-952429 X-HE-Meta: U2FsdGVkX18mQGnLlZEV3dDUJ0tmN0qA5OOwoGIai8mym7zRQLYgJOo1f4Dg6pqwUYCT3U8B6Tmrppcm1vt16+DNNukEjtj4njUkvOTgcfScecZlwzZuJrGfhEfbejv14DuvU14fSggzn4Kz/SGL3r95vPbQ+yL1F2YZxyo74ORS8OaHF3F1zKrSXdButZIKJdRALwXA+dvlZGKuusPd5rIKMv9dIwbVQqMjXjiKaxfLGQmb7d00z8xlUbvheVz7zFYRPw19mFfAnjEnpWV/40oIBIYbsuO05Ed+ZFuRtPc0yc977rOGvW3KyVuKl+X0NhMEDIO5zABjOmDpBleatGLQZCa/Shl+Q2W6IwvjZWwjzIZcldOb6rQeEmmeMrya5AR0yDcoBuaa9fkhrIjfORhcoyoCfv8WE0XM2sy1yWrMydr4S5IuuEWCkTfbkx41QyWdfguWWHyV88m6khIVQfUT0QkSAmJfug28bPuHOchddI0tzglcngg/dv4lXI/XkatkYeLPUyoy7HdXz1rVqPTfm5VNdnDyVZCZSDco+B+MueB7qFlWkaqJm5QU7rC1m11GFroj/+4+21Vg/svB6HJ+ioFc63UT0gOFsP1MH6XTqT7oKozceuj4ilPM0u+Lf7K7JnTxZZhFsNT3HPmzvBgPlBPHy2PxXsi3ZOE/0il2Vwy7pSJZ71H+TJcRNRo7wJxfA/gM6GIDmIvg0p2NpzPaCZOSQo3sYn1KEwKcsC03zxcAegfGmzDRqbgGqJf0qJGLwC30DgsHWhYnLNA+fXeT8aosoIv8ZUtRlzaA4f/JEQNFHLLPKyKy2XeMLZD1ocgYuOvFvFUUFyWwXuW35BcLH/V7n9ostjToBklO/s6SbXy/wmh4VKznXzB+AFdRSEZkj6/vYImWllyfwhl1y3YQEqPP9PY0IalnHpGKPZpdgXsR+g9wIq632E16AC32rohaWc/tnkYxhwak1FB wk1Zws2d fwhnUQGfed2X+5aNmT0g4FqAAkj4tmKkh6schO2ux/hLlTMvcmYDLTIt+zg== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Jan 17, 2023 at 12:34 AM Hillf Danton wrote: > > On Mon, 16 Jan 2023 20:52:45 -0800 Suren Baghdasaryan > > On Mon, Jan 16, 2023 at 7:16 PM Hillf Danton wrote: > > > No you are not. > > > > I'm not wrong or the other way around? Please expand a bit. > > You are not wrong. Ok, I think if I rewrite the vma_read_trylock() we should be fine?: static inline bool vma_read_trylock(struct vm_area_struct *vma) { int count, new; /* Check before locking. A race might cause false locked result. */ if (READ_ONCE(vma->vm_lock->lock_seq) == READ_ONCE(vma->vm_mm->mm_lock_seq)) return false; count = atomic_read(&vma->vm_lock->count); for (;;) { /* * Is VMA is write-locked? Overflow might produce false locked result. * False unlocked result is impossible because we modify and check * vma->vm_lock_seq under vma->vm_lock protection and mm->mm_lock_seq * modification invalidates all existing locks. */ if (count < 0) return false; new = count + 1; /* If atomic_t overflows, fail to lock. */ if (new < 0) return false; /* * Atomic RMW will provide implicit mb on success to pair with smp_wmb in * vma_write_lock, on failure we retry. */ new = atomic_cmpxchg(&vma->vm_lock->count, count, new); if (new == count) break; count = new; cpu_relax(); } if (unlikely(READ_ONCE(vma->vm_lock->lock_seq) == READ_ONCE(vma->vm_mm->mm_lock_seq))) { if (atomic_dec_and_test(&vma->vm_lock->count)) wake_up(&vma->vm_mm->vma_writer_wait); return false; } return true; } > > > > > > If the writer lock owner is preempted by a reader while releasing lock, > > > > > > set count to zero > > > <-- preempt > > > wake up waiters > > > > > > then lock is owned by reader but with read waiters. > > > > > > That is buggy if write waiter starvation is allowed in this patchset. > > > > I don't quite understand your point here. Readers don't wait, so there > > can't be "read waiters". Could you please expand with a race diagram > > maybe? > > cpu3 cpu2 > --- --- > taskA bond to cpu3 > down_write(&mm->mmap_lock); > vma_write_lock L > taskB fail to take L for read > taskC fail to take mmap_lock for write > taskD fail to take L for read > vma_write_unlock_mm(mm); > > preempted by taskE > taskE take L for read and > read waiters of L, taskB and taskD, > should be woken up > > up_write(&mm->mmap_lock); Readers never wait for vma lock, that's why we have only vma_read_trylock and no vma_read_lock. In your scenario taskB and taskD will fall back to taking mmap_lock for read after they failed vma_read_trylock. Once taskA does up_write(mmap_lock) they will be woken up since they are blocked on taking mmap_lock for read. >