* [PATCH v2 18/21] mmc: tmio: Make use of the new sg_map helper function
From: Logan Gunthorpe @ 2017-04-25 18:21 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
linux-scsi-u79uwXL29TY76Z2rM5mHXA,
open-iscsi-/JYPxA39Uh5TLH3MbocFFw,
megaraidlinux.pdl-dY08KVG/lbpWk0Htik3J/w,
sparmaintainer-GLv8BlqOqDDQT0dZR+AlfA,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
target-devel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA
Cc: Jens Axboe, Ulf Hansson, James E.J. Bottomley, Martin K. Petersen,
Matthew Wilcox, Greg Kroah-Hartman, Sumit Semwal, Wolfram Sang,
Christoph Hellwig
In-Reply-To: <1493144468-22493-1-git-send-email-logang-OTvnGxWRz7hWk0Htik3J/w@public.gmane.org>
Straightforward conversion to sg_map helper. Seeing there is no
cleare error path, SG_MAP_MUST_NOT_FAIL which may BUG_ON in certain
cases in the future.
Signed-off-by: Logan Gunthorpe <logang-OTvnGxWRz7hWk0Htik3J/w@public.gmane.org>
Cc: Wolfram Sang <wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/@public.gmane.org>
Cc: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
drivers/mmc/host/tmio_mmc.h | 7 +++++--
drivers/mmc/host/tmio_mmc_pio.c | 12 ++++++++++++
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index d0edb57..bc43eb0 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -202,17 +202,20 @@ void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
irqreturn_t tmio_mmc_irq(int irq, void *devid);
+/* Note: this function may return PTR_ERR and must be checked! */
static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg,
unsigned long *flags)
{
+ void *ret;
+
local_irq_save(*flags);
- return kmap_atomic(sg_page(sg)) + sg->offset;
+ return sg_map(sg, 0, SG_KMAP_ATOMIC | SG_MAP_MUST_NOT_FAIL);
}
static inline void tmio_mmc_kunmap_atomic(struct scatterlist *sg,
unsigned long *flags, void *virt)
{
- kunmap_atomic(virt - sg->offset);
+ sg_unmap(sg, virt, 0, SG_KMAP_ATOMIC);
local_irq_restore(*flags);
}
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index a2d92f1..bbb4f19 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -506,6 +506,18 @@ static void tmio_mmc_check_bounce_buffer(struct tmio_mmc_host *host)
if (host->sg_ptr == &host->bounce_sg) {
unsigned long flags;
void *sg_vaddr = tmio_mmc_kmap_atomic(host->sg_orig, &flags);
+ if (IS_ERR(sg_vaddr)) {
+ /*
+ * This should really never happen unless
+ * the code is changed to use memory that is
+ * not mappable in the sg. Seeing there doesn't
+ * seem to be any error path out of here,
+ * we can only WARN.
+ */
+ WARN(1, "Non-mappable memory used in sg!");
+ return;
+ }
+
memcpy(sg_vaddr, host->bounce_buf, host->bounce_sg.length);
tmio_mmc_kunmap_atomic(host->sg_orig, &flags, sg_vaddr);
}
--
2.1.4
^ permalink raw reply related
* [PATCH v2 19/21] mmc: sdricoh_cs: Make use of the new sg_map helper function
From: Logan Gunthorpe @ 2017-04-25 18:21 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
linux-scsi-u79uwXL29TY76Z2rM5mHXA,
open-iscsi-/JYPxA39Uh5TLH3MbocFFw,
megaraidlinux.pdl-dY08KVG/lbpWk0Htik3J/w,
sparmaintainer-GLv8BlqOqDDQT0dZR+AlfA,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
target-devel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA
Cc: Jens Axboe, Ulf Hansson, James E.J. Bottomley, Martin K. Petersen,
Matthew Wilcox, Greg Kroah-Hartman, Sumit Semwal, Sascha Sommer,
Christoph Hellwig
In-Reply-To: <1493144468-22493-1-git-send-email-logang-OTvnGxWRz7hWk0Htik3J/w@public.gmane.org>
This is a straightforward conversion to the new function.
Signed-off-by: Logan Gunthorpe <logang-OTvnGxWRz7hWk0Htik3J/w@public.gmane.org>
Cc: Sascha Sommer <saschasommer-KuiJ5kEpwI6ELgA04lAiVw@public.gmane.org>
Cc: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
drivers/mmc/host/sdricoh_cs.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/host/sdricoh_cs.c b/drivers/mmc/host/sdricoh_cs.c
index 5ff26ab..03225c3 100644
--- a/drivers/mmc/host/sdricoh_cs.c
+++ b/drivers/mmc/host/sdricoh_cs.c
@@ -319,16 +319,20 @@ static void sdricoh_request(struct mmc_host *mmc, struct mmc_request *mrq)
for (i = 0; i < data->blocks; i++) {
size_t len = data->blksz;
u8 *buf;
- struct page *page;
int result;
- page = sg_page(data->sg);
- buf = kmap(page) + data->sg->offset + (len * i);
+ buf = sg_map(data->sg, (len * i), SG_KMAP);
+ if (IS_ERR(buf)) {
+ cmd->error = PTR_ERR(buf);
+ break;
+ }
+
result =
sdricoh_blockio(host,
data->flags & MMC_DATA_READ, buf, len);
- kunmap(page);
- flush_dcache_page(page);
+ sg_unmap(data->sg, buf, (len * i), SG_KMAP);
+
+ flush_dcache_page(sg_page(data->sg));
if (result) {
dev_err(dev, "sdricoh_request: cmd %i "
"block transfer failed\n", cmd->opcode);
--
2.1.4
^ permalink raw reply related
* [PATCH v2 20/21] mmc: tifm_sd: Make use of the new sg_map helper function
From: Logan Gunthorpe @ 2017-04-25 18:21 UTC (permalink / raw)
To: linux-kernel, linux-crypto, linux-media, dri-devel, intel-gfx,
linux-raid, linux-mmc, linux-nvdimm, linux-scsi, open-iscsi,
megaraidlinux.pdl, sparmaintainer, devel, target-devel, netdev,
linux-rdma, dm-devel
Cc: Jens Axboe, Ulf Hansson, James E.J. Bottomley, Alex Dubov,
Martin K. Petersen, Matthew Wilcox, Greg Kroah-Hartman,
Sumit Semwal, Ross Zwisler, Dan Williams, Stephen Bates,
Logan Gunthorpe, Christoph Hellwig
In-Reply-To: <1493144468-22493-1-git-send-email-logang@deltatee.com>
This conversion is a bit complicated. We modiy the read_fifo,
write_fifo and copy_page functions to take a scatterlist instead of a
page. Thus we can use sg_map instead of kmap_atomic. There's a bit of
accounting that needed to be done for the offset for this to work.
(Seeing sg_map takes care of the offset but it's already added and
used earlier in the code.)
There's also no error path, so we use SG_MAP_MUST_NOT_FAIL which may
BUG_ON in certain cases in the future.
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Alex Dubov <oakad@yahoo.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/host/tifm_sd.c | 50 +++++++++++++++++++++++++++-------------------
1 file changed, 29 insertions(+), 21 deletions(-)
diff --git a/drivers/mmc/host/tifm_sd.c b/drivers/mmc/host/tifm_sd.c
index 93c4b40..e64345a 100644
--- a/drivers/mmc/host/tifm_sd.c
+++ b/drivers/mmc/host/tifm_sd.c
@@ -111,14 +111,16 @@ struct tifm_sd {
};
/* for some reason, host won't respond correctly to readw/writew */
-static void tifm_sd_read_fifo(struct tifm_sd *host, struct page *pg,
+static void tifm_sd_read_fifo(struct tifm_sd *host, struct scatterlist *sg,
unsigned int off, unsigned int cnt)
{
struct tifm_dev *sock = host->dev;
unsigned char *buf;
unsigned int pos = 0, val;
- buf = kmap_atomic(pg) + off;
+ buf = sg_map(sg, off - sg->offset,
+ SG_KMAP_ATOMIC | SG_MAP_MUST_NOT_FAIL);
+
if (host->cmd_flags & DATA_CARRY) {
buf[pos++] = host->bounce_buf_data[0];
host->cmd_flags &= ~DATA_CARRY;
@@ -134,17 +136,19 @@ static void tifm_sd_read_fifo(struct tifm_sd *host, struct page *pg,
}
buf[pos++] = (val >> 8) & 0xff;
}
- kunmap_atomic(buf - off);
+ sg_unmap(sg, buf, off - sg->offset, SG_KMAP_ATOMIC);
}
-static void tifm_sd_write_fifo(struct tifm_sd *host, struct page *pg,
+static void tifm_sd_write_fifo(struct tifm_sd *host, struct scatterlist *sg,
unsigned int off, unsigned int cnt)
{
struct tifm_dev *sock = host->dev;
unsigned char *buf;
unsigned int pos = 0, val;
- buf = kmap_atomic(pg) + off;
+ buf = sg_map(sg, off - sg->offset,
+ SG_KMAP_ATOMIC | SG_MAP_MUST_NOT_FAIL);
+
if (host->cmd_flags & DATA_CARRY) {
val = host->bounce_buf_data[0] | ((buf[pos++] << 8) & 0xff00);
writel(val, sock->addr + SOCK_MMCSD_DATA);
@@ -161,7 +165,7 @@ static void tifm_sd_write_fifo(struct tifm_sd *host, struct page *pg,
val |= (buf[pos++] << 8) & 0xff00;
writel(val, sock->addr + SOCK_MMCSD_DATA);
}
- kunmap_atomic(buf - off);
+ sg_unmap(sg, buf, off - sg->offset, SG_KMAP_ATOMIC);
}
static void tifm_sd_transfer_data(struct tifm_sd *host)
@@ -170,7 +174,6 @@ static void tifm_sd_transfer_data(struct tifm_sd *host)
struct scatterlist *sg = r_data->sg;
unsigned int off, cnt, t_size = TIFM_MMCSD_FIFO_SIZE * 2;
unsigned int p_off, p_cnt;
- struct page *pg;
if (host->sg_pos == host->sg_len)
return;
@@ -192,33 +195,39 @@ static void tifm_sd_transfer_data(struct tifm_sd *host)
}
off = sg[host->sg_pos].offset + host->block_pos;
- pg = nth_page(sg_page(&sg[host->sg_pos]), off >> PAGE_SHIFT);
p_off = offset_in_page(off);
p_cnt = PAGE_SIZE - p_off;
p_cnt = min(p_cnt, cnt);
p_cnt = min(p_cnt, t_size);
if (r_data->flags & MMC_DATA_READ)
- tifm_sd_read_fifo(host, pg, p_off, p_cnt);
+ tifm_sd_read_fifo(host, &sg[host->sg_pos], p_off,
+ p_cnt);
else if (r_data->flags & MMC_DATA_WRITE)
- tifm_sd_write_fifo(host, pg, p_off, p_cnt);
+ tifm_sd_write_fifo(host, &sg[host->sg_pos], p_off,
+ p_cnt);
t_size -= p_cnt;
host->block_pos += p_cnt;
}
}
-static void tifm_sd_copy_page(struct page *dst, unsigned int dst_off,
- struct page *src, unsigned int src_off,
+static void tifm_sd_copy_page(struct scatterlist *dst, unsigned int dst_off,
+ struct scatterlist *src, unsigned int src_off,
unsigned int count)
{
- unsigned char *src_buf = kmap_atomic(src) + src_off;
- unsigned char *dst_buf = kmap_atomic(dst) + dst_off;
+ unsigned char *src_buf, *dst_buf;
+
+ src_off -= src->offset;
+ dst_off -= dst->offset;
+
+ src_buf = sg_map(src, src_off, SG_KMAP_ATOMIC | SG_MAP_MUST_NOT_FAIL);
+ dst_buf = sg_map(dst, dst_off, SG_KMAP_ATOMIC | SG_MAP_MUST_NOT_FAIL);
memcpy(dst_buf, src_buf, count);
- kunmap_atomic(dst_buf - dst_off);
- kunmap_atomic(src_buf - src_off);
+ sg_unmap(dst, dst_buf, dst_off, SG_KMAP_ATOMIC);
+ sg_unmap(src, src_buf, src_off, SG_KMAP_ATOMIC);
}
static void tifm_sd_bounce_block(struct tifm_sd *host, struct mmc_data *r_data)
@@ -227,7 +236,6 @@ static void tifm_sd_bounce_block(struct tifm_sd *host, struct mmc_data *r_data)
unsigned int t_size = r_data->blksz;
unsigned int off, cnt;
unsigned int p_off, p_cnt;
- struct page *pg;
dev_dbg(&host->dev->dev, "bouncing block\n");
while (t_size) {
@@ -241,18 +249,18 @@ static void tifm_sd_bounce_block(struct tifm_sd *host, struct mmc_data *r_data)
}
off = sg[host->sg_pos].offset + host->block_pos;
- pg = nth_page(sg_page(&sg[host->sg_pos]), off >> PAGE_SHIFT);
p_off = offset_in_page(off);
p_cnt = PAGE_SIZE - p_off;
p_cnt = min(p_cnt, cnt);
p_cnt = min(p_cnt, t_size);
if (r_data->flags & MMC_DATA_WRITE)
- tifm_sd_copy_page(sg_page(&host->bounce_buf),
+ tifm_sd_copy_page(&host->bounce_buf,
r_data->blksz - t_size,
- pg, p_off, p_cnt);
+ &sg[host->sg_pos], p_off, p_cnt);
else if (r_data->flags & MMC_DATA_READ)
- tifm_sd_copy_page(pg, p_off, sg_page(&host->bounce_buf),
+ tifm_sd_copy_page(&sg[host->sg_pos], p_off,
+ &host->bounce_buf,
r_data->blksz - t_size, p_cnt);
t_size -= p_cnt;
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related
* [PATCH v2 21/21] memstick: Make use of the new sg_map helper function
From: Logan Gunthorpe @ 2017-04-25 18:21 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
linux-scsi-u79uwXL29TY76Z2rM5mHXA,
open-iscsi-/JYPxA39Uh5TLH3MbocFFw,
megaraidlinux.pdl-dY08KVG/lbpWk0Htik3J/w,
sparmaintainer-GLv8BlqOqDDQT0dZR+AlfA,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
target-devel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA
Cc: Jens Axboe, James E.J. Bottomley, Alex Dubov, Martin K. Petersen,
Matthew Wilcox, Greg Kroah-Hartman, Sumit Semwal,
Christoph Hellwig
In-Reply-To: <1493144468-22493-1-git-send-email-logang-OTvnGxWRz7hWk0Htik3J/w@public.gmane.org>
Straightforward conversion, but we have to make use of
SG_MAP_MUST_NOT_FAIL which may BUG_ON in certain cases
in the future.
Signed-off-by: Logan Gunthorpe <logang-OTvnGxWRz7hWk0Htik3J/w@public.gmane.org>
Cc: Alex Dubov <oakad-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
---
drivers/memstick/host/jmb38x_ms.c | 11 ++++++-----
drivers/memstick/host/tifm_ms.c | 11 ++++++-----
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/memstick/host/jmb38x_ms.c b/drivers/memstick/host/jmb38x_ms.c
index 48db922..9019e37 100644
--- a/drivers/memstick/host/jmb38x_ms.c
+++ b/drivers/memstick/host/jmb38x_ms.c
@@ -303,7 +303,6 @@ static int jmb38x_ms_transfer_data(struct jmb38x_ms_host *host)
unsigned int off;
unsigned int t_size, p_cnt;
unsigned char *buf;
- struct page *pg;
unsigned long flags = 0;
if (host->req->long_data) {
@@ -318,14 +317,14 @@ static int jmb38x_ms_transfer_data(struct jmb38x_ms_host *host)
unsigned int uninitialized_var(p_off);
if (host->req->long_data) {
- pg = nth_page(sg_page(&host->req->sg),
- off >> PAGE_SHIFT);
p_off = offset_in_page(off);
p_cnt = PAGE_SIZE - p_off;
p_cnt = min(p_cnt, length);
local_irq_save(flags);
- buf = kmap_atomic(pg) + p_off;
+ buf = sg_map(&host->req->sg,
+ off - host->req->sg.offset,
+ SG_KMAP_ATOMIC | SG_MAP_MUST_NOT_FAIL);
} else {
buf = host->req->data + host->block_pos;
p_cnt = host->req->data_len - host->block_pos;
@@ -341,7 +340,9 @@ static int jmb38x_ms_transfer_data(struct jmb38x_ms_host *host)
: jmb38x_ms_read_reg_data(host, buf, p_cnt);
if (host->req->long_data) {
- kunmap_atomic(buf - p_off);
+ sg_unmap(&host->req->sg, buf,
+ off - host->req->sg.offset,
+ SG_KMAP_ATOMIC);
local_irq_restore(flags);
}
diff --git a/drivers/memstick/host/tifm_ms.c b/drivers/memstick/host/tifm_ms.c
index 7bafa72..304985d 100644
--- a/drivers/memstick/host/tifm_ms.c
+++ b/drivers/memstick/host/tifm_ms.c
@@ -186,7 +186,6 @@ static unsigned int tifm_ms_transfer_data(struct tifm_ms *host)
unsigned int off;
unsigned int t_size, p_cnt;
unsigned char *buf;
- struct page *pg;
unsigned long flags = 0;
if (host->req->long_data) {
@@ -203,14 +202,14 @@ static unsigned int tifm_ms_transfer_data(struct tifm_ms *host)
unsigned int uninitialized_var(p_off);
if (host->req->long_data) {
- pg = nth_page(sg_page(&host->req->sg),
- off >> PAGE_SHIFT);
p_off = offset_in_page(off);
p_cnt = PAGE_SIZE - p_off;
p_cnt = min(p_cnt, length);
local_irq_save(flags);
- buf = kmap_atomic(pg) + p_off;
+ buf = sg_map(&host->req->sg,
+ off - host->req->sg.offset,
+ SG_KMAP_ATOMIC | SG_MAP_MUST_NOT_FAIL);
} else {
buf = host->req->data + host->block_pos;
p_cnt = host->req->data_len - host->block_pos;
@@ -221,7 +220,9 @@ static unsigned int tifm_ms_transfer_data(struct tifm_ms *host)
: tifm_ms_read_data(host, buf, p_cnt);
if (host->req->long_data) {
- kunmap_atomic(buf - p_off);
+ sg_unmap(&host->req->sg, buf,
+ off - host->req->sg.offset,
+ SG_KMAP_ATOMIC | SG_MAP_MUST_NOT_FAIL);
local_irq_restore(flags);
}
--
2.1.4
^ permalink raw reply related
* Re: Recover array after I panicked
From: Wols Lists @ 2017-04-25 18:22 UTC (permalink / raw)
To: Patrik Dahlström, Andreas Klauer; +Cc: Brad Campbell, linux-raid
In-Reply-To: <CAHKno4BfQG61pYMtcUf8NrzPt3dtNM=xe_DAb7Ypt4Qy_=1QkQ@mail.gmail.com>
On 25/04/17 12:37, Patrik Dahlström wrote:
> I'm not sure how I would backup 20 TB of data. I'll backup what I can,
> of course. The most essential.
Buy some cheap disks :-)
A quick skim gives me £111 for a 4TB drive, but that'll be tricky to
find. It is a Barracuda, however ...
https://www.eclipsecomputers.com/Product/4TB-Seagate-Barracuda-5900rpm-SATA3-64Mb-HardDrive/
Better are 8TB (£260) or 10TB (£365, £420) drives, but they're pricy.
https://www.eclipsecomputers.com/Product/8Tb-Seagate-SkyHawk-7200rpm-SATA3-256Mb-Hard-Drive/
https://www.eclipsecomputers.com/Product/10Tb-Seagate-IronWolf-7200rpm-SATA3-256Mb-Hard-Drive/
https://www.eclipsecomputers.com/Product/10Tb-Seagate-IronWolf-Pro-7200rpm-SATA3-256Mb-Hard-Drive/
If it's just a temporary backup, I'd try and get another couple of
drives same as your raid, and beg borrow or steal as many drives from
your friends as you need. Your local little computer shop might have a
load of drives salvaged from dead pc's that you can borrow (or maybe not :-(
Cheers,
Wol
^ permalink raw reply
* 875 linux-raid
From: citydesk @ 2017-04-25 18:41 UTC (permalink / raw)
To: linux-raid
[-- Attachment #1: 40891313.zip --]
[-- Type: application/zip, Size: 1701 bytes --]
^ permalink raw reply
* Re: Recover array after I panicked
From: Patrik Dahlström @ 2017-04-25 23:01 UTC (permalink / raw)
To: Andreas Klauer; +Cc: Brad Campbell, linux-raid
In-Reply-To: <20170424110455.GA4949@metamorpher.de>
On 04/24/2017 01:04 PM, Andreas Klauer wrote:
> On Mon, Apr 24, 2017 at 09:34:04AM +0200, Patrik Dahlström wrote:
> Now create two RAID sets:
>
> # losetup -D
> # for f in ? ; do cp "$f" "$f".a ; done;
> # for f in ? ; do cp "$f" "$f".b ; done;
> # for a in *.a ; do losetup --find --show "$a" ; done
> # for b in *.b ; do losetup --find --show "$b" ; done
> # mdadm --create /dev/md42 --assume-clean --level=5 --raid-devices=5 /dev/loop{0,1,2,3,4}
> # mdadm --create /dev/md42 --assume-clean --level=5 --raid-devices=6 /dev/loop{5,6,7,8,9,10}
>
> # cat /proc/mdstat
> Personalities : [linear] [raid0] [raid1] [raid10] [raid6] [raid5] [raid4]
> md42 : active raid5 loop4[4] loop3[3] loop2[2] loop1[1] loop0[0]
> 405504 blocks super 1.2 level 5, 512k chunk, algorithm 2 [5/5] [UUUUU]
>
> md43 : active raid5 loop10[5] loop9[4] loop8[3] loop7[2] loop6[1] loop5[0]
> 506880 blocks super 1.2 level 5, 512k chunk, algorithm 2 [6/6] [UUUUUU]
>
> And compare:
>
> # hexdump -C -n 64 /dev/md42
> 00000000 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 0a |000000000000000.|
> 00000010 30 30 30 30 30 30 30 30 30 30 30 30 30 31 30 0a |000000000000010.|
> 00000020 30 30 30 30 30 30 30 30 30 30 30 30 30 32 30 0a |000000000000020.|
> 00000030 30 30 30 30 30 30 30 30 30 30 30 30 30 33 30 0a |000000000000030.|
> 00000040
> # hexdump -C -n 64 /dev/md43
> 00000000 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 0a |000000000000000.|
> 00000010 30 30 30 30 30 30 30 30 30 30 30 30 30 31 30 0a |000000000000010.|
> 00000020 30 30 30 30 30 30 30 30 30 30 30 30 30 32 30 0a |000000000000020.|
> 00000030 30 30 30 30 30 30 30 30 30 30 30 30 30 33 30 0a |000000000000030.|
> 00000040
>
> This is identical because in this example, the offset didn't change.
>
> # hexdump -C -n 64 -s 80808080 /dev/md42
> 04d10890 30 30 30 30 30 30 30 30 35 66 31 30 38 39 30 0a |000000005f10890.|
> 04d108a0 30 30 30 30 30 30 30 30 35 66 31 30 38 61 30 0a |000000005f108a0.|
> 04d108b0 30 30 30 30 30 30 30 30 35 66 31 30 38 62 30 0a |000000005f108b0.|
> 04d108c0 30 30 30 30 30 30 30 30 35 66 31 30 38 63 30 0a |000000005f108c0.|
> 04d108d0
> # hexdump -C -n 64 -s 80808080 /dev/md43
> 04d10890 30 30 30 30 30 30 30 30 34 64 31 30 38 39 30 0a |000000004d10890.|
> 04d108a0 30 30 30 30 30 30 30 30 34 64 31 30 38 61 30 0a |000000004d108a0.|
> 04d108b0 30 30 30 30 30 30 30 30 34 64 31 30 38 62 30 0a |000000004d108b0.|
> 04d108c0 30 30 30 30 30 30 30 30 34 64 31 30 38 63 30 0a |000000004d108c0.|
> 04d108d0
>
> For this offset, md42 was wrong, md43 is correct.
>
> # hexdump -C -n 64 -s 300808080 /dev/md42
> 11edf790 30 30 30 30 30 30 30 31 31 65 64 66 37 39 30 0a |000000011edf790.|
> 11edf7a0 30 30 30 30 30 30 30 31 31 65 64 66 37 61 30 0a |000000011edf7a0.|
> 11edf7b0 30 30 30 30 30 30 30 31 31 65 64 66 37 62 30 0a |000000011edf7b0.|
> 11edf7c0 30 30 30 30 30 30 30 31 31 65 64 66 37 63 30 0a |000000011edf7c0.|
> 11edf7d0
> # hexdump -C -n 64 -s 300808080 /dev/md43
> 11edf790 30 30 30 30 30 30 30 31 31 65 64 66 37 39 30 0a |000000011edf790.|
> 11edf7a0 30 30 30 30 30 30 30 31 31 65 64 66 37 61 30 0a |000000011edf7a0.|
> 11edf7b0 30 30 30 30 30 30 30 31 31 65 64 66 37 62 30 0a |000000011edf7b0.|
> 11edf7c0 30 30 30 30 30 30 30 31 31 65 64 66 37 63 30 0a |000000011edf7c0.|
> 11edf7d0
>
> For this offset, md42 and md43 overlapped. Grow progressed that far yet
> without writing into the original data of the 5disk raid5. This could be
> a suitable merge point for a linear device mapping.
>
> # hexdump -C -n 64 -s 400008080 /dev/md42
> 17d7a390 30 30 30 30 30 30 30 31 37 64 37 61 33 39 30 0a |000000017d7a390.|
> 17d7a3a0 30 30 30 30 30 30 30 31 37 64 37 61 33 61 30 0a |000000017d7a3a0.|
> 17d7a3b0 30 30 30 30 30 30 30 31 37 64 37 61 33 62 30 0a |000000017d7a3b0.|
> 17d7a3c0 30 30 30 30 30 30 30 31 37 64 37 61 33 63 30 0a |000000017d7a3c0.|
> 17d7a3d0
> # hexdump -C -n 64 -s 400008080 /dev/md43
> 17d7a390 30 30 30 30 30 30 30 31 33 31 37 61 33 39 30 0a |00000001317a390.|
> 17d7a3a0 30 30 30 30 30 30 30 31 33 31 37 61 33 61 30 0a |00000001317a3a0.|
> 17d7a3b0 30 30 30 30 30 30 30 31 33 31 37 61 33 62 30 0a |00000001317a3b0.|
> 17d7a3c0 30 30 30 30 30 30 30 31 33 31 37 61 33 63 30 0a |00000001317a3c0.|
> 17d7a3d0
>
> For this offset, md42 is correct and md43 is wrong.
> Grow did not progress that far.
>
> That's the general outline of the idea.
> The problem in your case is of course, your data is not that easy to verify.
I've been experimenting with this idea today by writing a small program
that looks for a mkv (matroska) header in a file, prints the offset and
then exit. I then extract 10 MB from that offset and try to play the
file with mpv.
$ ./find_matroska /dev/md1 0x202C0000000
Offset: 0x202C0000000 (2059 GB)
Offset: 0x20300000000 (2060 GB)
Found magic @ 0x2030CFCDDD7
Not matroska
Offset: 0x20340000000 (2061 GB)
Offset: 0x20380000000 (2062 GB)
Found magic @ 0x203A0005A49
Not matroska
Found magic @ 0x203AA800000
It's matroska
$ dd if=/dev/md1 bs=524288 count=20 skip=$((0x203AA800000/524288)) of=/tmp/raw.mkv
20+0 records in
20+0 records out
10485760 bytes (10 MB, 10 MiB) copied, 0.0748999 s, 140 MB/s
(copy to laptop and play)
On the 6 disk raid, I had no problem finding an mkv file at relatively
low offset (< 4 TB) that would play the whole 10 MB without issues.
Somewhere between 4-5 TB I start to get corrupted videos again. I
actually stumbled upon the exact same video on offsets:
0x400C1800000 (4 TB) : Intact, grow has come here
0x500F2B00000 (5 TB) : Corrupted, grow did not progress here yet
However, I was not able to do that on the 5 disk raid. It didn't matter
if I started the search at 1 TB or 16 TB, it would always have errors.
Would this mean that my data offset is wrong for the 5 disk raid?
Best regards
// Patrik
^ permalink raw reply
* [mdadm PATCH] Create: move STOP_ARRAY to abort_locked
From: Zhilong Liu @ 2017-04-26 7:03 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: neilb, linux-raid, Zhilong Liu
The sysfs entry and devnm would be created once create_mddev()
performed successfully, but the creating isn't completed here,
move STOP_ARRAY to abort_locked, the purpose is to cleanup the
partially created array.
Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
Create.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/Create.c b/Create.c
index 6ca0924..fe0ab7e 100644
--- a/Create.c
+++ b/Create.c
@@ -904,10 +904,8 @@ int Create(struct supertype *st, char *mddev,
remove_partitions(fd);
if (st->ss->add_to_super(st, &inf->disk,
fd, dv->devname,
- dv->data_offset)) {
- ioctl(mdfd, STOP_ARRAY, NULL);
+ dv->data_offset))
goto abort_locked;
- }
st->ss->getinfo_super(st, inf, NULL);
safe_mode_delay = inf->safe_mode_delay;
@@ -1008,7 +1006,6 @@ int Create(struct supertype *st, char *mddev,
sysfs_set_safemode(&info, safe_mode_delay);
if (err) {
pr_err("failed to activate array.\n");
- ioctl(mdfd, STOP_ARRAY, NULL);
goto abort;
}
} else if (c->readonly &&
@@ -1018,7 +1015,6 @@ int Create(struct supertype *st, char *mddev,
"array_state", "readonly") < 0) {
pr_err("Failed to start array: %s\n",
strerror(errno));
- ioctl(mdfd, STOP_ARRAY, NULL);
goto abort;
}
} else {
@@ -1030,7 +1026,6 @@ int Create(struct supertype *st, char *mddev,
if (info.array.chunk_size & (info.array.chunk_size-1)) {
cont_err("Problem may be that chunk size is not a power of 2\n");
}
- ioctl(mdfd, STOP_ARRAY, NULL);
goto abort;
}
/* if start_ro module parameter is set, array is
@@ -1061,7 +1056,9 @@ int Create(struct supertype *st, char *mddev,
map_remove(&map, fd2devnm(mdfd));
map_unlock(&map);
- if (mdfd >= 0)
+ if (mdfd >= 0) {
+ ioctl(mdfd, STOP_ARRAY, NULL);
close(mdfd);
+ }
return 1;
}
--
2.6.6
^ permalink raw reply related
* Re: [PATCH v2 15/21] xen-blkfront: Make use of the new sg_map helper function
From: Roger Pau Monné @ 2017-04-26 7:37 UTC (permalink / raw)
To: Logan Gunthorpe
Cc: Boris Ostrovsky, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
target-devel-u79uwXL29TY76Z2rM5mHXA, Christoph Hellwig,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b, James E.J. Bottomley,
linux-scsi-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, Sumit Semwal,
open-iscsi-/JYPxA39Uh5TLH3MbocFFw,
linux-media-u79uwXL29TY76Z2rM5mHXA, Juergen Gross, Julien Grall,
intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
sparmaintainer-GLv8BlqOqDDQT0dZR+AlfA,
linux-raid-u79uwXL29TY76Z2rM5mHXA,
megaraidlinux.pdl-dY08KVG/lbpWk0Htik3J/w, Jens Axboe,
Martin K. Petersen, netdev-u79uwXL29TY76Z2rM5mHXA, Matthew Wilcox,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman
In-Reply-To: <1493144468-22493-16-git-send-email-logang-OTvnGxWRz7hWk0Htik3J/w@public.gmane.org>
On Tue, Apr 25, 2017 at 12:21:02PM -0600, Logan Gunthorpe wrote:
> Straightforward conversion to the new helper, except due to the lack
> of error path, we have to use SG_MAP_MUST_NOT_FAIL which may BUG_ON in
> certain cases in the future.
>
> Signed-off-by: Logan Gunthorpe <logang-OTvnGxWRz7hWk0Htik3J/w@public.gmane.org>
> Cc: Boris Ostrovsky <boris.ostrovsky-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Cc: Juergen Gross <jgross-IBi9RG/b67k@public.gmane.org>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Cc: "Roger Pau Monné" <roger.pau-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org>
> ---
> drivers/block/xen-blkfront.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index 3945963..ed62175 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -816,8 +816,9 @@ static int blkif_queue_rw_req(struct request *req, struct blkfront_ring_info *ri
> BUG_ON(sg->offset + sg->length > PAGE_SIZE);
>
> if (setup.need_copy) {
> - setup.bvec_off = sg->offset;
> - setup.bvec_data = kmap_atomic(sg_page(sg));
> + setup.bvec_off = 0;
> + setup.bvec_data = sg_map(sg, 0, SG_KMAP_ATOMIC |
> + SG_MAP_MUST_NOT_FAIL);
I assume that sg_map already adds sg->offset to the address?
Also wondering whether we can get rid of bvec_off and just increment bvec_data,
adding Julien who IIRC added this code.
> }
>
> gnttab_foreach_grant_in_range(sg_page(sg),
> @@ -827,7 +828,7 @@ static int blkif_queue_rw_req(struct request *req, struct blkfront_ring_info *ri
> &setup);
>
> if (setup.need_copy)
> - kunmap_atomic(setup.bvec_data);
> + sg_unmap(sg, setup.bvec_data, 0, SG_KMAP_ATOMIC);
> }
> if (setup.segments)
> kunmap_atomic(setup.segments);
> @@ -1053,7 +1054,7 @@ static int xen_translate_vdev(int vdevice, int *minor, unsigned int *offset)
> case XEN_SCSI_DISK5_MAJOR:
> case XEN_SCSI_DISK6_MAJOR:
> case XEN_SCSI_DISK7_MAJOR:
> - *offset = (*minor / PARTS_PER_DISK) +
> + *offset = (*minor / PARTS_PER_DISK) +
> ((major - XEN_SCSI_DISK1_MAJOR + 1) * 16) +
> EMULATED_SD_DISK_NAME_OFFSET;
> *minor = *minor +
> @@ -1068,7 +1069,7 @@ static int xen_translate_vdev(int vdevice, int *minor, unsigned int *offset)
> case XEN_SCSI_DISK13_MAJOR:
> case XEN_SCSI_DISK14_MAJOR:
> case XEN_SCSI_DISK15_MAJOR:
> - *offset = (*minor / PARTS_PER_DISK) +
> + *offset = (*minor / PARTS_PER_DISK) +
> ((major - XEN_SCSI_DISK8_MAJOR + 8) * 16) +
> EMULATED_SD_DISK_NAME_OFFSET;
> *minor = *minor +
> @@ -1119,7 +1120,7 @@ static int xlvbd_alloc_gendisk(blkif_sector_t capacity,
> if (!VDEV_IS_EXTENDED(info->vdevice)) {
> err = xen_translate_vdev(info->vdevice, &minor, &offset);
> if (err)
> - return err;
> + return err;
Cosmetic changes should go in a separate patch please.
Roger.
^ permalink raw reply
* Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions
From: Christoph Hellwig @ 2017-04-26 7:44 UTC (permalink / raw)
To: Logan Gunthorpe
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
linux-scsi-u79uwXL29TY76Z2rM5mHXA,
open-iscsi-/JYPxA39Uh5TLH3MbocFFw,
megaraidlinux.pdl-dY08KVG/lbpWk0Htik3J/w,
sparmaintainer-GLv8BlqOqDDQT0dZR+AlfA,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
target-devel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA, Christoph Hellwig,
Martin K. Petersen, James E.J. Bottomley, Jens Axboe,
Greg Kroah-Hartman, Dan Williams, Ross Zwisler, Matthew Wilcox,
Sumit Semwal
In-Reply-To: <1493144468-22493-2-git-send-email-logang-OTvnGxWRz7hWk0Htik3J/w@public.gmane.org>
On Tue, Apr 25, 2017 at 12:20:48PM -0600, Logan Gunthorpe wrote:
> This patch introduces functions which kmap the pages inside an sgl.
> These functions replace a common pattern of kmap(sg_page(sg)) that is
> used in more than 50 places within the kernel.
>
> The motivation for this work is to eventually safely support sgls that
> contain io memory. In order for that to work, any access to the contents
> of an iomem SGL will need to be done with iomemcpy or hit some warning.
> (The exact details of how this will work have yet to be worked out.)
I think we'll at least need a draft of those to make sense of these
patches. Otherwise they just look very clumsy.
> + * Use this function to map a page in the scatterlist at the specified
> + * offset. sg->offset is already added for you. Note: the semantics of
> + * this function are that it may fail. Thus, its output should be checked
> + * with IS_ERR and PTR_ERR. Otherwise, a pointer to the specified offset
> + * in the mapped page is returned.
> + *
> + * Flags can be any of:
> + * * SG_KMAP - Use kmap to create the mapping
> + * * SG_KMAP_ATOMIC - Use kmap_atomic to map the page atommically.
> + * Thus, the rules of that function apply: the
> + * cpu may not sleep until it is unmaped.
> + * * SG_MAP_MUST_NOT_FAIL - Indicate that sg_map must not fail.
> + * If it does, it will issue a BUG_ON instead.
> + * This is intended for legacy code only, it
> + * is not to be used in new code.
I'm sorry but this API is just a trainwreck. Right now we have the
nice little kmap_atomic API, which never fails and has a very nice
calling convention where we just pass back the return address, but does
not support sleeping inside the critical section.
And kmap, whіch may fail and requires the original page to be passed
back. Anything that mixes these two concepts up is simply a non-starter.
--
You received this message because you are subscribed to the Google Groups "open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To post to this group, send email to open-iscsi-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH v2 02/21] libiscsi: Add an internal error code
From: Christoph Hellwig @ 2017-04-26 7:48 UTC (permalink / raw)
To: Logan Gunthorpe
Cc: dri-devel, Stephen Bates, dm-devel, target-devel,
Christoph Hellwig, devel, James E.J. Bottomley, linux-scsi,
linux-nvdimm, linux-rdma, Sumit Semwal, Ross Zwisler, open-iscsi,
linux-media, intel-gfx, sparmaintainer, linux-raid, Dan Williams,
megaraidlinux.pdl, Jens Axboe, Martin K. Petersen, netdev,
Matthew Wilcox, linux-mmc, linux-kernel, linux-crypto,
Greg Kroah-Hartman
In-Reply-To: <1493144468-22493-3-git-send-email-logang@deltatee.com>
On Tue, Apr 25, 2017 at 12:20:49PM -0600, Logan Gunthorpe wrote:
> This is a prep patch to add a new error code to libiscsi. We want to
> rework some kmap calls to be able to fail. When we do, we'd like to
> use this error code.
The kmap case in iscsi_tcp_segment_map can already fail. Please add
handling of that failure to this patch, and we should get it merged
ASAP.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* Reshape didn't stop when two disks was removed
From: 彭昶勳 @ 2017-04-26 7:58 UTC (permalink / raw)
To: linux-raid
Hi all,
I built a raid5 md on 3 disks, than I added two disks to reshape this
md to a five disks raid5 md.
When reshaping the md, two added disks disconneted, but reshape didn't
stop. The md didn't crash, just degraded.
I traced code and I found this problem may be caused by function
error() in raid5.c
In error(), it set faulty bit after calc_degraded.
therefore, when clac_degraded() checking faulty, it didn't count
degrade of the first removed disk.
Should it set_bit faulty before clac_degrade function?
thanks,
Allen
^ permalink raw reply
* Re: [MD PATCH 1/1] Don't add nr_pending for resync requests
From: Coly Li @ 2017-04-26 8:40 UTC (permalink / raw)
To: Shaohua Li, Xiao Ni; +Cc: linux-raid, ncroxon, neilb
In-Reply-To: <20170425171034.peeh6uuvwycqtise@kernel.org>
On 2017/4/26 上午1:10, Shaohua Li wrote:
> Hi,
> Cc: Coly and Neil.
>
> On Mon, Apr 24, 2017 at 04:47:15PM +0800, Xiao Ni wrote:
>> In newer barrier codes, raise_barrier waits if conf->nr_pending[idx] is not zero.
>> After all the conditions are true, the resync request can go on be handling. But
>> it adds conf->nr_pending[idx] again. The next resync request hit the same bucket
>> idx need to wait the resync request which is submitted before. The performance
>> of resync/recovery is degraded.
>>
>> I did some tests:
>> 1. Before removing the codes, the resync performance is:
>> Device: rrqm/s wrqm/s r/s w/s rMB/s wMB/s avgrq-sz avgqu-sz await r_await w_await svctm %util
>> sdc 0.00 0.00 0.00 164.00 0.00 10.13 126.46 0.95 5.80 0.00 5.80 5.82 95.40
>> sdb 0.00 0.00 163.00 2.00 10.19 0.00 126.47 0.04 0.25 0.18 5.50 0.25 4.10
>> 2. After removing the codes, the resync performance is:
>> Device: rrqm/s wrqm/s r/s w/s rMB/s wMB/s avgrq-sz avgqu-sz await r_await w_await svctm %util
>> sdc 0.00 2164.00 0.00 751.00 0.00 182.06 496.49 7.49 9.95 0.00 9.95 1.33 100.00
>> sdb 2162.00 0.00 752.00 0.00 182.25 0.00 496.34 0.56 0.74 0.74 0.00 0.65 48.80
>
> A big resync regression, thanks to check it! Your analysis makes sense to me,
> but the patch doesn't. freeze_array() depends on nr_pending to wait for all
> normal IO and resync. This change will make the wait for resync disappear.
>
> I think the problem is we are abusing nr_pending here. freeze_array() waits for
> a condition which doesn't need to be maintained by nr_pending. It makes sense
> raise_barrier to wait for nr_pending caused by normal IO, but it doesn't make
> sense to wait for other raise_barrier.
>
> We could add another vairable, increase it in raise_barrier, decrease it in
> lower_barrier and let freeze_array() check the new variable. That should fix
> the problem and not expose race condition for freeze_array().
Shaohua,
Adding another variable to recode pending resync I/O is good idea. We
can do something like this,
@@ -869,7 +869,7 @@ static void raise_barrier()
- atomic_inc(&conf->nr_pending[idx]);
+ atomic_inc(&conf->nr_resync_pending[idx]);
spin_unlock_irq(&conf->resync_lock);
}
@@ -880,7 +880,7 @@ static void lower_barrier()
atomic_dec(&conf->barrier[idx]);
- atomic_dec(&conf->nr_pending[idx]);
+ atomic_dec(&conf->nr_resync_pending[idx]);
wake_up(&conf->wait_barrier);
}
@@ -1018,7 +1018,8 @@ static int get_unqueued_pending()
int idx, ret;
for (ret = 0, idx = 0; idx < BARRIER_BUCKETS_NR; idx++)
- ret += atomic_read(&conf->nr_pending[idx]) -
+ ret += atomic_read(&conf->nr_pending[idx]) +
+ atomic_read(&conf->nr_resync_pending[idx]) -
atomic_read(&conf->nr_queued[idx]);
return ret;
Xiao,
Nice catch! From your email, it seems resync request on raid1 device can
be non-linear, otherwise, it won't hit same barrier bucket by different
resync I/O to raise barrier. This is some thing I don't know. Thank you !
The reason to increase conf->nr_pending[idx] is from 'commit 34e97f17
("md/raid1: count resync requests in nr_pending.")', because normal I/O
and resync I/O can happen at same time on same raid1 device. And both of
them can be queued by reschedule_retry(), raise_barrier() need to
increase conf->nr_pending[idx] to make sure pending I/Os will be equal
to queued I/Os when array is frozen. We cannot remove
conf->nr_pending[idx] operations from raise_barrier()/lower_barrier().
Would you like to compose a patch by Shaohua's suggestion ?
Thanks.
Coly Li
^ permalink raw reply
* Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions
From: Christian König @ 2017-04-26 8:59 UTC (permalink / raw)
To: Logan Gunthorpe, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
linux-scsi-u79uwXL29TY76Z2rM5mHXA,
open-iscsi-/JYPxA39Uh5TLH3MbocFFw,
megaraidlinux.pdl-dY08KVG/lbpWk0Htik3J/w,
sparmaintainer-GLv8BlqOqDDQT0dZR+AlfA,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
target-devel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA
Cc: Jens Axboe, James E.J. Bottomley, Martin K. Petersen,
Matthew Wilcox, Greg Kroah-Hartman, Christoph Hellwig
In-Reply-To: <1493144468-22493-2-git-send-email-logang-OTvnGxWRz7hWk0Htik3J/w@public.gmane.org>
Am 25.04.2017 um 20:20 schrieb Logan Gunthorpe:
> This patch introduces functions which kmap the pages inside an sgl.
> These functions replace a common pattern of kmap(sg_page(sg)) that is
> used in more than 50 places within the kernel.
>
> The motivation for this work is to eventually safely support sgls that
> contain io memory. In order for that to work, any access to the contents
> of an iomem SGL will need to be done with iomemcpy or hit some warning.
> (The exact details of how this will work have yet to be worked out.)
> Having all the kmaps in one place is just a first step in that
> direction. Additionally, seeing this helps cut down the users of sg_page,
> it should make any effort to go to struct-page-less DMAs a little
> easier (should that idea ever swing back into favour again).
>
> A flags option is added to select between a regular or atomic mapping so
> these functions can replace kmap(sg_page or kmap_atomic(sg_page.
> Future work may expand this to have flags for using page_address or
> vmap. We include a flag to require the function not to fail to
> support legacy code that has no easy error path. Much further in the
> future, there may be a flag to allocate memory and copy the data
> from/to iomem.
>
> We also add the semantic that sg_map can fail to create a mapping,
> despite the fact that the current code this is replacing is assumed to
> never fail and the current version of these functions cannot fail. This
> is to support iomem which may either have to fail to create the mapping or
> allocate memory as a bounce buffer which itself can fail.
>
> Also, in terms of cleanup, a few of the existing kmap(sg_page) users
> play things a bit loose in terms of whether they apply sg->offset
> so using these helper functions should help avoid such issues.
>
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> ---
Good to know that somebody is working on this. Those problems troubled
us as well.
Patch is Acked-by: Christian König <christian.koenig@amd.com>.
Regards,
Christian.
> include/linux/scatterlist.h | 85 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 85 insertions(+)
>
> diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
> index cb3c8fe..fad170b 100644
> --- a/include/linux/scatterlist.h
> +++ b/include/linux/scatterlist.h
> @@ -5,6 +5,7 @@
> #include <linux/types.h>
> #include <linux/bug.h>
> #include <linux/mm.h>
> +#include <linux/highmem.h>
> #include <asm/io.h>
>
> struct scatterlist {
> @@ -126,6 +127,90 @@ static inline struct page *sg_page(struct scatterlist *sg)
> return (struct page *)((sg)->page_link & ~0x3);
> }
>
> +#define SG_KMAP (1 << 0) /* create a mapping with kmap */
> +#define SG_KMAP_ATOMIC (1 << 1) /* create a mapping with kmap_atomic */
> +#define SG_MAP_MUST_NOT_FAIL (1 << 2) /* indicate sg_map should not fail */
> +
> +/**
> + * sg_map - kmap a page inside an sgl
> + * @sg: SG entry
> + * @offset: Offset into entry
> + * @flags: Flags for creating the mapping
> + *
> + * Description:
> + * Use this function to map a page in the scatterlist at the specified
> + * offset. sg->offset is already added for you. Note: the semantics of
> + * this function are that it may fail. Thus, its output should be checked
> + * with IS_ERR and PTR_ERR. Otherwise, a pointer to the specified offset
> + * in the mapped page is returned.
> + *
> + * Flags can be any of:
> + * * SG_KMAP - Use kmap to create the mapping
> + * * SG_KMAP_ATOMIC - Use kmap_atomic to map the page atommically.
> + * Thus, the rules of that function apply: the
> + * cpu may not sleep until it is unmaped.
> + * * SG_MAP_MUST_NOT_FAIL - Indicate that sg_map must not fail.
> + * If it does, it will issue a BUG_ON instead.
> + * This is intended for legacy code only, it
> + * is not to be used in new code.
> + *
> + * Also, consider carefully whether this function is appropriate. It is
> + * largely not recommended for new code and if the sgl came from another
> + * subsystem and you don't know what kind of memory might be in the list
> + * then you definitely should not call it. Non-mappable memory may be in
> + * the sgl and thus this function may fail unexpectedly. Consider using
> + * sg_copy_to_buffer instead.
> + **/
> +static inline void *sg_map(struct scatterlist *sg, size_t offset, int flags)
> +{
> + struct page *pg;
> + unsigned int pg_off;
> + void *ret;
> +
> + offset += sg->offset;
> + pg = nth_page(sg_page(sg), offset >> PAGE_SHIFT);
> + pg_off = offset_in_page(offset);
> +
> + if (flags & SG_KMAP_ATOMIC)
> + ret = kmap_atomic(pg) + pg_off;
> + else if (flags & SG_KMAP)
> + ret = kmap(pg) + pg_off;
> + else
> + ret = ERR_PTR(-EINVAL);
> +
> + /*
> + * In theory, this can't happen yet. Once we start adding
> + * unmapable memory, it also shouldn't happen unless developers
> + * start putting unmappable struct pages in sgls and passing
> + * it to code that doesn't support it.
> + */
> + BUG_ON(flags & SG_MAP_MUST_NOT_FAIL && IS_ERR(ret));
> +
> + return ret;
> +}
> +
> +/**
> + * sg_unmap - unmap a page that was mapped with sg_map_offset
> + * @sg: SG entry
> + * @addr: address returned by sg_map_offset
> + * @offset: Offset into entry (same as specified for sg_map)
> + * @flags: Flags, which are the same specified for sg_map
> + *
> + * Description:
> + * Unmap the page that was mapped with sg_map_offset
> + **/
> +static inline void sg_unmap(struct scatterlist *sg, void *addr,
> + size_t offset, int flags)
> +{
> + struct page *pg = nth_page(sg_page(sg), offset >> PAGE_SHIFT);
> + unsigned int pg_off = offset_in_page(offset);
> +
> + if (flags & SG_KMAP_ATOMIC)
> + kunmap_atomic(addr - sg->offset - pg_off);
> + else if (flags & SG_KMAP)
> + kunmap(pg);
> +}
> +
> /**
> * sg_set_buf - Set sg entry to point at given data
> * @sg: SG entry
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply
* [PATCH 1/1] IMSM: Initialize my_vol_raid_dev_num during vol creation
From: Pawel Baldysiak @ 2017-04-26 9:08 UTC (permalink / raw)
To: linux-raid; +Cc: jes.serensen, Pawel Baldysiak
This field was not initialized so far. This ID needs to be unique
for every newly created array in container.
Signed-off-by: Pawel Baldysiak <pawel.baldysiak@intel.com>
---
super-intel.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index 0aed57c..d5c77c0 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -232,8 +232,13 @@ struct imsm_super {
__u32 orig_family_num; /* 0x40 - 0x43 original family num */
__u32 pwr_cycle_count; /* 0x44 - 0x47 simulated power cycle count for array */
__u32 bbm_log_size; /* 0x48 - 0x4B - size of bad Block Mgmt Log in bytes */
-#define IMSM_FILLERS 35
- __u32 filler[IMSM_FILLERS]; /* 0x4C - 0xD7 RAID_MPB_FILLERS */
+ __u16 num_raid_devs_created; /* 0x4C - 0x4D Used for generating unique
+ * volume IDs for raid_dev created in this array
+ * (starts at 1)
+ */
+ __u16 filler1; /* 0x4E - 0x4F */
+#define IMSM_FILLERS 34
+ __u32 filler[IMSM_FILLERS]; /* 0x50 - 0xD7 RAID_MPB_FILLERS */
struct imsm_disk disk[1]; /* 0xD8 diskTbl[numDisks] */
/* here comes imsm_dev[num_raid_devs] */
/* here comes BBM logs */
@@ -5368,6 +5373,8 @@ static int init_super_imsm_volume(struct supertype *st, mdu_array_info_t *info,
set_imsm_ord_tbl_ent(map, i, IMSM_ORD_REBUILD);
}
mpb->num_raid_devs++;
+ mpb->num_raid_devs_created++;
+ dev->my_vol_raid_dev_num = mpb->num_raid_devs_created;
if (s->consistency_policy == UnSet ||
s->consistency_policy == CONSISTENCY_POLICY_RESYNC ||
--
2.9.3
^ permalink raw reply related
* Re: [MD PATCH 1/1] Don't add nr_pending for resync requests
From: Xiao Ni @ 2017-04-26 9:22 UTC (permalink / raw)
To: Coly Li; +Cc: Shaohua Li, linux-raid, ncroxon, neilb
In-Reply-To: <599ae320-3375-8bd6-f1cd-4091ecc0a529@suse.de>
----- Original Message -----
> From: "Coly Li" <colyli@suse.de>
> To: "Shaohua Li" <shli@kernel.org>, "Xiao Ni" <xni@redhat.com>
> Cc: linux-raid@vger.kernel.org, ncroxon@redhat.com, neilb@suse.com
> Sent: Wednesday, April 26, 2017 4:40:20 PM
> Subject: Re: [MD PATCH 1/1] Don't add nr_pending for resync requests
>
> On 2017/4/26 上午1:10, Shaohua Li wrote:
> > Hi,
> > Cc: Coly and Neil.
> >
> > On Mon, Apr 24, 2017 at 04:47:15PM +0800, Xiao Ni wrote:
> >> In newer barrier codes, raise_barrier waits if conf->nr_pending[idx] is
> >> not zero.
> >> After all the conditions are true, the resync request can go on be
> >> handling. But
> >> it adds conf->nr_pending[idx] again. The next resync request hit the same
> >> bucket
> >> idx need to wait the resync request which is submitted before. The
> >> performance
> >> of resync/recovery is degraded.
> >>
> >> I did some tests:
> >> 1. Before removing the codes, the resync performance is:
> >> Device: rrqm/s wrqm/s r/s w/s rMB/s wMB/s avgrq-sz
> >> avgqu-sz await r_await w_await svctm %util
> >> sdc 0.00 0.00 0.00 164.00 0.00 10.13 126.46
> >> 0.95 5.80 0.00 5.80 5.82 95.40
> >> sdb 0.00 0.00 163.00 2.00 10.19 0.00 126.47
> >> 0.04 0.25 0.18 5.50 0.25 4.10
> >> 2. After removing the codes, the resync performance is:
> >> Device: rrqm/s wrqm/s r/s w/s rMB/s wMB/s avgrq-sz
> >> avgqu-sz await r_await w_await svctm %util
> >> sdc 0.00 2164.00 0.00 751.00 0.00 182.06 496.49
> >> 7.49 9.95 0.00 9.95 1.33 100.00
> >> sdb 2162.00 0.00 752.00 0.00 182.25 0.00 496.34
> >> 0.56 0.74 0.74 0.00 0.65 48.80
> >
> > A big resync regression, thanks to check it! Your analysis makes sense to
> > me,
> > but the patch doesn't. freeze_array() depends on nr_pending to wait for all
> > normal IO and resync. This change will make the wait for resync disappear.
> >
> > I think the problem is we are abusing nr_pending here. freeze_array() waits
> > for
> > a condition which doesn't need to be maintained by nr_pending. It makes
> > sense
> > raise_barrier to wait for nr_pending caused by normal IO, but it doesn't
> > make
> > sense to wait for other raise_barrier.
> >
> > We could add another vairable, increase it in raise_barrier, decrease it in
> > lower_barrier and let freeze_array() check the new variable. That should
> > fix
> > the problem and not expose race condition for freeze_array().
>
> Shaohua,
>
> Adding another variable to recode pending resync I/O is good idea. We
> can do something like this,
>
> @@ -869,7 +869,7 @@ static void raise_barrier()
>
> - atomic_inc(&conf->nr_pending[idx]);
> + atomic_inc(&conf->nr_resync_pending[idx]);
> spin_unlock_irq(&conf->resync_lock);
> }
>
> @@ -880,7 +880,7 @@ static void lower_barrier()
>
> atomic_dec(&conf->barrier[idx]);
> - atomic_dec(&conf->nr_pending[idx]);
> + atomic_dec(&conf->nr_resync_pending[idx]);
> wake_up(&conf->wait_barrier);
> }
>
> @@ -1018,7 +1018,8 @@ static int get_unqueued_pending()
> int idx, ret;
>
> for (ret = 0, idx = 0; idx < BARRIER_BUCKETS_NR; idx++)
> - ret += atomic_read(&conf->nr_pending[idx]) -
> + ret += atomic_read(&conf->nr_pending[idx]) +
> + atomic_read(&conf->nr_resync_pending[idx]) -
> atomic_read(&conf->nr_queued[idx]);
>
> return ret;
>
> Xiao,
>
> Nice catch! From your email, it seems resync request on raid1 device can
> be non-linear, otherwise, it won't hit same barrier bucket by different
> resync I/O to raise barrier. This is some thing I don't know. Thank you !
Hi Coly and shaohua
Sorry I don't understand "non-linear raid1 device". What's the meaning about this?
In raid1_sync_request, it can handle RESYNC_PAGES pages. It's 64KB. So there is a
chance that different resync requests hit the same bucket id. Because the bucket unit
is 64MB. Am I right?
>
> The reason to increase conf->nr_pending[idx] is from 'commit 34e97f17
> ("md/raid1: count resync requests in nr_pending.")', because normal I/O
> and resync I/O can happen at same time on same raid1 device. And both of
> them can be queued by reschedule_retry(), raise_barrier() need to
> increase conf->nr_pending[idx] to make sure pending I/Os will be equal
> to queued I/Os when array is frozen. We cannot remove
> conf->nr_pending[idx] operations from raise_barrier()/lower_barrier().
>
> Would you like to compose a patch by Shaohua's suggestion ?
>
Sure, I'll re-send the patch later.
At first I didn't understand the relationship about nr_pending and nr_queued.
If I remove the action adding nr_pending for resync requests. There maybe one
deadlock in freeze_array. Thanks for the explanation.
Regards
Xiao
^ permalink raw reply
* Re: [mdadm PATCH 4/4] Create: tell udev device is not ready when first created.
From: Peter Rajnoha @ 2017-04-26 10:19 UTC (permalink / raw)
To: NeilBrown, Jes Sorensen; +Cc: linux-raid, dm-devel
In-Reply-To: <87mvbasqx6.fsf@notabene.neil.brown.name>
On 04/20/2017 11:35 PM, NeilBrown wrote:
> On Thu, Apr 20 2017, Jes Sorensen wrote:
...
>> Second, isn't this going to be racey if you have multiple arrays
>> running? I am wondering if we cannot find a solution that relies on a
>> permanently installed udev rule that we enable/disable with systemctl
>> and use the device name as an argument?
>
> There would only be a problematic race of an array as being created at
> the same time that another array is being assembled. Is that at all
> likely? They tend to happen at different parts of the usage cycle... I
> guess we shouldn't assume though.
>
> I admit that blocking *all* arrays was a short cut. We need to create
> the udev rule before the new device is first activated, and keep it in
> existence until after the array has been started and the fd on /dev/mdXX
> has been closed - and then a bit more because udev doesn't respond
> instantly.
> In some cases we don't know the name of the md device until
> create_mddev() chooses on with find_free_devnum().
> So if we want to block udev from handling a device, we need to put the
> block into effect (e.g. create the rules.d file) in mddev_create() just
> before the call to open_dev_excl().
>
> If we wanted an more permanent udev rule, we would need to record the
> devices that should be ignored in the filesystem somewhere else.
> Maybe in /run/mdadm.
> e.g.
>
> KERNEL=="md*", TEST="/run/mdadm/creating-$kernel", ENV{SYSTEMD_READY}="0"
>
> Then we could have something like the following (untested) in mdadm.
> Does that seem more suitable?
>
Yes, please, if possible, go for a permanent udev rule surely - this
will make it much easier for other foreign tools to hook in properly if
needed and it would also be much easier to debug.
But, wouldn't it be better if we could just pass this information ("not
initialized yet") as RUN_ARRAY md ioctl parameter? In that case the md
driver in kernel could add the variable to the uevent it generates which
userspace udev rules could check for easily. This way, we don't need to
hassle with creating files in filesystem and the information would be
directly a part of the uevent the md kernel driver generates (so
directly accessible in udev rules too). Also, possibly adding more
variables for other future scenarios if needed.
We use something similar in device-mapper already where we pass flags
(besides other things) as a device-mapper ioctl parameter which then
gets into the uevent as DM_COOKIE uevent variable (which in turn the
udev rules can decode into individual flags). This method proved to be
working well for us to solve the problem of uninitialized data area so
we don't get into trouble with scans from udev (see also dm_ioctl
structure in usr/include/linux/dm-ioctl.h and the dm_kobject_uevent
function in drivers/md/dm.c for the kernel part; in udev rules the flags
are then various DM_UDEV_* variables we check for)
--
Peter
^ permalink raw reply
* [MD PATCH 1/1] Use a new variable to count flighting sync requests
From: Xiao Ni @ 2017-04-26 13:32 UTC (permalink / raw)
To: linux-raid; +Cc: shli, colyli, neilb, ncroxon
In new barrier codes, raise_barrier waits if conf->nr_pending[idx] is not zero.
After all the conditions are true, the resync request can go on be handled. But
it adds conf->nr_pending[idx] again. The next resync request hit the same bucket
idx need to wait the resync request which is submitted before. The performance
of resync/recovery is degraded.
So we should use a new variable to count sync requests which are in flight.
Suggested-by: Shaohua Li <shli@kernel.org>
Suggested-by: Coly Li <colyli@suse.de>
Signed-off-by: Xiao Ni <xni@redhat.com>
---
drivers/md/raid1.c | 14 +++++++++++---
drivers/md/raid1.h | 1 +
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index a34f587..3c304ef 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -869,7 +869,7 @@ static void raise_barrier(struct r1conf *conf, sector_t sector_nr)
atomic_read(&conf->barrier[idx]) < RESYNC_DEPTH,
conf->resync_lock);
- atomic_inc(&conf->nr_pending[idx]);
+ atomic_inc(&conf->nr_sync_pending[idx]);
spin_unlock_irq(&conf->resync_lock);
}
@@ -880,7 +880,7 @@ static void lower_barrier(struct r1conf *conf, sector_t sector_nr)
BUG_ON(atomic_read(&conf->barrier[idx]) <= 0);
atomic_dec(&conf->barrier[idx]);
- atomic_dec(&conf->nr_pending[idx]);
+ atomic_dec(&conf->nr_sync_pending[idx]);
wake_up(&conf->wait_barrier);
}
@@ -1018,7 +1018,8 @@ static int get_unqueued_pending(struct r1conf *conf)
int idx, ret;
for (ret = 0, idx = 0; idx < BARRIER_BUCKETS_NR; idx++)
- ret += atomic_read(&conf->nr_pending[idx]) -
+ ret += atomic_read(&conf->nr_pending[idx]) +
+ atomic_read(&conf->nr_sync_pending[idx]) -
atomic_read(&conf->nr_queued[idx]);
return ret;
@@ -3024,6 +3025,11 @@ static struct r1conf *setup_conf(struct mddev *mddev)
if (!conf->nr_pending)
goto abort;
+ conf->nr_sync_pending = kcalloc(BARRIER_BUCKETS_NR,
+ sizeof(atomic_t), GFP_KERNEL);
+ if (!conf->nr_sync_pending)
+ goto abort;
+
conf->nr_waiting = kcalloc(BARRIER_BUCKETS_NR,
sizeof(atomic_t), GFP_KERNEL);
if (!conf->nr_waiting)
@@ -3136,6 +3142,7 @@ static struct r1conf *setup_conf(struct mddev *mddev)
kfree(conf->mirrors);
safe_put_page(conf->tmppage);
kfree(conf->poolinfo);
+ kfree(conf->nr_sync_pending);
kfree(conf->nr_pending);
kfree(conf->nr_waiting);
kfree(conf->nr_queued);
@@ -3241,6 +3248,7 @@ static void raid1_free(struct mddev *mddev, void *priv)
kfree(conf->mirrors);
safe_put_page(conf->tmppage);
kfree(conf->poolinfo);
+ kfree(conf->nr_sync_pending);
kfree(conf->nr_pending);
kfree(conf->nr_waiting);
kfree(conf->nr_queued);
diff --git a/drivers/md/raid1.h b/drivers/md/raid1.h
index dd22a37..a3580ee 100644
--- a/drivers/md/raid1.h
+++ b/drivers/md/raid1.h
@@ -85,6 +85,7 @@ struct r1conf {
wait_queue_head_t wait_barrier;
spinlock_t resync_lock;
atomic_t *nr_pending;
+ atomic_t *nr_sync_pending;
atomic_t *nr_waiting;
atomic_t *nr_queued;
atomic_t *barrier;
--
2.7.4
^ permalink raw reply related
* Re: [MD PATCH 1/1] Use a new variable to count flighting sync requests
From: Coly Li @ 2017-04-26 13:55 UTC (permalink / raw)
To: Xiao Ni, linux-raid; +Cc: shli, neilb, ncroxon
In-Reply-To: <1493213539-15970-1-git-send-email-xni@redhat.com>
On 2017/4/26 下午9:32, Xiao Ni wrote:
> In new barrier codes, raise_barrier waits if conf->nr_pending[idx] is not zero.
> After all the conditions are true, the resync request can go on be handled. But
> it adds conf->nr_pending[idx] again. The next resync request hit the same bucket
> idx need to wait the resync request which is submitted before. The performance
> of resync/recovery is degraded.
> So we should use a new variable to count sync requests which are in flight.
>
> Suggested-by: Shaohua Li <shli@kernel.org>
> Suggested-by: Coly Li <colyli@suse.de>
> Signed-off-by: Xiao Ni <xni@redhat.com>
Hi Xiao,
The patch looks good to me. But I do have interest to have a look on the
performance number, does this patch help to improve resync throughput ?
Thanks.
Coly Li
> ---
> drivers/md/raid1.c | 14 +++++++++++---
> drivers/md/raid1.h | 1 +
> 2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index a34f587..3c304ef 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -869,7 +869,7 @@ static void raise_barrier(struct r1conf *conf, sector_t sector_nr)
> atomic_read(&conf->barrier[idx]) < RESYNC_DEPTH,
> conf->resync_lock);
>
> - atomic_inc(&conf->nr_pending[idx]);
> + atomic_inc(&conf->nr_sync_pending[idx]);
> spin_unlock_irq(&conf->resync_lock);
> }
>
> @@ -880,7 +880,7 @@ static void lower_barrier(struct r1conf *conf, sector_t sector_nr)
> BUG_ON(atomic_read(&conf->barrier[idx]) <= 0);
>
> atomic_dec(&conf->barrier[idx]);
> - atomic_dec(&conf->nr_pending[idx]);
> + atomic_dec(&conf->nr_sync_pending[idx]);
> wake_up(&conf->wait_barrier);
> }
>
> @@ -1018,7 +1018,8 @@ static int get_unqueued_pending(struct r1conf *conf)
> int idx, ret;
>
> for (ret = 0, idx = 0; idx < BARRIER_BUCKETS_NR; idx++)
> - ret += atomic_read(&conf->nr_pending[idx]) -
> + ret += atomic_read(&conf->nr_pending[idx]) +
> + atomic_read(&conf->nr_sync_pending[idx]) -
> atomic_read(&conf->nr_queued[idx]);
>
> return ret;
> @@ -3024,6 +3025,11 @@ static struct r1conf *setup_conf(struct mddev *mddev)
> if (!conf->nr_pending)
> goto abort;
>
> + conf->nr_sync_pending = kcalloc(BARRIER_BUCKETS_NR,
> + sizeof(atomic_t), GFP_KERNEL);
> + if (!conf->nr_sync_pending)
> + goto abort;
> +
> conf->nr_waiting = kcalloc(BARRIER_BUCKETS_NR,
> sizeof(atomic_t), GFP_KERNEL);
> if (!conf->nr_waiting)
> @@ -3136,6 +3142,7 @@ static struct r1conf *setup_conf(struct mddev *mddev)
> kfree(conf->mirrors);
> safe_put_page(conf->tmppage);
> kfree(conf->poolinfo);
> + kfree(conf->nr_sync_pending);
> kfree(conf->nr_pending);
> kfree(conf->nr_waiting);
> kfree(conf->nr_queued);
> @@ -3241,6 +3248,7 @@ static void raid1_free(struct mddev *mddev, void *priv)
> kfree(conf->mirrors);
> safe_put_page(conf->tmppage);
> kfree(conf->poolinfo);
> + kfree(conf->nr_sync_pending);
> kfree(conf->nr_pending);
> kfree(conf->nr_waiting);
> kfree(conf->nr_queued);
> diff --git a/drivers/md/raid1.h b/drivers/md/raid1.h
> index dd22a37..a3580ee 100644
> --- a/drivers/md/raid1.h
> +++ b/drivers/md/raid1.h
> @@ -85,6 +85,7 @@ struct r1conf {
> wait_queue_head_t wait_barrier;
> spinlock_t resync_lock;
> atomic_t *nr_pending;
> + atomic_t *nr_sync_pending;
> atomic_t *nr_waiting;
> atomic_t *nr_queued;
> atomic_t *barrier;
>
^ permalink raw reply
* Re: [MD PATCH 1/1] Use a new variable to count flighting sync requests
From: Xiao Ni @ 2017-04-26 14:01 UTC (permalink / raw)
To: Coly Li; +Cc: linux-raid, shli, neilb, ncroxon
In-Reply-To: <38fd88c3-4ecd-2778-e6a0-187861faa183@suse.de>
----- Original Message -----
> From: "Coly Li" <colyli@suse.de>
> To: "Xiao Ni" <xni@redhat.com>, linux-raid@vger.kernel.org
> Cc: shli@kernel.org, neilb@suse.com, ncroxon@redhat.com
> Sent: Wednesday, April 26, 2017 9:55:30 PM
> Subject: Re: [MD PATCH 1/1] Use a new variable to count flighting sync requests
>
> On 2017/4/26 下午9:32, Xiao Ni wrote:
> > In new barrier codes, raise_barrier waits if conf->nr_pending[idx] is not
> > zero.
> > After all the conditions are true, the resync request can go on be handled.
> > But
> > it adds conf->nr_pending[idx] again. The next resync request hit the same
> > bucket
> > idx need to wait the resync request which is submitted before. The
> > performance
> > of resync/recovery is degraded.
> > So we should use a new variable to count sync requests which are in flight.
> >
> > Suggested-by: Shaohua Li <shli@kernel.org>
> > Suggested-by: Coly Li <colyli@suse.de>
> > Signed-off-by: Xiao Ni <xni@redhat.com>
>
> Hi Xiao,
>
> The patch looks good to me. But I do have interest to have a look on the
> performance number, does this patch help to improve resync throughput ?
> Thanks.
>
> Coly Li
Hi Coly
The server which I did test is down now. I'll give the test result ASAP.
Regards
Xiao
>
> > ---
> > drivers/md/raid1.c | 14 +++++++++++---
> > drivers/md/raid1.h | 1 +
> > 2 files changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> > index a34f587..3c304ef 100644
> > --- a/drivers/md/raid1.c
> > +++ b/drivers/md/raid1.c
> > @@ -869,7 +869,7 @@ static void raise_barrier(struct r1conf *conf, sector_t
> > sector_nr)
> > atomic_read(&conf->barrier[idx]) < RESYNC_DEPTH,
> > conf->resync_lock);
> >
> > - atomic_inc(&conf->nr_pending[idx]);
> > + atomic_inc(&conf->nr_sync_pending[idx]);
> > spin_unlock_irq(&conf->resync_lock);
> > }
> >
> > @@ -880,7 +880,7 @@ static void lower_barrier(struct r1conf *conf, sector_t
> > sector_nr)
> > BUG_ON(atomic_read(&conf->barrier[idx]) <= 0);
> >
> > atomic_dec(&conf->barrier[idx]);
> > - atomic_dec(&conf->nr_pending[idx]);
> > + atomic_dec(&conf->nr_sync_pending[idx]);
> > wake_up(&conf->wait_barrier);
> > }
> >
> > @@ -1018,7 +1018,8 @@ static int get_unqueued_pending(struct r1conf *conf)
> > int idx, ret;
> >
> > for (ret = 0, idx = 0; idx < BARRIER_BUCKETS_NR; idx++)
> > - ret += atomic_read(&conf->nr_pending[idx]) -
> > + ret += atomic_read(&conf->nr_pending[idx]) +
> > + atomic_read(&conf->nr_sync_pending[idx]) -
> > atomic_read(&conf->nr_queued[idx]);
> >
> > return ret;
> > @@ -3024,6 +3025,11 @@ static struct r1conf *setup_conf(struct mddev
> > *mddev)
> > if (!conf->nr_pending)
> > goto abort;
> >
> > + conf->nr_sync_pending = kcalloc(BARRIER_BUCKETS_NR,
> > + sizeof(atomic_t), GFP_KERNEL);
> > + if (!conf->nr_sync_pending)
> > + goto abort;
> > +
> > conf->nr_waiting = kcalloc(BARRIER_BUCKETS_NR,
> > sizeof(atomic_t), GFP_KERNEL);
> > if (!conf->nr_waiting)
> > @@ -3136,6 +3142,7 @@ static struct r1conf *setup_conf(struct mddev *mddev)
> > kfree(conf->mirrors);
> > safe_put_page(conf->tmppage);
> > kfree(conf->poolinfo);
> > + kfree(conf->nr_sync_pending);
> > kfree(conf->nr_pending);
> > kfree(conf->nr_waiting);
> > kfree(conf->nr_queued);
> > @@ -3241,6 +3248,7 @@ static void raid1_free(struct mddev *mddev, void
> > *priv)
> > kfree(conf->mirrors);
> > safe_put_page(conf->tmppage);
> > kfree(conf->poolinfo);
> > + kfree(conf->nr_sync_pending);
> > kfree(conf->nr_pending);
> > kfree(conf->nr_waiting);
> > kfree(conf->nr_queued);
> > diff --git a/drivers/md/raid1.h b/drivers/md/raid1.h
> > index dd22a37..a3580ee 100644
> > --- a/drivers/md/raid1.h
> > +++ b/drivers/md/raid1.h
> > @@ -85,6 +85,7 @@ struct r1conf {
> > wait_queue_head_t wait_barrier;
> > spinlock_t resync_lock;
> > atomic_t *nr_pending;
> > + atomic_t *nr_sync_pending;
> > atomic_t *nr_waiting;
> > atomic_t *nr_queued;
> > atomic_t *barrier;
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [MD PATCH 1/1] Use a new variable to count flighting sync requests
From: Shaohua Li @ 2017-04-26 15:08 UTC (permalink / raw)
To: Xiao Ni; +Cc: linux-raid, colyli, neilb, ncroxon
In-Reply-To: <1493213539-15970-1-git-send-email-xni@redhat.com>
On Wed, Apr 26, 2017 at 09:32:19PM +0800, Xiao Ni wrote:
> In new barrier codes, raise_barrier waits if conf->nr_pending[idx] is not zero.
> After all the conditions are true, the resync request can go on be handled. But
> it adds conf->nr_pending[idx] again. The next resync request hit the same bucket
> idx need to wait the resync request which is submitted before. The performance
> of resync/recovery is degraded.
> So we should use a new variable to count sync requests which are in flight.
>
> Suggested-by: Shaohua Li <shli@kernel.org>
> Suggested-by: Coly Li <colyli@suse.de>
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---
> drivers/md/raid1.c | 14 +++++++++++---
> drivers/md/raid1.h | 1 +
> 2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index a34f587..3c304ef 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -869,7 +869,7 @@ static void raise_barrier(struct r1conf *conf, sector_t sector_nr)
> atomic_read(&conf->barrier[idx]) < RESYNC_DEPTH,
> conf->resync_lock);
>
> - atomic_inc(&conf->nr_pending[idx]);
> + atomic_inc(&conf->nr_sync_pending[idx]);
Any reason why nr_sync_pending is an array? Looks a single atomic is enough to me.
Thanks,
Shaohua
> spin_unlock_irq(&conf->resync_lock);
> }
>
> @@ -880,7 +880,7 @@ static void lower_barrier(struct r1conf *conf, sector_t sector_nr)
> BUG_ON(atomic_read(&conf->barrier[idx]) <= 0);
>
> atomic_dec(&conf->barrier[idx]);
> - atomic_dec(&conf->nr_pending[idx]);
> + atomic_dec(&conf->nr_sync_pending[idx]);
> wake_up(&conf->wait_barrier);
> }
>
> @@ -1018,7 +1018,8 @@ static int get_unqueued_pending(struct r1conf *conf)
> int idx, ret;
>
> for (ret = 0, idx = 0; idx < BARRIER_BUCKETS_NR; idx++)
> - ret += atomic_read(&conf->nr_pending[idx]) -
> + ret += atomic_read(&conf->nr_pending[idx]) +
> + atomic_read(&conf->nr_sync_pending[idx]) -
> atomic_read(&conf->nr_queued[idx]);
>
> return ret;
> @@ -3024,6 +3025,11 @@ static struct r1conf *setup_conf(struct mddev *mddev)
> if (!conf->nr_pending)
> goto abort;
>
> + conf->nr_sync_pending = kcalloc(BARRIER_BUCKETS_NR,
> + sizeof(atomic_t), GFP_KERNEL);
> + if (!conf->nr_sync_pending)
> + goto abort;
> +
> conf->nr_waiting = kcalloc(BARRIER_BUCKETS_NR,
> sizeof(atomic_t), GFP_KERNEL);
> if (!conf->nr_waiting)
> @@ -3136,6 +3142,7 @@ static struct r1conf *setup_conf(struct mddev *mddev)
> kfree(conf->mirrors);
> safe_put_page(conf->tmppage);
> kfree(conf->poolinfo);
> + kfree(conf->nr_sync_pending);
> kfree(conf->nr_pending);
> kfree(conf->nr_waiting);
> kfree(conf->nr_queued);
> @@ -3241,6 +3248,7 @@ static void raid1_free(struct mddev *mddev, void *priv)
> kfree(conf->mirrors);
> safe_put_page(conf->tmppage);
> kfree(conf->poolinfo);
> + kfree(conf->nr_sync_pending);
> kfree(conf->nr_pending);
> kfree(conf->nr_waiting);
> kfree(conf->nr_queued);
> diff --git a/drivers/md/raid1.h b/drivers/md/raid1.h
> index dd22a37..a3580ee 100644
> --- a/drivers/md/raid1.h
> +++ b/drivers/md/raid1.h
> @@ -85,6 +85,7 @@ struct r1conf {
> wait_queue_head_t wait_barrier;
> spinlock_t resync_lock;
> atomic_t *nr_pending;
> + atomic_t *nr_sync_pending;
> atomic_t *nr_waiting;
> atomic_t *nr_queued;
> atomic_t *barrier;
> --
> 2.7.4
>
^ permalink raw reply
* Re: [MD PATCH 1/1] Use a new variable to count flighting sync requests
From: Coly Li @ 2017-04-26 15:38 UTC (permalink / raw)
To: Shaohua Li, Xiao Ni; +Cc: linux-raid, neilb, ncroxon
In-Reply-To: <20170426150850.lxe2ktt4uhljcp5y@kernel.org>
On 2017/4/26 下午11:08, Shaohua Li wrote:
> On Wed, Apr 26, 2017 at 09:32:19PM +0800, Xiao Ni wrote:
>> In new barrier codes, raise_barrier waits if conf->nr_pending[idx] is not zero.
>> After all the conditions are true, the resync request can go on be handled. But
>> it adds conf->nr_pending[idx] again. The next resync request hit the same bucket
>> idx need to wait the resync request which is submitted before. The performance
>> of resync/recovery is degraded.
>> So we should use a new variable to count sync requests which are in flight.
>>
>> Suggested-by: Shaohua Li <shli@kernel.org>
>> Suggested-by: Coly Li <colyli@suse.de>
>> Signed-off-by: Xiao Ni <xni@redhat.com>
>> ---
>> drivers/md/raid1.c | 14 +++++++++++---
>> drivers/md/raid1.h | 1 +
>> 2 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>> index a34f587..3c304ef 100644
>> --- a/drivers/md/raid1.c
>> +++ b/drivers/md/raid1.c
>> @@ -869,7 +869,7 @@ static void raise_barrier(struct r1conf *conf, sector_t sector_nr)
>> atomic_read(&conf->barrier[idx]) < RESYNC_DEPTH,
>> conf->resync_lock);
>>
>> - atomic_inc(&conf->nr_pending[idx]);
>> + atomic_inc(&conf->nr_sync_pending[idx]);
>
> Any reason why nr_sync_pending is an array? Looks a single atomic is enough to me.
>
Yes, you are right. A single atomic works fine :-)
Coly
^ permalink raw reply
* Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions
From: Logan Gunthorpe @ 2017-04-26 18:11 UTC (permalink / raw)
To: Christoph Hellwig
Cc: dri-devel, Stephen Bates, dm-devel, target-devel, Sumit Semwal,
devel, James E.J. Bottomley, linux-scsi, linux-nvdimm, linux-rdma,
Ross Zwisler, open-iscsi, linux-media, intel-gfx, sparmaintainer,
linux-raid, Dan Williams, megaraidlinux.pdl, Jens Axboe,
Martin K. Petersen, netdev, Matthew Wilcox, linux-mmc,
linux-kernel, linux-crypto, Greg Kroah-Hartman
In-Reply-To: <20170426074416.GA7936@lst.de>
On 26/04/17 01:44 AM, Christoph Hellwig wrote:
> I think we'll at least need a draft of those to make sense of these
> patches. Otherwise they just look very clumsy.
Ok, I'll work up a draft proposal and send it in a couple days. But
without a lot of cleanup such as this series it's not going to even be
able to compile.
> I'm sorry but this API is just a trainwreck. Right now we have the
> nice little kmap_atomic API, which never fails and has a very nice
> calling convention where we just pass back the return address, but does
> not support sleeping inside the critical section.
>
> And kmap, whіch may fail and requires the original page to be passed
> back. Anything that mixes these two concepts up is simply a non-starter.
Ok, well for starters I think you are mistaken about kmap being able to
fail. I'm having a hard time finding many users of that function that
bother to check for an error when calling it. The main difficulty we
have now is that neither of those functions are expected to fail and we
need them to be able to in cases where the page doesn't map to system
RAM. This patch series is trying to address it for users of scatterlist.
I'm certainly open to other suggestions.
I also have to disagree that kmap and kmap_atomic are all that "nice".
Except for the sleeping restriction and performance, they effectively do
the same thing. And it was necessary to write a macro wrapper around
kunmap_atomic to ensure that users of that function don't screw it up.
(See 597781f3e5.) I'd say the kmap/kmap_atomic functions are the
trainwreck and I'm trying to do my best to cleanup a few cases.
There are a fair number of cases in the kernel that do something like:
if (something)
x = kmap(page);
else
x = kmap_atomic(page);
...
if (something)
kunmap(page)
else
kunmap_atomic(x)
Which just seems cumbersome to me.
In any case, if you can accept an sg_kmap and sg_kmap_atomic api just
say so and I'll make the change. But I'll still need a flags variable
for SG_MAP_MUST_NOT_FAIL to support legacy cases that have no fail path
and both of those functions will need to be pretty nearly replicas of
each other.
Logan
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply
* [bug report] md-cluster: Fix adding of new disk with new reload code
From: Dan Carpenter @ 2017-04-26 19:59 UTC (permalink / raw)
To: rgoldwyn; +Cc: linux-raid
Hello Goldwyn Rodrigues,
The patch dbb64f8635f5: "md-cluster: Fix adding of new disk with new
reload code" from Oct 1, 2015, leads to the following static checker
warning:
drivers/md/md-cluster.c:1341 add_new_disk()
warn: inconsistent returns 'cinfo->recv_mutex'.
Locked on : 1315,1341
Unlocked on: 1341
drivers/md/md-cluster.c
1300 static int add_new_disk(struct mddev *mddev, struct md_rdev *rdev)
1301 {
1302 struct md_cluster_info *cinfo = mddev->cluster_info;
1303 struct cluster_msg cmsg;
1304 int ret = 0;
1305 struct mdp_superblock_1 *sb = page_address(rdev->sb_page);
1306 char *uuid = sb->device_uuid;
1307
1308 memset(&cmsg, 0, sizeof(cmsg));
1309 cmsg.type = cpu_to_le32(NEWDISK);
1310 memcpy(cmsg.uuid, uuid, 16);
1311 cmsg.raid_slot = cpu_to_le32(rdev->desc_nr);
1312 lock_comm(cinfo, 1);
^^^^^^^^^^^^^^^^^^^
We take the lock here.
1313 ret = __sendmsg(cinfo, &cmsg);
1314 if (ret)
1315 return ret;
Should we unlock on failure here?
1316 cinfo->no_new_dev_lockres->flags |= DLM_LKF_NOQUEUE;
1317 ret = dlm_lock_sync(cinfo->no_new_dev_lockres, DLM_LOCK_EX);
1318 cinfo->no_new_dev_lockres->flags &= ~DLM_LKF_NOQUEUE;
1319 /* Some node does not "see" the device */
1320 if (ret == -EAGAIN)
1321 ret = -ENOENT;
1322 if (ret)
1323 unlock_comm(cinfo);
Because we do here. I think we're only supposed to hold the lock on
success but how this all works with cancel etc looked slightly
complicated so I decided to ask instead of sending a patch.
1324 else {
1325 dlm_lock_sync(cinfo->no_new_dev_lockres, DLM_LOCK_CR);
1326 /* Since MD_CHANGE_DEVS will be set in add_bound_rdev which
1327 * will run soon after add_new_disk, the below path will be
1328 * invoked:
1329 * md_wakeup_thread(mddev->thread)
1330 * -> conf->thread (raid1d)
1331 * -> md_check_recovery -> md_update_sb
1332 * -> metadata_update_start/finish
1333 * MD_CLUSTER_SEND_LOCKED_ALREADY will be cleared eventually.
1334 *
1335 * For other failure cases, metadata_update_cancel and
1336 * add_new_disk_cancel also clear below bit as well.
1337 * */
1338 set_bit(MD_CLUSTER_SEND_LOCKED_ALREADY, &cinfo->state);
1339 wake_up(&cinfo->wait);
1340 }
1341 return ret;
1342 }
regards,
dan carpenter
^ permalink raw reply
* Loan Offers
From: Financial Services @ 2017-04-26 21:07 UTC (permalink / raw)
To: Recipients
Do you need a loan to pay up bill or to start a business? Email Us
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox