Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751758AbdGaWRX (ORCPT ); Mon, 31 Jul 2017 18:17:23 -0400 Received: from mail.kernel.org ([198.145.29.99]:46874 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751717AbdGaWRU (ORCPT ); Mon, 31 Jul 2017 18:17:20 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7C7D222B6C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=sstabellini@kernel.org Date: Mon, 31 Jul 2017 15:17:17 -0700 (PDT) From: Stefano Stabellini X-X-Sender: sstabellini@sstabellini-ThinkPad-X260 To: Boris Ostrovsky cc: Stefano Stabellini , xen-devel@lists.xen.org, linux-kernel@vger.kernel.org, jgross@suse.com, Stefano Stabellini Subject: Re: [PATCH v2 05/13] xen/pvcalls: implement bind command In-Reply-To: Message-ID: References: <1501017730-12797-1-git-send-email-sstabellini@kernel.org> <1501017730-12797-5-git-send-email-sstabellini@kernel.org> <5978AD87.20504@oracle.com> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1067 Lines: 31 On Thu, 27 Jul 2017, Boris Ostrovsky wrote: > >> This all looks very similar to previous patches. Can it be factored out? > > You are right that the pattern is the same for all commands: > > - get a request > > - fill the request > > - possibly do something else > > - wait > > however each request is different, the struct and fields are different. > > There are spin_lock and spin_unlock calls intermingled. I am not sure I > > can factor out much of this. Maybe I could create a static inline or > > macro as a syntactic sugar to replace the wait call, but that's pretty > > much it I think. > > Maybe you could factor out common fragments, not necessarily the whole > thing at once? > > For example, > > static inline int get_request(*bedata, int *req_id) > { > > *req_id = bedata->ring.req_prod_pvt & (RING_SIZE(&bedata->ring) - 1); > if (RING_FULL(&bedata->ring) || > READ_ONCE(bedata->rsp[*req_id].req_id) != PVCALLS_INVALID_ID) { > return -EAGAIN; > return 0; > } > > (or some such) You are right, the code looks better this way. I'll add it.