[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20080126220404.5c128ca3.akpm@linux-foundation.org>
Date: Sat, 26 Jan 2008 22:04:04 -0800
From: Andrew Morton <akpm@...ux-foundation.org>
To: Kyungmin Park <kmpark@...radead.org>
Cc: linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH] CRAMFS: Uncompressed files support
> On Tue, 22 Jan 2008 11:23:45 +0900 Kyungmin Park <kmpark@...radead.org> wrote:
> Hi,
>
> This patch enables the uncompressed files support in cramfs.
>
> The word 'uncompressed file' is from linear cramfs (aka Application XIP).
> In linear cramfs, it is used to suport XIP on NOR. However it is also helpful on OneNAND. It makes a filesystem faster by removing compression overhead.
> In XIP mode it runs XIP, But non-XIP mode. It copies data to ram and runs.
>
> In my simple test, copy busybox (compressed or uncompressed).
> It reduces the about 50% time saving from 0.40s to 0.19s.
> Yes, it incrases the file system size, but nowadays flash has big capacity.
> It's trade-off between size and performance.
>
> Also this patch uses the page cache directly.
> In previous implementation, it used the local buffer. why?
> It's already uncompressed and fits to page size. So It uses the page directly to remove useless memory copy.
>
> It's compatible the existing linear cramfs image and original one.
>
> Any comments are welcome.
>
> diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c
> index 3d194a2..edba28f 100644
> --- a/fs/cramfs/inode.c
> +++ b/fs/cramfs/inode.c
I don't know what kernel this is against but it generates a lot of rejects
against present mainline.
> @@ -40,6 +40,7 @@ static DEFINE_MUTEX(read_mutex);
> #define CRAMINO(x) (((x)->offset && (x)->size)?(x)->offset<<2:1)
> #define OFFSET(x) ((x)->i_ino)
>
> +#define CRAMFS_INODE_IS_XIP(x) ((x)->i_mode & S_ISVTX)
>
> static int cramfs_iget5_test(struct inode *inode, void *opaque)
> {
> @@ -143,8 +144,9 @@ static int next_buffer;
> /*
> * Returns a pointer to a buffer containing at least LEN bytes of
> * filesystem starting at byte offset OFFSET into the filesystem.
> + * If the @pg has the page, it returns the page buffer address
> */
> -static void *cramfs_read(struct super_block *sb, unsigned int offset, unsigned int len)
> +static void *cramfs_read(struct super_block *sb, unsigned int offset, unsigned int len, struct page **pg)
> {
> struct address_space *mapping = sb->s_bdev->bd_inode->i_mapping;
> struct page *pages[BLKS_PER_BUF];
> @@ -174,6 +176,22 @@ static void *cramfs_read(struct super_block *sb, unsigned int offset, unsigned i
>
> devsize = mapping->host->i_size >> PAGE_CACHE_SHIFT;
>
> + /*
> + * Use page directly either
> + * - uncompressed page or
> + * - comprssed page which has all required data
> + */
> + if (pg && offset + len <= PAGE_CACHE_SIZE) {
> + struct page *page = NULL;
> + page = read_mapping_page(mapping, blocknr, NULL);
> + if (!IS_ERR(page)) {
> + *pg = page;
> + data = kmap(page);
> + data += offset;
> + return data;
> + }
> + }
Are you sure about the kmap/kunmap handling in this patch? It looks like
it might be unbalanced.
--
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/
Powered by blists - more mailing lists