netdev - Re: [RFC][PATCH 3/3] enic: add h/w interfaces

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Fri, 29 Aug 2008 11:58:17 -0700
From:	Roland Dreier <rdreier@...co.com>
To:	Scott Feldman <scofeldm@...co.com>
Cc:	netdev@...r.kernel.org
Subject: Re: [RFC][PATCH 3/3] enic: add h/w interfaces

 > > spinning for 100 usecs is pretty nasty... can this be changed to
 > > usleep()?
 > 
 > No because we can't sleep in some of the calling contexts.

I don't think it's a merge-blocker but it is definitely worth thinking
about how to avoid udelay(100) -- a 100+ microsecond latency is pretty
nasty for lots of cases.

 > > not sure why you're making this volatile here... I suspect it doesn't do
 > > what you really want on architectures with a weak memory ordering model,
 > > so it would be better to make things explicit with a memory barrier plus
 > > a comment explaining what you're doing.
 > 
 > We're using volatile here to make the color bit in the descriptor is
 > read first before the any of the other desc fields.  The hardware
 > guarantees the color bit is the last byte (bit) written on the
 > descriptor.  I'll put in a comment explaining what we're doing.

So then volatile doesn't actually do what you want.  You need rmb() to
make sure CPUs with weak ordering models don't reorder things.  Volatile
makes sure the compiler doesn't reorder things but out-of-order CPUs can
easily execute the reads in a different order than that.

 > > This looks way too big to inline.
 > 
 > It's called in the performance path in several places.

My guess is that the benefit of having only one copy in I$ outweighs the
function call overhead.  I guess you could see if you could measure a
difference either way.  And if things are equal then the smaller code is
preferable because it reduces the cache pressure on other code that
might be running.

 - R.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html