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 03B46EB64D7 for ; Wed, 28 Jun 2023 12:14:49 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 63DA98D0002; Wed, 28 Jun 2023 08:14:48 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5ED778D0001; Wed, 28 Jun 2023 08:14:48 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4DC3D8D0002; Wed, 28 Jun 2023 08:14:48 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 3D48F8D0001 for ; Wed, 28 Jun 2023 08:14:48 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 02538160B49 for ; Wed, 28 Jun 2023 12:14:47 +0000 (UTC) X-FDA: 80952050256.24.1A96F32 Received: from www262.sakura.ne.jp (www262.sakura.ne.jp [202.181.97.72]) by imf29.hostedemail.com (Postfix) with ESMTP id 9EFF8120016 for ; Wed, 28 Jun 2023 12:14:44 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=none; dmarc=none; spf=none (imf29.hostedemail.com: domain of penguin-kernel@I-love.SAKURA.ne.jp has no SPF policy when checking 202.181.97.72) smtp.mailfrom=penguin-kernel@I-love.SAKURA.ne.jp ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1687954485; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=RInM3wg/yJuoQRcE1qQgZHhP4mRN3JrVlKl+zraaYhE=; b=s3cY/Ybfhc9pCvRzHAqWvu75kIvyMfQ9TDUWrm7arCUMqWw1tSoc2Y6XKSmaCsXeP3QTiI pNpfU4KW5ZLOVPTZIkn14hsxSYuIMcwMxhazy2uQSY+qw0RVknXfato4VX9Esr10HdsieI bn9DXzg1P7xCUnro9H2hSSkAapthVMI= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=none; dmarc=none; spf=none (imf29.hostedemail.com: domain of penguin-kernel@I-love.SAKURA.ne.jp has no SPF policy when checking 202.181.97.72) smtp.mailfrom=penguin-kernel@I-love.SAKURA.ne.jp ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1687954485; a=rsa-sha256; cv=none; b=A5VyznJqZDlp68v3sdEGcF21p4thRaOeKw06VVDdn4L2L8X76GTiAF11JLEBbelRtS+G2Q ODmdkifvyDAKd+MKJG0VVdn+3EAdSH+bECx5LUvaP6tbkc2I5QzpfkTX/yRMjZOBJMg04A JO00BxhUqQcgUY46ugpr6vAWCYdc1f4= Received: from fsav112.sakura.ne.jp (fsav112.sakura.ne.jp [27.133.134.239]) by www262.sakura.ne.jp (8.15.2/8.15.2) with ESMTP id 35SCEHJP077978; Wed, 28 Jun 2023 21:14:17 +0900 (JST) (envelope-from penguin-kernel@I-love.SAKURA.ne.jp) Received: from www262.sakura.ne.jp (202.181.97.72) by fsav112.sakura.ne.jp (F-Secure/fsigk_smtp/550/fsav112.sakura.ne.jp); Wed, 28 Jun 2023 21:14:17 +0900 (JST) X-Virus-Status: clean(F-Secure/fsigk_smtp/550/fsav112.sakura.ne.jp) Received: from [192.168.1.6] (M106072142033.v4.enabler.ne.jp [106.72.142.33]) (authenticated bits=0) by www262.sakura.ne.jp (8.15.2/8.15.2) with ESMTPSA id 35SCEG6P077973 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NO); Wed, 28 Jun 2023 21:14:16 +0900 (JST) (envelope-from penguin-kernel@I-love.SAKURA.ne.jp) Message-ID: Date: Wed, 28 Jun 2023 21:14:16 +0900 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: [PATCH v2 1/2] seqlock: Do the lockdep annotation before locking in do_write_seqcount_begin_nested() Content-Language: en-US To: Petr Mladek , Sebastian Andrzej Siewior Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, "Luis Claudio R. Goncalves" , Andrew Morton , Boqun Feng , Ingo Molnar , John Ogness , Mel Gorman , Michal Hocko , Peter Zijlstra , Thomas Gleixner , Waiman Long , Will Deacon References: <20230623171232.892937-1-bigeasy@linutronix.de> <20230623171232.892937-2-bigeasy@linutronix.de> <20230626081254.XmorFrhs@linutronix.de> From: Tetsuo Handa In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspam-User: X-Stat-Signature: swbzgw8g3ryq16aw71bqspedw8dhjs8a X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 9EFF8120016 X-HE-Tag: 1687954484-196237 X-HE-Meta: U2FsdGVkX1+Wzndot1Qq3BG/IMZmWIEgRQdifSkwIz9BDyr70Y/otjctV/p2u5A6HjEQNzZFxKuACxYb/VCqhxkFrd5Ysq72cAPuf/iIHAqzyn4omAjerGu1QWWinRn40mAXwaE24/hWlkaJ3ANqqhLUGlMoN7vFxcPy5/4mk922GXGh912gBES88/lsUg/D8E46/bsqQc2q2v9C2d/l8OSyr2JC8ukFm57gpfa+1gJWgm3cIJqa+NIT5udwJOI37i2CW8KCB2nAEyAV+QzOmgL0JynzNuJCzMCjSaZAxo+976qqaz7A+yxbULDifcJAi6TMUdLyWjVtAYxywBP2I51fSZITaoH3FtRDNvL9vgC3vSFrWkNhGamCseN79j7mLjlm4f8WWh6qKUekV/gATESqgkAPFYDtjI3rUxX8B+jHHrUUVjr7cR8MmyMUJaXn9GewesGFF1/5mKjMILbOznc0U3lNL1Zn39w8Gt5Qrp/VTDcfKE2akVtXokrdhBh1uOAY29Kj6zIwL8oo3TvKt6Bct0Jt2GtOwiip0SgliXfS/VF9DEqrpp7/lI+SMoyCZUZleywEyy4qTEe+n2YGGNxTr5rBtHa4Vq6TwKo9rmbLS9tgxSgeu36IYeg5Teb0o5+uDeUp8/R0rAYJQZNJzPuOg7P4u5UkL+xpWWxk+jSsfgE0FFBGJ6nQSniOIYL2BeXDJUfk5LN2HibVtHAqhiL/yqpwQ4yfeTuCFGkbVnhl9HkLnAtyaNJcs/3NFR6bN8PEMBa6aaG+sKbx7ekBPxkS82QEORn1JuZyvRnERIql9Wl2LAklryY5B/qOyoWZJAtDcObosXzMY6fhed38hUCql7YY9EI+Sr68fJ7zqCh2xGDXNYIK1kNP6MZHYPkjpkayPvRgOYptVeOtIaoGrUwdKtkel+MSNqPPRWPhEy1HK1z1DYf7v0X1ItNYJnk7ojhSJsGMBYRtObd8KSG 4N0s2Ewd aUcFWv82Iwjs+4MvqO3eyfchMjDR0ER334HreWrB+Bu0YUwyEeHjkR3Fu3pDta80VNYq7apTzpo13ah9HlYOKqPzBZBzMLrwlWIKIKHGHgEZ3ai68wQQG1Tk6b3T59TW5ImTyijWuLhOH/W/13XhzaySW4O0e8OB+8dcDpKTzzMobsyrPxadRiR8N4nIGRygRoPJRuE4V+kSXNYxTctTJGvdApto2/1dqBxZsbPb4P/XjmHURHHPdiwFwRl1WbxEDnF2UWCpt+/+D++XHm19LEkAntDchq637Dz3+UDEC/DlZcyRvkF2RCc3QgELnFoeUphG2Ht963eLsLCqZGRHw0uLmeg== 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 2023/06/26 23:44, Petr Mladek wrote: > On Mon 2023-06-26 10:12:54, Sebastian Andrzej Siewior wrote: >> On 2023-06-24 15:54:12 [+0900], Tetsuo Handa wrote: >>> Why not to do the same on the end side? >>> >>> static inline void do_write_seqcount_end(seqcount_t *s) >>> { >>> - seqcount_release(&s->dep_map, _RET_IP_); >>> do_raw_write_seqcount_end(s); >>> + seqcount_release(&s->dep_map, _RET_IP_); >>> } >> >> I don't have a compelling argument for doing it. It is probably better >> to release the lock from lockdep's point of view and then really release >> it (so it can't be acquired before it is released). > > If this is true then we should not change the ordering on the _begin > side either. I mean that we should call the lockdep code only > after the lock is taken. Anyway, both sides should be symmetric. > > That said, lockdep is about chains of locks and not about timing. > We must not call lockdep annotation when the lock is still available > for a nested context. So the ordering is probably important only when > the lock might be taken from both normal and interrupt context. > > Anyway, please do not do this change only because of printk(). > IMHO, the current ordering is more logical and the printk() problem > should be solved another way. Then, since [PATCH 1/2] cannot be applied, [PATCH 2/2] is automatically rejected. I found /* * Locking a pcp requires a PCP lookup followed by a spinlock. To avoid * a migration causing the wrong PCP to be locked and remote memory being * potentially allocated, pin the task to the CPU for the lookup+lock. * preempt_disable is used on !RT because it is faster than migrate_disable. * migrate_disable is used on RT because otherwise RT spinlock usage is * interfered with and a high priority task cannot preempt the allocator. */ #ifndef CONFIG_PREEMPT_RT #define pcpu_task_pin() preempt_disable() #define pcpu_task_unpin() preempt_enable() #else #define pcpu_task_pin() migrate_disable() #define pcpu_task_unpin() migrate_enable() #endif in mm/page_alloc.c . Thus, I think that calling migrate_disable() if CONFIG_PREEMPT_RT=y and calling local_irq_save() if CONFIG_PREEMPT_RT=n (i.e. Alternative 3) will work. But thinking again, since CONFIG_PREEMPT_RT=y uses special printk() approach where messages are printed from a dedicated kernel thread, do we need to call printk_deferred_enter() if CONFIG_PREEMPT_RT=y ? That is, isn't the fix as straightforward as below? ---------------------------------------- diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 47421bedc12b..a2a3bfa69a12 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -5805,6 +5805,7 @@ static void __build_all_zonelists(void *data) int nid; int __maybe_unused cpu; pg_data_t *self = data; +#ifndef CONFIG_PREEMPT_RT unsigned long flags; /* @@ -5820,6 +5821,7 @@ static void __build_all_zonelists(void *data) * calling kmalloc(GFP_ATOMIC | __GFP_NOWARN) with port->lock held. */ printk_deferred_enter(); +#endif write_seqlock(&zonelist_update_seq); #ifdef CONFIG_NUMA @@ -5858,8 +5860,10 @@ static void __build_all_zonelists(void *data) } write_sequnlock(&zonelist_update_seq); +#ifndef CONFIG_PREEMPT_RT printk_deferred_exit(); local_irq_restore(flags); +#endif } static noinline void __init ----------------------------------------