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;