All of lore.kernel.org
 help / color / mirror / Atom feed
From: Roland Dreier <rdreier-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
To: Eli Cohen <eli-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
Cc: Linux RDMA list
	<linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Eli Cohen <eli-VPRAkNaXOzVS1MOuV/RT9w@public.gmane.org>,
	ewg <ewg-ZwoEplunGu1OwGhvXhtEPSCwEArCW2h5@public.gmane.org>,
	general-list
	<general-ZwoEplunGu1OwGhvXhtEPSCwEArCW2h5@public.gmane.org>
Subject: Re: [PATCH] mlx4: remove limitation on LSO header size
Date: Mon, 12 Oct 2009 10:03:03 -0700	[thread overview]
Message-ID: <adaiqekh7eg.fsf@cisco.com> (raw)
In-Reply-To: <20091011100015.GB4929@mtls03> (Eli Cohen's message of "Sun, 11 Oct 2009 12:00:15 +0200")


 > >  > +	*blh = unlikely(halign > 64) ? 1 : 0;

 > > This idiom of "(boolean condition) ? 1 : 0" looks odd to me... doesn't
 > > (halign > 64) already evaluate to 1 or 0 anyway?  Does the unlikely()
 > > actually affect code generation here?

 > True, (halign > 64) is the same and is cleaner. As for the unlikely()
 > -- well it's already been there and besides, we're never sure if it
 > will improve anything so the same question could be asked for other
 > places in the code.

I was just wondering in this case where you are just assigning the
boolean value of the expression to a variable how unlikely affects
things.  I can understand for conditional jumps how the compiler can
choose to make the likely case more efficient, but when there are no
jumps then I was just curious how the hint could help.

 > > I assume this initialization is to avoid a compiler warning.  But the
 > > code is actually correct without initializing blh -- so I think that we
 > > save a tiny bit of code by doing uninitialized_var() instead?

 > We must initialize blh since it is used for any send request and not
 > just LSO opcodes. 

So then this patch was buggy because blh was not reinitialized as we
loop through multiple work requests?  eg an LSO request followed by a
non-LSO request?

Anyway I'll look over the newer patch.

  reply	other threads:[~2009-10-12 17:03 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-30  9:07 [PATCH] mlx4: remove limitation on LSO header size Eli Cohen
2009-10-07 22:45 ` Roland Dreier
     [not found]   ` <adaocoiombn.fsf-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
2009-10-11 10:00     ` Eli Cohen
2009-10-12 17:03       ` Roland Dreier [this message]
     [not found] ` <4AC858E0.2010506@voltaire.com>
     [not found]   ` <4AC858E0.2010506-smomgflXvOZWk0Htik3J/w@public.gmane.org>
2009-10-11  9:47     ` [ewg] " Eli Cohen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=adaiqekh7eg.fsf@cisco.com \
    --to=rdreier-fyb4gu1cfyuavxtiumwx3w@public.gmane.org \
    --cc=eli-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org \
    --cc=eli-VPRAkNaXOzVS1MOuV/RT9w@public.gmane.org \
    --cc=ewg-ZwoEplunGu1OwGhvXhtEPSCwEArCW2h5@public.gmane.org \
    --cc=general-ZwoEplunGu1OwGhvXhtEPSCwEArCW2h5@public.gmane.org \
    --cc=linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.