Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753783AbbKLRvb (ORCPT ); Thu, 12 Nov 2015 12:51:31 -0500 Received: from smtp.citrix.com ([66.165.176.89]:51222 "EHLO SMTP.CITRIX.COM" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753718AbbKLRva (ORCPT ); Thu, 12 Nov 2015 12:51:30 -0500 X-IronPort-AV: E=Sophos;i="5.20,282,1444694400"; d="scan'208";a="311875517" Subject: Re: [Xen-devel] [PATCH v2 2/2] block/xen-blkfront: Handle non-indirect grant with 64KB pages To: Julien Grall , , References: <1445264354-23648-1-git-send-email-julien.grall@citrix.com> <1445264354-23648-3-git-send-email-julien.grall@citrix.com> <5644C111.2020006@citrix.com> <5644CCA8.4090003@citrix.com> CC: Boris Ostrovsky , David Vrabel From: =?UTF-8?Q?Roger_Pau_Monn=c3=a9?= X-Enigmail-Draft-Status: N1110 Message-ID: <5644D19B.4000900@citrix.com> Date: Thu, 12 Nov 2015 18:51:23 +0100 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <5644CCA8.4090003@citrix.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit X-DLP: MIA1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2025 Lines: 58 El 12/11/15 a les 18.30, Julien Grall ha escrit: > Hi, > > On 12/11/15 16:40, Roger Pau Monné wrote: >>> [1] http://lists.xen.org/archives/html/xen-devel/2015-08/msg02200.html >>> >>> Signed-off-by: Julien Grall >> >> LGTM, only a couple of typos and a simplification: >> >> Signed-off-by: Roger Pau Monné > > Do you mean Acked-by? ;) Yes, I also had problems with smtp, so I thought this one was actually not sent. You have another one with a proper Ack :). >>> + >>> + /* The I/O request may be split in two */ >>> + if (unlikely(s->associated_id != NO_ASSOCIATED_ID)) { >>> + struct blk_shadow *s2 = &info->shadow[s->associated_id]; >>> + >>> + /* Keep the status of the current response in shadow */ >>> + s->status = (bret->status == BLKIF_RSP_OKAY) ? >>> + REQ_DONE : REQ_FAIL; >>> + >>> + /* Wait the second response if not yet here */ >>> + if (s2->status == REQ_WAITING) >>> + return 0; >>> + >>> + /* >>> + * The status of the current response will be used in >>> + * order to know if the request has failed. >>> + * Update the current response status only if has not >>> + * failed. >>> + */ >>> + if (bret->status == BLKIF_RSP_OKAY && s2->status == REQ_FAIL) >> >> This could be simplified by only checking if s2->status == REQ_FAIL. > > I didn't do it because bret->status may be different than > BLKIF_RSP_ERROR (for instance BLKIF_RSP_EOPNOTSUPP). I think this is not actually possible in practice, but what if bret->status == BLKIF_RSP_OKAY and the bret from s2 actually had BLKIF_RSP_EOPNOTSUPP, wouldn't we loose the EOPNOTSUPP by unconditionally setting BLKIF_RSP_ERROR? Should s->status be able to store all the possible return codes from the response (OK/ERROR/NOTSUPP)? Roger. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/