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 77B2BC3065C for ; Tue, 2 Jul 2024 10:16:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C7FB66B00AE; Tue, 2 Jul 2024 06:16:13 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C30076B00B2; Tue, 2 Jul 2024 06:16:13 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AF7616B00B3; Tue, 2 Jul 2024 06:16:13 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 93C666B00AE for ; Tue, 2 Jul 2024 06:16:13 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 16289C20CB for ; Tue, 2 Jul 2024 10:16:13 +0000 (UTC) X-FDA: 82294407426.08.4CB2C8C Received: from mout-p-102.mailbox.org (mout-p-102.mailbox.org [80.241.56.152]) by imf13.hostedemail.com (Postfix) with ESMTP id B6E7B2000E for ; Tue, 2 Jul 2024 10:16:09 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=pankajraghav.com header.s=MBO0001 header.b=FF4DRMl+; spf=pass (imf13.hostedemail.com: domain of kernel@pankajraghav.com designates 80.241.56.152 as permitted sender) smtp.mailfrom=kernel@pankajraghav.com; dmarc=pass (policy=quarantine) header.from=pankajraghav.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1719915347; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=hGA34wKwysx5NC5w2uPw2pO5+dBtQxqRiC/Wy1SKQaw=; b=pgQcAzs4yGUJv9kobaJcidTLuJttd8G2LFW4Hg0SAQ9Wsus8hjuY5HgAiWUyeFkZnxFc3C yfLjD13YhkDEccwgk7orcKLg56xTqZ5K5gaiq7SjVb6x/k9a+hARbSyRfOrXMmbR7GJbg8 Fqlo8Cu1C3f31R7HSnuluZ0ldzO/UsQ= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1719915347; a=rsa-sha256; cv=none; b=cYrrD98hs3dS4Wz90rBFQ3KgjK3z9aQ+kA9fJ4cmF6I5QR51LOZeArHcjAIc+TNLeo+9h0 je13nZKg1R7QndRTrd+GtL8+nYx9Z6oWH8OlKHhpcsVPW/h/prD5A/gfeokfKclVW/v1/Z c+QuVlGcjb8Td2SqZUbHvPYlqZXNmX8= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=pankajraghav.com header.s=MBO0001 header.b=FF4DRMl+; spf=pass (imf13.hostedemail.com: domain of kernel@pankajraghav.com designates 80.241.56.152 as permitted sender) smtp.mailfrom=kernel@pankajraghav.com; dmarc=pass (policy=quarantine) header.from=pankajraghav.com Received: from smtp2.mailbox.org (smtp2.mailbox.org [IPv6:2001:67c:2050:b231:465::2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-102.mailbox.org (Postfix) with ESMTPS id 4WCzMS3Z0Kz9sQ6; Tue, 2 Jul 2024 12:16:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pankajraghav.com; s=MBO0001; t=1719915364; 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: in-reply-to:in-reply-to:references:references; bh=hGA34wKwysx5NC5w2uPw2pO5+dBtQxqRiC/Wy1SKQaw=; b=FF4DRMl+i63yqfv5l5ySVR46sxgKZK3PxHrh7stGuzCkk65s4Cb6QPPwRLa9e112elGwQY wFUIrQf85JyE+Qw9huGuvKNmS8dJtZI0wn/YmDK+sr8f/K5/QsI+FrKiOmsonLkSybqi0D 4AS7g1s6MyC+BRw1PAbOw65J3doVTw4+TAIef+7RMyXgB9gj5gYQdGJyramKdyyFyxourJ jZ9HXGRpsMxstbhmDmQy5vPyWKvGPScGog37mvQG+69cUVpXLxG1llo16HbPr9AqtOIrCy c0zu3BgehargAVk+XWlN5MQ4ZVawpxs9RQpzN4WfQvKCxHuG3OfgMchJ/5lpfQ== Date: Tue, 2 Jul 2024 10:15:56 +0000 From: "Pankaj Raghav (Samsung)" To: Christoph Hellwig Cc: david@fromorbit.com, willy@infradead.org, chandan.babu@oracle.com, djwong@kernel.org, brauner@kernel.org, akpm@linux-foundation.org, linux-kernel@vger.kernel.org, yang@os.amperecomputing.com, linux-mm@kvack.org, john.g.garry@oracle.com, linux-fsdevel@vger.kernel.org, hare@suse.de, p.raghav@samsung.com, mcgrof@kernel.org, gost.dev@samsung.com, cl@os.amperecomputing.com, linux-xfs@vger.kernel.org, Zi Yan Subject: Re: [PATCH v8 06/10] iomap: fix iomap_dio_zero() for fs bs > system page size Message-ID: <20240702101556.jdi5anyr3v5zngnv@quentin> References: <20240625114420.719014-1-kernel@pankajraghav.com> <20240625114420.719014-7-kernel@pankajraghav.com> <20240702074203.GA29410@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240702074203.GA29410@lst.de> X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: B6E7B2000E X-Stat-Signature: m887z5u8peqeffeiz6565qs7e6x5xzbp X-HE-Tag: 1719915369-665977 X-HE-Meta: U2FsdGVkX19uY16AyUSqqwDtVQbzIuzV2btddsVMcBacvF/sdGIM63B5/K90XYfWq4kkkgzM7eVLiCkSzC1/x9LpHVyHP6B+ZV6tFlbdseiDvVYX/riatm4qSuasUctx/P3Io3twPryAm9BqHRU66HLTNGrCnaHjjcK79lqgRjJ1/njTRcFWCYQiMKU3V4cXJuQDjkpuJHvh589l9aF/TRHEcv55y/cKn3s9zA0mu6AYQ5sY14Q7EN/zZJvZ45VY+SgmSRIJ3dHUnJGRdGgFwBCI1nh3tTUoIx2ON8It0w6X9B8RcPgi5y10Qg31c/jKVvfYNtE6dGKUagANztVjndpvQG9er9Tqg981ZOotALQ82Pp5qxITo9/O0c++PsPJ1beS1qHLJB7/gK3951pk3iRExVe2hg8IOXkapi3/LveqzxIBb0Md1M69nAxAV8XVxZ9ZCfZaT//UVLsYactYrh1mfOA0Tyww4tguTmEpgs51IgNlIKqsZGMJEOHdTLpqDeE+qf9hUilr9qqO7A+8/+gcAKk0rAmm6QjAb0j0xgccojGXCkiR0e1V6kleEO5s/iBav3oANXlwaRLVXBw9szu1Grz5E5MC3diTz+UQKPPUF9+o7P5u0Wzhajq06wgzP4bg4LbIm8T8n8dUh/4dl8MkY8m2J74AD3cgR0mai2USkLN6pjdJ1RO4Wtc+TRCHHrAZlhiJTkufadAW/UL+lr5roDdrnjTW1fetQ1+EthLCah8rtnc4An7ZLXIgh1XLrZfghZZJpjg7SIb4/W0EzNkbf9jAJS5AS165qCwNtl84wfCAnh6NtbK/fyvqICJZt64g1+dEpAixDX0XMLJw2TldT6xBF68WES9hDRgOPCntU66204oJZv8o4p2hYas8GPsQcN4yz6oc0TTCX+F5h6JpAeiWwDfIZm7uBckqMsSjcoRRAQW+xbSRMwTUbehOOm/SJtBQlDiOclh6+ah oBbhH5vs cVVz+5LrCQLtOEjkByNkgIkLhktcPfMRrP2SRvbK2gI1vff3CDBfbzvHAdj6krpyX809ZZv1nOALJQ1HI+kNp7/lUAN1Rliqi5L53b/dvwj4DULDoIgsWwGsrxIly0mNnavJcWLf5VDX4KhflBl4BcvupHcp6MMXK7ZgjWb80oe9k+6a9F9MSumsIyStxFoClIOm574YENvJBIHoqMqtBfysgOeuObmLNVW7H9w4byanOajyDVoN9qPH7IgoFcr8zvWN6eENBOCS9z6X3Gn4cDqUIwT1USjx9I/98QrwM6naB0W/uAZPu3KrIf2v7GsNvMLBpzGQe+2zm8bs+oqQFFsqKH3vPraWPK0Zb1CGl9sAqliqp3G247yffhCIBl44P22K+ovpsqjttGBUliYfKOga0wdVTgQ+6kZHL 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: List-Subscribe: List-Unsubscribe: > > +fs_initcall(iomap_pagecache_init); > > s/iomap_pagecache_init/iomap_buffered_init/ > > We don't use pagecache naming anywhere else in the file. Got it. > > > +/* > > + * Used for sub block zeroing in iomap_dio_zero() > > + */ > > +#define ZERO_PAGE_64K_SIZE (65536) > > just use SZ_64K > > > +#define ZERO_PAGE_64K_ORDER (get_order(ZERO_PAGE_64K_SIZE)) > > No really point in having this. Hmm, I used it twice, hence the define. But if we decide to get rid of set_memory_ro(), then this does not make sense. > > > +static struct page *zero_page_64k; > > This should be a folio. Encoding the size in the name is also really > weird and just creates churn when we have to increase it. Willy suggested we could use raw pages as we don't need the metadata from using a folio. [0] > > > > + /* > > + * Max block size supported is 64k > > + */ > > + WARN_ON_ONCE(len > ZERO_PAGE_64K_SIZE); > > > A WARN_ON without actually erroring out here is highly dangerous. I agree but I think we decided that we are safe with 64k for now as fs that uses iomap will not have a block size > 64k. But this function needs some changes when we decide to go beyond 64k by returning error instead of not returning anything. Until then WARN_ON_ONCE would be a good stop gap for people developing the feature to go beyond 64k block size[1]. > > > + > > bio = iomap_dio_alloc_bio(iter, dio, 1, REQ_OP_WRITE | REQ_SYNC | REQ_IDLE); > > Overly long line here. > Not a part of my change, so I didn't bother reformatting it. :) > > + > > +static int __init iomap_dio_init(void) > > +{ > > + zero_page_64k = alloc_pages(GFP_KERNEL | __GFP_ZERO, > > + ZERO_PAGE_64K_ORDER); > > > + > > + if (!zero_page_64k) > > + return -ENOMEM; > > + > > + set_memory_ro((unsigned long)page_address(zero_page_64k), > > + 1U << ZERO_PAGE_64K_ORDER); > > What's the point of the set_memory_ro here? Yes, we won't write to > it, but it's hardly an attack vector and fragments the direct map. That is a good point. Darrick suggested why not add a ro tag as we don't write to it but I did not know the consequence of direct map fragmentation when this is added. So probably there is no value calling set_memory_ro here. -- Pankaj [0] https://lore.kernel.org/linux-fsdevel/ZkT46AsZ3WghOArL@casper.infradead.org/ [1] I spent a lot of time banging my head why I was getting FS corruption when I was doing direct io in XFS while adding LBS support before I found the PAGE_SIZE assumption here.