From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from verein.lst.de ([213.95.11.211]:46764 "EHLO newverein.lst.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726487AbeIEXX2 (ORCPT ); Wed, 5 Sep 2018 19:23:28 -0400 Date: Wed, 5 Sep 2018 20:55:39 +0200 From: Christoph Hellwig To: Eric Sandeen Cc: Carlos Maiolino , linux-fsdevel@vger.kernel.org, hch@lst.de, david@fromorbit.com Subject: Re: [PATCH 1/2] fs: Replace direct ->bmap calls by bmap() Message-ID: <20180905185539.GA423@lst.de> References: <20180905135748.30098-1-cmaiolino@redhat.com> <20180905135748.30098-2-cmaiolino@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Wed, Sep 05, 2018 at 09:23:26AM -0500, Eric Sandeen wrote: > > block0 = page->index; > > block0 <<= shift; > > > > - block = inode->i_mapping->a_ops->bmap(inode->i_mapping, block0); > > + block = bmap(inode, block0); > > Prior to this there's an ASSERT that inode->i_mapping->a_ops->bmap > exists. Should that stay, if the goal is to move all ->bmap use out > of calling code? I think it needs to go away. > OTOH, what will this code do if bmap() finds that there > is no ->bmap present and returns 0? I think we just need to fix the bmap() prototype so that it can return error, which would be very useful for various reasons. Something like this: int bmap(struct address_space *mapping, sector_t *block) { if (!mapping->a_ops->bmap) return -EINVAL; *block = mapping->a_ops->bmap(mapping, *block); return 0; } then add fiemap support with real error handling later.