From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-f68.google.com ([209.85.214.68]:35175 "EHLO mail-it0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752069AbeGBPFU (ORCPT ); Mon, 2 Jul 2018 11:05:20 -0400 Received: by mail-it0-f68.google.com with SMTP id l16-v6so12422464ita.0 for ; Mon, 02 Jul 2018 08:05:20 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20180702125230.GB3366@lst.de> References: <20180615130209.1970-2-hch@lst.de> <20180629144444.28826-1-agruenba@redhat.com> <20180701062141.GA27281@lst.de> <20180702125230.GB3366@lst.de> From: Andreas Gruenbacher Date: Mon, 2 Jul 2018 17:05:18 +0200 Message-ID: Subject: Re: [PATCH] iomap: Add inline data support to iomap_readpage_actor To: Christoph Hellwig Cc: cluster-devel , linux-ext4 , linux-fsdevel Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 2 July 2018 at 14:52, Christoph Hellwig wrote: > On Sun, Jul 01, 2018 at 11:43:43PM +0200, Andreas Gruenbacher wrote: >> > + WARN_ON_ONCE(plen != PAGE_SIZE); >> >> Inline mappings only extend to the end of the file (iomap->offset == 0 >> && iomap->length == inode->i_size), so length and plen will be >> inode->i_size here. This assertion should just be removed. > > But we should generally still return PAGE_SIZE from iomap_begin > because we cover the whole page and zero the rest, don't we? iomap_begin doesn't know when it's being called for readpage. Direct I/O reads don't pad to the page size, for example, so handling the padding in the caller seems reasonable. If you compare with regular block-based mappings, those are also block but not necessarily page aligned. For stuffed files, iomap_begin will return an IOMAP_HOLE mapping for offsets beyond the end of the file, so iterating beyond the end of the file is fine (even if readpage doesn't do that). > Either way, I can remove the assert from now. Yes, thanks. Andreas From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andreas Gruenbacher Subject: Re: [PATCH] iomap: Add inline data support to iomap_readpage_actor Date: Mon, 2 Jul 2018 17:05:18 +0200 Message-ID: References: <20180615130209.1970-2-hch@lst.de> <20180629144444.28826-1-agruenba@redhat.com> <20180701062141.GA27281@lst.de> <20180702125230.GB3366@lst.de> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: cluster-devel , linux-fsdevel , linux-ext4 To: Christoph Hellwig Return-path: In-Reply-To: <20180702125230.GB3366@lst.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: cluster-devel-bounces@redhat.com Errors-To: cluster-devel-bounces@redhat.com List-Id: linux-ext4.vger.kernel.org On 2 July 2018 at 14:52, Christoph Hellwig wrote: > On Sun, Jul 01, 2018 at 11:43:43PM +0200, Andreas Gruenbacher wrote: >> > + WARN_ON_ONCE(plen != PAGE_SIZE); >> >> Inline mappings only extend to the end of the file (iomap->offset == 0 >> && iomap->length == inode->i_size), so length and plen will be >> inode->i_size here. This assertion should just be removed. > > But we should generally still return PAGE_SIZE from iomap_begin > because we cover the whole page and zero the rest, don't we? iomap_begin doesn't know when it's being called for readpage. Direct I/O reads don't pad to the page size, for example, so handling the padding in the caller seems reasonable. If you compare with regular block-based mappings, those are also block but not necessarily page aligned. For stuffed files, iomap_begin will return an IOMAP_HOLE mapping for offsets beyond the end of the file, so iterating beyond the end of the file is fine (even if readpage doesn't do that). > Either way, I can remove the assert from now. Yes, thanks. Andreas From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andreas Gruenbacher Date: Mon, 2 Jul 2018 17:05:18 +0200 Subject: [Cluster-devel] [PATCH] iomap: Add inline data support to iomap_readpage_actor In-Reply-To: <20180702125230.GB3366@lst.de> References: <20180615130209.1970-2-hch@lst.de> <20180629144444.28826-1-agruenba@redhat.com> <20180701062141.GA27281@lst.de> <20180702125230.GB3366@lst.de> Message-ID: List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On 2 July 2018 at 14:52, Christoph Hellwig wrote: > On Sun, Jul 01, 2018 at 11:43:43PM +0200, Andreas Gruenbacher wrote: >> > + WARN_ON_ONCE(plen != PAGE_SIZE); >> >> Inline mappings only extend to the end of the file (iomap->offset == 0 >> && iomap->length == inode->i_size), so length and plen will be >> inode->i_size here. This assertion should just be removed. > > But we should generally still return PAGE_SIZE from iomap_begin > because we cover the whole page and zero the rest, don't we? iomap_begin doesn't know when it's being called for readpage. Direct I/O reads don't pad to the page size, for example, so handling the padding in the caller seems reasonable. If you compare with regular block-based mappings, those are also block but not necessarily page aligned. For stuffed files, iomap_begin will return an IOMAP_HOLE mapping for offsets beyond the end of the file, so iterating beyond the end of the file is fine (even if readpage doesn't do that). > Either way, I can remove the assert from now. Yes, thanks. Andreas