All of lore.kernel.org
 help / color / mirror / Atom feed
From: Damien Le Moal <Damien.LeMoal@wdc.com>
To: Mike Snitzer <snitzer@redhat.com>,
	"Luis R. Rodriguez" <mcgrof@kernel.org>
Cc: "hare@suse.de" <hare@suse.de>,
	"axboe@kernel.dk" <axboe@kernel.dk>,
	"jaegeuk@kernel.org" <jaegeuk@kernel.org>,
	"yuchao0@huawei.com" <yuchao0@huawei.com>,
	"ghe@suse.com" <ghe@suse.com>,
	"mwilck@suse.com" <mwilck@suse.com>,
	"tchvatal@suse.com" <tchvatal@suse.com>,
	"zren@suse.com" <zren@suse.com>,
	"agk@redhat.com" <agk@redhat.com>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>
Subject: Re: dm-zoned-tools: add zoned disk udev rules for scheduler / dmsetup
Date: Fri, 15 Jun 2018 09:00:47 +0000	[thread overview]
Message-ID: <e9765fe6-ceb4-db27-fd92-be5ea58b4cfc@wdc.com> (raw)
In-Reply-To: <20180614123806.GA32162@redhat.com>

TWlrZSwNCg0KT24gNi8xNC8xOCAyMTozOCwgTWlrZSBTbml0emVyIHdyb3RlOg0KPiBPbiBXZWQs
IEp1biAxMyAyMDE4IGF0ICA4OjExcG0gLTA0MDAsDQo+IEx1aXMgUi4gUm9kcmlndWV6IDxtY2dy
b2ZAa2VybmVsLm9yZz4gd3JvdGU6DQo+IA0KPj4gU2V0dGluZyB1cCBhIHpvbmVkIGRpc2tzIGlu
IGEgZ2VuZXJpYyBmb3JtIGlzIG5vdCBzbyB0cml2aWFsLiBUaGVyZQ0KPj4gaXMgYWxzbyBxdWl0
ZSBhIGJpdCBvZiB0cmliYWwga25vd2xlZGdlIHdpdGggdGhlc2UgZGV2aWNlcyB3aGljaCBpcyBu
b3QNCj4+IGVhc3kgdG8gZmluZC4NCj4+DQo+PiBUaGUgY3VycmVudGx5IHN1cHBsaWVkIGRlbW8g
c2NyaXB0IHdvcmtzIGJ1dCBpdCBpcyBub3QgZ2VuZXJpYyBlbm91Z2ggdG8gYmUNCj4+IHByYWN0
aWNhbCBmb3IgTGludXggZGlzdHJpYnV0aW9ucyBvciBldmVuIGRldmVsb3BlcnMgd2hpY2ggb2Z0
ZW4gbW92ZQ0KPj4gZnJvbSBvbmUga2VybmVsIHRvIGFub3RoZXIuDQo+Pg0KPj4gVGhpcyB0cmll
cyB0byBwdXQgYSBiaXQgb2YgdGhpcyB0cmliYWwga25vd2xlZGdlIGludG8gYW4gaW5pdGlhbCB1
ZGV2DQo+PiBydWxlIGZvciBkZXZlbG9wbWVudCB3aXRoIHRoZSBob3BlcyBMaW51eCBkaXN0cmli
dXRpb25zIGNhbiBsYXRlcg0KPj4gZGVwbG95LiBUaHJlZSBydWxlIGFyZSBhZGRlZC4gT25lIHJ1
bGUgaXMgb3B0aW9uYWwgZm9yIG5vdywgaXQgc2hvdWxkIGJlDQo+PiBleHRlbmRlZCBsYXRlciB0
byBiZSBtb3JlIGRpc3RyaWJ1dGlvbi1mcmllbmRseSBhbmQgdGhlbiBJIHRoaW5rIHRoaXMNCj4+
IG1heSBiZSByZWFkeSBmb3IgY29uc2lkZXJhdGlvbiBmb3IgaW50ZWdyYXRpb24gb24gZGlzdHJp
YnV0aW9ucy4NCj4+DQo+PiAxKSBzY2hlZHVsZXIgc2V0dXANCj4gDQo+IFRoaXMgaXMgd3Jvbmcu
LiBpZiB6b25lZCBkZXZpY2VzIGFyZSBzbyBkZXBlbmRlbnQgb24gZGVhZGxpbmUgb3INCj4gbXEt
ZGVhZGxpbmUgdGhlbiB0aGUga2VybmVsIHNob3VsZCBhbGxvdyB0aGVtIHRvIGJlIGhhcmRjb2Rl
ZC4gIEkga25vdw0KPiBKZW5zIHJlbW92ZWQgdGhlIEFQSSB0byBkbyBzbyBidXQgdGhlIGZhY3Qg
dGhhdCBkcml2ZXJzIG5lZWQgdG8gcmVseSBvbg0KPiBoYWNrcyBsaWtlIHRoaXMgdWRldiBydWxl
IHRvIGdldCBhIGZ1bmN0aW9uYWwgZGV2aWNlIGlzIHByb29mIHdlIG5lZWQgdG8NCj4gYWxsb3cg
ZHJpdmVycyB0byBpbXBvc2UgdGhlIHNjaGVkdWxlciB1c2VkLg0KDQpJIGFncmVlLiBTd2l0Y2hp
bmcgc2NoZWR1bGVyIGluIHRoZSBrZXJuZWwgZHVyaW5nIGRldmljZSBwcm9iZS9icmluZy11cA0K
d291bGQgYmUgbXkgcHJlZmVycmVkIGNob2ljZS4gQnV0IHVudGlsIHdlIGNvbWUgdG8gYSBjb25z
ZW5zdXMgb24gdGhlDQpiZXN0IHdheSB0byBkbyB0aGlzLCBJIHRoaW5rIHRoYXQgdGhpcyB1ZGV2
IHJ1bGUgaXMgdXNlZnVsIHNpbmNlIHRoZQ0KInNjaGVkdWxlcj0iIGtlcm5lbCBwYXJhbWV0ZXIg
aXMgcmF0aGVyIGhlYXZ5IGhhbmRlZCBhbmQgYXBwbGllcyB0byBhbGwNCnNpbmdsZSBxdWV1ZSBk
ZXZpY2VzLiBOb3QgdG8gbWVudGlvbiB0aGF0IGFkZGluZyB0aGlzIHBhcmFtZXRlciB0byB0aGUN
Cmtlcm5lbCBhcmd1bWVudHMgaXMgaW4gZXNzZW5jZSBzaW1pbGFyIHRvIHRoZSB1ZGV2IHJ1bGUg
YWRkaXRpb246IGFjdGlvbg0KZnJvbSB0aGUgc3lzdGVtIHVzZXIgaXMgbmVjZXNzYXJ5IHRvIGFj
aGlldmUgYSBjb3JyZWN0IGNvbmZpZ3VyYXRpb24NCmV2ZW4gdGhvdWdoIHRoYXQgY291bGQgZWFz
aWx5IGJlIGRvbmUgYXV0b21hdGljYWxseSBmcm9tIHdpdGhpbiB0aGUNCmJsb2NrIGxheWVyLg0K
DQpJbiB0aGUgbWVhbnRpbWUsIGRvY3VtZW50aW5nIHByb3Blcmx5IHRoYXQgdGhlIGRlYWRsaW5l
IHNjaGVkdWxlciBoYXMgYQ0Kc3BlY2lhbCByZWxhdGlvbnNoaXAgd2l0aCB6b25lZCBibG9jayBk
ZXZpY2VzIGlzIHN0aWxsIGEgbmljZSB0aGluZyB0bw0KZG8uIEJ1dCB5ZXMsIGRtLXpvbmVkLXRv
b2xzIG1heSBub3QgYmUgdGhlIGJlc3QgcGxhY2UgdG8gZG8gdGhhdC4gQQ0Kc2ltcGxlIHRleHQg
ZmlsZSB1bmRlciBEb2N1bWVudGF0aW9uL2Jsb2NrIG1heSBiZSBiZXR0ZXIuIEF0IHRoZSB2ZXJ5
DQpsZWFzdCwgRG9jdW1lbnRhdGlvbi9ibG9jay9kZWFkbGluZS1pb3NjaGVkLnR4dCBzaG91bGQg
aGF2ZSBzb21lIG1lbnRpb24NCm9mIGl0cyBhbG1vc3QgbWFuZGF0b3J5IHVzZSB3aXRoIHpvbmVk
IGJsb2NrIGRldmljZXMuIEkgd2lsbCB3b3JrIG9uIHRoYXQuDQoNCj4+IDIpIGJhY2tsaXN0IGYy
ZnMgZGV2aWNlcw0KPiANCj4gVGhlcmUgc2hvdWxkIHBvcmJhYmx5IGJlIHN1cHBvcnQgaW4gZG0t
em9uZWQgZm9yIGRldGVjdGluZyB3aGV0aGVyIGENCj4gem9uZWQgZGV2aWNlIHdhcyBmb3JtYXR0
ZWQgd2l0aCBmMmZzIChhc3N1bWluZyB0aGVyZSBpcyBhIGtub3duIGYyZnMNCj4gc3VwZXJibG9j
ayk/DQoNClRoYXQgd291bGQgY2VydGFpbmx5IGJlIG5pY2UgdG8gaGF2ZSBhbmQgd291bGQgbm90
IGJlIHRvbyBoYXJkIHRvIGNvZGUNCmluIGRtemFkbS4gSSBjYW4gYWRkIHRoYXQgYW5kIGRvIHdo
YXQgZm9yIGluc3RhbmNlIG1rZnMuZXh0NCBvciBta2ZzLnhmcw0KZG8sIHdoaWNoIGlzIGFzayBm
b3IgY29uZmlybWF0aW9uIG9yIGJhaWwgb3V0IGlmIGFuIGV4aXN0aW5nIHZhbGlkIEZTDQpmb3Jt
YXQgaXMgZGV0ZWN0ZWQgb24gdGhlIGRpc2suDQoNClRoYXQgc2FpZCwgc3VjaCB0ZXN0IGFyZSBm
YXIgZnJvbSBjb21tb24uIG1kYWRtIGZvciBpbnN0YW5jZSB3aWxsDQpoYXBwaWx5IGZvcm1hdCBh
bmQgc3RhcnQgYW4gYXJyYXkgdXNpbmcgdW5tb3VudGVkIGRpc2tzIHdpdGggdmFsaWQgZmlsZQ0K
c3lzdGVtcyBvbiB0aGVtLg0KDQo+PiAzKSBydW4gZG1zZXR1cCBmb3IgdGhlIHJlc3Qgb2YgZGV2
aWNlcw0KPiANCj4gYXV0b21hZ2ljYWxseSBydW5uaW5nIGRtc2V0dXAgZGlyZWN0bHkgZnJvbSB1
ZGV2IHRvIGNyZWF0ZSBhIGRtLXpvbmVkDQo+IHRhcmdldCBpcyB2ZXJ5IG11Y2ggd3JvbmcuICBJ
dCBqdXN0IGdldHMgaW4gdGhlIHdheSBvZiBwcm9wZXIgc3VwcG9ydA0KPiB0aGF0IHNob3VsZCBi
ZSBhZGQgdG8gYXBwcm9wcmlhdGUgdG9vbHMgdGhhdCBhZG1pbnMgdXNlIHRvIHNldHVwIHRoZWly
DQo+IHpvbmVkIGRldmljZXMuICBGb3IgaW5zdGFuY2UsIHBlcnNpc3RlbnQgdXNlIG9mIGRtLXpv
bmVkIHRhcmdldCBzaG91bGQNCj4gYmUgbWFkZSByZWxpYWJsZSB3aXRoIGEgdm9sdW1lIG1hbmFn
ZXIuLg0KPiANCj4gSW4gZ2VuZXJhbCB0aGlzIHVkZXYgc2NyaXB0IGlzIHVud2VsY29tZSBhbmQg
bWFrZXMgdGhpbmdzIHdheSB3b3JzZSBmb3INCj4gdGhlIGxvbmctdGVybSBzdWNjZXNzIG9mIHpv
bmVkIGRldmljZXMuDQo+IA0KPiBJIGRvbid0IGRpc3B1dGUgdGhlcmUgaXMgYW4gb2J2aW91cyB2
b2lkIGZvciBob3cgdG8gcHJvcGVybHkgc2V0dXAgem9uZWQNCj4gZGV2aWNlcywgYnV0IHRoaXMg
c2NyaXB0IGlzIF9ub3RfIHdoYXQgc2hvdWxkIGZpbGwgdGhhdCB2b2lkLg0KDQpGYWlyIHBvaW50
cy4gSSBhZ3JlZSB0aGF0IGl0IGlzIGhhY2tpc2guIFRoZSBpbnRlbnQgd2FzIGFzIHlvdSBzYXkg
dG8NCnRlbXBvcmFyaWx5IGZpbGwgYSB2b2lkLiBCdXQgZ3JhbnRlZCwgdGVtcG9yYXJ5IGhhY2tz
IHRlbmQgdG8gc3RheSAodG9vKQ0KbG9uZyBhcm91bmQgYW5kIGNhbiBpbiB0aGUgZW5kIGdldCBp
biB0aGUgd2F5IG9mIGNsZWFuIHNvbHV0aW9ucy4gSSB3aWxsDQpzdGFydCBsb29raW5nIGludG8g
YSBwcm9wZXIgZml4IGZvciBkbS16b25lZCBzZXR1cCBwZXJzaXN0ZW5jZS4NCg0KPiBTbyBhIGhl
YXJ0ZmVsdDoNCj4gDQo+IE5hY2tlZC1ieTogTWlrZSBTbml0emVyIDxzbml0emVyQHJlZGhhdC5j
b20+DQoNClVuZGVyc3Rvb2QuIEkgd2lsbCByZXZlcnQgYW5kIG5vdCBkb2N1bWVudCB0aGlzIHVk
ZXYgcnVsZSBpbg0KZG0tem9uZWQtdG9vbHMuIEkgd2FzIGEgbGl0dGxlIHRvbyBxdWljayBpbiBh
cHBseWluZyB0aGlzIHBhdGNoIGFuZCBkaWQNCm5vdCB3YWl0IHRvIHNlZSBjb21tZW50cyBmaXJz
dC4gTXkgYmFkLg0KDQpUaGFuayB5b3UgZm9yIHlvdXIgY29tbWVudHMuDQoNCkJlc3QgcmVnYXJk
cy4NCg0KLS0gDQpEYW1pZW4gTGUgTW9hbCwNCldlc3Rlcm4gRGlnaXRhbA==

WARNING: multiple messages have this Message-ID (diff)
From: Damien Le Moal <Damien.LeMoal@wdc.com>
To: Mike Snitzer <snitzer@redhat.com>,
	"Luis R. Rodriguez" <mcgrof@kernel.org>
Cc: "hare@suse.de" <hare@suse.de>,
	"axboe@kernel.dk" <axboe@kernel.dk>,
	"jaegeuk@kernel.org" <jaegeuk@kernel.org>,
	"yuchao0@huawei.com" <yuchao0@huawei.com>,
	"ghe@suse.com" <ghe@suse.com>,
	"mwilck@suse.com" <mwilck@suse.com>,
	"tchvatal@suse.com" <tchvatal@suse.com>,
	"zren@suse.com" <zren@suse.com>,
	"agk@redhat.com" <agk@redhat.com>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>
Subject: Re: dm-zoned-tools: add zoned disk udev rules for scheduler / dmsetup
Date: Fri, 15 Jun 2018 09:00:47 +0000	[thread overview]
Message-ID: <e9765fe6-ceb4-db27-fd92-be5ea58b4cfc@wdc.com> (raw)
In-Reply-To: <20180614123806.GA32162@redhat.com>

Mike,

On 6/14/18 21:38, Mike Snitzer wrote:
> On Wed, Jun 13 2018 at  8:11pm -0400,
> Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> 
>> Setting up a zoned disks in a generic form is not so trivial. There
>> is also quite a bit of tribal knowledge with these devices which is not
>> easy to find.
>>
>> The currently supplied demo script works but it is not generic enough to be
>> practical for Linux distributions or even developers which often move
>> from one kernel to another.
>>
>> This tries to put a bit of this tribal knowledge into an initial udev
>> rule for development with the hopes Linux distributions can later
>> deploy. Three rule are added. One rule is optional for now, it should be
>> extended later to be more distribution-friendly and then I think this
>> may be ready for consideration for integration on distributions.
>>
>> 1) scheduler setup
> 
> This is wrong.. if zoned devices are so dependent on deadline or
> mq-deadline then the kernel should allow them to be hardcoded.  I know
> Jens removed the API to do so but the fact that drivers need to rely on
> hacks like this udev rule to get a functional device is proof we need to
> allow drivers to impose the scheduler used.

I agree. Switching scheduler in the kernel during device probe/bring-up
would be my preferred choice. But until we come to a consensus on the
best way to do this, I think that this udev rule is useful since the
"scheduler=" kernel parameter is rather heavy handed and applies to all
single queue devices. Not to mention that adding this parameter to the
kernel arguments is in essence similar to the udev rule addition: action
from the system user is necessary to achieve a correct configuration
even though that could easily be done automatically from within the
block layer.

In the meantime, documenting properly that the deadline scheduler has a
special relationship with zoned block devices is still a nice thing to
do. But yes, dm-zoned-tools may not be the best place to do that. A
simple text file under Documentation/block may be better. At the very
least, Documentation/block/deadline-iosched.txt should have some mention
of its almost mandatory use with zoned block devices. I will work on that.

>> 2) backlist f2fs devices
> 
> There should porbably be support in dm-zoned for detecting whether a
> zoned device was formatted with f2fs (assuming there is a known f2fs
> superblock)?

That would certainly be nice to have and would not be too hard to code
in dmzadm. I can add that and do what for instance mkfs.ext4 or mkfs.xfs
do, which is ask for confirmation or bail out if an existing valid FS
format is detected on the disk.

That said, such test are far from common. mdadm for instance will
happily format and start an array using unmounted disks with valid file
systems on them.

>> 3) run dmsetup for the rest of devices
> 
> automagically running dmsetup directly from udev to create a dm-zoned
> target is very much wrong.  It just gets in the way of proper support
> that should be add to appropriate tools that admins use to setup their
> zoned devices.  For instance, persistent use of dm-zoned target should
> be made reliable with a volume manager..
> 
> In general this udev script is unwelcome and makes things way worse for
> the long-term success of zoned devices.
> 
> I don't dispute there is an obvious void for how to properly setup zoned
> devices, but this script is _not_ what should fill that void.

Fair points. I agree that it is hackish. The intent was as you say to
temporarily fill a void. But granted, temporary hacks tend to stay (too)
long around and can in the end get in the way of clean solutions. I will
start looking into a proper fix for dm-zoned setup persistence.

> So a heartfelt:
> 
> Nacked-by: Mike Snitzer <snitzer@redhat.com>

Understood. I will revert and not document this udev rule in
dm-zoned-tools. I was a little too quick in applying this patch and did
not wait to see comments first. My bad.

Thank you for your comments.

Best regards.

-- 
Damien Le Moal,
Western Digital

  parent reply	other threads:[~2018-06-15  9:00 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-14  0:11 [PATCH] dm-zoned-tools: add zoned disk udev rules for scheduler / dmsetup Luis R. Rodriguez
2018-06-14 10:01 ` Damien Le Moal
2018-06-14 10:01   ` Damien Le Moal
2018-06-14 13:39   ` Bart Van Assche
2018-06-14 13:39     ` Bart Van Assche
2018-06-14 13:42     ` Christoph Hellwig
2018-06-15 11:07       ` Martin Wilck
2018-06-15 11:07         ` Martin Wilck
2018-06-14 12:38 ` Mike Snitzer
2018-06-14 16:23   ` Bart Van Assche
2018-06-14 16:23     ` Bart Van Assche
2018-06-14 17:37   ` Luis R. Rodriguez
2018-06-14 17:46     ` Luis R. Rodriguez
2018-06-14 17:58     ` Mike Snitzer
2018-06-15  9:59       ` Damien Le Moal
2018-06-15  9:59         ` Damien Le Moal
2018-06-15 14:50         ` Mike Snitzer
2018-06-15  9:00   ` Damien Le Moal [this message]
2018-06-15  9:00     ` Damien Le Moal
2018-06-14 16:19 ` [PATCH] " Bart Van Assche
2018-06-14 16:19   ` Bart Van Assche
2018-06-14 17:44   ` Luis R. Rodriguez

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=e9765fe6-ceb4-db27-fd92-be5ea58b4cfc@wdc.com \
    --to=damien.lemoal@wdc.com \
    --cc=agk@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=ghe@suse.com \
    --cc=hare@suse.de \
    --cc=jaegeuk@kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=mwilck@suse.com \
    --cc=snitzer@redhat.com \
    --cc=tchvatal@suse.com \
    --cc=yuchao0@huawei.com \
    --cc=zren@suse.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.