From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-170.mta0.migadu.com (out-170.mta0.migadu.com [91.218.175.170]) (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 76F883A6B8E for ; Wed, 29 Jul 2026 05:59:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.170 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785304779; cv=none; b=t7zKEn5JNVfRMxLmWRpX2UP++XlWt5+HuZMUhPSgiPt3rpYCngx/npz+yjUxpDmZZ4AZIuJdcT3CUTVb2aXRELbuXWw1n/PLbJbj8cnZFzpvfHfhfL2I8Ukh8IwnUkiW+iOS2F2C5S+DnzHSVQCkFoS1EOLfxL6PSSMcQNuLdh4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785304779; c=relaxed/simple; bh=q1XqTljWgTtyKE6anPd+YW80sP1WpzRZVSW/UxmPDFQ=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=D+7W3hAy26XSVv8Uds3Po2OcxPNu00r5LZbDRLBeKVZsXOeGM45V8+jzGBw4JUAbkjTj8Ww8LIg8L9SdMsUVp9F35BXH+RjlrCLmYsjK/HkCVsLD9zvPYEKHs9DcjONi2NouvDpMSX/KoapmGznZyUVT8jmPx6yt8n6Ll3y65wc= 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=c0uZa9Ju; arc=none smtp.client-ip=91.218.175.170 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="c0uZa9Ju" Message-ID: <93f5a87d-bf82-4b56-a778-a39d03ba4e3c@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1785304775; 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=nh9P/FxHkvTjdaHZyx9SWlPOELOUresg7sNd6fW5k+o=; b=c0uZa9JuXoZhW8DQ4+rotcSAIpNKliQLTVni8KYRqiL1Y0tLO6LMxjF9sLzGGzXhvwGtJ+ D+lxDHAvMO63pTpKgNVmO+6aH+YtYnvBRcQsD7+Yz1XDAm/lYbXZuRHOjnxLAiejiZzxRl y1YA5EinoFzuYnZRELCWGwhZlssRqxM= Date: Wed, 29 Jul 2026 13:59:18 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Subject: Re: [PATCH v2 2/2] mm: shmem: make unused huge shrinker memcg aware To: Baolin Wang , hughd@google.com, akpm@linux-foundation.org, usama.arif@linux.dev, Qi Zheng Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <889581093179462979dbfb5458620fcca531787b.1784621804.git.zhengqi.arch@bytedance.com> <313c4dc7-9d03-40da-b6a0-170a774010b3@linux.alibaba.com> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Qi Zheng In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT Hi Baolin, On 7/29/26 11:29 AM, Baolin Wang wrote: >>>> @@ -2532,28 +2727,6 @@ static int shmem_get_folio_gfp(struct inode >>>> *inode, pgoff_t index, >>>>   alloced: >>>>       alloced = true; >>>> -    if (folio_test_large(folio) && >>>> -        DIV_ROUND_UP(i_size_read(inode), PAGE_SIZE) < >>>> -                    folio_next_index(folio)) { >>>> -        struct shmem_sb_info *sbinfo = SHMEM_SB(inode->i_sb); >>>> -        struct shmem_inode_info *info = SHMEM_I(inode); >>>> -        /* >>>> -         * Part of the large folio is beyond i_size: subject >>>> -         * to shrink under memory pressure. >>>> -         */ >>>> -        spin_lock(&sbinfo->shrinklist_lock); >>>> -        /* >>>> -         * _careful to defend against unlocked access to >>>> -         * ->shrink_list in shmem_unused_huge_shrink() >>>> -         */ >>>> -        if (list_empty_careful(&info->shrinklist)) { >>>> -            list_add_tail(&info->shrinklist, >>>> -                      &sbinfo->shrinklist); >>>> -            sbinfo->shrinklist_len++; >>>> -        } >>>> -        spin_unlock(&sbinfo->shrinklist_lock); >>>> -    } >>> >>> Why move this additional logic down? >> >> Because under the following condition: >> >>          if (sgp <= SGP_CACHE && >>          ((loff_t)index << PAGE_SHIFT) >= i_size_read(inode)) { >>          error = -EINVAL; >>          goto unlock; >>      } >> >> we will jump to unlock and remove the folio from page cache. >> >> With the new memcg-aware design, shmem_unused_huge_add() takes a memcg >> reference. If the folio is then removed by the error path, the inode >> sits on the shrinker list holding a stale memcg reference and pointing >> at an i_size that no longer matches the folio. >> >> So we should ensure the inode is only queued when the folio is fully set >> up and about to be returned successfully. > > Right. But I think this deserves a separate preparation patch with above > explanation, moving the original shrinklist addition logic to after all > checks are completed. Agree, will split this into a separate preparation patch. Thanks, Qi