Maybe I'm beating a dead horse, but here's what I came up with:

include/linux/mmzone.h:640:22: warning: potentially expensive pointer 
subtraction

Using arch/x86/mm/highmem_32.o as an example, line 82, PageHighMem
expands to is_highmem.

        if (!PageHighMem(page))
                return page_address(page);

I've run through the current code, one of Linus' suggestions and Linus'
alternate version of my original patch without the stupid unsigned
long casts, using char *.  The best version appears to be the char *
version.  Code size will increase by one byte for each use of
is_highmem, but there is one fewer instruction (saves a sar over the
base code).  The reason for the code size increase is the two cmps
use 32-bit compares rather than 16 bit compares in the original due
to the sar beforehand. (-3 bytes for no sar +4 bytes for longer cmp)

It's a small thing, if you agree that the new code is better, I'll
send a patch.

Original:
static inline int is_highmem(struct zone *zone)
{
#ifdef CONFIG_HIGHMEM
        int zone_idx = zone - zone->zone_pgdat->node_zones;
        return zone_idx == ZONE_HIGHMEM ||
                (zone_idx == ZONE_MOVABLE && zone_movable_is_highmem());
#else
        return 0;
#endif
}

 207:   2b 80 8c 07 00 00       sub    0x78c(%eax),%eax
 20d:   c1 f8 0b                sar    $0xb,%eax
 210:   83 f8 02                cmp    $0x2,%eax
 213:   74 16                   je     22b <kmap_atomic_prot+0x144>
 215:   83 f8 03                cmp    $0x3,%eax
 218:   0f 85 8f 00 00 00       jne    2ad <kmap_atomic_prot+0x1c6>
 21e:   83 3d 00 00 00 00 02    cmpl   $0x2,0x0
 225:   0f 85 82 00 00 00       jne    2ad <kmap_atomic_prot+0x1c6>
 22b:   64 a1 00 00 00 00       mov    %fs:0x0,%eax

Linus' suggestion:
static inline int is_highmem(struct zone *zone)
{
#ifdef CONFIG_HIGHMEM
        struct zone *base = zone->zone_pgdat->node_zones;
        return zone == base + ZONE_HIGHMEM ||
                (zone == base + ZONE_MOVABLE && zone_movable_is_highmem());
#else
        return 0;
#endif
}

 202:   8d 90 00 00 00 00       lea    0x0(%eax),%edx
 208:   8b 8a 8c 07 00 00       mov    0x78c(%edx),%ecx
 20e:   8d 81 00 10 00 00       lea    0x1000(%ecx),%eax
 214:   39 c2                   cmp    %eax,%edx
 216:   74 1b                   je     233 <kmap_atomic_prot+0x14c>
 218:   8d 81 00 18 00 00       lea    0x1800(%ecx),%eax
 21e:   39 c2                   cmp    %eax,%edx
 220:   0f 85 8f 00 00 00       jne    2b5 <kmap_atomic_prot+0x1ce>
 226:   83 3d 00 00 00 00 02    cmpl   $0x2,0x0
 22d:   0f 85 82 00 00 00       jne    2b5 <kmap_atomic_prot+0x1ce>
 233:   64 a1 00 00 00 00       mov    %fs:0x0,%eax
 
An alternate suggestion (excuse the long line):

static inline int is_highmem(struct zone *zone)
{
#ifdef CONFIG_HIGHMEM
        int zone_off = (char *)zone - (char *)zone->zone_pgdat->node_zones;
        return zone_off == ZONE_HIGHMEM * sizeof(*zone) ||
                (zone_off == ZONE_MOVABLE * sizeof(*zone) && 
zone_movable_is_highmem());
#else
        return 0;
#endif
}

 207:   2b 80 8c 07 00 00       sub    0x78c(%eax),%eax
 20d:   3d 00 10 00 00          cmp    $0x1000,%eax
 212:   74 18                   je     22c <kmap_atomic_prot+0x145>
 214:   3d 00 18 00 00          cmp    $0x1800,%eax
 219:   0f 85 8f 00 00 00       jne    2ae <kmap_atomic_prot+0x1c7>
 21f:   83 3d 00 00 00 00 02    cmpl   $0x2,0x0
 226:   0f 85 82 00 00 00       jne    2ae <kmap_atomic_prot+0x1c7>
 22c:   64 a1 00 00 00 00       mov    %fs:0x0,%eax

Cheers,

Harvey

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to