LKML Archive on
help / color / mirror / Atom feed
From: "Jörn Engel" <>
To: Stephane Chazelas <>
Cc: "Jörn Engel" <>,,,
	"Arnd Bergmann" <>
Subject: Re: [PATCH 2.6.24] block2mtd: removing a device and typo fixes
Date: Tue, 19 Feb 2008 16:08:22 +0100	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

[ Just returned home. ]

On Tue, 12 February 2008 16:10:34 +0000, Stephane Chazelas wrote:
> OK, I can do that. Would the "simple" fix go to the
> Trivial Patch Monkey?

That or to me or to dwmw2...  I'm not too picky about the path, as long
as it gets merged eventually.

> > 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.
> Well, yes that raised a concern to me, the "exit" function
> returns "void". If the del_mtd_device fails with EBUSY at the
> moment (such as when a /dev/mtdblock<x> is mounted), we ignore
> it and go on with freeing everything leaving a dangling mtd.
> Is there a way around that?

For module unload we should be safe.  Errors are only returned if the
device doesn't exist (a clear bug) or if the device is still being used.
Module refcounting should prevent the latter case, so either way we have
a bug if del_mtd_device returns an error.

When removing the device via your proposed interface we should simply
return the error to userspace, if the interface permits.  Sysfs doesn't
seem to permit error returns, so we should keep the device alive and do

> Another problem is that it's not easy to check whether a mtd
> creation was successful or not. Basically, you have to write to
> a /sys file and then parse /proc/mtd to get the result.

Agreed, sysfs' lack of error indication isn't ideal.

> What about having a /dev/block2mtd (with owner/permissions that
> could allow non-root users to use it), with 2 ioctls:
> - one to "link" a block dev to a mtd that would take as
>   parameter a fd to an open block dev (again allowing for
>   flexible permissions) and would return the number of the
>   allocated mtd and success/failure in errno. Upon sucess it
>   would increase the refcnt of block2mtd.
> - and one to "release" the link. That would fail if the mtd is
>   in use and decrease block2mtd's refcnt upon success.
> A bit like the loop devices (or /dev/ptmx) actually. What do you
> think?

Could work.  Passing the fd raises several alarm bells.  Arnd, any
comments from you?

In general I'd like to see some good reasons for adding an ioctl (or any
other) interface first.  Creating interfaces is hard and you rarely get
a second chance to fix the mess you created before you knew all the

> (also, with the /sys interface, isn't there a potential problem
> with chroots wrt the path given to the /sys file?)

There may be.  Can you check if there is?


Debugging is twice as hard as writing the code in the first place.
Therefore, if you write the code as cleverly as possible, you are,
by definition, not smart enough to debug it.
-- Brian W. Kernighan

  reply	other threads:[~2008-02-19 15:09 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
2008-02-12 16:10   ` Stephane Chazelas
2008-02-19 15:08     ` Jörn Engel [this message]
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:

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

  git send-email \ \ \ \ \ \ \
    --subject='Re: [PATCH 2.6.24] block2mtd: removing a device and typo fixes' \

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