From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ed1-f49.google.com (mail-ed1-f49.google.com [209.85.208.49]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 010042D97BB for ; Sun, 2 Aug 2026 10:30:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.49 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785666617; cv=none; b=buQjL4GRHHc9sZkqiwk6npZNozLbxLhKhLvEOuh3GUlBjGXGdnG5IZYd5DhesYJfq8Oaw3dhg4KMsfCo8lqcnBg4HNrYvaiMHsbtNbeAMW2FWx8vTXdtmbGyxZ2ULblwweCeR6idUk0RORT+7vNfu6Ci7woP2O5ERPKSFakQFHQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785666617; c=relaxed/simple; bh=KCoDI5ESWHtORyzqca/rH+I6IC9kneGwg0J/LBYN6N0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=D+Cx6cP/q78RXzUjhy5uGj236OrC6neFqAIULrLS2iO0pB+F9zW1d2YPlWAo7K0oTMfbcbVF+08z4eih1FQvP1Ht8altrjRrJBR2r4+pFQdaby1x5MFjSOumIKGZlLhxNXXbsfRkYTOaRaCxoxfrpl6kDSawmDHyb1s0EfdEjgw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com; spf=pass smtp.mailfrom=suse.com; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b=EZ7OAtrZ; arc=none smtp.client-ip=209.85.208.49 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b="EZ7OAtrZ" Received: by mail-ed1-f49.google.com with SMTP id 4fb4d7f45d1cf-69edc72e513so507080a12.3 for ; Sun, 02 Aug 2026 03:30:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1785666614; x=1786271414; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=PqCQcjOei1MEqWMExYN6Cl7+Q3LMozj2JD/0MEjkPCQ=; b=EZ7OAtrZWjs9gQiMzg82aQJnfzLKvvZcrAw5OyscHC6fcXuCai8t6u4PcWlMPVI8xw aOURPO0Fea+MWVsTw0jCVFV8r4yL6/NFzfpI9mdOH6ToFA6Zy/DxdMNNhwU3BnW9/tu9 /SFrCwtXMuEY4Os0qS+KYlrmPV9Yf9EdKaP5wQnb5ZfklNcCH5cNuA9fzqzNwqciJINW dB5cib1jVG8/nbn3xXRy2VN08yic9FcpGL34Rpaw43NVVVvEGv7HxZ/OWqrdHk5ICD9d 2GDt9GtFJNLZJFzSYyicXgP5kGCrDA7REqpE6bn/TgzzxO2DKY9nNrqq+bJ0BQ+/PBOQ r9qg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1785666614; x=1786271414; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=PqCQcjOei1MEqWMExYN6Cl7+Q3LMozj2JD/0MEjkPCQ=; b=BArN1b667PmE6lomoYimgMg8Sud56Du2DaNSKIn5j+6Es6KdICTzNgPS7OO5UNNGUt idqQmLLWRBU1Hu9/sRu282eqazxMEcpY41hDjmZj0+9YoViXGLgMGUSkYUNUE44Cm+O9 GmDSfwkbDhOPs0zm0dEjSW8yfxh30FSU0bg59YC3UYgEttL0kdux8OzFUuGvXPrkvG9b bnypWqDZCThsIJ3psfMxL2nSB7K/yPNNb5p/LwwREjscc/+Oj92jgWPChWYDIv8EBp6n 4IUXdCEfyeWRdLK+1MZRSwPBlsuyu0w5U/+xxWjsvEH7InnkKTjHWDHW89tCoNgVHwiP 5Wwg== X-Forwarded-Encrypted: i=1; AHgh+RqFSr4TG+hHFSEp7077eDO5KnnEECa6uaiAvXRwnUwG1UKPBLvAL+a/T+T0A8z42yQ4L2nDPw87lUU387U=@vger.kernel.org X-Gm-Message-State: AOJu0Yx/Upm+AO366ftpNZDNlnr0mC+Ilqnp9AEGMW58NqMRGuL05htw WHlNrsQmoKaRkI/KIm/gK6KcjJp9StSYfgEWEdeVZf4h7TkS6iEPFa0XyIhCYSmZKdolxereqET pVIb5WhI= X-Gm-Gg: AR+sD12VMDBT5eUcMTFlHUf3HqA1vO3ajRx4i83wdnajXP3EKL1VIYT5ND8CJmNImDs ttY0KGRuTXCNusbMjCLkwfGWlR4FpvkWtTXkV9DwwMVQF4Lh7OiDf44E4ZWCogIhgxC8rT36wLA cbws7u4Eio6S/7o0PZlZkuAVR6XJ/C3nDn1k64Slndul5jrV+fbQS2TO8J/O/XW6+pGG+l5ovUZ Gw6VIrabyCMK1mZ6FY5j1lNHDaV6N7HuTIEItCM+GhEQrKO6RCXEmNi0GgimKhpRah1vZwk60XB zBEZbZMSZNAjndzsIjaRqjfkm3GEvLIUhDfovXd1wz/zmH6nSM/j+/210UPPUh9da3TgSMTeQKS 9a+H2bOXR6d4lSgodWOjrOGXySSGbSBMVIWXV4YtKmxnQ04kkx+sXDgWzG+F5VQBgST7PYHkBi+ 70K6Xe7+qlyhbTs595H16Fd2Y8FlMw98iX/i59ECseDHpnC567sw8gfTx4TCW82VNCnxXpZg== X-Received: by 2002:a05:6402:2111:b0:699:6415:751e with SMTP id 4fb4d7f45d1cf-6a0a7d096f8mr3056890a12.4.1785666614157; Sun, 02 Aug 2026 03:30:14 -0700 (PDT) Received: from localhost ([202.127.77.110]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-84edbd310b2sm2406396b3a.5.2026.08.02.03.30.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 02 Aug 2026 03:30:12 -0700 (PDT) Date: Sun, 2 Aug 2026 18:30:06 +0800 From: Heming Zhao To: Christoph Hellwig Cc: joseph.qi@linux.alibaba.com, mark@fasheh.com, jlbec@evilplan.org, ocfs2-devel@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH v2 2/4] ocfs2: switch dio read path from buffer_head to iomap Message-ID: References: <20260727061802.18485-1-heming.zhao@suse.com> <20260727061802.18485-3-heming.zhao@suse.com> <20260728041657.GB19573@lst.de> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260728041657.GB19573@lst.de> Sorry for the late reply, I am fighting with customer bug this week. On Tue, Jul 28, 2026 at 06:16:57AM +0200, Christoph Hellwig wrote: > On Mon, Jul 27, 2026 at 02:17:58PM +0800, Heming Zhao wrote: > > This patch migrates the DIO read path from the legacy buffer_head > > Please never start commit messages with "This patch", that context > is implied already. OK. > > > The rw cluster-lock level is not stashed in iocb->private: iomap DIO owns > > Explaining what you don't do is always a bit odd, I'd start with what > you do, and only really mention what not later if really needed. Will change in next version. > > > that field (it stores a bio there for polled I/O and unconditionally > > clears it before calling ->end_io), so any side-channel packed into > > iocb->private is corrupted and would trip the end_io lock-state check. > > Instead the unlock level is encoded by which iomap_dio_ops is passed - > > ocfs2_iomap_dio_ops_pr (PRMODE) or ocfs2_iomap_dio_ops_ex (EXMODE) - and > > the *_iter() functions use the split __iomap_dio_rw() + iomap_dio_complete() > > API so they can determine unambiguously, from the return value, whether the > > completion handler already dropped the lock (real I/O, sync or -EIOCBQUEUED) > > or whether the caller must drop it (no I/O issued, or buffered fallback). > > .. and a lot of this just seems to be about the locking, but very little > about the iomap changes. Will change in next version. > > > + if (flags & IOMAP_WRITE) { > > + /* todo */ > > should this be a WARN_ON for now? Will follow Joseph's review comment, replace with "return -NOTSUPP". > > > + if (direct_io && ocfs2_should_use_dio(iocb, to, inode)) { > > + struct iomap_dio *dio; > > + > > + dio = __iomap_dio_rw(iocb, to, &ocfs2_iomap_ops, > > + &ocfs2_iomap_dio_ops_r_pr, 0, NULL, 0); > > + if (dio == NULL) { > > + /* No I/O issued; rw_lock still held. */ > > + ret = 0; > > + } else if (IS_ERR(dio)) { > > + ret = PTR_ERR(dio); > > + if (ret == -EIOCBQUEUED) > > + rw_level = -1; > > + } else { > > + ret = iomap_dio_complete(dio); > > + if (ret != 0) > > + rw_level = -1; > > + } > > What information is missing if you use plain iomap_dio_rw here? ocfs2 should release the cluster lock in end_dio flow, where "rw_level = -1" indicates that end_dio is responsible for releasing it. Btw, I previously tried using iomap_dio_rw() here, but found that some xfstests triggered crashes because of this cluster lock issue. The current code (idea) is based on Joseph's patch, and the code work fine. > > > + /* > > + * A 0 result means the mapping bounced us back to buffered I/O > > + * (e.g. inline data); the rw_lock is still held. Clear > > + * IOCB_DIRECT so generic_file_read_iter() takes the buffered > > + * path rather than re-entering direct I/O. > > + */ > > + if (ret == 0) { > > + iocb->ki_flags &= ~IOCB_DIRECT; > > + ret = generic_file_read_iter(iocb, to); > > + } > > + } else { > > + iocb->ki_flags &= ~IOCB_DIRECT; > > + ret = generic_file_read_iter(iocb, to); > > + } > > Can you share this code somehow? > I can't follow what you mean. What code do you want to see? Thanks, Heming