From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-179.mta0.migadu.com (out-179.mta0.migadu.com [91.218.175.179]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EEC5A37C115 for ; Fri, 6 Mar 2026 16:19:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.179 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772813990; cv=none; b=g1pHd8gA7MJQF4hY0rhoxUTzHhiNhPKs+27JTwKeAW2AiTJurAdULHB3pI7sVLO4u5ue+qzXb/NdavhLDX1BVrcTNrP5Y8wflfGatgCKiPMGpgFUzt6hrRGnuL6xexi1tGtzO/TzZUvs/e1egBnHzTNIwaUV7diM+z4UYuzWIyQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772813990; c=relaxed/simple; bh=vYz4jOqZbm+eogOFtWMPvGd/4Zn46Qy2r+65tcIIsM0=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=kPeQ7kUshNNOnT2C1CPDMLdPMS3gdh/H5c1Pn2EkZ+WCYphd03q8Z5we4EvEtK+nPyMltH0jtsI/HR2ugKcjnysIIr964e7wd4P4RsBgwpGlFVgodbcxmNazs6eqAUH4R3u25xZby/GNa1Az4oqdYVyMQqdkTMu4gCYzPYCvAt8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=Jc6o3lQb; arc=none smtp.client-ip=91.218.175.179 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="Jc6o3lQb" Message-ID: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1772813987; h=from:from: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=rfl45dSj6u6b1uTEm79PLyjl61gJFmtiEwVDJ745b4E=; b=Jc6o3lQbGwLczRe8uFLm30vRrYb7lxgmz+kSvflDAzVVSm5SwcDalU/rFLxY4tkcDrlnWU nXyldCwgaby7uujeFolu9k6GgjyRUtqvYbMC/OXs+1o5C29cXln4EweScov/8VoTmOy8rb e9jPyLZth/a0Yhm4YVnvVKZ/RC4IaL4= Date: Fri, 6 Mar 2026 19:19:38 +0300 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Subject: Re: [PATCH] mm: migrate: requeue destination folio on deferred split queue Content-Language: en-GB To: Matthew Wilcox Cc: "David Hildenbrand (Arm)" , Andrew Morton , npache@redhat.com, ziy@nvidia.com, linux-mm@kvack.org, matthew.brost@intel.com, joshua.hahnjy@gmail.com, hannes@cmpxchg.org, rakie.kim@sk.com, byungchul@sk.com, gourry@gourry.net, ying.huang@linux.alibaba.com, apopple@nvidia.com, linux-kernel@vger.kernel.org, kernel-team@meta.com References: <20260306133556.2051251-1-usama.arif@linux.dev> <64051a59-680f-40ae-b291-b884aeb7c77b@linux.dev> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Usama Arif In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Migadu-Flow: FLOW_OUT On 06/03/2026 16:08, Matthew Wilcox wrote: > On Fri, Mar 06, 2026 at 05:12:38PM +0300, Usama Arif wrote: >> + /* >> + * Record whether the source folio is on the deferred split queue >> + * before move_to_new_folio(), which unqueues it via >> + * __folio_migrate_mapping(). >> + */ >> + if (folio_test_large(src) && folio_test_large_rmappable(src) && >> + !data_race(list_empty(&src->_deferred_list))) > > Why do you need data_race() here? list_empty() contains a READ_ONCE() > so shouldn't be necessary? Ah mainly because we dont acquire split_queue_lock before accessing, similar to what we do in folio_unqueue_deferred_split(). > >> + old_page_state |= PAGE_WAS_ON_DEFERRED_SPLIT; > > You've done a great job of the naming. So much so that the comment > seems entirely unnecessary? > >> + /* >> + * Requeue the destination folio on the deferred split queue if >> + * the source was on the queue. The source is unqueued in >> + * __folio_migrate_mapping(), so we record and check the state >> + * from before move_to_new_folio(). >> + */ >> + if (old_page_state & PAGE_WAS_ON_DEFERRED_SPLIT) >> + deferred_split_folio(dst, false); > > Again, I'm not sure the comment says anything that the code doesn't? Yeah there is a much simpler version in reply to Zi's review in [1]. I found the whole migrate_folio_move() function quite verbose already, was keeping up with the theme of that function lol. I will see if I can cut down on the comment in [1] as well. Thanks! [1] https://lore.kernel.org/all/28e48b47-f215-4e4a-b55a-01dbf293ff35@linux.dev/