All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Wagner <david.wagner@free-electrons.com>
To: dedekind1@gmail.com
Cc: Arnd Bergmann <arnd@arndb.de>,
	linux-mtd <linux-mtd@lists.infradead.org>,
	linux-embedded <linux-embedded@vger.kernel.org>,
	lkml <linux-kernel@vger.kernel.org>,
	Tim Bird <tim.bird@am.sony.com>,
	David Woodhouse <dwmw2@infradead.org>
Subject: Re: [PATCHv3] UBI: new module ubiblk: block layer on top of UBI
Date: Fri, 09 Sep 2011 16:41:53 +0200	[thread overview]
Message-ID: <4E6A25B1.8070400@free-electrons.com> (raw)
In-Reply-To: <1315569206.7905.41.camel@sauron>

On 09/09/2011 01:53 PM, Artem Bityutskiy wrote:
> On Thu, 2011-09-08 at 17:26 +0200, Arnd Bergmann wrote:
>> On Tuesday 06 September 2011, Artem Bityutskiy wrote:
>>> Not sure about the bus approach - David, could you take a look at it
>>> please? If we can handle errors there - then we could indeed re-use the
>>> UBI control device. We could even re-use the ioctl data structures for
>>> UBI volumes creation/removal - we have plenty of space there reserved
>>> for future extensions.
>>

@Arnd:
> * Use the existing UBI control device for the block devices as
>   well and just add two more ioctls to create the devices.
>   You can add a logical bus_type for this so that the ubi block
>   driver gets automatically loaded matched with the device when
>   one is created using the control device.

I certainly miss some background, I'm not sure I understand how this
works: bus_type seems suitable for pluggable devices that possess a
device ID which matches against a driver that will then get loaded.  But
ubiblk devices are created by ubiblk.
So, are you suggesting to move ubiblk_create() to UBI and add a
MODULE_ALIAS to ubiblk (actually, I don't know what it would contain) ?

(I just saw that you sent an email while I was writing this one ;
however, I still understand. I'll try and read how scsi does that).


@Artem:
> Sorry, I wonted to talk about situations when someone opens an ubiblk
> device while the underlying UBI volume is being removed, but then though
> this is trivial and forgot to erase the last sentence.

Ah, yes, I guess we need to hold a vol_lock in ubiblk_remove() ?


> Anyway, I suggest the following algorithm:
> 
> 1. Stick with the own cdev approach - the driver becomes very simple
>    in this case - we review it.

This is the way it's implemented in v4, right ?

BTW, those are the changes made so far since v4:
 * Add missing headers (they are included by other headers but it seems
   to be good practice not to rely on that).

 * Remove an macro rendered useless with the linked lists

 * correct some formatting in kerneldoc comments

 * introduce refcounting to avoid multiple opens or closing a UBI
   volume while still in use

 * make checkpatch happy about assignation inside a condition

 * use DEFINE_MUTEX for devlist_lock


	Best Regards,
	David

-- 
David Wagner, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

WARNING: multiple messages have this Message-ID (diff)
From: David Wagner <david.wagner@free-electrons.com>
To: dedekind1@gmail.com
Cc: Arnd Bergmann <arnd@arndb.de>,
	linux-embedded <linux-embedded@vger.kernel.org>,
	lkml <linux-kernel@vger.kernel.org>,
	linux-mtd <linux-mtd@lists.infradead.org>,
	Tim Bird <tim.bird@am.sony.com>,
	David Woodhouse <dwmw2@infradead.org>
Subject: Re: [PATCHv3] UBI: new module ubiblk: block layer on top of UBI
Date: Fri, 09 Sep 2011 16:41:53 +0200	[thread overview]
Message-ID: <4E6A25B1.8070400@free-electrons.com> (raw)
In-Reply-To: <1315569206.7905.41.camel@sauron>

On 09/09/2011 01:53 PM, Artem Bityutskiy wrote:
> On Thu, 2011-09-08 at 17:26 +0200, Arnd Bergmann wrote:
>> On Tuesday 06 September 2011, Artem Bityutskiy wrote:
>>> Not sure about the bus approach - David, could you take a look at it
>>> please? If we can handle errors there - then we could indeed re-use the
>>> UBI control device. We could even re-use the ioctl data structures for
>>> UBI volumes creation/removal - we have plenty of space there reserved
>>> for future extensions.
>>

@Arnd:
> * Use the existing UBI control device for the block devices as
>   well and just add two more ioctls to create the devices.
>   You can add a logical bus_type for this so that the ubi block
>   driver gets automatically loaded matched with the device when
>   one is created using the control device.

I certainly miss some background, I'm not sure I understand how this
works: bus_type seems suitable for pluggable devices that possess a
device ID which matches against a driver that will then get loaded.  But
ubiblk devices are created by ubiblk.
So, are you suggesting to move ubiblk_create() to UBI and add a
MODULE_ALIAS to ubiblk (actually, I don't know what it would contain) ?

(I just saw that you sent an email while I was writing this one ;
however, I still understand. I'll try and read how scsi does that).


@Artem:
> Sorry, I wonted to talk about situations when someone opens an ubiblk
> device while the underlying UBI volume is being removed, but then though
> this is trivial and forgot to erase the last sentence.

Ah, yes, I guess we need to hold a vol_lock in ubiblk_remove() ?


> Anyway, I suggest the following algorithm:
> 
> 1. Stick with the own cdev approach - the driver becomes very simple
>    in this case - we review it.

This is the way it's implemented in v4, right ?

BTW, those are the changes made so far since v4:
 * Add missing headers (they are included by other headers but it seems
   to be good practice not to rely on that).

 * Remove an macro rendered useless with the linked lists

 * correct some formatting in kerneldoc comments

 * introduce refcounting to avoid multiple opens or closing a UBI
   volume while still in use

 * make checkpatch happy about assignation inside a condition

 * use DEFINE_MUTEX for devlist_lock


	Best Regards,
	David

-- 
David Wagner, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

WARNING: multiple messages have this Message-ID (diff)
From: David Wagner <david.wagner@free-electrons.com>
To: dedekind1@gmail.com
Cc: Arnd Bergmann <arnd@arndb.de>,
	linux-embedded <linux-embedded@vger.kernel.org>,
	lkml <linux-kernel@vger.kernel.org>,
	linux-mtd <linux-mtd@lists.infradead.org>,
	Tim Bird <tim.bird@am.sony.com>,
	David Woodhouse <dwmw2@infradead.org>
Subject: Re: [PATCHv3] UBI: new module ubiblk: block layer on top of UBI
Date: Fri, 09 Sep 2011 16:41:53 +0200	[thread overview]
Message-ID: <4E6A25B1.8070400@free-electrons.com> (raw)
In-Reply-To: <1315569206.7905.41.camel@sauron>

On 09/09/2011 01:53 PM, Artem Bityutskiy wrote:
> On Thu, 2011-09-08 at 17:26 +0200, Arnd Bergmann wrote:
>> On Tuesday 06 September 2011, Artem Bityutskiy wrote:
>>> Not sure about the bus approach - David, could you take a look at it
>>> please? If we can handle errors there - then we could indeed re-use the
>>> UBI control device. We could even re-use the ioctl data structures for
>>> UBI volumes creation/removal - we have plenty of space there reserved
>>> for future extensions.
>>

@Arnd:
> * Use the existing UBI control device for the block devices as
>   well and just add two more ioctls to create the devices.
>   You can add a logical bus_type for this so that the ubi block
>   driver gets automatically loaded matched with the device when
>   one is created using the control device.

I certainly miss some background, I'm not sure I understand how this
works: bus_type seems suitable for pluggable devices that possess a
device ID which matches against a driver that will then get loaded.  But
ubiblk devices are created by ubiblk.
So, are you suggesting to move ubiblk_create() to UBI and add a
MODULE_ALIAS to ubiblk (actually, I don't know what it would contain) ?

(I just saw that you sent an email while I was writing this one ;
however, I still understand. I'll try and read how scsi does that).


@Artem:
> Sorry, I wonted to talk about situations when someone opens an ubiblk
> device while the underlying UBI volume is being removed, but then though
> this is trivial and forgot to erase the last sentence.

Ah, yes, I guess we need to hold a vol_lock in ubiblk_remove() ?


> Anyway, I suggest the following algorithm:
> 
> 1. Stick with the own cdev approach - the driver becomes very simple
>    in this case - we review it.

This is the way it's implemented in v4, right ?

BTW, those are the changes made so far since v4:
 * Add missing headers (they are included by other headers but it seems
   to be good practice not to rely on that).

 * Remove an macro rendered useless with the linked lists

 * correct some formatting in kerneldoc comments

 * introduce refcounting to avoid multiple opens or closing a UBI
   volume while still in use

 * make checkpatch happy about assignation inside a condition

 * use DEFINE_MUTEX for devlist_lock


	Best Regards,
	David

-- 
David Wagner, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  parent reply	other threads:[~2011-09-09 14:37 UTC|newest]

Thread overview: 118+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-24 13:34 [RFC] ubiblk: read-only block layer on top of UBI david.wagner
2011-06-24 13:34 ` david.wagner
2011-06-24 13:34 ` [PATCH] UBI: new module ubiblk: " david.wagner
2011-06-24 13:34   ` david.wagner
2011-06-27 19:26   ` Artem Bityutskiy
2011-06-27 19:26     ` Artem Bityutskiy
2011-06-28 11:35     ` David Wagner
2011-06-28 11:35       ` David Wagner
2011-06-29  6:52       ` Artem Bityutskiy
2011-06-29  6:52         ` Artem Bityutskiy
2011-06-28 14:50     ` Matthieu CASTET
2011-06-28 14:50       ` Matthieu CASTET
2011-06-28 15:32       ` David Wagner
2011-06-28 15:32         ` David Wagner
2011-06-29  6:25         ` Artem Bityutskiy
2011-06-29  6:25           ` Artem Bityutskiy
2011-06-24 13:45 ` [Addendum][RFC] ubiblk: read-only " David Wagner
2011-06-27 19:14 ` [RFC] " Artem Bityutskiy
2011-06-27 19:14   ` Artem Bityutskiy
2011-06-28 15:24 ` [RFC PATCHv2] UBI: new module ubiblk: " david.wagner
2011-06-28 15:24   ` david.wagner
2011-06-28 15:24   ` david.wagner
2011-06-29  6:54   ` Artem Bityutskiy
2011-06-29  6:54     ` Artem Bityutskiy
2011-07-26 12:27 ` [PATCH] " David Wagner
2011-07-26 12:27   ` David Wagner
2011-07-26 12:34   ` Christoph Hellwig
2011-07-26 12:34     ` Christoph Hellwig
2011-07-26 12:34     ` Christoph Hellwig
2011-07-26 12:58     ` David Wagner
2011-07-26 12:58       ` David Wagner
2011-07-28  6:14   ` Artem Bityutskiy
2011-07-28  6:14     ` Artem Bityutskiy
2011-08-15 11:56   ` Artem Bityutskiy
2011-08-15 11:56     ` Artem Bityutskiy
2011-08-17 13:17 ` [PATCHv3] " david.wagner
2011-08-17 13:17   ` david.wagner
2011-08-17 14:20   ` [PATCH] Tools for controling ubiblk David Wagner
2011-08-17 14:20     ` David Wagner
2011-08-22  8:17     ` Artem Bityutskiy
2011-08-22  8:17       ` Artem Bityutskiy
2011-08-22  7:39   ` [PATCHv3] UBI: new module ubiblk: block layer on top of UBI Artem Bityutskiy
2011-08-22  7:39     ` Artem Bityutskiy
2011-08-22  7:42   ` Artem Bityutskiy
2011-08-22  7:42     ` Artem Bityutskiy
2011-08-24 16:23     ` Arnd Bergmann
2011-08-24 16:23       ` Arnd Bergmann
2011-08-25  7:06       ` Artem Bityutskiy
2011-08-25  7:06         ` Artem Bityutskiy
2011-08-25 15:12         ` Arnd Bergmann
2011-08-25 15:12           ` Arnd Bergmann
2011-08-25 15:12           ` Arnd Bergmann
2011-09-01 12:55           ` David Wagner
2011-09-01 12:55             ` David Wagner
2011-09-01 12:55             ` David Wagner
2011-09-06  3:44           ` Artem Bityutskiy
2011-09-06  3:44             ` Artem Bityutskiy
2011-09-06  4:10             ` Artem Bityutskiy
2011-09-06  4:10               ` Artem Bityutskiy
2011-09-06  4:10               ` Artem Bityutskiy
2011-09-06  4:29               ` Artem Bityutskiy
2011-09-06  4:29                 ` Artem Bityutskiy
2011-09-08 15:26               ` Arnd Bergmann
2011-09-08 15:26                 ` Arnd Bergmann
2011-09-08 15:26                 ` Arnd Bergmann
2011-09-09 11:53                 ` Artem Bityutskiy
2011-09-09 11:53                   ` Artem Bityutskiy
2011-09-09 12:02                   ` Artem Bityutskiy
2011-09-09 12:02                     ` Artem Bityutskiy
2011-09-09 14:25                   ` Arnd Bergmann
2011-09-09 14:25                     ` Arnd Bergmann
2011-09-09 15:27                     ` Artem Bityutskiy
2011-09-09 15:27                       ` Artem Bityutskiy
2011-09-09 14:41                   ` David Wagner [this message]
2011-09-09 14:41                     ` David Wagner
2011-09-09 14:41                     ` David Wagner
2011-09-09 14:51                     ` Arnd Bergmann
2011-09-09 14:51                       ` Arnd Bergmann
2011-09-11 10:18                     ` Artem Bityutskiy
2011-09-11 10:18                       ` Artem Bityutskiy
2011-09-11 10:18                       ` Artem Bityutskiy
2011-09-11 10:35                       ` David Wagner
2011-09-11 10:35                         ` David Wagner
2011-08-24 16:15 ` [PATCHv4] " david.wagner
2011-08-24 16:15   ` david.wagner
2011-08-24 16:21   ` [PATCH] document ubiblk's usage of the same ioctl magic as a part " David Wagner
2011-08-24 16:21     ` David Wagner
2011-09-06  4:58     ` Artem Bityutskiy
2011-09-06  4:58       ` Artem Bityutskiy
2011-09-06  4:55   ` [PATCHv4] UBI: new module ubiblk: block layer on top " Artem Bityutskiy
2011-09-06  4:55     ` Artem Bityutskiy
2011-09-12  9:51 ` [PATCHv5] " David Wagner
2011-09-12  9:51   ` David Wagner
2011-09-12  9:51   ` David Wagner
2011-09-19  4:50   ` Artem Bityutskiy
2011-09-19  4:50     ` Artem Bityutskiy
2011-09-22  7:58 ` [PATCHv6] " David Wagner
2011-09-22  7:58   ` David Wagner
2011-09-23 10:58   ` Artem Bityutskiy
2011-09-23 10:58     ` Artem Bityutskiy
2011-09-26 12:58     ` David Wagner
2011-09-26 12:58       ` David Wagner
2011-09-26  9:17   ` Ricard Wanderlof
2011-09-26  9:17     ` Ricard Wanderlof
2011-09-26  9:17     ` Ricard Wanderlof
2011-09-26 12:11   ` Ricard Wanderlof
2011-09-26 12:38 ` [PATCHv7] " David Wagner
2011-09-26 12:38   ` David Wagner
2011-09-26 13:20   ` Artem Bityutskiy
2011-09-26 13:20     ` Artem Bityutskiy
2011-09-26 14:25 ` [PATCHv8] " David Wagner
2011-09-26 14:25   ` David Wagner
2011-09-26 14:36   ` Artem Bityutskiy
2011-09-26 14:36     ` Artem Bityutskiy
2011-09-26 14:40 ` [PATCHv9] " David Wagner
2011-09-26 14:40   ` David Wagner
2011-10-01 14:08   ` Artem Bityutskiy
2011-10-01 14:08     ` 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:
  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=4E6A25B1.8070400@free-electrons.com \
    --to=david.wagner@free-electrons.com \
    --cc=arnd@arndb.de \
    --cc=dedekind1@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=linux-embedded@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=tim.bird@am.sony.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.