From patchwork Thu Jan 13 11:19:34 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bart Van Assche X-Patchwork-Id: 475541 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.3) with ESMTP id p0DBJuh0017280 for ; Thu, 13 Jan 2011 11:19:57 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756600Ab1AMLT4 (ORCPT ); Thu, 13 Jan 2011 06:19:56 -0500 Received: from mail-iy0-f174.google.com ([209.85.210.174]:43287 "EHLO mail-iy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756321Ab1AMLTz convert rfc822-to-8bit (ORCPT ); Thu, 13 Jan 2011 06:19:55 -0500 Received: by iyj18 with SMTP id 18so1409350iyj.19 for ; Thu, 13 Jan 2011 03:19:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:in-reply-to:references:from :date:x-google-sender-auth:message-id:subject:to:cc:content-type :content-transfer-encoding; bh=B5srUJjRUId/ywF3NhsTABzR9PSpHyh/XLedur9tJAI=; b=w0HHPUXASvVVjHEYLlzuMGZxvRmfM0MHHg9TN4nJU/CHdabnyaxkG451Dc4bo2yZNP ycJtlm/q2PNJ+r72efXSNX4Utnj7BeXk7QYfWWpuNK8K4bZ++g0CFOZ73VoUoCqJuCqI AFVN/+hEO5UKyFg2s6Hr1/mbceq5WQWDkZVlk= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:from:date :x-google-sender-auth:message-id:subject:to:cc:content-type :content-transfer-encoding; b=UO+ppIhDf4CU5jT6lsF8Va1J49Cw2kdN1ELJSSMBDfvE2IABIhsJ6HP8CCoEUax+kf cCPC45mZRSlJtXbPJbGNJHF3Ot4zvh8sO0Zxd0hG+Gl2VdxC1XMyRNf/3e9fyWIA0O2s c+q+ZcmqYMHmRHip7HhnFEdoQqDGjQ24c5FK0= Received: by 10.231.149.194 with SMTP id u2mr2299845ibv.32.1294917594921; Thu, 13 Jan 2011 03:19:54 -0800 (PST) MIME-Version: 1.0 Received: by 10.231.30.199 with HTTP; Thu, 13 Jan 2011 03:19:34 -0800 (PST) In-Reply-To: <1294866460.15826.41.camel@lap75545.ornl.gov> References: <1294854748.15826.26.camel@lap75545.ornl.gov> <1294861341.15826.38.camel@lap75545.ornl.gov> <1294866460.15826.41.camel@lap75545.ornl.gov> From: Bart Van Assche Date: Thu, 13 Jan 2011 12:19:34 +0100 X-Google-Sender-Auth: sElfYhYJti8tYMJcFA8fN72STjY Message-ID: Subject: Re: srp warning To: David Dillow Cc: Roland Dreier , "linux-rdma@vger.kernel.org" Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Thu, 13 Jan 2011 11:19:57 +0000 (UTC) diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index 4b62105..cc29d57 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -1113,7 +1113,7 @@ static void srp_send_completion(struct ib_cq *cq, void *target_ptr) static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd) { struct srp_target_port *target = host_to_target(shost); - struct srp_request *req; + struct srp_request *uninitialized_var(req); struct srp_iu *iu; struct srp_cmd *cmd; struct ib_device *dev; While I don't doubt that that patch has been tested properly and that it has been carefully selected over the possible alternatives, I have been wondering why an approach like the one below (patch has not been tested) has not been chosen ? Not only should that patch avoid triggering a warning about the variable 'req' not being initialized but it also eliminates an if-statement and hence might improve performance a little. Bart. diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index 4b62105..3164c4d 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -1132,16 +1132,12 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd) spin_lock_irqsave(&target->lock, flags); iu = __srp_get_tx_iu(target, SRP_IU_CMD); - if (iu) { - req = list_first_entry(&target->free_reqs, struct srp_request, - list); - list_del(&req->list); - } + if (unlikely(!iu)) + goto err_unlock; + req = list_first_entry(&target->free_reqs, struct srp_request, list); + list_del(&req->list); spin_unlock_irqrestore(&target->lock, flags); - if (!iu) - goto err; - dev = target->srp_host->srp_dev->dev; ib_dma_sync_single_for_cpu(dev, iu->dma, srp_max_iu_len,