LKML Archive on
help / color / mirror / Atom feed
From: Artem Bityutskiy <>
To: Richard Purdie <>
Cc: linux-mtd <>,
	LKML <>
Subject: Re: [PATCH/RFC] oops and panic message logging to MTD
Date: Tue, 19 Jun 2007 13:29:22 +0300	[thread overview]
Message-ID: <1182248962.4403.60.camel@sauron> (raw)
In-Reply-To: <1182247254.5760.17.camel@localhost.localdomain>

On Tue, 2007-06-19 at 11:00 +0100, Richard Purdie wrote:
> On Tue, 2007-06-19 at 10:55 +0300, Artem Bityutskiy wrote:
> > On Mon, 2007-06-18 at 17:31 +0100, Richard Purdie wrote:
> > > +	if (mtd->erasesize < OOPS_PAGE_SIZE)
> > > +		erase.len = OOPS_PAGE_SIZE;
> > 
> > It seems to me that your code won't work if mtd->erasesize <
> > OOPS_PAGE_SIZE anyway, so this check should not be here I guess.
> Its just a sanity check...

Then do this once in the "add" notifier.

> > Well, mtd->block_isbad() may return error, unlikely, bu still. You also
> > ignore the error at other places.
> Ignoring that is deliberate since it doesn't really matter if the block
> is bad or we can't read from it, the action is the same...

No, bad EB is a perfectly legal thing and you should deal with it. Error
code means that something very bad and sever is going on and you have to
just refuse working with this device.

> > Also, do not ignore error code of mtd->block_markbad()
> All we can do is print a warning, the action will be the same...

No, the action should be returning an error and refuse doing more work.

> > Also, could you please use wait_event_interruptible() instead of
> > set_current_state() which looks better (indeed, you have wait queue, 
> > so use wq calls).
> That piece of code is in keeping with the rest of the mtd erase handling
> code. Looking through the various wq macros, I don't see any which help
> particularly in this case...

Well, it is matter of taste so I do not insist. But I think
wait_event_interruptible() is much nicer. Glance at drivers/ubi/io.c how
it looks.

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

  reply	other threads:[~2007-06-19 10:30 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-06-18 16:31 [PATCH/RFC] oops and panic message logging to MTD Richard Purdie
2007-06-19  7:55 ` Artem Bityutskiy
2007-06-19 10:00   ` Richard Purdie
2007-06-19 10:29     ` Artem Bityutskiy [this message]
2007-06-19 10:52       ` Richard Purdie
2007-06-19 11:05         ` Artem Bityutskiy
2007-07-03  9:47     ` Jarkko Lavinen
2007-07-04  8:54       ` Richard Purdie
2007-06-19  8:07 ` Artem Bityutskiy

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1182248962.4403.60.camel@sauron \ \ \ \ \

* 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;
as well as URLs for NNTP newsgroup(s).