LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Jörn Engel" <joern@logfs.org>
To: Stephane Chazelas <stephane.chazelas@emerson.com>
Cc: linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org
Subject: Re: [PATCH 2.6.24] block2mtd: removing a device and typo fixes
Date: Tue, 12 Feb 2008 16:21:24 +0100	[thread overview]
Message-ID: <20080212152124.GA21878@lazybastard.org> (raw)
In-Reply-To: <chaz20080212134751.GI30304@artesyncp.com>

On Tue, 12 February 2008 13:47:51 +0000, Stephane Chazelas wrote:
> 
> this patch addresses a number of small issues mainly regarding
> the output made by this driver to dmesg:
>   - Some of the "blkmtd"'s had not been changed to "block2mtd"
>     which caused display problem
>   - the parse_err() macro was displaying "block2mtd: " twice

Fairly obvious fixes.

> Also, one can add a block2mtd mtd device with things like:
> 
> echo /dev/loop3,$((256*1024)) |
>   sudo tee /sys/module/block2mtd/parameters/block2mtd
> 
> but individual mtds cannot be removed. You can only do a
> modprobe -r block2mtd to remove *all* the block2mtd mtds.
> 
> This patch proposes to add the cabability with:
> 
> echo /dev/loop3,remove |
>   sudo tee /sys/module/block2mtd/parameters/block2mtd

Sounds sane enough.  But I do have some reservations about the
implementation.  It would be best if you split the patch in two.  One
with the obvious stuff above and one for this.

The core of remove_device_by_name() is shared with block2mtd_exit(),
so a common helper would be good.  Your error handling is better, so
let's keep that version.

And independently of your patch a mutex protecting the device list from
simultaneous modifications would be good to have.

Side note: I may not have internet access until 19th or so.

Jörn

-- 
Rules of Optimization:
Rule 1: Don't do it.
Rule 2 (for experts only): Don't do it yet.
-- M.A. Jackson

  reply	other threads:[~2008-02-12 15:21 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-12 13:47 Stephane Chazelas
2008-02-12 15:21 ` Jörn Engel [this message]
2008-02-12 16:10   ` Stephane Chazelas
2008-02-19 15:08     ` Jörn Engel
2008-02-19 22:33       ` Arnd Bergmann
2008-02-20  6:43         ` Jörn Engel
2008-02-20 14:43         ` Stephane Chazelas
2008-02-20 16:30           ` Jörn Engel
2008-02-20 17:02             ` Stephane Chazelas
2008-02-20 17:22               ` Jörn Engel
2008-02-20 17:30                 ` Stephane Chazelas
2008-02-23 15:33                   ` Jörn Engel
2008-02-20 14:36       ` Stephane Chazelas
2008-02-20 16:42         ` Jörn Engel
2008-02-20 16:55           ` Stephane Chazelas

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=20080212152124.GA21878@lazybastard.org \
    --to=joern@logfs.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=stephane.chazelas@emerson.com \
    --subject='Re: [PATCH 2.6.24] block2mtd: removing a device and typo fixes' \
    /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

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