Re: [PATCH 3/3] xfs: alignment check bio buffers

From: Christoph Hellwig <hch@infradead.org>
To: Dave Chinner <david@fromorbit.com>
Cc: Christoph Hellwig <hch@infradead.org>,
	linux-xfs@vger.kernel.org, Jens Axboe <axboe@kernel.dk>,
	linux-block@vger.kernel.org
Subject: Re: [PATCH 3/3] xfs: alignment check bio buffers
Date: Thu, 22 Aug 2019 01:03:12 -0700	[thread overview]
Message-ID: <20190822080312.GB31346@infradead.org> (raw)
In-Reply-To: <20190822003745.GS1119@dread.disaster.area>

On Thu, Aug 22, 2019 at 10:37:45AM +1000, Dave Chinner wrote:
> > I know Jens disagree, but with the amount of bugs we've been hitting
> > thangs to slub (and I'm pretty sure we have a more hiding outside of
> > XFS) I think we need to add the blk_rq_aligned check to bio_add_page.
> 
> ... I'm not prepared to fight this battle to get this initial fix
> into the code. Get the fix merged, then we can 

Well, the initial fix are the first two patches.  This patch really
just adds a safety belt.  I'll happily take over the effort to get
sensible checks in the block code if you give me a couple weeks,
in the meantime I'd prefer if we could skip this third patch for now.

> > Note that all current callers of bio_add_page can only really check
> > for the return value != the added len anyway, so it is not going to
> > make anything worse.
> 
> It does make things worse - it turns multi-bio chaining loops like
> the one xfs_rw_bdev() into an endless loop as they don't make
> progress - they just keep allocating a new bio and retrying the same
> badly aligned buffer and failing. So if we want an alignment failure
> to error out, callers need to handle the failure, not treat it like
> a full bio.

True.  And we need to improve the interface here, which I'm going
to try to fit into my series that lift some common bio filling code
to the core.  I'll add you to the cc list.