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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 39EFFC7EE30 for ; Tue, 23 May 2023 22:27:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238619AbjEWW1W (ORCPT ); Tue, 23 May 2023 18:27:22 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57546 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238185AbjEWW1U (ORCPT ); Tue, 23 May 2023 18:27:20 -0400 Received: from mail-pf1-x432.google.com (mail-pf1-x432.google.com [IPv6:2607:f8b0:4864:20::432]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3BC1DDA for ; Tue, 23 May 2023 15:27:18 -0700 (PDT) Received: by mail-pf1-x432.google.com with SMTP id d2e1a72fcca58-64d44b198baso55270b3a.0 for ; Tue, 23 May 2023 15:27:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fromorbit-com.20221208.gappssmtp.com; s=20221208; t=1684880837; x=1687472837; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=o+Mu9QUDJUPxCQ+nxaGfODHItInO1Mx21+1cY8P0+ko=; b=myd4lDfvuaaTHVDxIZueGM+k5E70olrh5Bi4kawJ2EqxlzdG3jx2hNsJKzBv1ExQrZ aZIqungadEPCy/hk+arz6SCHAhumLQ2exHtv9ezxiuWyl92MWzYKy8HK8quRSz97EAMo 8HDOmtAd6fKra/CsSjrI/dK+pfZCIsI+1uCq1qHYsi0xHo3WYArXZwYWudExTIr6Fj27 3dDJ033Vg+M4TLAaJI5DgBCEbpaEmjNrP7x2LDAEYwxTkMPe/qI246iat844prVielH/ N1rZ8XOfFOa41MmZaZKsz9PVPHf7hGTl1NypGa83NrTRXoIwS26AGhLJnHwQKKez3ITY DvPQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684880837; x=1687472837; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=o+Mu9QUDJUPxCQ+nxaGfODHItInO1Mx21+1cY8P0+ko=; b=QVE/b3zPxhPJANJffuqJdsW626YJt1iPPEtxQfFjZE/b0kKY21dGXKMjqVLK7wbq9j MUoKckeVwjrAie+nmtvCVHWZK6b/XNo/CjArTjJyuCmP0omL0YL+G64dR3mpM9qmqmYY dIuXOKQ50+XlTXU7IYtmJs5smgpk584Zp/5jwEOJ3XF4yydmUMXWBOng2LR+kOoGmZ/8 45av68J0uDbQEJ/8skVI/AkHcXcmXq6d2CRXxTmbRib5/lgo0pMCpRJRDpyAUBW+MMcT vSZeYAJ6cDq2IkJXqmCJE/6Y6t/FtmD+JIIvus08PQEDpQK3IxciW2ZPA3P6hwGdvRq5 H5gg== X-Gm-Message-State: AC+VfDyWSguWz+l6LJ9J8RSmUNimm74jocc0OHjY6XEPXtTcuVhOFGb3 ez49ONT/ZHRI8NDqRNdD0k4V0A== X-Google-Smtp-Source: ACHHUZ5+wDLkzWl5hqADeJv/K/pEdU5ftVoYvaXjN5yUsSSnEKzwjNvqAoa0WrZdkbq9aiPARCtcUw== X-Received: by 2002:a05:6a20:3c8d:b0:10c:d5dd:c223 with SMTP id b13-20020a056a203c8d00b0010cd5ddc223mr1532145pzj.15.1684880837655; Tue, 23 May 2023 15:27:17 -0700 (PDT) Received: from dread.disaster.area (pa49-179-0-188.pa.nsw.optusnet.com.au. [49.179.0.188]) by smtp.gmail.com with ESMTPSA id d11-20020a63fd0b000000b005308b255502sm6443395pgh.68.2023.05.23.15.27.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 May 2023 15:27:16 -0700 (PDT) Received: from dave by dread.disaster.area with local (Exim 4.96) (envelope-from ) id 1q1aTJ-0036Bu-2Z; Wed, 24 May 2023 08:27:13 +1000 Date: Wed, 24 May 2023 08:27:13 +1000 From: Dave Chinner To: Hannes Reinecke Cc: Christoph Hellwig , Jens Axboe , Miklos Szeredi , "Darrick J. Wong" , Andrew Morton , David Howells , Matthew Wilcox , linux-block@vger.kernel.org, linux-fsdevel@vger.kernel.org, ceph-devel@vger.kernel.org, linux-ext4@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, cluster-devel@redhat.com, linux-xfs@vger.kernel.org, linux-nfs@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 16/17] block: use iomap for writes to block devices Message-ID: References: <20230424054926.26927-1-hch@lst.de> <20230424054926.26927-17-hch@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org On Fri, May 19, 2023 at 04:22:01PM +0200, Hannes Reinecke wrote: > On 4/24/23 07:49, Christoph Hellwig wrote: > > Use iomap in buffer_head compat mode to write to block devices. > > > > Signed-off-by: Christoph Hellwig > > --- > > block/Kconfig | 1 + > > block/fops.c | 33 +++++++++++++++++++++++++++++---- > > 2 files changed, 30 insertions(+), 4 deletions(-) > > > > diff --git a/block/Kconfig b/block/Kconfig > > index 941b2dca70db73..672b08f0096ab4 100644 > > --- a/block/Kconfig > > +++ b/block/Kconfig > > @@ -5,6 +5,7 @@ > > menuconfig BLOCK > > bool "Enable the block layer" if EXPERT > > default y > > + select IOMAP > > select SBITMAP > > help > > Provide block layer support for the kernel. > > diff --git a/block/fops.c b/block/fops.c > > index 318247832a7bcf..7910636f8df33b 100644 > > --- a/block/fops.c > > +++ b/block/fops.c > > @@ -15,6 +15,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include "blk.h" > > @@ -386,6 +387,27 @@ static ssize_t blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter) > > return __blkdev_direct_IO(iocb, iter, bio_max_segs(nr_pages)); > > } > > +static int blkdev_iomap_begin(struct inode *inode, loff_t offset, loff_t length, > > + unsigned int flags, struct iomap *iomap, struct iomap *srcmap) > > +{ > > + struct block_device *bdev = I_BDEV(inode); > > + loff_t isize = i_size_read(inode); > > + > > + iomap->bdev = bdev; > > + iomap->offset = ALIGN_DOWN(offset, bdev_logical_block_size(bdev)); > > + if (WARN_ON_ONCE(iomap->offset >= isize)) > > + return -EIO; > > I'm hitting this during booting: > [ 5.016324] > [ 5.030256] iomap_iter+0x11a/0x350 > [ 5.030264] iomap_readahead+0x1eb/0x2c0 > [ 5.030272] read_pages+0x5d/0x220 > [ 5.030279] page_cache_ra_unbounded+0x131/0x180 > [ 5.030284] filemap_get_pages+0xff/0x5a0 Why is filemap_get_pages() using unbounded readahead? Surely readahead should be limited to reading within EOF.... > [ 5.030292] filemap_read+0xca/0x320 > [ 5.030296] ? aa_file_perm+0x126/0x500 > [ 5.040216] ? touch_atime+0xc8/0x150 > [ 5.040224] blkdev_read_iter+0xb0/0x150 > [ 5.040228] vfs_read+0x226/0x2d0 > [ 5.040234] ksys_read+0xa5/0xe0 > [ 5.040238] do_syscall_64+0x5b/0x80 > > Maybe we should consider this patch: > > diff --git a/block/fops.c b/block/fops.c > index 524b8a828aad..d202fb663f25 100644 > --- a/block/fops.c > +++ b/block/fops.c > @@ -386,10 +386,13 @@ static int blkdev_iomap_begin(struct inode *inode, > loff_t offset, loff_t length, > > iomap->bdev = bdev; > iomap->offset = ALIGN_DOWN(offset, bdev_logical_block_size(bdev)); > - if (WARN_ON_ONCE(iomap->offset >= isize)) > - return -EIO; > - iomap->type = IOMAP_MAPPED; > - iomap->addr = iomap->offset; > + if (WARN_ON_ONCE(iomap->offset >= isize)) { > + iomap->type = IOMAP_HOLE; > + iomap->addr = IOMAP_NULL_ADDR; > + } else { > + iomap->type = IOMAP_MAPPED; > + iomap->addr = iomap->offset; > + } I think Christoph's code is correct. IMO, any attempt to read beyond the end of the device should throw out a warning and return an error, not silently return zeros. If readahead is trying to read beyond the end of the device, then it really seems to me like the problem here is readahead, not the iomap code detecting the OOB read request.... Cheers, Dave. -- Dave Chinner david@fromorbit.com