All of lore.kernel.org
 help / color / mirror / Atom feed
From: Segher Boessenkool <segher@kernel.crashing.org>
To: Mel Gorman <mel@csn.ul.ie>
Cc: Andrew Morton <akpm@osdl.org>,
	linuxppc-dev@ozlabs.org, vgoyal@in.ibm.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Compile failure fix for ppc on 2.6.17-rc4-mm3 (2nd attempt)
Date: Mon, 29 May 2006 19:56:21 +0200	[thread overview]
Message-ID: <c6414fc4b2c627791a49085bf8eea7e8@kernel.crashing.org> (raw)
In-Reply-To: <Pine.LNX.4.64.0605291825500.11234@skynet.skynet.ie>

>>>  #define push_end(res, size) do { unsigned long __sz = (size) ; \
>>> -	res->end = ((res->end + __sz) / (__sz + 1)) * (__sz + 1) + __sz; \
>>> +	resource_size_t farEnd = (res->end + __sz); \
>>> +	do_div(farEnd, (__sz + 1)); \
>>> +	res->end = farEnd * (__sz + 1) + __sz; \
>>>      } while (0)
>>
>> Size here is a) a misnomer (size + 1 is the actual size) and b) 
>> always a power
>> of two minus one.  So instead, do
>>
>> #define push_end(res, mask) res->end = -(-res->end & ~(unsigned 
>> long)mask)
>>
>> (with a do { } while(0) armour if you prefer).
>>
>
> It's not doing the same as the old code so is your suggested fix a 
> correct replacement?
>
> For example, given 0xfff for size the current code rounds res->end to 
> the next 0x1000 boundary and adds 0xfff. Your propose fix just rounds 
> it to the next 0x1000 if I'm reading it correctly but is what the code 
> was meant to do in the first place? Using masks, the equivilant of the 
> current code is something like;
>
> #define push_end(res, mask) do { \
> 	res->end = -(-res->end & ~(unsigned long)mask); \
> 	res->end += mask; \
> } while (0)

Yeah forgot a bit, this looks fine.


Segher

WARNING: multiple messages have this Message-ID (diff)
From: Segher Boessenkool <segher@kernel.crashing.org>
To: Mel Gorman <mel@csn.ul.ie>
Cc: vgoyal@in.ibm.com, Andrew Morton <akpm@osdl.org>,
	linuxppc-dev@ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Compile failure fix for ppc on 2.6.17-rc4-mm3 (2nd attempt)
Date: Mon, 29 May 2006 19:56:21 +0200	[thread overview]
Message-ID: <c6414fc4b2c627791a49085bf8eea7e8@kernel.crashing.org> (raw)
In-Reply-To: <Pine.LNX.4.64.0605291825500.11234@skynet.skynet.ie>

>>>  #define push_end(res, size) do { unsigned long __sz = (size) ; \
>>> -	res->end = ((res->end + __sz) / (__sz + 1)) * (__sz + 1) + __sz; \
>>> +	resource_size_t farEnd = (res->end + __sz); \
>>> +	do_div(farEnd, (__sz + 1)); \
>>> +	res->end = farEnd * (__sz + 1) + __sz; \
>>>      } while (0)
>>
>> Size here is a) a misnomer (size + 1 is the actual size) and b) 
>> always a power
>> of two minus one.  So instead, do
>>
>> #define push_end(res, mask) res->end = -(-res->end & ~(unsigned 
>> long)mask)
>>
>> (with a do { } while(0) armour if you prefer).
>>
>
> It's not doing the same as the old code so is your suggested fix a 
> correct replacement?
>
> For example, given 0xfff for size the current code rounds res->end to 
> the next 0x1000 boundary and adds 0xfff. Your propose fix just rounds 
> it to the next 0x1000 if I'm reading it correctly but is what the code 
> was meant to do in the first place? Using masks, the equivilant of the 
> current code is something like;
>
> #define push_end(res, mask) do { \
> 	res->end = -(-res->end & ~(unsigned long)mask); \
> 	res->end += mask; \
> } while (0)

Yeah forgot a bit, this looks fine.


Segher


  reply	other threads:[~2006-05-29 17:54 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-05-26 15:12 [PATCH] Compile failure fix for ppc on 2.6.17-rc4-mm3 (2nd attempt) Mel Gorman
2006-05-26 15:12 ` Mel Gorman
2006-05-26 16:49 ` Andrew Morton
2006-05-29 15:49   ` Mel Gorman
2006-05-29 15:49     ` Mel Gorman
2006-05-29 16:22     ` Segher Boessenkool
2006-05-29 16:22       ` Segher Boessenkool
2006-05-29 17:38       ` Mel Gorman
2006-05-29 17:38         ` Mel Gorman
2006-05-29 17:56         ` Segher Boessenkool [this message]
2006-05-29 17:56           ` Segher Boessenkool
2006-05-29 19:05           ` Mel Gorman
2006-05-29 19:05             ` Mel Gorman
2006-06-09  9:36             ` Paul Mackerras
2006-06-09  9:36               ` Paul Mackerras
2006-06-13 16:49               ` Mel Gorman
2006-06-13 16:49                 ` Mel Gorman

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=c6414fc4b2c627791a49085bf8eea7e8@kernel.crashing.org \
    --to=segher@kernel.crashing.org \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=mel@csn.ul.ie \
    --cc=vgoyal@in.ibm.com \
    /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.