linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Christoph Hellwig <hch@lst.de>,
	linux-fsdevel@vger.kernel.org, adilger@dilger.ca,
	jaegeuk@kernel.org, miklos@szeredi.hu, rpeterso@redhat.com,
	linux-xfs@vger.kernel.org
Subject: Re: [PATCH 4/9] fibmap: Use bmap instead of ->bmap method in ioctl_fibmap
Date: Tue, 6 Aug 2019 07:48:00 -0700	[thread overview]
Message-ID: <20190806144800.GN7138@magnolia> (raw)
In-Reply-To: <20190806120723.eb72ykmukgjejiku@pegasus.maiolino.io>

On Tue, Aug 06, 2019 at 02:07:24PM +0200, Carlos Maiolino wrote:
> On Tue, Aug 06, 2019 at 07:38:40AM +0200, Christoph Hellwig wrote:
> > On Mon, Aug 05, 2019 at 08:12:58AM -0700, Darrick J. Wong wrote:
> > > > returned. And IIRC, iomap is the only interface now that cares about issuing a
> > > > warning.
> > > >
> > > > I think the *best* we could do here, is to make the new bmap() to issue the same
> > > > kind of WARN() iomap does, but we can't really change the end result.
> > > 
> > > I'd rather we break legacy code than corrupt filesystems.
> > 
> 
> Yes, I have the same feeling, but this patchset does not have the goal to fix
> the broken api.
> 
> > This particular patch should keep existing behavior as is, as the intent
> > is to not change functionality.  Throwing in another patch to have saner
> > error behavior now that we have a saner in-kernel interface that cleary
> > documents what it is breaking and why on the other hand sounds like a
> > very good idea.
> 
> I totally agree here, and to be honest, I think such change should be in a
> different patchset rather than a new patch in this series. I can do it for sure,
> but this discussion IMHO should be done not only here in linux-fsdevel, but also
> in linux-api, which well, I don't think cc'ing this whole patchset there will do
> any good other than keep the change discussion more complicated than it should
> be. I'd rather finish the design and implementation of this patchset, and I'll
> follow-up it, once it's all set, with a new patch to change the truncation
> behavior, it will make the discussion way easier than mixing up subjects. What
> you guys think?

I probably would've fixed the truncation behavior in the old code and
based the fiemap-fibmap conversion on that so that anyone who wants to
backport the behavior change to an old kernel has an easier time of it.

But afterwards probably works just as well since I don't feel like tying
ourselves in more knots over an old interface. ;)

--D

> Cheers
> 
> 
> -- 
> Carlos

  reply	other threads:[~2019-08-06 14:48 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-31 14:12 [PATCH 0/9 V4] New ->fiemap infrastructure and ->bmap removal Carlos Maiolino
2019-07-31 14:12 ` [PATCH 1/9] fs: Enable bmap() function to properly return errors Carlos Maiolino
2019-07-31 14:12 ` [PATCH 2/9] cachefiles: drop direct usage of ->bmap method Carlos Maiolino
2019-07-31 14:12 ` [PATCH 3/9] ecryptfs: drop direct calls to ->bmap Carlos Maiolino
2019-07-31 14:12 ` [PATCH 4/9] fibmap: Use bmap instead of ->bmap method in ioctl_fibmap Carlos Maiolino
2019-07-31 23:12   ` Darrick J. Wong
2019-08-02  9:19     ` Carlos Maiolino
2019-08-02 15:14       ` Darrick J. Wong
2019-08-05 10:27         ` Carlos Maiolino
2019-08-05 15:12           ` Darrick J. Wong
2019-08-06  5:38             ` Christoph Hellwig
2019-08-06 12:07               ` Carlos Maiolino
2019-08-06 14:48                 ` Darrick J. Wong [this message]
2019-08-08  7:17                   ` Carlos Maiolino
2019-08-06 12:02             ` Carlos Maiolino
2019-08-06 22:41             ` Luis Chamberlain
2019-08-07 14:42               ` Darrick J. Wong
2019-08-08  7:12               ` Carlos Maiolino
2019-08-08 18:53                 ` Andreas Dilger
2019-08-19 10:10                   ` Carlos Maiolino
2019-07-31 14:12 ` [PATCH 5/9] fs: Move start and length fiemap fields into fiemap_extent_info Carlos Maiolino
2019-07-31 23:28   ` Darrick J. Wong
2019-08-02  9:51     ` Carlos Maiolino
2019-08-02 15:15       ` Darrick J. Wong
2019-08-05  9:40         ` Carlos Maiolino
2019-08-06  5:39       ` Christoph Hellwig
2019-07-31 14:12 ` [PATCH 6/9] iomap: Remove length and start fields from iomap_fiemap Carlos Maiolino
2019-07-31 23:24   ` Darrick J. Wong
2019-07-31 14:12 ` [PATCH 7/9] fiemap: Use a callback to fill fiemap extents Carlos Maiolino
2019-07-31 23:26   ` Darrick J. Wong
2019-07-31 14:12 ` [PATCH 8/9] Use FIEMAP for FIBMAP calls Carlos Maiolino
2019-07-31 14:12   ` Carlos Maiolino
2019-07-31 23:22   ` Darrick J. Wong
2019-07-31 23:31     ` Darrick J. Wong
2019-08-02 13:52       ` Carlos Maiolino
2019-08-06  5:41       ` Christoph Hellwig
2019-08-02 13:48     ` Carlos Maiolino
2019-08-02 15:29       ` Darrick J. Wong
2019-08-05 10:38         ` Carlos Maiolino
2019-08-06  5:46         ` Christoph Hellwig
2019-07-31 14:12 ` [PATCH 9/9] xfs: Get rid of ->bmap Carlos Maiolino
2019-07-31 23:30   ` Darrick J. Wong
2019-08-02 10:20 ` [PATCH 0/9 V4] New ->fiemap infrastructure and ->bmap removal Carlos Maiolino
2019-08-08  8:27 [PATCH 0/9 V5] " Carlos Maiolino
2019-08-08  8:27 ` [PATCH 4/9] fibmap: Use bmap instead of ->bmap method in ioctl_fibmap Carlos Maiolino
2019-08-08 20:38   ` kbuild test robot
2019-08-14 11:01     ` Carlos Maiolino
2019-08-14 11:08       ` Christoph Hellwig
2019-09-11 13:43 [PATCH 0/9 V6] New ->fiemap infrastructure and ->bmap removal Carlos Maiolino
2019-09-11 13:43 ` [PATCH 4/9] fibmap: Use bmap instead of ->bmap method in ioctl_fibmap Carlos Maiolino

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190806144800.GN7138@magnolia \
    --to=darrick.wong@oracle.com \
    --cc=adilger@dilger.ca \
    --cc=hch@lst.de \
    --cc=jaegeuk@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=rpeterso@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).