<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
<div class="moz-cite-prefix">On 10/15/2014 11:00 PM, NeilBrown
wrote:<br>
</div>
<blockquote cite="mid:20141016080054.7c906901@notabene.brown"
type="cite">
<pre wrap="">On Wed, 15 Oct 2014 09:13:08 -0400 Mike Snitzer <a class="moz-txt-link-rfc2396E" href="mailto:snitzer@redhat.com"><snitzer@redhat.com></a> wrote:
</pre>
<blockquote type="cite">
<pre wrap="">On Tue, Oct 14 2014 at 11:40pm -0400,
NeilBrown <a class="moz-txt-link-rfc2396E" href="mailto:neilb@suse.de"><neilb@suse.de></a> wrote:
</pre>
<blockquote type="cite">
<pre wrap="">On Tue, 14 Oct 2014 22:55:50 -0400 Mike Snitzer <a class="moz-txt-link-rfc2396E" href="mailto:snitzer@redhat.com"><snitzer@redhat.com></a> wrote:
</pre>
<blockquote type="cite">
<pre wrap="">On Tue, Oct 14 2014 at 9:19pm -0400,
NeilBrown <a class="moz-txt-link-rfc2396E" href="mailto:neilb@suse.de"><neilb@suse.de></a> wrote:
</pre>
<blockquote type="cite">
<pre wrap="">
dm_raid_superblock is 512.
Reading or writing this on a 512-byte sector works fine.
On a 4096-byte sector device, this fails.
If we round up rdev->sb_size to match the block size of
the device, all IO will work correctly.
Reported-by: "Liuhua Wang" <a class="moz-txt-link-rfc2396E" href="mailto:lwang@suse.com"><lwang@suse.com></a>
Signed-off-by: NeilBrown <a class="moz-txt-link-rfc2396E" href="mailto:neilb@suse.de"><neilb@suse.de></a>
---
this issue has been discussed already a bit. See email thread
Subject: Re: [dm-devel] [PATCH] fix mirror device creation with lvcreate failed
I think this is the best fix. It handles boths read and writes, and (I think)
at the best level.
Thanks,
NeilBrown
diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index 4880b69e2e9e..31bdd73bc368 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -858,7 +858,8 @@ static int super_load(struct md_rdev *rdev, struct md_rdev *refdev)
uint64_t events_sb, events_refsb;
rdev->sb_start = 0;
- rdev->sb_size = sizeof(*sb);
+ rdev->sb_size = roundup(sizeof(*sb),
+ bdev_logical_block_size(rdev->meta_bdev));
ret = read_disk_sb(rdev, rdev->sb_size);
if (ret)
</pre>
</blockquote>
<pre wrap="">
Wouldn't it be better to use bdev_physical_block_size()?
Even on a 4K device that emulates 512b logical sectors it is better to
use the physical block size (4K).
</pre>
</blockquote>
<pre wrap="">
_logical_ is the smallest value for which the IO actually works.
And the goal of the change is to make it work.
I don't object to using _physical_, but it isn't clear to me how I would
justify that as "correct".
A big question in my mind is: how much space does LVM reserve in this device
for the metadata? It seems reasonable to assume that it reserves at least
1 logical block. If the API guarantees that at least one physical block is
reserved, then that would justify using _physical_.
</pre>
</blockquote>
</blockquote>
</blockquote>
<br>
dm-raid uses 512 bytes for its superblock including padding in the
current code<br>
(with way less payload), followed by the bitmap at offset 4096
bytes.<br>
<br>
With Neil's proposal, the padding will be avoided alltogether.<br>
<br>
So logical looks fine to me, because physical may well be more than
4K in the future.<br>
Admittingly a longer time in the future though.<br>
<br>
If that happens, we'd end up with the bitmap start at offset 4096
bytes in the same physical sector<br>
and would have to cope with it plus the issues involved with 4K page
sizes.<br>
<br>
In order to prevent this to occur unrecognized in dm/md later,<br>
we should have an "if (rdev->sb_size > PAGE_SIZE) return
-EINVAL;" after the setting<br>
of rdev->sb_size in dm-raid.c and appropriate checks in md.c.<br>
<br>
BTW: even with additions to the dm-raid superblock I have to make in
order to allow for reshaping etc.,<br>
it's paylod is going to stay < 512 bytes.<br>
<br>
Heinz<br>
<br>
<blockquote cite="mid:20141016080054.7c906901@notabene.brown"
type="cite">
<blockquote type="cite">
<pre wrap="">
I'll have to check with Jon and/or Heinz on this point.
</pre>
<blockquote type="cite">
<pre wrap="">A quick look at the code shows that the bitmap superblock is placed 4K after
the start of the metadata.
</pre>
</blockquote>
<pre wrap="">
"the code" being the MD kernel code right? Any reason not to export a
#define that reflects the space MD reserves and just have dm-raid use that?
</pre>
</blockquote>
<pre wrap="">
No, "the code" being
mddev->bitmap_info.offset = 4096 >> 9; /* Enable bitmap creation */
rdev->mddev->bitmap_info.default_offset = 4096 >> 9;
in super_validate in dm-raid.c.
i.e. dm-raid specific code.
md doesn't reserve space, it just uses what it is told to. Told either by
mdadm via the md superblock or by dm-raid.
NeilBrown
</pre>
<blockquote type="cite">
<pre wrap="">
Starting to feel like hardcoding 4K is the right thing to do given the
current code.
</pre>
</blockquote>
<pre wrap="">
</pre>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">--
dm-devel mailing list
<a class="moz-txt-link-abbreviated" href="mailto:dm-devel@redhat.com">dm-devel@redhat.com</a>
<a class="moz-txt-link-freetext" href="https://www.redhat.com/mailman/listinfo/dm-devel">https://www.redhat.com/mailman/listinfo/dm-devel</a></pre>
</blockquote>
<br>
</body>
</html>