Fw: Re: [PATCH] fix page aging (2.4.9-ac12)

Fw: Re: [PATCH] fix page aging (2.4.9-ac12)

[Stephan von Krawczynski]

On Thu, 20 Sep 2001 15:32:26 +0200 Daniel Phillips <phillips@bonn-fries.net>
wrote:

> > Perhaps the following would be better, just in case anyone sets
> > PAGE_AGE_DECL to something other than 1.
> > 
> >     static inline void age_page_down(struct page *page)
> >     {
> > 	unsigned long age = page->age;
> > 	if (age > PAGE_AGE_DECL)
> > 		age -= PAGE_AGE_DECL;
> > 	else
> > 		age = 0;
> > 	page->age = age;
> >     }
> 
> 
>      static inline void age_page_down(struct page *page)
>      {
>  	page->age = max((int) (age - PAGE_AGE_DECL), 0);
>      }
> 
> ;-)

Aehm, Daniel. Just for a hint of what I know about C:

IF age is unsigned long (like declared above) and PAGE_AGE_DECL is a define,
then age-PAGE_AGE_DECL is of type unsigned long, which means it will not go
below 0 but instead be huge positive, so your cast (int) before will not do any
good, and you will not end up with 0 but somewhere above the clouds.
So you just made Linus' max/min/cast point of view _very_ clear, I guess it was
something like "people don't really think about using max/min and related
problems".

;-))

I guess you meant:

page->age = max(((int)age - (int)PAGE_AGE_DECL),0);

Written without actually caring about the real definition of PAGE_AGE_DECL.

Regards,
Stephan

PS: please anyone don't tell me what gnu c actually thinks about this, I don't
care, I read K&R.
And, of course, the whole thing is not worth discussing, it just hit me ...

Re: Fw: Re: [PATCH] fix page aging (2.4.9-ac12)

[Ville Herva]

On Thu, Sep 20, 2001 at 03:48:06PM +0200, you [Stephan von Krawczynski] claimed:
> On Thu, 20 Sep 2001 15:32:26 +0200 Daniel Phillips <phillips@bonn-fries.net>
> wrote:
> 
> >      static inline void age_page_down(struct page *page)
> >      {
> >  	page->age = max((int) (age - PAGE_AGE_DECL), 0);
> >      }
> 
> Aehm, Daniel. Just for a hint of what I know about C:
> 
> IF age is unsigned long (like declared above) and PAGE_AGE_DECL is a define,
> then age-PAGE_AGE_DECL is of type unsigned long, which means it will not go
> below 0 but instead be huge positive, so your cast (int) before will not do any
> good, and you will not end up with 0 but somewhere above the clouds.
> So you just made Linus' max/min/cast point of view _very_ clear, I guess it was
> something like "people don't really think about using max/min and related
> problems".

age - PAGE_AGE_DECL may be a 2^32-1 or so, but when you cast it back to int,
it is at most 2^31 again. It rolls over, so you get the sign bit back.
Witness:

vherva@linux:/home/vherva>cat n.c
#define FOUR 4
void main()
{
   unsigned three = 3;
   printf("%u\n", three - FOUR);
   printf("%i\n", (int)(three - FOUR));
   printf("%i\n", (int)three - (int)FOUR);
}

vherva@linux:/home/vherva>./a.out
4294967295
-1
-1

Perhaps a lucky incidence, but it works as Daniel wrote it. (At least on
32-bit architecture.)

I agree that Rik's variant is much more readable.


-- v --

v@iki.fi

Re: Fw: Re: [PATCH] fix page aging (2.4.9-ac12)

[Stephan von Krawczynski]

On Thu, 20 Sep 2001 17:03:49 +0300 Ville Herva <vherva@niksula.hut.fi> wrote:

> age - PAGE_AGE_DECL may be a 2^32-1 or so, but when you cast it back to int,
> it is at most 2^31 again. It rolls over, so you get the sign bit back.
> Witness:
> 
> vherva@linux:/home/vherva>cat n.c
> #define FOUR 4
> void main()
> {
>    unsigned three = 3;
>    printf("%u\n", three - FOUR);
>    printf("%i\n", (int)(three - FOUR));
>    printf("%i\n", (int)three - (int)FOUR);
> }
> 
> vherva@linux:/home/vherva>./a.out
> 4294967295
> -1
> -1
> 
> Perhaps a lucky incidence, but it works as Daniel wrote it. (At least on
> 32-bit architecture.)

Aha, you name it, and how do you find these goodies when moving the kernel over
to some new hardware platform? Even wrong things have long lives (see communist
block), nevertheless fate sometimes strikes back...

Regards,
Stephan

Re: Fw: Re: [PATCH] fix page aging (2.4.9-ac12)

[Ville Herva]

On Thu, Sep 20, 2001 at 04:20:54PM +0200, you [Stephan von Krawczynski] claimed:
> On Thu, 20 Sep 2001 17:03:49 +0300 Ville Herva <vherva@niksula.hut.fi> wrote:
> 
> > vherva@linux:/home/vherva>./a.out
> > 4294967295
> > -1
> > -1
> 
> Aha, you name it, and how do you find these goodies when moving the kernel over
> to some new hardware platform? 

Tracing an oops for a couple of hours, I assume...

To be clear, I'm not advocating the (int)(a - b) variant, in fact I don't
think I'm qualified to advocate anything. (But I think Rik's variant was the
nicest.)


-- v --

v@iki.fi

Re: Fw: Re: [PATCH] fix page aging (2.4.9-ac12)

[Oliver Xymoron]

On Thu, 20 Sep 2001, Stephan von Krawczynski wrote:

> On Thu, 20 Sep 2001 15:32:26 +0200 Daniel Phillips <phillips@bonn-fries.net>
> wrote:
>
> > > Perhaps the following would be better, just in case anyone sets
> > > PAGE_AGE_DECL to something other than 1.
> > >
> > >     static inline void age_page_down(struct page *page)
> > >     {
> > > 	unsigned long age = page->age;
> > > 	if (age > PAGE_AGE_DECL)
> > > 		age -= PAGE_AGE_DECL;
> > > 	else
> > > 		age = 0;
> > > 	page->age = age;
> > >     }
> >
> >
> >      static inline void age_page_down(struct page *page)
> >      {
> >  	page->age = max((int) (age - PAGE_AGE_DECL), 0);
> >      }
> >
> > ;-)
>
> Aehm, Daniel. Just for a hint of what I know about C:
>
> IF age is unsigned long (like declared above) and PAGE_AGE_DECL is a define,
> then age-PAGE_AGE_DECL is of type unsigned long, which means it will not go
> below 0 but instead be huge positive, so your cast (int) before will not do any
> good, and you will not end up with 0 but somewhere above the clouds.

Bzzt. Conversion from unsigned to signed where the value won't fit is
implementation defined. In practice it's done by truncation which in the
case of twos-complement arithmetic means it becomes negative again, and it
all works. More problematic is when age is just above 2^31 and suddenly
becomes zero - not likely to show up though.

A better paradigm:

 age = max(age, PAGE_AGE_DECL) - PAGE_AGE_DECL;
 age -= min(age, PAGE_AGE_DECL);

--
 "Love the dolphins," she advised him. "Write by W.A.S.T.E.."