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

Christoph Hellwig
2019-08-22T08:03:14Z

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.