From: Ahmad Fatoum <a.fatoum@pengutronix.de>
To: Sascha Hauer <s.hauer@pengutronix.de>,
Barebox List <barebox@lists.infradead.org>
Subject: Re: [PATCH 08/11] fs: ramfs: Use dynamically sized chunks
Date: Thu, 2 Jul 2020 16:28:26 +0200 [thread overview]
Message-ID: <e6dc5288-a62c-48e9-df4d-7e5fde191eb8@pengutronix.de> (raw)
In-Reply-To: <20200615060229.7533-9-s.hauer@pengutronix.de>
On 6/15/20 8:02 AM, Sascha Hauer wrote:
> This changes the way ramfs stores its data. So far we used equally sized
> chunks, this patch changes it to use chunks in a size that fits our
> needs. The chunks are always allocated in the size they are needed for
> the current truncation. Only if we fail to allocate all desired memory
> at once we fall back to allocating smaller chunks. Together with using
> the generic list implementation this results in smaller code and has
> the advantage that many image files end up being contiguously in memory
> and thus we can provide a memmap for them. Files will end up
> contiguously in memory when they are first created, then truncated to
> the final size and then filled up with data. This is something which
> is normally easily achievable when desired.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> fs/ramfs.c | 307 +++++++++++++++++++++++++++--------------------------
> 1 file changed, 159 insertions(+), 148 deletions(-)
>
> diff --git a/fs/ramfs.c b/fs/ramfs.c
> index 2b6df07996..ebe03de736 100644
> --- a/fs/ramfs.c
> +++ b/fs/ramfs.c
> @@ -23,12 +23,15 @@
> #include <errno.h>
> #include <linux/stat.h>
> #include <xfuncs.h>
> +#include <linux/sizes.h>
>
> #define CHUNK_SIZE (4096 * 2)
>
> struct ramfs_chunk {
> char *data;
> - struct ramfs_chunk *next;
> + unsigned long ofs;
> + int size;
> + struct list_head list;
> };
>
> struct ramfs_inode {
> @@ -37,12 +40,14 @@ struct ramfs_inode {
> char *symlink;
> ulong mode;
>
> - ulong size;
> - struct ramfs_chunk *data;
> + /* bytes used in this inode */
> + unsigned long size;
> + /* bytes currently allocated for this inode */
> + unsigned long alloc_size;
> +
> + struct list_head data;
>
> - /* Points to recently used chunk */
> - int recent_chunk;
> - struct ramfs_chunk *recent_chunkp;
> + struct ramfs_chunk *current_chunk;
> };
>
> static inline struct ramfs_inode *to_ramfs_inode(struct inode *inode)
> @@ -89,18 +94,25 @@ static struct inode *ramfs_get_inode(struct super_block *sb, const struct inode
> return inode;
> }
>
> -static struct ramfs_chunk *ramfs_get_chunk(void)
> +#define MIN_SIZE SZ_8K
> +
> +static struct ramfs_chunk *ramfs_get_chunk(unsigned long size)
> {
> struct ramfs_chunk *data = malloc(sizeof(struct ramfs_chunk));
> +
> if (!data)
> return NULL;
>
> - data->data = calloc(CHUNK_SIZE, 1);
> + if (size < MIN_SIZE)
> + size = MIN_SIZE;
> +
> + data->data = calloc(size, 1);
> if (!data->data) {
> free(data);
> return NULL;
> }
> - data->next = NULL;
> +
> + data->size = size;
>
> return data;
> }
> @@ -160,23 +172,6 @@ static int ramfs_symlink(struct inode *dir, struct dentry *dentry,
>
> static int ramfs_unlink(struct inode *dir, struct dentry *dentry)
> {
> - struct inode *inode = d_inode(dentry);
> -
> - if (inode) {
> - struct ramfs_inode *node = to_ramfs_inode(inode);
> - struct ramfs_chunk *chunk = node->data;
> -
> - node->data = NULL;
> -
> - while (chunk) {
> - struct ramfs_chunk *tmp = chunk;
> -
> - chunk = chunk->next;
> -
> - ramfs_put_chunk(tmp);
> - }
> - }
> -
> return simple_unlink(dir, dentry);
> }
>
> @@ -200,80 +195,57 @@ static const struct inode_operations ramfs_dir_inode_operations =
> .create = ramfs_create,
> };
>
> -static struct ramfs_chunk *ramfs_find_chunk(struct ramfs_inode *node, int chunk)
> +static struct ramfs_chunk *ramfs_find_chunk(struct ramfs_inode *node,
> + unsigned long pos, int *ofs, int *len)
> {
> - struct ramfs_chunk *data;
> - int left = chunk;
> + struct ramfs_chunk *data, *cur = node->current_chunk;
>
> - if (chunk == 0)
> - return node->data;
> + if (cur && pos >= cur->ofs)
> + data = cur;
> + else
> + data = list_first_entry(&node->data, struct ramfs_chunk, list);
>
> - if (node->recent_chunk == chunk)
> - return node->recent_chunkp;
> + list_for_each_entry_from(data, &node->data, list) {
> + if (data->ofs + data->size > pos) {
> + *ofs = pos - data->ofs;
> + *len = data->ofs + data->size - pos;
>
> - if (node->recent_chunk < chunk && node->recent_chunk != 0) {
> - /* Start at last known chunk */
> - data = node->recent_chunkp;
> - left -= node->recent_chunk;
> - } else {
> - /* Start at first chunk */
> - data = node->data;
> - }
> + node->current_chunk = data;
>
> - while (left--)
> - data = data->next;
> + return data;
> + }
> + }
>
> - node->recent_chunkp = data;
> - node->recent_chunk = chunk;
> + pr_err("%s: no chunk for pos %ld found\n", __func__, pos);
>
> - return data;
> + return NULL;
> }
>
> static int ramfs_read(struct device_d *_dev, FILE *f, void *buf, size_t insize)
> {
> struct inode *inode = f->f_inode;
> struct ramfs_inode *node = to_ramfs_inode(inode);
> - int chunk;
> struct ramfs_chunk *data;
> - int ofs;
> - int now;
> - int pos = f->pos;
> + int ofs, len, now;
> + unsigned long pos = f->pos;
> int size = insize;
>
> - chunk = pos / CHUNK_SIZE;
> - debug("%s: reading from chunk %d\n", __FUNCTION__, chunk);
> + debug("%s: %p %d @ %lld\n", __func__, node, insize, f->pos);
> +
> + while (size) {
> + data = ramfs_find_chunk(node, pos, &ofs, &len);
> + if (!data)
> + return -EINVAL;
>
> - /* Position ourself in stream */
> - data = ramfs_find_chunk(node, chunk);
> - ofs = pos % CHUNK_SIZE;
> + debug("%s: pos: %ld ofs: %d len: %d\n", __func__, pos, ofs, len);
> +
> + now = min(size, len);
>
> - /* Read till end of current chunk */
> - if (ofs) {
> - now = min(size, CHUNK_SIZE - ofs);
> - debug("Reading till end of node. size: %d\n", size);
> memcpy(buf, data->data + ofs, now);
> +
> size -= now;
> - pos += now;
> buf += now;
> - if (pos > node->size)
> - node->size = now;
> - data = data->next;
> - }
> -
> - /* Do full chunks */
> - while (size >= CHUNK_SIZE) {
> - debug("do full chunk. size: %d\n", size);
> - memcpy(buf, data->data, CHUNK_SIZE);
> - data = data->next;
> - size -= CHUNK_SIZE;
> - pos += CHUNK_SIZE;
> - buf += CHUNK_SIZE;
> - }
> -
> - /* And the rest */
> - if (size) {
> - debug("do rest. size: %d\n", size);
> - memcpy(buf, data->data, size);
> + pos += now;
> }
>
> return insize;
> @@ -283,100 +255,135 @@ static int ramfs_write(struct device_d *_dev, FILE *f, const void *buf, size_t i
> {
> struct inode *inode = f->f_inode;
> struct ramfs_inode *node = to_ramfs_inode(inode);
> - int chunk;
> struct ramfs_chunk *data;
> - int ofs;
> - int now;
> - int pos = f->pos;
> + int ofs, len, now;
> + unsigned long pos = f->pos;
On 32-bit systems, you are truncating a 64-bit pos to 32-bit here. Is this intended?
> int size = insize;
>
> - chunk = f->pos / CHUNK_SIZE;
> - debug("%s: writing to chunk %d\n", __FUNCTION__, chunk);
> + debug("%s: %p %d @ %lld\n", __func__, node, insize, f->pos);
> +
> + while (size) {
> + data = ramfs_find_chunk(node, pos, &ofs, &len);
> + if (!data)
> + return -EINVAL;
> +
> + debug("%s: pos: %ld ofs: %d len: %d\n", __func__, pos, ofs, len);
>
> - /* Position ourself in stream */
> - data = ramfs_find_chunk(node, chunk);
> - ofs = f->pos % CHUNK_SIZE;
> + now = min(size, len);
>
> - /* Write till end of current chunk */
> - if (ofs) {
> - now = min(size, CHUNK_SIZE - ofs);
> - debug("writing till end of node. size: %d\n", size);
> memcpy(data->data + ofs, buf, now);
> +
> size -= now;
> - pos += now;
> buf += now;
> - if (pos > node->size)
> - node->size = now;
> - data = data->next;
> + pos += now;
> + }
> +
> + return insize;
> +}
> +
> +static void ramfs_truncate_down(struct ramfs_inode *node, unsigned long size)
> +{
> + struct ramfs_chunk *data, *tmp;
> +
> + list_for_each_entry_safe(data, tmp, &node->data, list) {
> + if (data->ofs >= size) {
> + list_del(&data->list);
> + node->alloc_size -= data->size;
> + ramfs_put_chunk(data);
> + }
> }
>
> - /* Do full chunks */
> - while (size >= CHUNK_SIZE) {
> - debug("do full chunk. size: %d\n", size);
> - memcpy(data->data, buf, CHUNK_SIZE);
> - data = data->next;
> - size -= CHUNK_SIZE;
> - pos += CHUNK_SIZE;
> - buf += CHUNK_SIZE;
> + node->current_chunk = NULL;
> +}
> +
> +static int ramfs_truncate_up(struct ramfs_inode *node, unsigned long size)
> +{
> + struct ramfs_chunk *data, *tmp;
> + LIST_HEAD(list);
> + unsigned long add = size - node->alloc_size;
> + unsigned long chunksize = add;
> + unsigned long alloc_size = 0;
> +
> + if (node->alloc_size >= size)
> + return 0;
> +
> + /*
> + * We first try to allocate all space we need in a single chunk.
> + * This may fail because of fragmented memory, so in case we cannot
> + * allocate memory we successively decrease the chunk size until
> + * we have enough allocations made.
> + */
> + while (1) {
> + unsigned long now = min(chunksize, add);
> +
> + data = ramfs_get_chunk(now);
> + if (!data) {
> + /* No luck, try with smaller chunk size */
> + chunksize >>= 1;
> +
> + /* If we do not have even 128KiB then go out */
> + if (chunksize < SZ_128K)
> + goto out;
> +
> + continue;
> + }
> +
> + data->ofs = node->alloc_size + alloc_size;
> +
> + alloc_size += data->size;
> +
> + list_add_tail(&data->list, &list);
> +
> + if (add <= data->size)
> + break;
> +
> + add -= data->size;
> }
>
> - /* And the rest */
> - if (size) {
> - debug("do rest. size: %d\n", size);
> - memcpy(data->data, buf, size);
> + list_splice_tail(&list, &node->data);
> +
> + node->alloc_size += alloc_size;
> +
> + return 0;
> +
> +out:
> + list_for_each_entry_safe(data, tmp, &list, list) {
> + list_del(&data->list);
> + ramfs_put_chunk(data);
> }
>
> - return insize;
> + return -ENOSPC;
> }
>
> static int ramfs_truncate(struct device_d *dev, FILE *f, loff_t size)
> {
> struct inode *inode = f->f_inode;
> struct ramfs_inode *node = to_ramfs_inode(inode);
> - int oldchunks, newchunks;
> - struct ramfs_chunk *data = node->data;
> -
> - newchunks = (size + CHUNK_SIZE - 1) / CHUNK_SIZE;
> - oldchunks = (node->size + CHUNK_SIZE - 1) / CHUNK_SIZE;
> -
> - if (newchunks < oldchunks) {
> - if (!newchunks)
> - node->data = NULL;
> - while (newchunks--)
> - data = data->next;
> - while (data) {
> - struct ramfs_chunk *tmp;
> - tmp = data->next;
> - ramfs_put_chunk(data);
> - data = tmp;
> - }
> - if (node->recent_chunk > newchunks)
> - node->recent_chunk = 0;
> - }
> + int ret;
>
> - if (newchunks > oldchunks) {
> - if (data) {
> - data = ramfs_find_chunk(node, oldchunks - 1);
> - } else {
> - node->data = ramfs_get_chunk();
> - if (!node->data)
> - return -ENOSPC;
> - data = node->data;
> - oldchunks = 1;
> - }
> + /*
> + * This is a malloc based filesystem, no need to support more
> + * memory than fits into unsigned long.
> + */
> + if (size > ULONG_MAX)
> + return -ENOSPC;
>
> - while (data->next)
> - data = data->next;
> + debug("%s: %p cur: %ld new: %lld alloc: %ld\n", __func__, node,
> + node->size, size, node->alloc_size);
>
> - while (newchunks > oldchunks) {
> - data->next = ramfs_get_chunk();
> - if (!data->next)
> - return -ENOSPC;
> - data = data->next;
> - oldchunks++;
> - }
> + if (size == node->size)
> + return 0;
> +
> + if (size < node->size) {
> + ramfs_truncate_down(node, size);
> + } else {
> + ret = ramfs_truncate_up(node, size);
> + if (ret)
> + return ret;
> }
> +
> node->size = size;
> +
> return 0;
> }
>
> @@ -386,6 +393,8 @@ static struct inode *ramfs_alloc_inode(struct super_block *sb)
>
> node = xzalloc(sizeof(*node));
>
> + INIT_LIST_HEAD(&node->data);
> +
> return &node->inode;
> }
>
> @@ -393,6 +402,8 @@ static void ramfs_destroy_inode(struct inode *inode)
> {
> struct ramfs_inode *node = to_ramfs_inode(inode);
>
> + ramfs_truncate_down(node, 0);
> +
> free(node);
> }
>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
next prev parent reply other threads:[~2020-07-02 14:28 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-15 6:02 [PATCH 00/11] " Sascha Hauer
2020-06-15 6:02 ` [PATCH 01/11] update list.h from Linux-5.7 Sascha Hauer
2020-06-15 6:02 ` [PATCH 02/11] fs: Add destroy_inode callbacks to filesystems Sascha Hauer
2020-06-15 6:02 ` [PATCH 03/11] fs: Make iput() accept NULL pointers Sascha Hauer
2020-06-15 6:02 ` [PATCH 04/11] fs: free inodes we no longer need Sascha Hauer
2020-08-03 22:02 ` Ahmad Fatoum
2020-08-10 11:13 ` Sascha Hauer
2020-06-15 6:02 ` [PATCH 05/11] digest: Drop usage of memmap Sascha Hauer
2020-06-15 6:02 ` [PATCH 06/11] fs: ramfs: Return -ENOSPC Sascha Hauer
2020-06-15 6:02 ` [PATCH 07/11] fs: ramfs: Drop dead code Sascha Hauer
2020-06-15 6:02 ` [PATCH 08/11] fs: ramfs: Use dynamically sized chunks Sascha Hauer
2020-07-02 14:28 ` Ahmad Fatoum [this message]
2020-07-05 14:14 ` Sascha Hauer
2020-06-15 6:02 ` [PATCH 09/11] fs: ramfs: Implement memmap Sascha Hauer
2020-06-15 6:02 ` [PATCH 10/11] libfile: copy_file: Fix calling discard_range Sascha Hauer
2020-06-15 6:02 ` [PATCH 11/11] libfile: copy_file: explicitly truncate to final size Sascha Hauer
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=e6dc5288-a62c-48e9-df4d-7e5fde191eb8@pengutronix.de \
--to=a.fatoum@pengutronix.de \
--cc=barebox@lists.infradead.org \
--cc=s.hauer@pengutronix.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox