On Wed, Jan 12, 2011 at 10:07 PM, David Dillow <[email protected]> wrote: > On Wed, 2011-01-12 at 14:50 -0500, Roland Dreier wrote: >> > Attached -- I'm using kind of a minimal config for build testing, so >> > maybe I have some strange optimization enabled >> (CONFIG_CC_OPTIMIZE_FOR_SIZE=y?) >> >> Indeed... unsetting CONFIG_CC_OPTIMIZE_FOR_SIZE seems to get rid of the >> warning. > > Pushed the uninitialized_var change to > > git://git.kernel.org/pub/scm/linux/kernel/git/dad/srp-initiator.git > srp-lock-scaling-warning
Hello Dave, I found this patch in the above location: IB/srp: surpress uninitialized variable warning with -Os Signed-off-by: David Dillow <[email protected]> --- 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, DMA_TO_DEVICE); @@ -1185,6 +1181,7 @@ err_iu: spin_lock_irqsave(&target->lock, flags); list_add(&req->list, &target->free_reqs); +err_unlock: spin_unlock_irqrestore(&target->lock, flags); err: -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
