lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 21 Jul 2015 12:12:12 +0100 From: Julien Grall <julien.grall@...rix.com> To: Roger Pau Monné <roger.pau@...rix.com>, <xen-devel@...ts.xenproject.org> CC: <ian.campbell@...rix.com>, <stefano.stabellini@...citrix.com>, <linux-kernel@...r.kernel.org>, David Vrabel <david.vrabel@...rix.com>, "Boris Ostrovsky" <boris.ostrovsky@...cle.com>, <linux-arm-kernel@...ts.infradead.org> Subject: Re: [Xen-devel] [PATCH v2 05/20] block/xen-blkfront: Split blkif_queue_request in 2 Hi Roger, On 21/07/15 10:54, Roger Pau Monné wrote: > El 09/07/15 a les 22.42, Julien Grall ha escrit: >> Currently, blkif_queue_request has 2 distinct execution path: >> - Send a discard request >> - Send a read/write request >> >> The function is also allocating grants to use for generating the >> request. Although, this is only used for read/write request. >> >> Rather than having a function with 2 distinct execution path, separate >> the function in 2. This will also remove one level of tabulation. >> >> Signed-off-by: Julien Grall <julien.grall@...rix.com> >> Cc: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com> >> Cc: Roger Pau Monné <roger.pau@...rix.com> >> Cc: Boris Ostrovsky <boris.ostrovsky@...cle.com> >> Cc: David Vrabel <david.vrabel@...rix.com> > > Patch looks fine, although with so much indentation changes it's kind of > hard to review. I wasn't sure how to make this patch more easy to review and it seems like diff is getting confused. It's mostly removing one indentation layer (the if (req->cmd_flags ...)) and move the discard code in a separate function. > Acked-by: Roger Pau Monné <roger.pau@...rix.com> Thank you. > Just one minor change below. > > [...] > >> @@ -595,6 +603,24 @@ static int blkif_queue_request(struct request *req) >> return 0; >> } >> >> +/* >> + * Generate a Xen blkfront IO request from a blk layer request. Reads >> + * and writes are handled as expected. >> + * >> + * @req: a request struct >> + */ >> +static int blkif_queue_request(struct request *req) >> +{ >> + struct blkfront_info *info = req->rq_disk->private_data; >> + >> + if (unlikely(info->connected != BLKIF_STATE_CONNECTED)) >> + return 1; >> + >> + if (unlikely(req->cmd_flags & (REQ_DISCARD | REQ_SECURE))) >> + return blkif_queue_discard_req(req); >> + else >> + return blkif_queue_rw_req(req); > > There's no need for the else clause. I find it more readable and obvious to understand than: if ( ... ) return return; when there is only one line in the else. IIRC, the resulting assembly will be the same. Anyway, I can drop the else if you really want. Regards, -- Julien Grall -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists