Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Received-SPF: Pass (protection.outlook.com: domain of nvidia.com designates
 216.228.117.161 as permitted sender) receiver=protection.outlook.com;
 client-ip=216.228.117.161; helo=mail.nvidia.com; pr=C
Message-ID: <6a5cd655-f3e6-49ef-e359-401ba84fca25@nvidia.com>
Date:   Wed, 21 Jun 2023 19:22:24 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Subject: Re: [PATCH] mm/memory_hotplug.c: don't fail hot unplug quite so
 eagerly
Content-Language: en-US
To:     David Hildenbrand <david@redhat.com>,
        Oscar Salvador <osalvador@suse.de>
CC:     Andrew Morton <akpm@linux-foundation.org>,
        LKML <linux-kernel@vger.kernel.org>, <linux-mm@kvack.org>
References: <20230620011719.155379-1-jhubbard@nvidia.com>
 <ed83df65-f785-7077-ddd0-4e53d6fa6056@redhat.com>
 <80e01fa9-28c0-37e8-57f8-5bb4ce9a9db7@nvidia.com>
 <83689f25-ca50-7ece-45f0-a936e704df7d@redhat.com>
From:   John Hubbard <jhubbard@nvidia.com>
In-Reply-To: <83689f25-ca50-7ece-45f0-a936e704df7d@redhat.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Jun 2023 02:22:33.6923
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 5d57c62a-ca5f-4be5-52e6-08db72c789c2
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=43083d15-7273-40c1-b7db-39efd9ccc17a;Ip=[216.228.117.161];Helo=[mail.nvidia.com]
X-MS-Exchange-CrossTenant-AuthSource: SN1PEPF0002636C.namprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR12MB6034
X-Spam-Status: No, score=-1.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        NICE_REPLY_A,SPF_HELO_PASS,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=no
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 6/21/23 01:11, David Hildenbrand wrote:
>> ...what about discerning between "user initiated offline_pages" and
>> "offline pages as part of a driver shutdown/unload"?
> 
> Makes sense to me.
> 
> There are two ways for triggering it directly from user space:
> 
> 1) drivers/base/core.c:online_store()
> 2) drivers/base/memory.c:state_store()
> 
> We cannot easily hook into 2) to indicate "we're offlining directly
> from user space". SO we might have to do it the other way around.
> 
> 
> Something along the following lines should do the trick (expect whitespace damage):
> 
> 
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 53ee7654f009..acd4b739505a 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -152,6 +152,13 @@ void put_online_mems(void)
> 
>   bool movable_node_enabled = false;
> 
> +/*
> + * Protected by the device hotplug lock. Indicates whether device offlining
> + * is triggered from try_offline_memory_block() such that we don't fail memory
> + * offlining if a signal is pending.
> + */
> +static bool mhp_in_try_offline_memory_block;
> +
>   #ifndef CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE
>   int mhp_default_online_type = MMOP_OFFLINE;
>   #else
> @@ -1860,7 +1867,8 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages,
>          do {
>                  pfn = start_pfn;
>                  do {
> -                       if (signal_pending(current)) {
> +                       if (!mhp_in_try_offline_memory_block &&
> +                           signal_pending(current)) {
>                                  ret = -EINTR;
>                                  reason = "signal backoff";
>                                  goto failed_removal_isolated;
> @@ -2177,7 +2185,9 @@ static int try_offline_memory_block(struct memory_block *mem, void *arg)
>          if (page && zone_idx(page_zone(page)) == ZONE_MOVABLE)
>                  online_type = MMOP_ONLINE_MOVABLE;
> 
> +       mhp_in_try_offline_memory_block = true;
>          rc = device_offline(&mem->dev);
> +       mhp_in_try_offline_memory_block = false;
>          /*
>           * Default is MMOP_OFFLINE - change it only if offlining succeeded,
>           * so try_reonline_memory_block() can do the right thing.
> 
> 
> 
> There is still arch/powerpc/platforms/pseries/hotplug-memory.c that calls
> device_offline() and would fail on signals (not sure if relevant, like for virtio-mem it
> shouldn't be that relevant).
> 
> I guess dlpar_remove_lmb() can now simply call offline_and_remove_memory().
> [I might craft a patch later]
> 

This direction looks good to me, I'd love to see a patch if you
put something together.


thanks,
-- 
John Hubbard
NVIDIA