linux-kernel - Re: [PATCH/RFC] oops and panic message logging to MTD

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]

Message-Id: <1182250350.5760.42.camel@localhost.localdomain>
Date:	Tue, 19 Jun 2007 11:52:29 +0100
From:	Richard Purdie <rpurdie@...nedhand.com>
To:	dedekind@...radead.org
Cc:	linux-mtd <linux-mtd@...ts.infradead.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH/RFC] oops and panic message logging to MTD

On Tue, 2007-06-19 at 13:29 +0300, Artem Bityutskiy wrote:
> On Tue, 2007-06-19 at 11:00 +0100, Richard Purdie wrote:
> > > 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.

The code does.

>  Error
> code means that something very bad and sever is going on and you have to
> just refuse working with this device.

In this case, it will just move on to the next EB. There is code to
handle no available EBs at which point it will refuse to work with the
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.

It will refuse to do more work if no usable EB is available but it will
try others first. It can be assumed mtdoops will only be used with small
partitions so trying to find a usable EB before giving a fatal error
shouldn't have much of an impact on the system and might just let it
keep working in some strange error cases (and since its an error logger,
that is good).

> > > 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.

I'll have a look.

Cheers,

Richard

-
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/