All of lore.kernel.org
 help / color / mirror / Atom feed
From: Artem Bityutskiy <dedekind1@gmail.com>
To: Grant Erickson <marathon96@gmail.com>
Cc: Jarkko Lavinen <jarkko.lavinen@nokia.com>, linux-mtd@lists.infradead.org
Subject: Re: [PATCH v3] Retry Large Buffer Allocations
Date: Thu, 07 Apr 2011 11:13:03 +0300	[thread overview]
Message-ID: <1302163983.2407.12.camel@localhost> (raw)
In-Reply-To: <1302105402-12990-1-git-send-email-marathon96@gmail.com>

On Wed, 2011-04-06 at 08:56 -0700, Grant Erickson wrote:
> When handling user space read or write requests via mtd_{read,write}
> or JFFS2 medium scan requests, exponentially back off on the size of
> the requested kernel transfer buffer until it succeeds or until the
> requested transfer buffer size falls below the page size.
> 
> This helps ensure the operation can succeed under low-memory,
> highly-fragmented situations albeit somewhat more slowly.
> 
>   v2: Incorporated coding style and comment feedback from Artem.
>   v3: Incorporated more feedback from Artem. Retargeted patch against
>       l2-mtd-2.6.
> 
> Signed-off-by: Grant Erickson <marathon96@gmail.com>

Looks good, but I have 2 requests still.

> ---
>  drivers/mtd/mtdchar.c   |   50 +++++++++++++++++++++-------------------------
>  drivers/mtd/mtdcore.c   |   41 ++++++++++++++++++++++++++++++++++++++
>  fs/jffs2/scan.c         |   11 +++++----
>  include/linux/mtd/mtd.h |    2 +
>  4 files changed, 72 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
> index 145b3d0d..8651a79 100644
> --- a/drivers/mtd/mtdchar.c
> +++ b/drivers/mtd/mtdchar.c
> @@ -166,10 +166,23 @@ static int mtd_close(struct inode *inode, struct file *file)
>  	return 0;
>  } /* mtd_close */
>  
> -/* FIXME: This _really_ needs to die. In 2.5, we should lock the
> -   userspace buffer down and use it directly with readv/writev.
> -*/
> -#define MAX_KMALLOC_SIZE 0x20000
> +/* Back in April 2005, Linus wrote:
> + *
> + *   FIXME: This _really_ needs to die. In 2.5, we should lock the
> + *   userspace buffer down and use it directly with readv/writev.
> + *
> + * The implementation below, using mtd_alloc_up_to, mitigates

1. s/mtd_alloc_up_to/mtd_kmalloc_up_to/ here as well.

BTW, It still does not apply on top of l2-mtd-2.6.git, the conflict is
in include/linux/mtd/mtd.h - but well, I can fix that conflict.

2. Please, fix compilation warning:

fs/jffs2/scan.c: In function ‘jffs2_scan_medium’:
fs/jffs2/scan.c:123: warning: passing argument 1 of ‘mtd_kmalloc_up_to’ from incompatible pointer type
include/linux/mtd/mtd.h:351: note: expected ‘size_t *’ but argument is of type ‘uint32_t *’

This is a bug in 64-bit machines because sizeof(size_t) is 8 there.

Could you please fix that, test, and then send the final version.

Thanks!

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

      reply	other threads:[~2011-04-07  8:15 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-06 15:56 [PATCH v3] Retry Large Buffer Allocations Grant Erickson
2011-04-07  8:13 ` Artem Bityutskiy [this message]

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=1302163983.2407.12.camel@localhost \
    --to=dedekind1@gmail.com \
    --cc=jarkko.lavinen@nokia.com \
    --cc=linux-mtd@lists.infradead.org \
    --cc=marathon96@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.