Re: [PATCH v2 16/16] iomap: Make readpage synchronous

2020-10-15 Thread Christoph Hellwig
On Thu, Oct 15, 2020 at 08:03:12PM +0100, Matthew Wilcox wrote: > I honestly don't see the problem. We have to assign the status > conditionally anyway so we don't overwrite an error with a subsequent > success. Yes, but having a potential NULL pointer to a common structure is just waiting for tr

Re: [PATCH v2 16/16] iomap: Make readpage synchronous

2020-10-15 Thread Matthew Wilcox
On Thu, Oct 15, 2020 at 06:58:48PM +0100, Christoph Hellwig wrote: > On Thu, Oct 15, 2020 at 05:43:33PM +0100, Matthew Wilcox wrote: > > I prefer assigning ctx conditionally to propagating the knowledge > > that !rac means synchronous. I've gone with this: > > And I really hate these kinds of con

Re: [PATCH v2 16/16] iomap: Make readpage synchronous

2020-10-15 Thread Christoph Hellwig
On Thu, Oct 15, 2020 at 05:43:33PM +0100, Matthew Wilcox wrote: > On Thu, Oct 15, 2020 at 10:42:03AM +0100, Christoph Hellwig wrote: > > > +static void iomap_read_page_end_io(struct bio_vec *bvec, > > > + struct completion *done, bool error) > > > > I really don't like the parameters here.

Re: [PATCH v2 16/16] iomap: Make readpage synchronous

2020-10-15 Thread Matthew Wilcox
On Thu, Oct 15, 2020 at 10:42:03AM +0100, Christoph Hellwig wrote: > > +static void iomap_read_page_end_io(struct bio_vec *bvec, > > + struct completion *done, bool error) > > I really don't like the parameters here. Part of the problem is > that ctx is only assigned to bi_private condi

Re: [PATCH v2 16/16] iomap: Make readpage synchronous

2020-10-15 Thread Christoph Hellwig
> +static void iomap_read_page_end_io(struct bio_vec *bvec, > + struct completion *done, bool error) I really don't like the parameters here. Part of the problem is that ctx is only assigned to bi_private conditionally, which can easily be fixed. The other part is the strange bool er

[PATCH v2 16/16] iomap: Make readpage synchronous

2020-10-09 Thread Matthew Wilcox (Oracle)
A synchronous readpage lets us report the actual errno instead of ineffectively setting PageError. Signed-off-by: Matthew Wilcox (Oracle) --- fs/iomap/buffered-io.c | 74 -- 1 file changed, 42 insertions(+), 32 deletions(-) diff --git a/fs/iomap/buffered-