From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from aserp2130.oracle.com ([141.146.126.79]:33290 "EHLO aserp2130.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726962AbfARGtJ (ORCPT ); Fri, 18 Jan 2019 01:49:09 -0500 Subject: Re: [PATCH v2 4/5] xfs: remove superfluous writeback mapping eof trimming References: <20190117192004.49346-1-bfoster@redhat.com> <20190117192004.49346-5-bfoster@redhat.com> From: Allison Henderson Message-ID: <6798de3d-def7-c75d-a61a-23dc04f874b1@oracle.com> Date: Thu, 17 Jan 2019 23:48:58 -0700 MIME-Version: 1.0 In-Reply-To: <20190117192004.49346-5-bfoster@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-xfs-owner@vger.kernel.org List-ID: List-Id: xfs To: Brian Foster , linux-xfs@vger.kernel.org On 1/17/19 12:20 PM, Brian Foster wrote: > Now that the cached writeback mapping is explicitly invalidated on > data fork changes, the EOF trimming band-aid is no longer necessary. > Remove xfs_trim_extent_eof() as well since it has no other users. > > Signed-off-by: Brian Foster > --- > fs/xfs/libxfs/xfs_bmap.c | 11 ----------- > fs/xfs/libxfs/xfs_bmap.h | 1 - > fs/xfs/xfs_aops.c | 15 --------------- > 3 files changed, 27 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > index 332eefa2700b..4c73927819c2 100644 > --- a/fs/xfs/libxfs/xfs_bmap.c > +++ b/fs/xfs/libxfs/xfs_bmap.c > @@ -3685,17 +3685,6 @@ xfs_trim_extent( > } > } > > -/* trim extent to within eof */ > -void > -xfs_trim_extent_eof( > - struct xfs_bmbt_irec *irec, > - struct xfs_inode *ip) > - > -{ > - xfs_trim_extent(irec, 0, XFS_B_TO_FSB(ip->i_mount, > - i_size_read(VFS_I(ip)))); > -} > - > /* > * Trim the returned map to the required bounds > */ > diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h > index 09d3ea97cc15..b4ff710d7250 100644 > --- a/fs/xfs/libxfs/xfs_bmap.h > +++ b/fs/xfs/libxfs/xfs_bmap.h > @@ -181,7 +181,6 @@ static inline bool xfs_bmap_is_real_extent(struct xfs_bmbt_irec *irec) > > void xfs_trim_extent(struct xfs_bmbt_irec *irec, xfs_fileoff_t bno, > xfs_filblks_t len); > -void xfs_trim_extent_eof(struct xfs_bmbt_irec *, struct xfs_inode *); > int xfs_bmap_add_attrfork(struct xfs_inode *ip, int size, int rsvd); > int xfs_bmap_set_attrforkoff(struct xfs_inode *ip, int size, int *version); > void xfs_bmap_local_to_extents_empty(struct xfs_inode *ip, int whichfork); > diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c > index 649e4ad76add..09d8f7690e9e 100644 > --- a/fs/xfs/xfs_aops.c > +++ b/fs/xfs/xfs_aops.c > @@ -357,19 +357,6 @@ xfs_map_blocks( > if (XFS_FORCED_SHUTDOWN(mp)) > return -EIO; > > - /* > - * We have to make sure the cached mapping is within EOF to protect > - * against eofblocks trimming on file release leaving us with a stale > - * mapping. Otherwise, a page for a subsequent file extending buffered > - * write could get picked up by this writeback cycle and written to the > - * wrong blocks. > - * > - * Note that what we really want here is a generic mapping invalidation > - * mechanism to protect us from arbitrary extent modifying contexts, not > - * just eofblocks. > - */ > - xfs_trim_extent_eof(&wpc->imap, ip); > - > /* > * COW fork blocks can overlap data fork blocks even if the blocks > * aren't shared. COW I/O always takes precedent, so we must always > @@ -482,7 +469,6 @@ xfs_map_blocks( > } > > wpc->imap = imap; > - xfs_trim_extent_eof(&wpc->imap, ip); > trace_xfs_map_blocks_found(ip, offset, count, wpc->io_type, &imap); > return 0; > allocate_blocks: > @@ -494,7 +480,6 @@ xfs_map_blocks( > ASSERT(whichfork == XFS_COW_FORK || cow_fsb == NULLFILEOFF || > imap.br_startoff + imap.br_blockcount <= cow_fsb); > wpc->imap = imap; > - xfs_trim_extent_eof(&wpc->imap, ip); > trace_xfs_map_blocks_alloc(ip, offset, count, wpc->io_type, &imap); > return 0; > } > I'll skip this one, it sounds from the last review we decided not to keep it. Thanks though! Allison