From patchwork Mon May 14 11:32:08 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 10398025 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 1A5B5600D0 for ; Mon, 14 May 2018 11:32:26 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 09A2E29103 for ; Mon, 14 May 2018 11:32:26 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id F180029112; Mon, 14 May 2018 11:32:25 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 65DD429103 for ; Mon, 14 May 2018 11:32:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752253AbeENLcK (ORCPT ); Mon, 14 May 2018 07:32:10 -0400 Received: from mail-yb0-f193.google.com ([209.85.213.193]:37837 "EHLO mail-yb0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751198AbeENLcJ (ORCPT ); Mon, 14 May 2018 07:32:09 -0400 Received: by mail-yb0-f193.google.com with SMTP id i13-v6so3999866ybl.4; Mon, 14 May 2018 04:32:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=N9xLY/7I8hyUm3pyYgzh03ArvqLzr6PpX0ywKpGIdQI=; b=tVPQv5nRkDKcvxydwelN2+UV5FMGyx0IQipPtkIsDXiRjZoV1BXI3em+bV1Gx/yML9 F/jfFedESmmFd0upJg0dVdb9cfMjzjcJ8Kyp8uox0fNpVOpTc0YIlTkOzx5R7eMKioOh wX8daFcRQ8Apcgy/z8AvOL1VPE5JaEF+RAWNZdmCD28efLv7cxxn1s3+U6AlewRIrwCp g29GHXQ4k+Otw/yyvg8bD+fC60sMIDMh/S33M9dDhYj74NHndvf5oCseGkkqoHmZR0Tp oKMfuUOzzWmA4XjTlELbGEipCbxIgZMyFFdxn6wCwpqOGkn8oEokyzPgn9W2wjPCkGRe rW9A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=N9xLY/7I8hyUm3pyYgzh03ArvqLzr6PpX0ywKpGIdQI=; b=aTBIVKto1mHZuEpKAleyBP0HNIFGOgliLDi5LyNEUDdw2d/9fqh43BXXXTLpOe5acv x0RAyLEqif69iIfjhUIR4TEnMWL/DHArpExrUnhaAz9ZgTgX8nVw4ChJ13v9OhAfV17e 80wAynVCEDZqP73qyuNDMTNNInQgqjuN7UV51pz3Tz+xTaHxCIkqjBj17+iugbavsLC9 NC3yAK1Itaplicla6D2Jp7SGoqaphBf108TjZwCqWGGrpcJgaQomRTBn2gpHusjkyRga Sq30KprJ2gDtIepKV24ypHT4gDc5yNRBwrkcJb9MwJOEz4jgUC4Vs9iGhGFlSmXQ3jFV hlNQ== X-Gm-Message-State: ALKqPwflO3YXArpvo9/76+cVePnf7AmvS593EaFGtjdPa7qXiKnAiXR3 pMnNupVBQt3MMDFUY5fmuElalxYiy+6jviMQ5w0= X-Google-Smtp-Source: AB8JxZqRHpJaPiQJqJEInLgrtVtIXtSCcn6TSO9BFWhLc1IVt5tkJj+wb90gCUWOUvxDuooxwGh0Okm1qQ4hRLBkdpA= X-Received: by 2002:a25:e485:: with SMTP id b127-v6mr6662341ybh.162.1526297528905; Mon, 14 May 2018 04:32:08 -0700 (PDT) MIME-Version: 1.0 Received: by 10.13.239.198 with HTTP; Mon, 14 May 2018 04:32:08 -0700 (PDT) In-Reply-To: <20180514095022.nfp7vuepz4kfshni@quack2.suse.cz> References: <20180513160404.GA19620@redhat.com> <20180514095022.nfp7vuepz4kfshni@quack2.suse.cz> From: Amir Goldstein Date: Mon, 14 May 2018 14:32:08 +0300 Message-ID: Subject: Re: DEBUG_RWSEMS warning from thaw_super() To: Jan Kara Cc: Oleg Nesterov , Waiman Long , Ingo Molnar , linux-fsdevel , linux-kernel , Al Viro Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, May 14, 2018 at 12:50 PM, Jan Kara wrote: > On Sun 13-05-18 18:04:04, Oleg Nesterov wrote: >> On 05/13, Amir Goldstein wrote: >> > >> > Since kernel v4.17-rc1 and DEBUG_RWSEMS, I see the >> > warning below after filesystem freeze/thaw. >> > >> > This is a case where one process acquires a bunch of rwsem >> > and another process releases them. >> > >> > To convey this use case to lockdep, freeze_super() calls >> > lockdep_sb_freeze_release() on exit and thaw_super() >> > calls lockdep_sb_freeze_acquire() on entry. >> >> This was already discussed, but I forgot the result... >> >> So once again, why we can't simply update percpu_rwsem_acquire() ? >> Or we can check CONFIG_RWSEM_SPIN_ON_OWNER to match percpu_rwsem_release(), >> but CONFIG_DEBUG_RWSEMS explains the purpose better. > > Yeah, what you suggests seems reasonable to me. So feel free to add: > > Acked-by: Jan Kara > How about this version? A bit more prudent and also addresses the TODO in commit 55cc156505f2 ("percpu-rwsem: introduce percpu_rwsem_release() and percpu_rwsem_acquire()") Sorry for spaces instead of tabs, I'll re-post properly if this is acked. Thanks, Amir. ----- diff --git a/include/linux/percpu-rwsem.h b/include/linux/percpu-rwsem.h index b1f37a89e368..323d5ba6a60d 100644 --- a/include/linux/percpu-rwsem.h +++ b/include/linux/percpu-rwsem.h @@ -127,13 +127,16 @@ extern void percpu_free_rwsem(struct percpu_rw_semaphore *); #define percpu_rwsem_assert_held(sem) \ lockdep_assert_held(&(sem)->rw_sem) +extern void percpu_rwsem_set_user_owned(struct percpu_rw_semaphore *sem); +extern void percpu_rwsem_set_owner(struct percpu_rw_semaphore *sem); + static inline void percpu_rwsem_release(struct percpu_rw_semaphore *sem, bool read, unsigned long ip) { lock_release(&sem->rw_sem.dep_map, 1, ip); #ifdef CONFIG_RWSEM_SPIN_ON_OWNER if (!read) - sem->rw_sem.owner = NULL; + percpu_rwsem_set_user_owned(sem); #endif } @@ -141,6 +144,10 @@ static inline void percpu_rwsem_acquire(struct percpu_rw_semaphore *sem, bool read, unsigned long ip) { lock_acquire(&sem->rw_sem.dep_map, 0, 1, read, 1, NULL, ip); +#ifdef CONFIG_RWSEM_SPIN_ON_OWNER + if (!read) + percpu_rwsem_set_owner(sem); +#endif } #endif diff --git a/kernel/locking/percpu-rwsem.c b/kernel/locking/percpu-rwsem.c index 883cf1b92d90..afa65915541f 100644 --- a/kernel/locking/percpu-rwsem.c +++ b/kernel/locking/percpu-rwsem.c @@ -7,6 +7,8 @@ #include #include +#include "rwsem.h" + int __percpu_init_rwsem(struct percpu_rw_semaphore *sem, const char *name, struct lock_class_key *rwsem_key) { @@ -190,3 +192,17 @@ void percpu_up_write(struct percpu_rw_semaphore *sem) rcu_sync_exit(&sem->rss); } EXPORT_SYMBOL_GPL(percpu_up_write); + +void percpu_rwsem_set_user_owned(struct percpu_rw_semaphore *sem) +{ + DEBUG_RWSEMS_WARN_ON(sem->rw_sem.owner != current); + sem->rw_sem.owner = RWSEM_USER_OWNED; +} +EXPORT_SYMBOL_GPL(percpu_rwsem_set_user_owned); + +void percpu_rwsem_set_owner(struct percpu_rw_semaphore *sem) +{ + DEBUG_RWSEMS_WARN_ON(sem->rw_sem.owner != RWSEM_USER_OWNED); + sem->rw_sem.owner = current; +} +EXPORT_SYMBOL_GPL(percpu_rwsem_set_user_owned); diff --git a/kernel/locking/rwsem.h b/kernel/locking/rwsem.h index a17cba8d94bb..f686596ec033 100644 --- a/kernel/locking/rwsem.h +++ b/kernel/locking/rwsem.h @@ -11,10 +11,14 @@ * 2) RWSEM_READER_OWNED * - lock is currently or previously owned by readers (lock is free * or not set by owner yet) - * 3) Other non-zero value + * 3) RWSEM_USER_OWNED + * - lock is currently owned by userspace (previously owned by writer + * and should be handed over to a new writer before being freed) + * 4) Other non-zero value * - a writer owns the lock */ #define RWSEM_READER_OWNED ((struct task_struct *)1UL) +#define RWSEM_USER_OWNED ((struct task_struct *)2UL) #ifdef CONFIG_DEBUG_RWSEMS # define DEBUG_RWSEMS_WARN_ON(c) DEBUG_LOCKS_WARN_ON(c)