Hideo AOKI wrote:
David Miller wrote:
From: Hideo AOKI <[EMAIL PROTECTED]>
Date: Tue, 13 Nov 2007 22:27:13 -0500
Herbert Xu wrote:
On Mon, Oct 29, 2007 at 05:23:10PM -0400, Hideo AOKI wrote:
+#define SK_DATAGRAM_MEM_QUANTUM ((int)PAGE_SIZE)
+
+static inline int sk_datagram_pages(int amt)
+{
+ return DIV_ROUND_UP(amt, SK_DATAGRAM_MEM_QUANTUM);
+}
Does this really have to be int? Unsigned would let the compiler
optimise this to a simple shift.
Thank you for the comment.
This inline function is used to calculate the first argument of
atomic_add()
and atomic_sub(). Since the argument is int, I believe that using int is
better than using unsigned int.
If you know the values will always be positive, as you will know here,
it is OK to us unsigned int here and avoids the unacceptable expensive
divide instruction.
Please fix this.
Thanks for your comments. I finally understood.
I'll fix it and resubmit the patch as soon as possible.
Let me propose the following code to use a shift instruction instead
of a divide instruction. I confirmed that the code could remove a
divide instruction, however, I would like to have comment on this
implementation.
+#define SK_DATAGRAM_MEM_QUANTUM ((unsigned int)PAGE_SIZE)
+
+static inline int sk_datagram_pages(int amt)
+{
+ /* Cast to unsigned as an optimization, since amt is always positive. */
+ return DIV_ROUND_UP((unsigned int)amt, SK_DATAGRAM_MEM_QUANTUM);
+}
+
Please let me know if there is proper coding style.
I'm re-testing the whole patch set right now. If there is no problem,
I'll resubmit new patch set, which includes this fix, tomorrow.
Many thanks,
Hideo
--
Hitachi Computer Products (America) Inc.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
