Messages in this thread | | | Date | Thu, 15 Oct 2020 20:03:12 +0100 | From | Matthew Wilcox <> | Subject | Re: [PATCH v2 16/16] iomap: Make readpage synchronous |
| |
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 conditional assignments. If the > ->rac check is too raw please just add an explicit > > bool synchronous : 1; > > flag.
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.
> True. I'd still prefer the AOP_UPDATED_PAGE as the fallthrough case > and an explicit goto out_unlock, though.
So this?
if (ctx.bio) { submit_bio(ctx.bio); wait_for_completion(&ctx.done); if (ret < 0) goto err; ret = blk_status_to_errno(ctx.status); }
if (ret < 0) goto err; return AOP_UPDATED_PAGE; err: unlock_page(page); return ret;
| |