linux-kernel - Re: [PATCH 1/8] Add yaffs2 file system: allocator, attribs, bitmap code

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] [thread-next>] [day] [month] [year] [list]
Date:	Thu, 2 Dec 2010 14:48:56 -0600
From:	kevin granade <kevin.granade@...il.com>
To:	Ryan Mallon <ryan@...ewatersys.com>
Cc:	Charles Manning <cdhmanning@...il.com>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/8] Add yaffs2 file system: allocator, attribs, bitmap code

On Thu, Dec 2, 2010 at 1:49 PM, Ryan Mallon <ryan@...ewatersys.com> wrote:
> On 12/01/2010 10:57 AM, Charles Manning wrote:
>> Signed-off-by: Charles Manning <cdhmanning@...il.com>
>> ---
>>  fs/yaffs2/yaffs_allocator.c |  397 +++++++++++++++++++++++++++++++++++++++++++
>>  fs/yaffs2/yaffs_allocator.h |   30 ++++
>>  fs/yaffs2/yaffs_attribs.c   |  124 ++++++++++++++
>>  fs/yaffs2/yaffs_attribs.h   |   28 +++
>>  fs/yaffs2/yaffs_bitmap.c    |  104 +++++++++++
>>  fs/yaffs2/yaffs_bitmap.h    |   33 ++++
>>  6 files changed, 716 insertions(+), 0 deletions(-)
>>  create mode 100644 fs/yaffs2/yaffs_allocator.c
>>  create mode 100644 fs/yaffs2/yaffs_allocator.h
>>  create mode 100644 fs/yaffs2/yaffs_attribs.c
>>  create mode 100644 fs/yaffs2/yaffs_attribs.h
>>  create mode 100644 fs/yaffs2/yaffs_bitmap.c
>>  create mode 100644 fs/yaffs2/yaffs_bitmap.h
>>
>> diff --git a/fs/yaffs2/yaffs_allocator.c b/fs/yaffs2/yaffs_allocator.c
>> new file mode 100644
>> index 0000000..b9fe31e
>> --- /dev/null
>> +++ b/fs/yaffs2/yaffs_allocator.c
>> @@ -0,0 +1,397 @@
>> +/*
>> + * YAFFS: Yet Another Flash File System. A NAND-flash specific file system.
>> + *
>> + * Copyright (C) 2002-2010 Aleph One Ltd.
>> + *   for Toby Churchill Ltd and Brightstar Engineering
>> + *
>> + * Created by Charles Manning <charles@...ph1.co.uk>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include "yaffs_allocator.h"
>> +#include "yaffs_guts.h"
>> +#include "yaffs_trace.h"
>> +#include "yportenv.h"
>> +
>> +#ifdef CONFIG_YAFFS_YMALLOC_ALLOCATOR
>
> This doesn't appear to be defined anywhere and is not in Kconfig. Is
> this just a wrapper for support on other operating systems or does it
> can it be used on Linux for debugging? In the former case it should
> probably removed and in the latter it should probably have a comment
> explaining why you would want to define it.
>
> <snip>
>
>> +
>> +#include "yaffs_bitmap.h"
>> +#include "yaffs_trace.h"
>> +/*
>> + * Chunk bitmap manipulations
>> + */
>> +
>> +static Y_INLINE u8 *yaffs_block_bits(struct yaffs_dev *dev, int blk)
>
> Should just use inline.
>
> <snip>
>
>> +
>> +int yaffs_still_some_chunks(struct yaffs_dev *dev, int blk)
>> +{
>> +     u8 *blk_bits = yaffs_block_bits(dev, blk);
>> +     int i;
>> +     for (i = 0; i < dev->chunk_bit_stride; i++) {
>
> Nitpick: You should have a blank line between variable declarations and
> start of code.
>
> <snip>
>
>> +int yaffs_count_chunk_bits(struct yaffs_dev *dev, int blk)
>> +{
>> +     u8 *blk_bits = yaffs_block_bits(dev, blk);
>> +     int i;
>> +     int n = 0;
>> +     for (i = 0; i < dev->chunk_bit_stride; i++) {
>> +             u8 x = *blk_bits;
>> +             while (x) {
>> +                     if (x & 1)
>> +                             n++;
>> +                     x >>= 1;
>> +             }
>> +
>> +             blk_bits++;
>> +     }
>> +     return n;
>> +}
>
>
> This is possibly more concise as a for loop. Also moving the definitions
> all to the top of the file and combining same types definitions on one
> line reduces this function to:
>
> int yaffs_count_chunk_bits(struct yaffs_dev *dev, int blk)
> {
>        u8 x, *blk_bits = yaffs_block_bits(dev, blk);
>        int i, n = 0;
>
>        for (i = 0; i < dev->chunk_bit_stride; i++, blk_bits++)
>                for (x = *blk_bits; x; x >>= 1)
>                        if (x & 1)
>                                n++;
>
>        return n;
> }

Actually, how about using the standard hamming weight function?
This adds a dependency on linux/bitops.h, but seems to be carefully
crafted to be as fast as possible based on the available hardware.

int yaffs_count_chunk_bits(struct yaffs_dev *dev, int blk)
{
       u8 *blk_bits = yaffs_block_bits(dev, blk);
       int i, n = 0;

       for (i = 0; i < dev->chunk_bit_stride; i++, blk_bits++)
               n += hweight8(*blk_bits);

       return n;
}

Kevin Granade


>
> ~Ryan
>
> --
> Bluewater Systems Ltd - ARM Technology Solution Centre
>
> Ryan Mallon                     5 Amuri Park, 404 Barbadoes St
> ryan@...ewatersys.com           PO Box 13 889, Christchurch 8013
> http://www.bluewatersys.com     New Zealand
> Phone: +64 3 3779127            Freecall: Australia 1800 148 751
> Fax:   +64 3 3779135                      USA 1800 261 2934
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/