* Re: RAID10 performance with 20 drives
From: David Brown @ 2017-05-31 13:32 UTC (permalink / raw)
To: Adam Goryachev, CoolCold, Linux RAID
In-Reply-To: <da03d962-2baf-66d9-f037-23de38c1ff50@websitemanagers.com.au>
On 31/05/17 15:14, Adam Goryachev wrote:
>
>
> On 31/5/17 22:20, CoolCold wrote:
>> top stat:
>> top - 12:09:03 up 4:55, 2 users, load average: 3.33, 3.18, 2.88
>> Tasks: 487 total, 4 running, 483 sleeping, 0 stopped, 0 zombie
>> %Cpu(s): 0.0 us, 4.5 sy, 0.0 ni, 95.3 id, 0.0 wa, 0.0 hi, 0.1
>> si, 0.0 st
>> KiB Mem : 13174918+total, 13005539+free, 1191212 used, 502584
>> buff/cache
>> KiB Swap: 9764860 total, 9764860 free, 0 used. 13020440+avail
>> Mem
>>
>> PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+
>> COMMAND
>> 22275 root 20 0 0 0 0 R 99.0 0.0 7:01.01
>> md1_raid10
>>
>> this cpu usage 99-100% is constant.
>>
> Sorry, but doesn't that say 95.3% idle?
>
> Do you have a multi core CPU? Is it multi threaded? What type of CPU is it?
>
> When running top, press 1, it will then show you each individual core
> and the stats for it.
>
> You might find that creating 10 RAID1 devices, and then using linear
> raid to join them together will perform better, from hearsay and memory,
> this will allow you to use a CPU for each RAID1, and another CPU for the
> linear, so if you had 11 CPU's (or more) then this should get you the
> best possible outcome (from a CPU point of view). In fact, if you have
> more than one CPU it would help.
>
For some workloads, a linear concat (which takes no cpu work) of raid1
pairs will be much faster than a raid0 stripe of raid1 pairs. Maybe I
am lacking the imagination, but I can't see a use-case where a 20 disk
raid10 setup is going to be the most efficient. Raid 0 is good for
striped performance, but you would need /massive/ single file streamed
reads or writes to cover the loses of latencies on all these disks. And
what would you do with all that data? You would quickly saturate 10 Gb
network links - there is no point in trying to get data on or off disks
much faster than you can use it.
> Also, you might want to run a newer kernel, I think there was a lot of
> work done on the resync parts to optimise that. You might also prefer to
> focus on performance measurements *after* the resync has completed,
> since that would be your "normal" status. Though in addition, you should
> test performance with one lost disk, and while replacing that disk to
> ensure that you are still able to sustain the required load during those
> events.
>
> Regards,
> Adam
^ permalink raw reply
* Re: RAID10 performance with 20 drives
From: Adam Goryachev @ 2017-05-31 13:14 UTC (permalink / raw)
To: CoolCold, Linux RAID
In-Reply-To: <CAGqmV7p4_r+ca+Wq_UXcY4RDa7WwE2nWkgXF+Sn3ChwLEmkurQ@mail.gmail.com>
On 31/5/17 22:20, CoolCold wrote:
> top stat:
> top - 12:09:03 up 4:55, 2 users, load average: 3.33, 3.18, 2.88
> Tasks: 487 total, 4 running, 483 sleeping, 0 stopped, 0 zombie
> %Cpu(s): 0.0 us, 4.5 sy, 0.0 ni, 95.3 id, 0.0 wa, 0.0 hi, 0.1 si, 0.0 st
> KiB Mem : 13174918+total, 13005539+free, 1191212 used, 502584 buff/cache
> KiB Swap: 9764860 total, 9764860 free, 0 used. 13020440+avail Mem
>
> PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND
> 22275 root 20 0 0 0 0 R 99.0 0.0 7:01.01 md1_raid10
>
> this cpu usage 99-100% is constant.
>
Sorry, but doesn't that say 95.3% idle?
Do you have a multi core CPU? Is it multi threaded? What type of CPU is it?
When running top, press 1, it will then show you each individual core
and the stats for it.
You might find that creating 10 RAID1 devices, and then using linear
raid to join them together will perform better, from hearsay and memory,
this will allow you to use a CPU for each RAID1, and another CPU for the
linear, so if you had 11 CPU's (or more) then this should get you the
best possible outcome (from a CPU point of view). In fact, if you have
more than one CPU it would help.
Also, you might want to run a newer kernel, I think there was a lot of
work done on the resync parts to optimise that. You might also prefer to
focus on performance measurements *after* the resync has completed,
since that would be your "normal" status. Though in addition, you should
test performance with one lost disk, and while replacing that disk to
ensure that you are still able to sustain the required load during those
events.
Regards,
Adam
^ permalink raw reply
* Re: RAID10 performance with 20 drives
From: Joe Landman @ 2017-05-31 12:36 UTC (permalink / raw)
To: Linux RAID
In-Reply-To: <CAGqmV7p4_r+ca+Wq_UXcY4RDa7WwE2nWkgXF+Sn3ChwLEmkurQ@mail.gmail.com>
On 05/31/2017 08:20 AM, CoolCold wrote:
> Hello!
> Got a new box, for image storage, playing around, created raid10 array
> with 20 1.8TB SATA drives, and found that we hit the cpu limit,
> details below.
>
[...]
> /proc/mdstat output:
> [root@spare-a17484327407661 rovchinnikov]# cat /proc/mdstat
> Personalities : [raid1] [raid10] [raid6] [raid5] [raid4]
> md1 : active raid10 sdx[19] sdw[18] sdv[17] sdu[16] sdt[15] sds[14]
> sdr[13] sdq[12] sdp[11] sdo[10] sdn[9] sdm[8] sdl[7] sdk[6] sdj[5]
> sdi[4] sdh[3] sdg[2] sdf[1] sde[0]
> 17580330880 blocks super 1.2 64K chunks 2 near-copies [20/20]
> [UUUUUUUUUUUUUUUUUUUU]
> [=>...................] resync = 6.4% (1133170368/17580330880)
> finish=192.6min speed=1423140K/sec
Note: you are syncing the drives at 1.4 GB/s.
[...]
> [root@spare-a17484327407661 rovchinnikov]# cat /proc/version
> Linux version 3.10.0-327.el7.x86_64 (builder@kbuilder.dev.centos.org)
> (gcc version 4.8.3 20140911 (Red Hat 4.8.3-9) (GCC) ) #1 SMP Thu Nov
> 19 22:10:57 UTC 2015
And you have an ancient kernel.
>
> So, the question is - why cpu usage is so high and I suppose is a limit here?
>
Without seeing 'vmstat 1' or 'dstat' output, I think all that is
possible is a guess.
If you have 20 drives, all connected over a single HBA going into an
expander, it is possible that this is one of the rate limiting factors
(and its around the same speed limit I've seen in other contexts for
expander based systems). Unfortunately, without more info, this is
going to be pure speculation.
1.4GB/s / 20 drives -> 70 MB/s. Without knowing what make/model drives
you have there, it would be hard to speculate what fraction of the
actual bandwidth you are getting. Most modern (e.g. new) drives can do
between 150-220 MB/s, so you could be anywhere from 33% to 50% of bandwidth.
Your HBA ... this matters tremendously to performance. Not all HBAs are
equivalent, and some are not very good at all. Which make and model,
how is it connected to the drives (direct or via expander), firmware
revs, etc. Since your kernel is ancient, chances are your HBA driver
is as well, so ...
Closely related are how many context switches and interrupts per second
you are seeing (hence the vmstat question). Also quite related is how
the irqs are being distributed for the HBA, or, as I've found many
times, "if" they are being distributed.
Also, something I've found quite often has to do with how the PCIe
devices actually negotiate their speeds. This has bitten me more many
times ... and I've written a tool to help answer that question:
https://github.com/joelandman/pcilist
Then there are questions on the disk config, such as "is the write cache
enabled", "is the read cache disabled".
sdparm | grep WCE
sdparm | grep RCD
And then the SD subsystem (read-ahead, ncq, etc.)
Basically you need to report far more information for anyone to give you
anything more than pure speculation.
--
Joe Landman
e: joe.landman@gmail.com
t: @hpcjoe
w: https://scalability.org
g: https://github.com/joelandman
^ permalink raw reply
* RAID10 performance with 20 drives
From: CoolCold @ 2017-05-31 12:20 UTC (permalink / raw)
To: Linux RAID
Hello!
Got a new box, for image storage, playing around, created raid10 array
with 20 1.8TB SATA drives, and found that we hit the cpu limit,
details below.
Creation (disable write intent bitmap, with bitmap all is much worse):
mdadm --create -c 64 -b none -n 20 -l 10 /dev/md1 /dev/sde /dev/sdf
/dev/sdg /dev/sdh /dev/sdi /dev/sdj /dev/sdk /dev/sdl /dev/sdm
/dev/sdn /dev/sdo /dev/sdp /dev/sdq /dev/sdr /dev/sds /dev/sdt
/dev/sdu /dev/sdv /dev/sdw /dev/sdx
/proc/mdstat output:
[root@spare-a17484327407661 rovchinnikov]# cat /proc/mdstat
Personalities : [raid1] [raid10] [raid6] [raid5] [raid4]
md1 : active raid10 sdx[19] sdw[18] sdv[17] sdu[16] sdt[15] sds[14]
sdr[13] sdq[12] sdp[11] sdo[10] sdn[9] sdm[8] sdl[7] sdk[6] sdj[5]
sdi[4] sdh[3] sdg[2] sdf[1] sde[0]
17580330880 blocks super 1.2 64K chunks 2 near-copies [20/20]
[UUUUUUUUUUUUUUUUUUUU]
[=>...................] resync = 6.4% (1133170368/17580330880)
finish=192.6min speed=1423140K/sec
top stat:
top - 12:09:03 up 4:55, 2 users, load average: 3.33, 3.18, 2.88
Tasks: 487 total, 4 running, 483 sleeping, 0 stopped, 0 zombie
%Cpu(s): 0.0 us, 4.5 sy, 0.0 ni, 95.3 id, 0.0 wa, 0.0 hi, 0.1 si, 0.0 st
KiB Mem : 13174918+total, 13005539+free, 1191212 used, 502584 buff/cache
KiB Swap: 9764860 total, 9764860 free, 0 used. 13020440+avail Mem
PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND
22275 root 20 0 0 0 0 R 99.0 0.0 7:01.01 md1_raid10
this cpu usage 99-100% is constant.
iostat shows drives have much more potential:
Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s
avgrq-sz avgqu-sz await r_await w_await svctm %util
sdi 0.00 0.00 2038.40 0.00 130457.60 0.00
128.00 0.48 0.24 0.24 0.00 0.13 27.00
sdf 0.00 0.00 2038.40 0.00 130457.60 0.00
128.00 0.38 0.19 0.19 0.00 0.12 24.76
sds 0.00 0.00 2038.40 0.00 130457.60 0.00
128.00 0.26 0.13 0.13 0.00 0.13 25.76
sde 0.00 0.00 2038.40 0.00 130457.60 0.00
128.00 0.35 0.17 0.17 0.00 0.12 24.08
sdh 0.00 0.00 2038.40 0.00 130457.60 0.00
128.00 0.31 0.15 0.15 0.00 0.14 29.32
sdj 0.00 0.00 2038.40 0.00 130457.60 0.00
128.00 0.35 0.17 0.17 0.00 0.13 26.46
sdt 0.00 0.00 2038.40 0.00 130457.60 0.00
128.00 0.31 0.15 0.15 0.00 0.13 26.36
sdn 0.00 0.00 2038.40 0.00 130457.60 0.00
128.00 0.56 0.28 0.28 0.00 0.14 28.40
sdq 0.00 0.00 2038.40 0.00 130457.60 0.00
128.00 0.60 0.29 0.29 0.00 0.13 26.28
sdl 0.00 0.00 2038.40 0.00 130457.60 0.00
128.00 0.43 0.21 0.21 0.00 0.15 29.94
sdm 0.00 0.00 2038.40 0.00 130457.60 0.00
128.00 0.31 0.15 0.15 0.00 0.13 26.02
sdk 0.00 0.00 2038.40 0.00 130457.60 0.00
128.00 0.33 0.16 0.16 0.00 0.14 28.74
sdv 0.00 0.00 2038.40 0.00 130457.60 0.00
128.00 0.38 0.19 0.19 0.00 0.11 23.32
sdo 0.00 0.00 2038.40 0.00 130457.60 0.00
128.00 0.48 0.24 0.24 0.00 0.15 29.80
sdr 0.00 0.00 2038.40 0.00 130457.60 0.00
128.00 0.34 0.16 0.16 0.00 0.12 25.10
sdg 0.00 0.00 2038.60 0.00 130470.40 0.00
128.00 0.56 0.27 0.27 0.00 0.15 30.66
sdu 0.00 0.00 2038.40 0.00 130457.60 0.00
128.00 0.27 0.13 0.13 0.00 0.11 22.02
sdp 0.00 0.00 2038.40 0.00 130457.60 0.00
128.00 0.32 0.16 0.16 0.00 0.14 28.70
sdw 0.00 0.00 2038.40 0.00 130457.60 0.00
128.00 0.30 0.15 0.15 0.00 0.12 25.38
sdx 0.00 0.00 2038.40 0.00 130457.60 0.00
128.00 0.40 0.20 0.20 0.00 0.13 26.48
kernel:
[root@spare-a17484327407661 rovchinnikov]# cat /proc/version
Linux version 3.10.0-327.el7.x86_64 (builder@kbuilder.dev.centos.org)
(gcc version 4.8.3 20140911 (Red Hat 4.8.3-9) (GCC) ) #1 SMP Thu Nov
19 22:10:57 UTC 2015
So, the question is - why cpu usage is so high and I suppose is a limit here?
--
Best regards,
[COOLCOLD-RIPN]
^ permalink raw reply
* [PATCH] md_u: Remove unused ioctl declaration of START_ARRAY
From: Zhilong Liu @ 2017-05-31 11:38 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: linux-raid, Zhilong Liu
START_ARRAY is no longer used in the code, so get rid of it.
MD commit: fbedac04fa11 ("[PATCH] md: the scheduled removal
of the START_ARRAY ioctl for md") merged in the year 2006.
Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
md_u.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/md_u.h b/md_u.h
index d59aa2d..2d66d52 100644
--- a/md_u.h
+++ b/md_u.h
@@ -34,7 +34,6 @@
/* usage */
#define RUN_ARRAY _IOW (MD_MAJOR, 0x30, mdu_param_t)
-#define START_ARRAY _IO (MD_MAJOR, 0x31)
#define STOP_ARRAY _IO (MD_MAJOR, 0x32)
#define STOP_ARRAY_RO _IO (MD_MAJOR, 0x33)
#define RESTART_ARRAY_RW _IO (MD_MAJOR, 0x34)
--
2.6.6
^ permalink raw reply related
* [PATCH v2] Get failed disk count from array state
From: Tomasz Majchrzak @ 2017-05-31 10:46 UTC (permalink / raw)
To: linux-raid; +Cc: jes.sorensen, Tomasz Majchrzak
In-Reply-To: <bc4470f4-20bf-c45a-9d96-385e4b46e1e3@gmail.com>
Recent commit has changed the way failed disks are counted. It breaks
recovery for external metadata arrays as failed disks are not part of
the array and have no corresponding entries is sysfs (they are only
reported for containers) so degraded arrays show no failed disks.
Recent commit overwrites GET_DEGRADED result prior to GET_STATE and it
is not set again if GET_STATE has not been requested. As GET_STATE
provides the same information as GET_DEGRADED, the latter is not needed
anymore. Remove GET_DEGRADED option and replace it with GET_STATE
option.
Don't count number of failed disks looking at sysfs entries but
calculate it at the end. Do it only for arrays as containers report
no disks, just spares.
Signed-off-by: Tomasz Majchrzak <tomasz.majchrzak@intel.com>
---
Incremental.c | 14 ++++----------
Monitor.c | 4 ++--
managemon.c | 4 ++--
mdadm.h | 1 -
raid6check.c | 2 +-
sysfs.c | 18 ++++++++----------
6 files changed, 17 insertions(+), 26 deletions(-)
v2:
adapted to apply to latest upstream version
diff --git a/Incremental.c b/Incremental.c
index 30dc7a2..6cf2174 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -886,16 +886,10 @@ static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
}
sra = sysfs_read(-1, mp->devnm,
GET_DEVS|GET_OFFSET|GET_SIZE|GET_STATE|
- GET_DEGRADED|GET_COMPONENT|GET_VERSION);
- if (!sra) {
- /* Probably a container - no degraded info */
- sra = sysfs_read(-1, mp->devnm,
- GET_DEVS|GET_OFFSET|GET_SIZE|GET_STATE|
- GET_COMPONENT|GET_VERSION);
- if (sra)
- sra->array.failed_disks = -1;
- }
- if (!sra)
+ GET_COMPONENT|GET_VERSION);
+ if (sra)
+ sra->array.failed_disks = -1;
+ else
continue;
if (st == NULL) {
int i;
diff --git a/Monitor.c b/Monitor.c
index 725f47d..bef2f1b 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -485,8 +485,8 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
if (st->devnm[0] == 0)
strcpy(st->devnm, fd2devnm(fd));
- sra = sysfs_read(-1, st->devnm, GET_LEVEL | GET_DISKS | GET_DEGRADED |
- GET_MISMATCH | GET_DEVS | GET_STATE);
+ sra = sysfs_read(-1, st->devnm, GET_LEVEL | GET_DISKS | GET_MISMATCH |
+ GET_DEVS | GET_STATE);
if (!sra)
goto disappeared;
diff --git a/managemon.c b/managemon.c
index a8df666..68f0c2d 100644
--- a/managemon.c
+++ b/managemon.c
@@ -685,8 +685,8 @@ static void manage_new(struct mdstat_ent *mdstat,
mdi = sysfs_read(-1, mdstat->devnm,
GET_LEVEL|GET_CHUNK|GET_DISKS|GET_COMPONENT|
- GET_DEGRADED|GET_SAFEMODE|
- GET_DEVS|GET_OFFSET|GET_SIZE|GET_STATE|GET_LAYOUT);
+ GET_SAFEMODE|GET_DEVS|GET_OFFSET|GET_SIZE|GET_STATE|
+ GET_LAYOUT);
if (!mdi)
return;
diff --git a/mdadm.h b/mdadm.h
index ec0a39e..ee9b837 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -637,7 +637,6 @@ enum sysfs_read_flags {
GET_MISMATCH = (1 << 5),
GET_VERSION = (1 << 6),
GET_DISKS = (1 << 7),
- GET_DEGRADED = (1 << 8),
GET_SAFEMODE = (1 << 9),
GET_BITMAP_LOCATION = (1 << 10),
diff --git a/raid6check.c b/raid6check.c
index 551f835..a8e6005 100644
--- a/raid6check.c
+++ b/raid6check.c
@@ -562,7 +562,7 @@ int main(int argc, char *argv[])
GET_LEVEL|
GET_LAYOUT|
GET_DISKS|
- GET_DEGRADED |
+ GET_STATE |
GET_COMPONENT|
GET_CHUNK|
GET_DEVS|
diff --git a/sysfs.c b/sysfs.c
index e47f5e4..78d2b52 100644
--- a/sysfs.c
+++ b/sysfs.c
@@ -162,18 +162,12 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
goto abort;
sra->array.layout = strtoul(buf, NULL, 0);
}
- if (options & GET_DISKS) {
+ if (options & (GET_DISKS|GET_STATE)) {
strcpy(base, "raid_disks");
if (load_sys(fname, buf, sizeof(buf)))
goto abort;
sra->array.raid_disks = strtoul(buf, NULL, 0);
}
- if (options & GET_DEGRADED) {
- strcpy(base, "degraded");
- if (load_sys(fname, buf, sizeof(buf)))
- goto abort;
- sra->array.failed_disks = strtoul(buf, NULL, 0);
- }
if (options & GET_COMPONENT) {
strcpy(base, "component_size");
if (load_sys(fname, buf, sizeof(buf)))
@@ -359,10 +353,9 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
strcpy(dbase, "state");
if (load_sys(fname, buf, sizeof(buf)))
goto abort;
- if (strstr(buf, "faulty")) {
+ if (strstr(buf, "faulty"))
dev->disk.state |= (1<<MD_DISK_FAULTY);
- sra->array.failed_disks++;
- } else {
+ else {
sra->array.working_disks++;
if (strstr(buf, "in_sync")) {
dev->disk.state |= (1<<MD_DISK_SYNC);
@@ -379,6 +372,11 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
dev->errors = strtoul(buf, NULL, 0);
}
}
+
+ if ((options & GET_STATE) && sra->array.raid_disks)
+ sra->array.failed_disks = sra->array.raid_disks -
+ sra->array.active_disks - sra->array.spare_disks;
+
closedir(dir);
return sra;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 01/22] Revert "afs: Move UUID struct to linux/uuid.h"
From: David Howells @ 2017-05-31 10:33 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA,
linux-raid-u79uwXL29TY76Z2rM5mHXA, Arnd Bergmann,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Mimi Zohar, Amir Goldstein,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
dhowells-H+wXaHxf7aLQT0dZR+AlfA,
linux-block-u79uwXL29TY76Z2rM5mHXA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, Andy Shevchenko, Shaohua Li,
Steven Whitehouse
In-Reply-To: <20170531094157.GA10511-jcswGhMUV9g@public.gmane.org>
Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org> wrote:
> Does the afs protocol require a v1 uuid or does it just use the formwat on
> the wire?
The format it uses on the wire is separate XDR encoding of each of the fields
in the uuid_v1 struct, counting the elements of the node[] array separately.
I'm not sure that the servers actually parse the contents of the UUID, but I
can't be sure as I don't have access to the source for all of them.
OpenAFS and AuriStor will at least accept the version field being 4, however.
David
^ permalink raw reply
* Re: [mdadm PATCH] Create: move STOP_ARRAY to abort_locked
From: Zhilong Liu @ 2017-05-31 10:25 UTC (permalink / raw)
To: Jes Sorensen; +Cc: linux-raid
In-Reply-To: <8275d868-ad51-209b-e46f-9d7f1c60d1a3@gmail.com>
On 05/09/2017 01:54 AM, Jes Sorensen wrote:
> On 05/07/2017 09:50 PM, Zhilong Liu wrote:
>>
>>
>> On 05/05/2017 11:31 AM, Liu Zhilong wrote:
>>>
>>>
>>> On 05/04/2017 10:54 PM, Jes Sorensen wrote:
>>>> On 05/04/2017 08:20 AM, Zhilong Liu wrote:
>>>>> Hi Jes,
>>>>>
>>>>> apply for review, this is a bug I ever encountered.
>>>>
>>>> Zhilong,
>>>>
>>>> Under what circumstances do you see this?
>>>>
>>>
>>> Issued the command:
>>> linux-g0sr:/home/test # ./mdadm -CR /dev/md0 -l1 -n2 -b internal
>>> /dev/loop[0-1] --size 63
>>> ... ... ...
>>> mdadm: Given bitmap chunk size not supported.
>>> linux-g0sr:/home/test # ls /dev/md0
>>> /dev/md0
>>> linux-g0sr:/home/test # ls /sys/block/md0/md/
>>> array_size bitmap component_size level metadata_version
>>> raid_disks reshape_position safe_mode_delay
>>> array_state chunk_size layout max_read_errors
>>> new_dev reshape_direction resync_start
>>>
>>> create_mddev() writes the devnm to
>>> /sys/module/md_mod/parameter/new_array,
>>> then in md.c, module_param_call() called the 'set' function of
>>> add_named_array(),
>>> md_alloc() init_and_add the kobject for devm, finally the devnm device
>>> has created
>>> and sysfs has registered after create_mddev executed successfully.
>>> Thus it's better
>>> to STOP_ARRAY in any case after create_mddev() invoked.
>>>
>>
>> this patch depends on the kernel commit:
>> 039b7225e6e9 ("md: allow creation of mdNNN arrays via
>> md_mod/parameters/new_array")
>> Neil's patch has set "mddev->hold_active = UNTIL_STOP", thus the
>> STOP_ARRAY can work
>> well on this situation.
>
> OK now I am confused - are you saying this change will only work after
> Neil's kernel patch has been applied? That would be no good, mdadm
> needs to work on older kernels too.
>
Sorry for the late reply for this.
Currently, the creating array method for later kernel(newer than v4.11),
it can avoid the race problem
via to set 'create_on_open' as N and write the mddev ->
/sys/module/md_mod/parameters/new_array,
then mdadm can send the ioctl to stop_array directly if creating array
fail, it won't happen the race.
refer to commit: 78b6350dcaad ("md: support disabling of create-on-open
semantics.")
And for older kernel, it's difficult to avoid the problem of the race
when use 'change', 'add' and 'remove'
udev rules in a short period of time via to ioctl commands.
For example, this case tested on my latest Tumbleweed(20170524) with
latest mdadm source.
Steps:
First part:
- open one terminal to monitor the udev:
Terminal 1: # udevadm monitor
monitor will print the received events for:
UDEV - the event which udev sends out after rule processing
KERNEL - the kernel uevent
- type the command in another terminal:
Terminal 2: # ./mdadm -CR /dev/md1 --bitmap=internal -l1 -n2 /dev/loop1
/dev/loop2 --size 63
add_internal_bitmap received the abnormal chunk_size( < 64k), and
return a failure.
but it left the partially created array and didn't cleanup. For this
test, the udev monitor prints:
Terminal 1: # udevadm monitor
... ...
KERNEL[146.077168] add /devices/virtual/bdi/9:1 (bdi)
KERNEL[146.077211] add /devices/virtual/block/md1 (block)
UDEV [146.078112] add /devices/virtual/bdi/9:1 (bdi)
UDEV [146.084163] add /devices/virtual/block/md1 (block)
Terminal 2: # ./mdadm -S /dev/md1
Terminal 1: # udevadm monitor
KERNEL[153.276209] remove /devices/virtual/bdi/9:1 (bdi)
KERNEL[153.276317] remove /devices/virtual/block/md1 (block)
UDEV [153.277034] remove /devices/virtual/bdi/9:1 (bdi)
UDEV [153.280801] remove /devices/virtual/block/md1 (block)
Second part:
add the ioctl(stop_array) and compiled to monitor the udevs.
# git diff
diff --git a/Create.c b/Create.c
index 239545f..21568ca 100644
--- a/Create.c
+++ b/Create.c
@@ -1065,7 +1065,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;
}
Terminal 2: # ./mdadm -CR /dev/md1 --bitmap=internal -l1 -n2 /dev/loop1
/dev/loop2 --size 63
Terminal 1: # udevadm monitor
... ...
KERNEL[171.964597] add /devices/virtual/bdi/9:1 (bdi)
KERNEL[171.965346] add /devices/virtual/block/md1 (block)
UDEV [171.965565] add /devices/virtual/bdi/9:1 (bdi)
KERNEL[171.984195] remove /devices/virtual/bdi/9:1 (bdi)
KERNEL[171.984555] remove /devices/virtual/block/md1 (block)
UDEV [171.984590] remove /devices/virtual/bdi/9:1 (bdi)
KERNEL[172.004504] add /devices/virtual/bdi/9:1 (bdi)
KERNEL[172.004890] add /devices/virtual/block/md1 (block)
UDEV [172.004923] add /devices/virtual/bdi/9:1 (bdi)
UDEV [172.005787] add /devices/virtual/block/md1 (block)
UDEV [172.009648] remove /devices/virtual/block/md1 (block)
UDEV [172.013232] add /devices/virtual/block/md1 (block)
So shall give udev a moment to process 'add' events?
mdadm can work as expect when adding usleep(100 * 1000) before perform
ioctl(STOP_ARRAY).
this is the udev monitor result tested by the following sample.
Terminal 1: # udevadm monitor
KERNEL[5476.780692] add /devices/virtual/bdi/9:1 (bdi)
KERNEL[5476.780976] add /devices/virtual/block/md1 (block)
UDEV [5476.782355] add /devices/virtual/bdi/9:1 (bdi)
UDEV [5476.786056] add /devices/virtual/block/md1 (block)
KERNEL[5476.896255] remove /devices/virtual/bdi/9:1 (bdi)
KERNEL[5476.896367] remove /devices/virtual/block/md1 (block)
UDEV [5476.896895] remove /devices/virtual/bdi/9:1 (bdi)
UDEV [5476.900752] remove /devices/virtual/block/md1 (block)
Such as like this:
diff --git a/Create.c b/Create.c
index 239545f..a07ace8 100644
--- a/Create.c
+++ b/Create.c
@@ -902,10 +902,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;
@@ -1006,7 +1004,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 &&
@@ -1016,7 +1013,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 {
@@ -1028,7 +1024,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
@@ -1065,7 +1060,11 @@ int Create(struct supertype *st, char *mddev,
map_remove(&map, fd2devnm(mdfd));
map_unlock(&map);
- if (mdfd >= 0)
+ if (mdfd >= 0) {
+ /* Give udev a moment to finish 'add' events. */
+ usleep(100*1000);
+ ioctl(mdfd, STOP_ARRAY, NULL);
close(mdfd);
+ }
return 1;
}
Thanks,
-Zhilong
> Jes
>
>
>
^ permalink raw reply related
* Re: [PATCH 01/22] Revert "afs: Move UUID struct to linux/uuid.h"
From: Christoph Hellwig @ 2017-05-31 9:41 UTC (permalink / raw)
To: David Howells
Cc: linux-raid-u79uwXL29TY76Z2rM5mHXA, Arnd Bergmann,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Mimi Zohar, Amir Goldstein,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Steven Whitehouse,
linux-xfs-u79uwXL29TY76Z2rM5mHXA,
linux-block-u79uwXL29TY76Z2rM5mHXA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, Andy Shevchenko, Shaohua Li,
Christoph Hellwig
In-Reply-To: <9377.1496138404-S6HVgzuS8uM4Awkfq6JHfwNdhmdF6hFW@public.gmane.org>
On Tue, May 30, 2017 at 11:00:04AM +0100, David Howells wrote:
> This isn't going to work. You've effectively changed the types of the fields
> in the UUID struct from BE to CPU-endian, but you're still calling
> generate_random_uuid(), which produces a BE UUID. You need to leave the
> struct members as __beXX or stop using the core UUID routines.
>
> Just move the struct uuid_v1 as-is to the afs headers and rename it to struct
> afs_uuid. You can then leave the (un)marshalling code alone.
That's one option. The other option would be to revert
"afs: Use core kernel UUID generation", as that also changed the
v1 UUID to a v4 uuid. Does the afs protocol require a v1 uuid
or does it just use the formwat on the wire?
^ permalink raw reply
* Re: [PATCH] bcache: Fix a sleep-in-atomic bug
From: Jia-Ju Bai @ 2017-05-31 8:31 UTC (permalink / raw)
To: Jia-Ju Bai
Cc: kent.overstreet, shli, axboe, mchristi, git, colyli, akpm,
wangyijing, mingo, mhocko, linux-bcache, linux-raid, linux-kernel
In-Reply-To: <1496215420-15819-1-git-send-email-baijiaju1990@163.com>
On 05/31/2017 03:23 PM, Jia-Ju Bai wrote:
> The driver may sleep under a spin lock, and the function call path is:
> journal_wait_for_write (acquire the lock by spin_lock)
> closure_sync
> schedule --> may sleep
>
> To fix it, the lock is released before "closure_sync", and the lock is
> acquired again after this function.
>
> Signed-off-by: Jia-Ju Bai<baijiaju1990@163.com>
> ---
> drivers/md/bcache/journal.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/md/bcache/journal.c b/drivers/md/bcache/journal.c
> index 1198e53..ad47c36 100644
> --- a/drivers/md/bcache/journal.c
> +++ b/drivers/md/bcache/journal.c
> @@ -724,6 +724,7 @@ static struct journal_write *journal_wait_for_write(struct cache_set *c,
> btree_flush_write(c);
> }
>
> + spin_unlock(&c->journal.lock);
> closure_sync(&cl);
> spin_lock(&c->journal.lock);
> wait = true;
Sorry, my patch is not correct, and it will cause double unlock.
Please ignore my patch.
Jia-Ju Bai
^ permalink raw reply
* Re: [PATCH] bcache: Fix a sleep-in-atomic bug
From: Kent Overstreet @ 2017-05-31 7:52 UTC (permalink / raw)
To: Jia-Ju Bai
Cc: shli, axboe, mchristi, git, colyli, akpm, wangyijing, mingo,
mhocko, linux-bcache, linux-raid, linux-kernel
In-Reply-To: <1496215420-15819-1-git-send-email-baijiaju1990@163.com>
On Wed, May 31, 2017 at 03:23:40PM +0800, Jia-Ju Bai wrote:
> The driver may sleep under a spin lock, and the function call path is:
> journal_wait_for_write (acquire the lock by spin_lock)
> closure_sync
> schedule --> may sleep
This patch is incorrect, you've introduced a double unlock.
Did you actually observe a sleep in atomic?
>
> To fix it, the lock is released before "closure_sync", and the lock is
> acquired again after this function.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@163.com>
> ---
> drivers/md/bcache/journal.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/md/bcache/journal.c b/drivers/md/bcache/journal.c
> index 1198e53..ad47c36 100644
> --- a/drivers/md/bcache/journal.c
> +++ b/drivers/md/bcache/journal.c
> @@ -724,6 +724,7 @@ static struct journal_write *journal_wait_for_write(struct cache_set *c,
> btree_flush_write(c);
> }
>
> + spin_unlock(&c->journal.lock);
> closure_sync(&cl);
> spin_lock(&c->journal.lock);
> wait = true;
> --
> 1.7.9.5
>
>
^ permalink raw reply
* [PATCH 2/2] md: Make flush bios explicitely sync
From: Jan Kara @ 2017-05-31 7:44 UTC (permalink / raw)
To: Shaohua Li; +Cc: Jan Kara, linux-raid, stable
In-Reply-To: <20170531074433.14298-1-jack@suse.cz>
Commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as
synchronous" removed REQ_SYNC flag from WRITE_{FUA|PREFLUSH|...}
definitions. generic_make_request_checks() however strips REQ_FUA and
REQ_PREFLUSH flags from a bio when the storage doesn't report volatile
write cache and thus write effectively becomes asynchronous which can
lead to performance regressions
Fix the problem by making sure all bios which are synchronous are
properly marked with REQ_SYNC.
CC: linux-raid@vger.kernel.org
CC: Shaohua Li <shli@kernel.org>
Fixes: b685d3d65ac791406e0dfd8779cc9b3707fea5a3
CC: stable@vger.kernel.org
Signed-off-by: Jan Kara <jack@suse.cz>
---
drivers/md/md.c | 2 +-
drivers/md/raid5-cache.c | 4 ++--
drivers/md/raid5-ppl.c | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 10367ffe92e3..212a6777ff31 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -765,7 +765,7 @@ void md_super_write(struct mddev *mddev, struct md_rdev *rdev,
test_bit(FailFast, &rdev->flags) &&
!test_bit(LastDev, &rdev->flags))
ff = MD_FAILFAST;
- bio->bi_opf = REQ_OP_WRITE | REQ_PREFLUSH | REQ_FUA | ff;
+ bio->bi_opf = REQ_OP_WRITE | REQ_SYNC | REQ_PREFLUSH | REQ_FUA | ff;
atomic_inc(&mddev->pending_writes);
submit_bio(bio);
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 4c00bc248287..0a7af8b0a80a 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -1782,7 +1782,7 @@ static int r5l_log_write_empty_meta_block(struct r5l_log *log, sector_t pos,
mb->checksum = cpu_to_le32(crc32c_le(log->uuid_checksum,
mb, PAGE_SIZE));
if (!sync_page_io(log->rdev, pos, PAGE_SIZE, page, REQ_OP_WRITE,
- REQ_FUA, false)) {
+ REQ_SYNC | REQ_FUA, false)) {
__free_page(page);
return -EIO;
}
@@ -2388,7 +2388,7 @@ r5c_recovery_rewrite_data_only_stripes(struct r5l_log *log,
mb->checksum = cpu_to_le32(crc32c_le(log->uuid_checksum,
mb, PAGE_SIZE));
sync_page_io(log->rdev, ctx->pos, PAGE_SIZE, page,
- REQ_OP_WRITE, REQ_FUA, false);
+ REQ_OP_WRITE, REQ_SYNC | REQ_FUA, false);
sh->log_start = ctx->pos;
list_add_tail(&sh->r5c, &log->stripe_in_journal_list);
atomic_inc(&log->stripe_in_journal_count);
diff --git a/drivers/md/raid5-ppl.c b/drivers/md/raid5-ppl.c
index 5d25bebf3328..ccce92e68d7f 100644
--- a/drivers/md/raid5-ppl.c
+++ b/drivers/md/raid5-ppl.c
@@ -907,8 +907,8 @@ static int ppl_write_empty_header(struct ppl_log *log)
pplhdr->checksum = cpu_to_le32(~crc32c_le(~0, pplhdr, PAGE_SIZE));
if (!sync_page_io(rdev, rdev->ppl.sector - rdev->data_offset,
- PPL_HEADER_SIZE, page, REQ_OP_WRITE | REQ_FUA, 0,
- false)) {
+ PPL_HEADER_SIZE, page, REQ_OP_WRITE | REQ_SYNC |
+ REQ_FUA, 0, false)) {
md_error(rdev->mddev, rdev);
ret = -EIO;
}
--
2.12.3
^ permalink raw reply related
* [PATCH 0/2 v2] Fix fallout from changes to FUA and PREFLUSH definitions
From: Jan Kara @ 2017-05-31 7:44 UTC (permalink / raw)
To: Shaohua Li; +Cc: Jan Kara, linux-raid, Mike Snitzer, dm-devel
Hello,
here are patches for DM/MD that address a possible performance issue caused by
commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as synchronous".
The core of the problem is that above mentioned commit removed REQ_SYNC flag
from WRITE_{FUA|PREFLUSH|...} definitions. generic_make_request_checks()
however strips REQ_FUA and REQ_PREFLUSH flags from a bio when the storage
doesn't report volatile write cache and thus write effectively becomes
asynchronous which can lead to performance regressions.
DM/MD guys, can you please have a look at patches and take them through your
trees? Thanks!
Honza
^ permalink raw reply
* [PATCH] bcache: Fix a sleep-in-atomic bug
From: Jia-Ju Bai @ 2017-05-31 7:23 UTC (permalink / raw)
To: kent.overstreet, shli, axboe, mchristi, git, colyli, akpm,
wangyijing, mingo, mhocko
Cc: linux-bcache, linux-raid, linux-kernel, Jia-Ju Bai
The driver may sleep under a spin lock, and the function call path is:
journal_wait_for_write (acquire the lock by spin_lock)
closure_sync
schedule --> may sleep
To fix it, the lock is released before "closure_sync", and the lock is
acquired again after this function.
Signed-off-by: Jia-Ju Bai <baijiaju1990@163.com>
---
drivers/md/bcache/journal.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/md/bcache/journal.c b/drivers/md/bcache/journal.c
index 1198e53..ad47c36 100644
--- a/drivers/md/bcache/journal.c
+++ b/drivers/md/bcache/journal.c
@@ -724,6 +724,7 @@ static struct journal_write *journal_wait_for_write(struct cache_set *c,
btree_flush_write(c);
}
+ spin_unlock(&c->journal.lock);
closure_sync(&cl);
spin_lock(&c->journal.lock);
wait = true;
--
1.7.9.5
^ permalink raw reply related
* Re: 4.11.2: reshape raid5 -> raid6 atop bcache deadlocks at start on md_attr_store / raid5_make_request
From: Shaohua Li @ 2017-05-30 17:41 UTC (permalink / raw)
To: NeilBrown; +Cc: Nix, linux-raid
In-Reply-To: <871sr8h8qx.fsf@notabene.neil.brown.name>
On Mon, May 29, 2017 at 09:17:10AM +1000, Neil Brown wrote:
> On Wed, May 24 2017, Shaohua Li wrote:
>
> > On Wed, May 24, 2017 at 11:24:21AM +1000, Neil Brown wrote:
> >> On Mon, May 22 2017, Nix wrote:
> >>
> >> > On 22 May 2017, NeilBrown told this:
> >> >
> >> >> Probably something like this:
> >> >>
> >> >> diff --git a/drivers/md/md.c b/drivers/md/md.c
> >> >> index f6ae1d67bcd0..dbca31be22a1 100644
> >> >> --- a/drivers/md/md.c
> >> >> +++ b/drivers/md/md.c
> >> >> @@ -8364,8 +8364,6 @@ static void md_start_sync(struct work_struct *ws)
> >> >> */
> >> >> void md_check_recovery(struct mddev *mddev)
> >> >> {
> >> >> - if (mddev->suspended)
> >> >> - return;
> >> >>
> >> >> if (mddev->bitmap)
> >> >> bitmap_daemon_work(mddev);
> >> >> @@ -8484,6 +8482,7 @@ void md_check_recovery(struct mddev *mddev)
> >> >> clear_bit(MD_RECOVERY_DONE, &mddev->recovery);
> >> >>
> >> >> if (!test_and_clear_bit(MD_RECOVERY_NEEDED, &mddev->recovery) ||
> >> >> + mddev->suspended ||
> >> >> test_bit(MD_RECOVERY_FROZEN, &mddev->recovery))
> >> >> goto not_running;
> >> >> /* no recovery is running.
> >> >>
> >> >> though it's late so don't trust anything I write.
> >> >>
> >> >> If you try again it will almost certainly succeed. I suspect this is a
> >> >> hard race to hit - well done!!!
> >> >
> >> > Definitely not a hard race to hit :( I just hit it again with this
> >> > patch.
> >> >
> >> > Absolutely identical hang:
> >> >
> >> > [ 495.833520] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> >> > [ 495.840618] mdadm D 0 2700 2537 0x00000000
> >> > [ 495.847762] Call Trace:
> >> > [ 495.854825] __schedule+0x290/0x810
> >> > [ 495.861905] schedule+0x36/0x80
> >> > [ 495.868934] mddev_suspend+0xb3/0xe0
> >> > [ 495.875926] ? wake_atomic_t_function+0x60/0x60
> >> > [ 495.882976] level_store+0x1a7/0x6c0
> >> > [ 495.889953] ? md_ioctl+0xb7/0x1c10
> >> > [ 495.896901] ? putname+0x53/0x60
> >> > [ 495.903807] md_attr_store+0x83/0xc0
> >> > [ 495.910684] sysfs_kf_write+0x37/0x40
> >> > [ 495.917547] kernfs_fop_write+0x110/0x1a0
> >> > [ 495.924429] __vfs_write+0x28/0x120
> >> > [ 495.931270] ? kernfs_iop_get_link+0x172/0x1e0
> >> > [ 495.938126] ? __alloc_fd+0x3f/0x170
> >> > [ 495.944906] vfs_write+0xb6/0x1d0
> >> > [ 495.951646] SyS_write+0x46/0xb0
> >> > [ 495.958338] entry_SYSCALL_64_fastpath+0x13/0x94
> >> >
> >> > Everything else hangs the same way, too. This was surprising enough that
> >> > I double-checked to be sure the patch was applied: it was. I suspect the
> >> > deadlock is somewhat different than you supposed... (and quite possibly
> >> > not a race at all, or I wouldn't be hitting it so consistently, every
> >> > time. I mean, I only need to miss it *once* and I'll have reshaped... :) )
> >> >
> >> > It seems I can reproduce this on demand, so if you want to throw a patch
> >> > with piles of extra printks my way, feel free.
> >>
> >> Did you have md_write_start being called by syslog-ng again?
> >> I wonder what syslog is logging - presumably something about the reshape
> >> starting.
> >> If you kill syslog-ng, can you start the reshape?
> >>
> >> Alternately, this might do it.
> >> I think the root problem is that it isn't safe to call mddev_suspend()
> >> while holding the reconfig_mutex.
> >> For complete safety I probably need to move the request_module() call
> >> earlier, as that could block if a device was suspended (no memory
> >> allocation allowed while device is suspended).
> >>
> >> Thanks,
> >> NeilBrown
> >>
> >>
> >>
> >> diff --git a/drivers/md/md.c b/drivers/md/md.c
> >> index 10367ffe92e3..a7b9c0576479 100644
> >> --- a/drivers/md/md.c
> >> +++ b/drivers/md/md.c
> >> @@ -324,8 +324,12 @@ static blk_qc_t md_make_request(struct request_queue *q, struct bio *bio)
> >> void mddev_suspend(struct mddev *mddev)
> >> {
> >> WARN_ON_ONCE(mddev->thread && current == mddev->thread->tsk);
> >> +
> >> if (mddev->suspended++)
> >> return;
> >> +#ifdef CONFIG_LOCKDEP
> >> + WARN_ON_ONCE(debug_locks && lockdep_is_held(&mddev->reconfig_mutex));
> >> +#endif
> >> synchronize_rcu();
> >> wait_event(mddev->sb_wait, atomic_read(&mddev->active_io) == 0);
> >> mddev->pers->quiesce(mddev, 1);
> >> @@ -3594,9 +3598,12 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
> >> if (slen == 0 || slen >= sizeof(clevel))
> >> return -EINVAL;
> >>
> >> + mddev_suspend(mddev);
> >> rv = mddev_lock(mddev);
> >> - if (rv)
> >> + if (rv) {
> >> + mddev_resume(mddev);
> >> return rv;
> >> + }
> >>
> >> if (mddev->pers == NULL) {
> >> strncpy(mddev->clevel, buf, slen);
> >> @@ -3687,7 +3694,6 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
> >> }
> >>
> >> /* Looks like we have a winner */
> >> - mddev_suspend(mddev);
> >> mddev_detach(mddev);
> >>
> >> spin_lock(&mddev->lock);
> >> @@ -3771,13 +3777,13 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
> >> blk_set_stacking_limits(&mddev->queue->limits);
> >> pers->run(mddev);
> >> set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
> >> - mddev_resume(mddev);
> >> if (!mddev->thread)
> >> md_update_sb(mddev, 1);
> >> sysfs_notify(&mddev->kobj, NULL, "level");
> >> md_new_event(mddev);
> >> rv = len;
> >> out_unlock:
> >> + mddev_resume(mddev);
> >> mddev_unlock(mddev);
> >> return rv;
> >> }
> >> @@ -4490,6 +4496,7 @@ action_store(struct mddev *mddev, const char *page, size_t len)
> >> int err;
> >> if (mddev->pers->start_reshape == NULL)
> >> return -EINVAL;
> >> + mddev_suspend(mddev);
> >> err = mddev_lock(mddev);
> >> if (!err) {
> >> if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery))
> >> @@ -4500,6 +4507,7 @@ action_store(struct mddev *mddev, const char *page, size_t len)
> >> }
> >> mddev_unlock(mddev);
> >> }
> >> + mddev_resume(mddev);
> >> if (err)
> >> return err;
> >> sysfs_notify(&mddev->kobj, NULL, "degraded");
> >
> > The analysis makes a lot of sense, thanks! The patch looks not solving the
> > problem though, because check_recovery will not write super if suspended isn't
> > 0.
> >
> > I'm thinking of alternative wayt to fix this issue. A request which stalls in
> > md_write_start reallys isn't an active IO. We could add another counter in
> > md_write_start to record stalled request there. Then in mddev_suspend, wait
> > event will wait for atomic_read(&mddev->active_io) == the_new_counter.
> >
>
> Sorry I didn't respond to this suggestion earlier.
>
> It is important that
> mddev_suspend(mddev)
> mddev_detach(mddev)
>
> results in the personality module being unused so it can be removed, as
> module_put() will soon be called.
> If a thread is stuck in md_write_start(), then when it is released it
> will return to the personality's make_request function and will execute
> code in the module. So we cannot safely remove call module_put() while
> code is blocked in md_write_start().
Ok, I thought md_suspend is only suspending the array. This makes sense,
thanks!
> We could possibly move the md_write_start() call into md_make_request(),
> so the personality code will never block.
> Maybe something like this?
> We probably also need to change all md_write_start() call is modules to
> md_write_inc().
Like this idea very much.
Thanks,
Shaohua
> Thanks,
> NeilBrown
>
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 10367ffe92e3..3e45bb429a22 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -277,6 +277,7 @@ static blk_qc_t md_make_request(struct request_queue *q, struct bio *bio)
> bio_endio(bio);
> return BLK_QC_T_NONE;
> }
> +check_suspended:
> smp_rmb(); /* Ensure implications of 'active' are visible */
> rcu_read_lock();
> if (mddev->suspended) {
> @@ -294,6 +295,13 @@ static blk_qc_t md_make_request(struct request_queue *q, struct bio *bio)
> }
> atomic_inc(&mddev->active_io);
> rcu_read_unlock();
> + if (rw == WRITE) {
> + md_write_start(mddev, bio);
> + if (test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags)) {
> + md_write_end(mddev);
> + goto check_suspended;
> + }
> + }
>
> /*
> * save the sectors now since our bio can
> @@ -309,6 +317,8 @@ static blk_qc_t md_make_request(struct request_queue *q, struct bio *bio)
> part_stat_add(cpu, &mddev->gendisk->part0, sectors[rw], sectors);
> part_stat_unlock();
>
> + md_write_end(mddev);
> +
> if (atomic_dec_and_test(&mddev->active_io) && mddev->suspended)
> wake_up(&mddev->sb_wait);
>
> @@ -327,6 +337,7 @@ void mddev_suspend(struct mddev *mddev)
> if (mddev->suspended++)
> return;
> synchronize_rcu();
> + wake_up(&mddev->sb_wait);
> wait_event(mddev->sb_wait, atomic_read(&mddev->active_io) == 0);
> mddev->pers->quiesce(mddev, 1);
>
> @@ -7979,7 +7990,7 @@ void md_write_start(struct mddev *mddev, struct bio *bi)
> if (did_change)
> sysfs_notify_dirent_safe(mddev->sysfs_state);
> wait_event(mddev->sb_wait,
> - !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags));
> + !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags) && !mddev->suspended);
> }
> EXPORT_SYMBOL(md_write_start);
>
^ permalink raw reply
* Re: [PATCH 01/22] Revert "afs: Move UUID struct to linux/uuid.h"
From: David Howells @ 2017-05-30 10:00 UTC (permalink / raw)
To: Christoph Hellwig
Cc: dhowells, Andy Shevchenko, Amir Goldstein, linux-fsdevel,
Shaohua Li, Dan Williams, Steven Whitehouse, Mimi Zohar,
linux-xfs, linux-raid, linux-nvdimm, linux-kernel, linux-block
In-Reply-To: <20170528102008.30276-2-hch@lst.de>
Christoph Hellwig <hch@lst.de> wrote:
> This reverts commit ff548773106ec7f8031bc6172e0234bd2a02c19c.
>
> The V1 uuid intrepreatation in struct form isn't really useful to the
> rest of the kernel, and not really compatible to it either, so move it
> back to AFS instead of polluting the global uuid.h.
This isn't going to work. You've effectively changed the types of the fields
in the UUID struct from BE to CPU-endian, but you're still calling
generate_random_uuid(), which produces a BE UUID. You need to leave the
struct members as __beXX or stop using the core UUID routines.
Just move the struct uuid_v1 as-is to the afs headers and rename it to struct
afs_uuid. You can then leave the (un)marshalling code alone.
David
^ permalink raw reply
* Re: mdadm --misc --readonly -> ENODEV indefinitely
From: Nix @ 2017-05-29 12:20 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid
In-Reply-To: <877f10hgx8.fsf@notabene.neil.brown.name>
On 28 May 2017, NeilBrown told this:
> On Sun, May 28 2017, Nix wrote:
>
>> So I was trying to flip on the RAID journal now I'd fully populated my
>> array and cut over to it. So, from early userspace, I did
>>
>> # first line done by initramfs scripting
>> /sbin/mdadm --assemble --scan --auto=md --freeze-reshape
>> mdadm --misc --readonly /dev/md/fast
>> mdadm --manage /dev/md/fast --add-journal /dev/ssd1
>> mdadm --misc --readwrite /dev/md/fast
>>
>> This did not work as planned, or indeed at all. After the first
>> --readonly, all requests for /dev/md/fast report "No such device or
>> address" until a reboot, though /proc/mdstat says the thing is still
>> there and /sys/block/md125/dev reports no change in major/minor numbers.
>> (I don't have udev in my early-userspace environment, but mdev reports
>> no change, either.)
[...]
> Commit: 065e519e71b2 ("md: MD_CLOSING needs to be cleared after called md_set_readonly or do_md_stop")
>
> Broken in v4.9-rc1
> Fixed in v4.12-rc1
Oh, apologies -- I should have checked master. :/ didn't think of it.
(And the fix hasn't hit stable in the two months since. I sort of
assumed promotion to stable was usually faster than that... but I guess
it depends on how much free time gregkh has.)
... my arrays are all humming along now, RAID-6ed and much faster than
they ever were with hardware RAID. I'm back in md land for good, where
bugs can actually get *fixed* without booting off a DOS floppy and doing
a reflash of horrible non-free firmware that if it goes wrong will make
my machine unusable.
The sense of freedom is undeniable :) thank you, mders one and all!
--
NULL && (void)
^ permalink raw reply
* Re: [PATCH v2 04/11] crypto: move drbg to generic async completion
From: Stephan Müller @ 2017-05-29 9:04 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: linux-security-module, Mike Snitzer, linux-doc, David Howells,
dm-devel, keyrings, linux-ima-devel, Pavel Shilovsky,
Alasdair Kergon, linux-cifs, Herbert Xu, Jonathan Corbet,
Mimi Zohar, Serge E. Hallyn, linux-ima-user, Eric Biggers,
linux-raid, linux-fscrypt, James Morris, Jaegeuk Kim, Ofir Drang,
Theodore Y. Ts'o, Dmitry Kasatkin, samba-technical,
linux-kernel, Steve French
In-Reply-To: <1496046180-21962-5-git-send-email-gilad@benyossef.com>
Am Montag, 29. Mai 2017, 10:22:51 CEST schrieb Gilad Ben-Yossef:
Hi Gilad,
> DRBG is starting an async. crypto op and waiting for it complete.
> Move it over to generic code doing the same.
>
> The code now also passes CRYPTO_TFM_REQ_MAY_SLEEP flag indicating
> crypto request memory allocation may use GFP_KERNEL which should
> be perfectly fine as the code is obviously sleeping for the
> completion of the request any way.
>
> Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Acked-by: Stephan Müller <smueller@chronox.de>
However, please note that there is a patch "crypto:drbg- Fixes panic in
wait_for_completion call." which adds another init_completion. I guess that
needs conversion to crypto_init_wait as well, once this patch set is accepted.
Ciao
Stephan
^ permalink raw reply
* [PATCH v2 11/11] crypto: adapt api sample to use async. op wait
From: Gilad Ben-Yossef @ 2017-05-29 8:22 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li, Steve French,
Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn
Cc: Eric Biggers, Ofir Drang, Pavel Shilovsky, linux-crypto,
linux-doc, linux-kernel, keyrings, linux-raid, linux-cifs,
samba-technical, linux-fscrypt, linux-ima-devel, linux-ima-user,
linux-security-module
In-Reply-To: <1496046180-21962-1-git-send-email-gilad@benyossef.com>
The code sample is waiting for an async. crypto op completion.
Adapt sample to use the new generic infrastructure to do the same.
This also fixes a possible data coruption bug created by the
use of wait_for_completion_interruptible() without dealing
correctly with an interrupt aborting the wait prior to the
async op finishing.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
Documentation/crypto/api-samples.rst | 52 +++++++-----------------------------
1 file changed, 10 insertions(+), 42 deletions(-)
diff --git a/Documentation/crypto/api-samples.rst b/Documentation/crypto/api-samples.rst
index d021fd9..73b466b 100644
--- a/Documentation/crypto/api-samples.rst
+++ b/Documentation/crypto/api-samples.rst
@@ -7,59 +7,27 @@ Code Example For Symmetric Key Cipher Operation
::
- struct tcrypt_result {
- struct completion completion;
- int err;
- };
-
/* tie all data structures together */
struct skcipher_def {
struct scatterlist sg;
struct crypto_skcipher *tfm;
struct skcipher_request *req;
- struct tcrypt_result result;
+ struct crypto_wait wait;
};
- /* Callback function */
- static void test_skcipher_cb(struct crypto_async_request *req, int error)
- {
- struct tcrypt_result *result = req->data;
-
- if (error == -EINPROGRESS)
- return;
- result->err = error;
- complete(&result->completion);
- pr_info("Encryption finished successfully\n");
- }
-
/* Perform cipher operation */
static unsigned int test_skcipher_encdec(struct skcipher_def *sk,
int enc)
{
- int rc = 0;
+ int rc;
if (enc)
- rc = crypto_skcipher_encrypt(sk->req);
+ rc = crypto_wait_req(crypto_skcipher_encrypt(sk->req), &sk->wait);
else
- rc = crypto_skcipher_decrypt(sk->req);
-
- switch (rc) {
- case 0:
- break;
- case -EINPROGRESS:
- case -EBUSY:
- rc = wait_for_completion_interruptible(
- &sk->result.completion);
- if (!rc && !sk->result.err) {
- reinit_completion(&sk->result.completion);
- break;
- }
- default:
- pr_info("skcipher encrypt returned with %d result %d\n",
- rc, sk->result.err);
- break;
- }
- init_completion(&sk->result.completion);
+ rc = crypto_wait_req(crypto_skcipher_decrypt(sk->req), &sk->wait);
+
+ if (rc)
+ pr_info("skcipher encrypt returned with result %d\n", rc);
return rc;
}
@@ -89,8 +57,8 @@ Code Example For Symmetric Key Cipher Operation
}
skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- test_skcipher_cb,
- &sk.result);
+ crypto_req_done,
+ &sk.wait);
/* AES 256 with random key */
get_random_bytes(&key, 32);
@@ -122,7 +90,7 @@ Code Example For Symmetric Key Cipher Operation
/* We encrypt one block */
sg_init_one(&sk.sg, scratchpad, 16);
skcipher_request_set_crypt(req, &sk.sg, &sk.sg, 16, ivdata);
- init_completion(&sk.result.completion);
+ crypto_init_wait(&sk.wait);
/* encrypt data */
ret = test_skcipher_encdec(&sk, 1);
--
2.1.4
^ permalink raw reply related
* [PATCH v2 10/11] ima: move to generic async completion
From: Gilad Ben-Yossef @ 2017-05-29 8:22 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li, Steve French,
Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn
Cc: Eric Biggers, Ofir Drang, Pavel Shilovsky, linux-crypto,
linux-doc, linux-kernel, keyrings, linux-raid, linux-cifs,
samba-technical, linux-fscrypt, linux-ima-devel, linux-ima-user,
linux-security-module
In-Reply-To: <1496046180-21962-1-git-send-email-gilad@benyossef.com>
ima starts several async crypto ops and waits for their completions.
Move it over to generic code doing the same.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
security/integrity/ima/ima_crypto.c | 56 +++++++++++--------------------------
1 file changed, 17 insertions(+), 39 deletions(-)
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 802d5d2..0e4db1fe 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -27,11 +27,6 @@
#include "ima.h"
-struct ahash_completion {
- struct completion completion;
- int err;
-};
-
/* minimum file size for ahash use */
static unsigned long ima_ahash_minsize;
module_param_named(ahash_minsize, ima_ahash_minsize, ulong, 0644);
@@ -196,30 +191,13 @@ static void ima_free_atfm(struct crypto_ahash *tfm)
crypto_free_ahash(tfm);
}
-static void ahash_complete(struct crypto_async_request *req, int err)
+static inline int ahash_wait(int err, struct crypto_wait *wait)
{
- struct ahash_completion *res = req->data;
- if (err == -EINPROGRESS)
- return;
- res->err = err;
- complete(&res->completion);
-}
+ err = crypto_wait_req(err, wait);
-static int ahash_wait(int err, struct ahash_completion *res)
-{
- switch (err) {
- case 0:
- break;
- case -EINPROGRESS:
- case -EBUSY:
- wait_for_completion(&res->completion);
- reinit_completion(&res->completion);
- err = res->err;
- /* fall through */
- default:
+ if (err)
pr_crit_ratelimited("ahash calculation failed: err: %d\n", err);
- }
return err;
}
@@ -233,7 +211,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
int rc, read = 0, rbuf_len, active = 0, ahash_rc = 0;
struct ahash_request *req;
struct scatterlist sg[1];
- struct ahash_completion res;
+ struct crypto_wait wait;
size_t rbuf_size[2];
hash->length = crypto_ahash_digestsize(tfm);
@@ -242,12 +220,12 @@ static int ima_calc_file_hash_atfm(struct file *file,
if (!req)
return -ENOMEM;
- init_completion(&res.completion);
+ crypto_init_wait(&wait);
ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
CRYPTO_TFM_REQ_MAY_SLEEP,
- ahash_complete, &res);
+ crypto_req_done, &wait);
- rc = ahash_wait(crypto_ahash_init(req), &res);
+ rc = ahash_wait(crypto_ahash_init(req), &wait);
if (rc)
goto out1;
@@ -288,7 +266,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
* read/request, wait for the completion of the
* previous ahash_update() request.
*/
- rc = ahash_wait(ahash_rc, &res);
+ rc = ahash_wait(ahash_rc, &wait);
if (rc)
goto out3;
}
@@ -304,7 +282,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
* read/request, wait for the completion of the
* previous ahash_update() request.
*/
- rc = ahash_wait(ahash_rc, &res);
+ rc = ahash_wait(ahash_rc, &wait);
if (rc)
goto out3;
}
@@ -318,7 +296,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
active = !active; /* swap buffers, if we use two */
}
/* wait for the last update request to complete */
- rc = ahash_wait(ahash_rc, &res);
+ rc = ahash_wait(ahash_rc, &wait);
out3:
if (read)
file->f_mode &= ~FMODE_READ;
@@ -327,7 +305,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
out2:
if (!rc) {
ahash_request_set_crypt(req, NULL, hash->digest, 0);
- rc = ahash_wait(crypto_ahash_final(req), &res);
+ rc = ahash_wait(crypto_ahash_final(req), &wait);
}
out1:
ahash_request_free(req);
@@ -527,7 +505,7 @@ static int calc_buffer_ahash_atfm(const void *buf, loff_t len,
{
struct ahash_request *req;
struct scatterlist sg;
- struct ahash_completion res;
+ struct crypto_wait wait;
int rc, ahash_rc = 0;
hash->length = crypto_ahash_digestsize(tfm);
@@ -536,12 +514,12 @@ static int calc_buffer_ahash_atfm(const void *buf, loff_t len,
if (!req)
return -ENOMEM;
- init_completion(&res.completion);
+ crypto_init_wait(&wait);
ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
CRYPTO_TFM_REQ_MAY_SLEEP,
- ahash_complete, &res);
+ crypto_req_done, &wait);
- rc = ahash_wait(crypto_ahash_init(req), &res);
+ rc = ahash_wait(crypto_ahash_init(req), &wait);
if (rc)
goto out;
@@ -551,10 +529,10 @@ static int calc_buffer_ahash_atfm(const void *buf, loff_t len,
ahash_rc = crypto_ahash_update(req);
/* wait for the update request to complete */
- rc = ahash_wait(ahash_rc, &res);
+ rc = ahash_wait(ahash_rc, &wait);
if (!rc) {
ahash_request_set_crypt(req, NULL, hash->digest, 0);
- rc = ahash_wait(crypto_ahash_final(req), &res);
+ rc = ahash_wait(crypto_ahash_final(req), &wait);
}
out:
ahash_request_free(req);
--
2.1.4
^ permalink raw reply related
* [PATCH v2 09/11] cifs: move to generic async completion
From: Gilad Ben-Yossef @ 2017-05-29 8:22 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li, Steve French,
Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn
Cc: Eric Biggers, Ofir Drang, Pavel Shilovsky, linux-crypto,
linux-doc, linux-kernel, keyrings, linux-raid, linux-cifs,
samba-technical, linux-fscrypt, linux-ima-devel, linux-ima-user,
linux-security-module
In-Reply-To: <1496046180-21962-1-git-send-email-gilad@benyossef.com>
cifs starts an async. crypto op and waits for their completion.
Move it over to generic code doing the same.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Acked-by: Pavel Shilovsky <pshilov@microsoft.com>
---
fs/cifs/smb2ops.c | 30 ++++--------------------------
1 file changed, 4 insertions(+), 26 deletions(-)
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index c586918..c0e1882 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -1707,22 +1707,6 @@ init_sg(struct smb_rqst *rqst, u8 *sign)
return sg;
}
-struct cifs_crypt_result {
- int err;
- struct completion completion;
-};
-
-static void cifs_crypt_complete(struct crypto_async_request *req, int err)
-{
- struct cifs_crypt_result *res = req->data;
-
- if (err == -EINPROGRESS)
- return;
-
- res->err = err;
- complete(&res->completion);
-}
-
static int
smb2_get_enc_key(struct TCP_Server_Info *server, __u64 ses_id, int enc, u8 *key)
{
@@ -1763,12 +1747,10 @@ crypt_message(struct TCP_Server_Info *server, struct smb_rqst *rqst, int enc)
struct aead_request *req;
char *iv;
unsigned int iv_len;
- struct cifs_crypt_result result = {0, };
+ DECLARE_CRYPTO_WAIT(wait);
struct crypto_aead *tfm;
unsigned int crypt_len = le32_to_cpu(tr_hdr->OriginalMessageSize);
- init_completion(&result.completion);
-
rc = smb2_get_enc_key(server, tr_hdr->SessionId, enc, key);
if (rc) {
cifs_dbg(VFS, "%s: Could not get %scryption key\n", __func__,
@@ -1826,14 +1808,10 @@ crypt_message(struct TCP_Server_Info *server, struct smb_rqst *rqst, int enc)
aead_request_set_ad(req, assoc_data_len);
aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- cifs_crypt_complete, &result);
+ crypto_req_done, &wait);
- rc = enc ? crypto_aead_encrypt(req) : crypto_aead_decrypt(req);
-
- if (rc == -EINPROGRESS || rc == -EBUSY) {
- wait_for_completion(&result.completion);
- rc = result.err;
- }
+ rc = crypto_wait_req(enc ? crypto_aead_encrypt(req)
+ : crypto_aead_decrypt(req), &wait);
if (!rc && enc)
memcpy(&tr_hdr->Signature, sign, SMB2_SIGNATURE_SIZE);
--
2.1.4
^ permalink raw reply related
* [PATCH v2 08/11] fscrypt: move to generic async completion
From: Gilad Ben-Yossef @ 2017-05-29 8:22 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li, Steve French,
Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn
Cc: linux-cifs, linux-doc, linux-ima-user, Eric Biggers,
samba-technical, linux-kernel, linux-raid, linux-fscrypt,
keyrings, linux-crypto, linux-security-module, linux-ima-devel,
Pavel Shilovsky, Ofir Drang
In-Reply-To: <1496046180-21962-1-git-send-email-gilad@benyossef.com>
fscrypt starts several async. crypto ops and waiting for them to
complete. Move it over to generic code doing the same.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
fs/crypto/crypto.c | 28 ++++------------------------
fs/crypto/fname.c | 36 ++++++------------------------------
fs/crypto/fscrypt_private.h | 10 ----------
fs/crypto/keyinfo.c | 21 +++------------------
4 files changed, 13 insertions(+), 82 deletions(-)
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 6d6eca3..227db85 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -125,21 +125,6 @@ struct fscrypt_ctx *fscrypt_get_ctx(const struct inode *inode, gfp_t gfp_flags)
}
EXPORT_SYMBOL(fscrypt_get_ctx);
-/**
- * page_crypt_complete() - completion callback for page crypto
- * @req: The asynchronous cipher request context
- * @res: The result of the cipher operation
- */
-static void page_crypt_complete(struct crypto_async_request *req, int res)
-{
- struct fscrypt_completion_result *ecr = req->data;
-
- if (res == -EINPROGRESS)
- return;
- ecr->res = res;
- complete(&ecr->completion);
-}
-
int fscrypt_do_page_crypto(const struct inode *inode, fscrypt_direction_t rw,
u64 lblk_num, struct page *src_page,
struct page *dest_page, unsigned int len,
@@ -150,7 +135,7 @@ int fscrypt_do_page_crypto(const struct inode *inode, fscrypt_direction_t rw,
u8 padding[FS_XTS_TWEAK_SIZE - sizeof(__le64)];
} xts_tweak;
struct skcipher_request *req = NULL;
- DECLARE_FS_COMPLETION_RESULT(ecr);
+ DECLARE_CRYPTO_WAIT(wait);
struct scatterlist dst, src;
struct fscrypt_info *ci = inode->i_crypt_info;
struct crypto_skcipher *tfm = ci->ci_ctfm;
@@ -168,7 +153,7 @@ int fscrypt_do_page_crypto(const struct inode *inode, fscrypt_direction_t rw,
skcipher_request_set_callback(
req, CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
- page_crypt_complete, &ecr);
+ crypto_req_done, &wait);
BUILD_BUG_ON(sizeof(xts_tweak) != FS_XTS_TWEAK_SIZE);
xts_tweak.index = cpu_to_le64(lblk_num);
@@ -180,14 +165,9 @@ int fscrypt_do_page_crypto(const struct inode *inode, fscrypt_direction_t rw,
sg_set_page(&src, src_page, len, offs);
skcipher_request_set_crypt(req, &src, &dst, len, &xts_tweak);
if (rw == FS_DECRYPT)
- res = crypto_skcipher_decrypt(req);
+ res = crypto_wait_req(crypto_skcipher_decrypt(req), &wait);
else
- res = crypto_skcipher_encrypt(req);
- if (res == -EINPROGRESS || res == -EBUSY) {
- BUG_ON(req->base.data != &ecr);
- wait_for_completion(&ecr.completion);
- res = ecr.res;
- }
+ res = crypto_wait_req(crypto_skcipher_encrypt(req), &wait);
skcipher_request_free(req);
if (res) {
printk_ratelimited(KERN_ERR
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index d1bb02b..d5732c9 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -15,21 +15,6 @@
#include "fscrypt_private.h"
/**
- * fname_crypt_complete() - completion callback for filename crypto
- * @req: The asynchronous cipher request context
- * @res: The result of the cipher operation
- */
-static void fname_crypt_complete(struct crypto_async_request *req, int res)
-{
- struct fscrypt_completion_result *ecr = req->data;
-
- if (res == -EINPROGRESS)
- return;
- ecr->res = res;
- complete(&ecr->completion);
-}
-
-/**
* fname_encrypt() - encrypt a filename
*
* The caller must have allocated sufficient memory for the @oname string.
@@ -40,7 +25,7 @@ static int fname_encrypt(struct inode *inode,
const struct qstr *iname, struct fscrypt_str *oname)
{
struct skcipher_request *req = NULL;
- DECLARE_FS_COMPLETION_RESULT(ecr);
+ DECLARE_CRYPTO_WAIT(ecr);
struct fscrypt_info *ci = inode->i_crypt_info;
struct crypto_skcipher *tfm = ci->ci_ctfm;
int res = 0;
@@ -76,17 +61,12 @@ static int fname_encrypt(struct inode *inode,
}
skcipher_request_set_callback(req,
CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
- fname_crypt_complete, &ecr);
+ crypto_req_done, &ecr);
sg_init_one(&sg, oname->name, cryptlen);
skcipher_request_set_crypt(req, &sg, &sg, cryptlen, iv);
/* Do the encryption */
- res = crypto_skcipher_encrypt(req);
- if (res == -EINPROGRESS || res == -EBUSY) {
- /* Request is being completed asynchronously; wait for it */
- wait_for_completion(&ecr.completion);
- res = ecr.res;
- }
+ res = crypto_wait_req(crypto_skcipher_encrypt(req), &ecr);
skcipher_request_free(req);
if (res < 0) {
printk_ratelimited(KERN_ERR
@@ -110,7 +90,7 @@ static int fname_decrypt(struct inode *inode,
struct fscrypt_str *oname)
{
struct skcipher_request *req = NULL;
- DECLARE_FS_COMPLETION_RESULT(ecr);
+ DECLARE_CRYPTO_WAIT(ecr);
struct scatterlist src_sg, dst_sg;
struct fscrypt_info *ci = inode->i_crypt_info;
struct crypto_skcipher *tfm = ci->ci_ctfm;
@@ -131,7 +111,7 @@ static int fname_decrypt(struct inode *inode,
}
skcipher_request_set_callback(req,
CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
- fname_crypt_complete, &ecr);
+ crypto_req_done, &ecr);
/* Initialize IV */
memset(iv, 0, FS_CRYPTO_BLOCK_SIZE);
@@ -140,11 +120,7 @@ static int fname_decrypt(struct inode *inode,
sg_init_one(&src_sg, iname->name, iname->len);
sg_init_one(&dst_sg, oname->name, oname->len);
skcipher_request_set_crypt(req, &src_sg, &dst_sg, iname->len, iv);
- res = crypto_skcipher_decrypt(req);
- if (res == -EINPROGRESS || res == -EBUSY) {
- wait_for_completion(&ecr.completion);
- res = ecr.res;
- }
+ res = crypto_wait_req(crypto_skcipher_decrypt(req), &ecr);
skcipher_request_free(req);
if (res < 0) {
printk_ratelimited(KERN_ERR
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 1e1f8a3..b2d130e 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -65,16 +65,6 @@ typedef enum {
#define FS_CTX_REQUIRES_FREE_ENCRYPT_FL 0x00000001
#define FS_CTX_HAS_BOUNCE_BUFFER_FL 0x00000002
-struct fscrypt_completion_result {
- struct completion completion;
- int res;
-};
-
-#define DECLARE_FS_COMPLETION_RESULT(ecr) \
- struct fscrypt_completion_result ecr = { \
- COMPLETION_INITIALIZER_ONSTACK((ecr).completion), 0 }
-
-
/* crypto.c */
extern int fscrypt_initialize(unsigned int cop_flags);
extern struct workqueue_struct *fscrypt_read_workqueue;
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 179e578..5661020 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -12,17 +12,6 @@
#include <linux/scatterlist.h>
#include "fscrypt_private.h"
-static void derive_crypt_complete(struct crypto_async_request *req, int rc)
-{
- struct fscrypt_completion_result *ecr = req->data;
-
- if (rc == -EINPROGRESS)
- return;
-
- ecr->res = rc;
- complete(&ecr->completion);
-}
-
/**
* derive_key_aes() - Derive a key using AES-128-ECB
* @deriving_key: Encryption key used for derivation.
@@ -37,7 +26,7 @@ static int derive_key_aes(u8 deriving_key[FS_AES_128_ECB_KEY_SIZE],
{
int res = 0;
struct skcipher_request *req = NULL;
- DECLARE_FS_COMPLETION_RESULT(ecr);
+ DECLARE_CRYPTO_WAIT(wait);
struct scatterlist src_sg, dst_sg;
struct crypto_skcipher *tfm = crypto_alloc_skcipher("ecb(aes)", 0, 0);
@@ -54,7 +43,7 @@ static int derive_key_aes(u8 deriving_key[FS_AES_128_ECB_KEY_SIZE],
}
skcipher_request_set_callback(req,
CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
- derive_crypt_complete, &ecr);
+ crypto_req_done, &wait);
res = crypto_skcipher_setkey(tfm, deriving_key,
FS_AES_128_ECB_KEY_SIZE);
if (res < 0)
@@ -64,11 +53,7 @@ static int derive_key_aes(u8 deriving_key[FS_AES_128_ECB_KEY_SIZE],
sg_init_one(&dst_sg, derived_key, FS_AES_256_XTS_KEY_SIZE);
skcipher_request_set_crypt(req, &src_sg, &dst_sg,
FS_AES_256_XTS_KEY_SIZE, NULL);
- res = crypto_skcipher_encrypt(req);
- if (res == -EINPROGRESS || res == -EBUSY) {
- wait_for_completion(&ecr.completion);
- res = ecr.res;
- }
+ res = crypto_wait_req(crypto_skcipher_encrypt(req), &wait);
out:
skcipher_request_free(req);
crypto_free_skcipher(tfm);
--
2.1.4
^ permalink raw reply related
* [PATCH v2 07/11] dm: move dm-verity to generic async completion
From: Gilad Ben-Yossef @ 2017-05-29 8:22 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li, Steve French,
Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn
Cc: Eric Biggers, Ofir Drang, Pavel Shilovsky, linux-crypto,
linux-doc, linux-kernel, keyrings, linux-raid, linux-cifs,
samba-technical, linux-fscrypt, linux-ima-devel, linux-ima-user,
linux-security-module
In-Reply-To: <1496046180-21962-1-git-send-email-gilad@benyossef.com>
dm-verity is starting async. crypto ops and waiting for them to complete.
Move it over to generic code doing the same.
This also fixes a possible data coruption bug created by the
use of wait_for_completion_interruptible() without dealing
correctly with an interrupt aborting the wait prior to the
async op finishing.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
drivers/md/dm-verity-target.c | 81 +++++++++++--------------------------------
drivers/md/dm-verity.h | 5 ---
2 files changed, 20 insertions(+), 66 deletions(-)
diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
index 1ec9b2c..f183b43 100644
--- a/drivers/md/dm-verity-target.c
+++ b/drivers/md/dm-verity-target.c
@@ -92,74 +92,33 @@ static sector_t verity_position_at_level(struct dm_verity *v, sector_t block,
return block >> (level * v->hash_per_block_bits);
}
-/*
- * Callback function for asynchrnous crypto API completion notification
- */
-static void verity_op_done(struct crypto_async_request *base, int err)
-{
- struct verity_result *res = (struct verity_result *)base->data;
-
- if (err == -EINPROGRESS)
- return;
-
- res->err = err;
- complete(&res->completion);
-}
-
-/*
- * Wait for async crypto API callback
- */
-static inline int verity_complete_op(struct verity_result *res, int ret)
-{
- switch (ret) {
- case 0:
- break;
-
- case -EINPROGRESS:
- case -EBUSY:
- ret = wait_for_completion_interruptible(&res->completion);
- if (!ret)
- ret = res->err;
- reinit_completion(&res->completion);
- break;
-
- default:
- DMERR("verity_wait_hash: crypto op submission failed: %d", ret);
- }
-
- if (unlikely(ret < 0))
- DMERR("verity_wait_hash: crypto op failed: %d", ret);
-
- return ret;
-}
-
static int verity_hash_update(struct dm_verity *v, struct ahash_request *req,
const u8 *data, size_t len,
- struct verity_result *res)
+ struct crypto_wait *wait)
{
struct scatterlist sg;
sg_init_one(&sg, data, len);
ahash_request_set_crypt(req, &sg, NULL, len);
- return verity_complete_op(res, crypto_ahash_update(req));
+ return crypto_wait_req(crypto_ahash_update(req), wait);
}
/*
* Wrapper for crypto_ahash_init, which handles verity salting.
*/
static int verity_hash_init(struct dm_verity *v, struct ahash_request *req,
- struct verity_result *res)
+ struct crypto_wait *wait)
{
int r;
ahash_request_set_tfm(req, v->tfm);
ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP |
CRYPTO_TFM_REQ_MAY_BACKLOG,
- verity_op_done, (void *)res);
- init_completion(&res->completion);
+ crypto_req_done, (void *)wait);
+ crypto_init_wait(wait);
- r = verity_complete_op(res, crypto_ahash_init(req));
+ r = crypto_wait_req(crypto_ahash_init(req), wait);
if (unlikely(r < 0)) {
DMERR("crypto_ahash_init failed: %d", r);
@@ -167,18 +126,18 @@ static int verity_hash_init(struct dm_verity *v, struct ahash_request *req,
}
if (likely(v->salt_size && (v->version >= 1)))
- r = verity_hash_update(v, req, v->salt, v->salt_size, res);
+ r = verity_hash_update(v, req, v->salt, v->salt_size, wait);
return r;
}
static int verity_hash_final(struct dm_verity *v, struct ahash_request *req,
- u8 *digest, struct verity_result *res)
+ u8 *digest, struct crypto_wait *wait)
{
int r;
if (unlikely(v->salt_size && (!v->version))) {
- r = verity_hash_update(v, req, v->salt, v->salt_size, res);
+ r = verity_hash_update(v, req, v->salt, v->salt_size, wait);
if (r < 0) {
DMERR("verity_hash_final failed updating salt: %d", r);
@@ -187,7 +146,7 @@ static int verity_hash_final(struct dm_verity *v, struct ahash_request *req,
}
ahash_request_set_crypt(req, NULL, digest, 0);
- r = verity_complete_op(res, crypto_ahash_final(req));
+ r = crypto_wait_req(crypto_ahash_final(req), wait);
out:
return r;
}
@@ -196,17 +155,17 @@ int verity_hash(struct dm_verity *v, struct ahash_request *req,
const u8 *data, size_t len, u8 *digest)
{
int r;
- struct verity_result res;
+ struct crypto_wait wait;
- r = verity_hash_init(v, req, &res);
+ r = verity_hash_init(v, req, &wait);
if (unlikely(r < 0))
goto out;
- r = verity_hash_update(v, req, data, len, &res);
+ r = verity_hash_update(v, req, data, len, &wait);
if (unlikely(r < 0))
goto out;
- r = verity_hash_final(v, req, digest, &res);
+ r = verity_hash_final(v, req, digest, &wait);
out:
return r;
@@ -389,7 +348,7 @@ int verity_hash_for_block(struct dm_verity *v, struct dm_verity_io *io,
* Calculates the digest for the given bio
*/
int verity_for_io_block(struct dm_verity *v, struct dm_verity_io *io,
- struct bvec_iter *iter, struct verity_result *res)
+ struct bvec_iter *iter, struct crypto_wait *wait)
{
unsigned int todo = 1 << v->data_dev_block_bits;
struct bio *bio = dm_bio_from_per_bio_data(io, v->ti->per_io_data_size);
@@ -414,7 +373,7 @@ int verity_for_io_block(struct dm_verity *v, struct dm_verity_io *io,
*/
sg_set_page(&sg, bv.bv_page, len, bv.bv_offset);
ahash_request_set_crypt(req, &sg, NULL, len);
- r = verity_complete_op(res, crypto_ahash_update(req));
+ r = crypto_wait_req(crypto_ahash_update(req), wait);
if (unlikely(r < 0)) {
DMERR("verity_for_io_block crypto op failed: %d", r);
@@ -482,7 +441,7 @@ static int verity_verify_io(struct dm_verity_io *io)
struct dm_verity *v = io->v;
struct bvec_iter start;
unsigned b;
- struct verity_result res;
+ struct crypto_wait wait;
for (b = 0; b < io->n_blocks; b++) {
int r;
@@ -507,17 +466,17 @@ static int verity_verify_io(struct dm_verity_io *io)
continue;
}
- r = verity_hash_init(v, req, &res);
+ r = verity_hash_init(v, req, &wait);
if (unlikely(r < 0))
return r;
start = io->iter;
- r = verity_for_io_block(v, io, &io->iter, &res);
+ r = verity_for_io_block(v, io, &io->iter, &wait);
if (unlikely(r < 0))
return r;
r = verity_hash_final(v, req, verity_io_real_digest(v, io),
- &res);
+ &wait);
if (unlikely(r < 0))
return r;
diff --git a/drivers/md/dm-verity.h b/drivers/md/dm-verity.h
index a59e0ad..b675bc0 100644
--- a/drivers/md/dm-verity.h
+++ b/drivers/md/dm-verity.h
@@ -90,11 +90,6 @@ struct dm_verity_io {
*/
};
-struct verity_result {
- struct completion completion;
- int err;
-};
-
static inline struct ahash_request *verity_io_hash_req(struct dm_verity *v,
struct dm_verity_io *io)
{
--
2.1.4
^ permalink raw reply related
* [PATCH v2 06/11] crypto: move testmgr to generic async completion
From: Gilad Ben-Yossef @ 2017-05-29 8:22 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li, Steve French,
Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn
Cc: linux-cifs, linux-doc, linux-ima-user, Eric Biggers,
samba-technical, linux-kernel, linux-raid, linux-fscrypt,
keyrings, linux-crypto, linux-security-module, linux-ima-devel,
Pavel Shilovsky, Ofir Drang
In-Reply-To: <1496046180-21962-1-git-send-email-gilad@benyossef.com>
testmgr is starting async. crypto ops and waiting for them to complete.
Move it over to generic code doing the same.
This also provides a test of the generic crypto async. wait code.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
crypto/testmgr.c | 184 +++++++++++++++++--------------------------------------
1 file changed, 56 insertions(+), 128 deletions(-)
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 6f5f3ed..4b779ca 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -76,11 +76,6 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
#define ENCRYPT 1
#define DECRYPT 0
-struct tcrypt_result {
- struct completion completion;
- int err;
-};
-
struct aead_test_suite {
struct {
const struct aead_testvec *vecs;
@@ -155,17 +150,6 @@ static void hexdump(unsigned char *buf, unsigned int len)
buf, len, false);
}
-static void tcrypt_complete(struct crypto_async_request *req, int err)
-{
- struct tcrypt_result *res = req->data;
-
- if (err == -EINPROGRESS)
- return;
-
- res->err = err;
- complete(&res->completion);
-}
-
static int testmgr_alloc_buf(char *buf[XBUFSIZE])
{
int i;
@@ -193,20 +177,10 @@ static void testmgr_free_buf(char *buf[XBUFSIZE])
free_page((unsigned long)buf[i]);
}
-static int wait_async_op(struct tcrypt_result *tr, int ret)
-{
- if (ret == -EINPROGRESS || ret == -EBUSY) {
- wait_for_completion(&tr->completion);
- reinit_completion(&tr->completion);
- ret = tr->err;
- }
- return ret;
-}
-
static int ahash_partial_update(struct ahash_request **preq,
struct crypto_ahash *tfm, const struct hash_testvec *template,
void *hash_buff, int k, int temp, struct scatterlist *sg,
- const char *algo, char *result, struct tcrypt_result *tresult)
+ const char *algo, char *result, struct crypto_wait *wait)
{
char *state;
struct ahash_request *req;
@@ -236,7 +210,7 @@ static int ahash_partial_update(struct ahash_request **preq,
}
ahash_request_set_callback(req,
CRYPTO_TFM_REQ_MAY_BACKLOG,
- tcrypt_complete, tresult);
+ crypto_req_done, wait);
memcpy(hash_buff, template->plaintext + temp,
template->tap[k]);
@@ -247,7 +221,7 @@ static int ahash_partial_update(struct ahash_request **preq,
pr_err("alg: hash: Failed to import() for %s\n", algo);
goto out;
}
- ret = wait_async_op(tresult, crypto_ahash_update(req));
+ ret = crypto_wait_req(crypto_ahash_update(req), wait);
if (ret)
goto out;
*preq = req;
@@ -272,7 +246,7 @@ static int __test_hash(struct crypto_ahash *tfm,
char *result;
char *key;
struct ahash_request *req;
- struct tcrypt_result tresult;
+ struct crypto_wait wait;
void *hash_buff;
char *xbuf[XBUFSIZE];
int ret = -ENOMEM;
@@ -286,7 +260,7 @@ static int __test_hash(struct crypto_ahash *tfm,
if (testmgr_alloc_buf(xbuf))
goto out_nobuf;
- init_completion(&tresult.completion);
+ crypto_init_wait(&wait);
req = ahash_request_alloc(tfm, GFP_KERNEL);
if (!req) {
@@ -295,7 +269,7 @@ static int __test_hash(struct crypto_ahash *tfm,
goto out_noreq;
}
ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- tcrypt_complete, &tresult);
+ crypto_req_done, &wait);
j = 0;
for (i = 0; i < tcount; i++) {
@@ -335,26 +309,26 @@ static int __test_hash(struct crypto_ahash *tfm,
ahash_request_set_crypt(req, sg, result, template[i].psize);
if (use_digest) {
- ret = wait_async_op(&tresult, crypto_ahash_digest(req));
+ ret = crypto_wait_req(crypto_ahash_digest(req), &wait);
if (ret) {
pr_err("alg: hash: digest failed on test %d "
"for %s: ret=%d\n", j, algo, -ret);
goto out;
}
} else {
- ret = wait_async_op(&tresult, crypto_ahash_init(req));
+ ret = crypto_wait_req(crypto_ahash_init(req), &wait);
if (ret) {
pr_err("alt: hash: init failed on test %d "
"for %s: ret=%d\n", j, algo, -ret);
goto out;
}
- ret = wait_async_op(&tresult, crypto_ahash_update(req));
+ ret = crypto_wait_req(crypto_ahash_update(req), &wait);
if (ret) {
pr_err("alt: hash: update failed on test %d "
"for %s: ret=%d\n", j, algo, -ret);
goto out;
}
- ret = wait_async_op(&tresult, crypto_ahash_final(req));
+ ret = crypto_wait_req(crypto_ahash_final(req), &wait);
if (ret) {
pr_err("alt: hash: final failed on test %d "
"for %s: ret=%d\n", j, algo, -ret);
@@ -420,22 +394,10 @@ static int __test_hash(struct crypto_ahash *tfm,
}
ahash_request_set_crypt(req, sg, result, template[i].psize);
- ret = crypto_ahash_digest(req);
- switch (ret) {
- case 0:
- break;
- case -EINPROGRESS:
- case -EBUSY:
- wait_for_completion(&tresult.completion);
- reinit_completion(&tresult.completion);
- ret = tresult.err;
- if (!ret)
- break;
- /* fall through */
- default:
- printk(KERN_ERR "alg: hash: digest failed "
- "on chunking test %d for %s: "
- "ret=%d\n", j, algo, -ret);
+ ret = crypto_wait_req(crypto_ahash_digest(req), &wait);
+ if (ret) {
+ pr_err("alg: hash: digest failed on chunking test %d for %s: ret=%d\n",
+ j, algo, -ret);
goto out;
}
@@ -486,13 +448,13 @@ static int __test_hash(struct crypto_ahash *tfm,
}
ahash_request_set_crypt(req, sg, result, template[i].tap[0]);
- ret = wait_async_op(&tresult, crypto_ahash_init(req));
+ ret = crypto_wait_req(crypto_ahash_init(req), &wait);
if (ret) {
pr_err("alt: hash: init failed on test %d for %s: ret=%d\n",
j, algo, -ret);
goto out;
}
- ret = wait_async_op(&tresult, crypto_ahash_update(req));
+ ret = crypto_wait_req(crypto_ahash_update(req), &wait);
if (ret) {
pr_err("alt: hash: update failed on test %d for %s: ret=%d\n",
j, algo, -ret);
@@ -503,7 +465,7 @@ static int __test_hash(struct crypto_ahash *tfm,
for (k = 1; k < template[i].np; k++) {
ret = ahash_partial_update(&req, tfm, &template[i],
hash_buff, k, temp, &sg[0], algo, result,
- &tresult);
+ &wait);
if (ret) {
pr_err("hash: partial update failed on test %d for %s: ret=%d\n",
j, algo, -ret);
@@ -511,7 +473,7 @@ static int __test_hash(struct crypto_ahash *tfm,
}
temp += template[i].tap[k];
}
- ret = wait_async_op(&tresult, crypto_ahash_final(req));
+ ret = crypto_wait_req(crypto_ahash_final(req), &wait);
if (ret) {
pr_err("alt: hash: final failed on test %d for %s: ret=%d\n",
j, algo, -ret);
@@ -580,7 +542,7 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
struct scatterlist *sg;
struct scatterlist *sgout;
const char *e, *d;
- struct tcrypt_result result;
+ struct crypto_wait wait;
unsigned int authsize, iv_len;
void *input;
void *output;
@@ -619,7 +581,7 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
else
e = "decryption";
- init_completion(&result.completion);
+ crypto_init_wait(&wait);
req = aead_request_alloc(tfm, GFP_KERNEL);
if (!req) {
@@ -629,7 +591,7 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
}
aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- tcrypt_complete, &result);
+ crypto_req_done, &wait);
iv_len = crypto_aead_ivsize(tfm);
@@ -709,7 +671,8 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
aead_request_set_ad(req, template[i].alen);
- ret = enc ? crypto_aead_encrypt(req) : crypto_aead_decrypt(req);
+ ret = crypto_wait_req(enc ? crypto_aead_encrypt(req)
+ : crypto_aead_decrypt(req), &wait);
switch (ret) {
case 0:
@@ -722,13 +685,6 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
goto out;
}
break;
- case -EINPROGRESS:
- case -EBUSY:
- wait_for_completion(&result.completion);
- reinit_completion(&result.completion);
- ret = result.err;
- if (!ret)
- break;
case -EBADMSG:
if (template[i].novrfy)
/* verification failure was expected */
@@ -866,7 +822,8 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
aead_request_set_ad(req, template[i].alen);
- ret = enc ? crypto_aead_encrypt(req) : crypto_aead_decrypt(req);
+ ret = crypto_wait_req(enc ? crypto_aead_encrypt(req)
+ : crypto_aead_decrypt(req), &wait);
switch (ret) {
case 0:
@@ -879,13 +836,6 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
goto out;
}
break;
- case -EINPROGRESS:
- case -EBUSY:
- wait_for_completion(&result.completion);
- reinit_completion(&result.completion);
- ret = result.err;
- if (!ret)
- break;
case -EBADMSG:
if (template[i].novrfy)
/* verification failure was expected */
@@ -1083,7 +1033,7 @@ static int __test_skcipher(struct crypto_skcipher *tfm, int enc,
struct scatterlist sg[8];
struct scatterlist sgout[8];
const char *e, *d;
- struct tcrypt_result result;
+ struct crypto_wait wait;
void *data;
char iv[MAX_IVLEN];
char *xbuf[XBUFSIZE];
@@ -1107,7 +1057,7 @@ static int __test_skcipher(struct crypto_skcipher *tfm, int enc,
else
e = "decryption";
- init_completion(&result.completion);
+ crypto_init_wait(&wait);
req = skcipher_request_alloc(tfm, GFP_KERNEL);
if (!req) {
@@ -1117,7 +1067,7 @@ static int __test_skcipher(struct crypto_skcipher *tfm, int enc,
}
skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- tcrypt_complete, &result);
+ crypto_req_done, &wait);
j = 0;
for (i = 0; i < tcount; i++) {
@@ -1164,23 +1114,12 @@ static int __test_skcipher(struct crypto_skcipher *tfm, int enc,
skcipher_request_set_crypt(req, sg, (diff_dst) ? sgout : sg,
template[i].ilen, iv);
- ret = enc ? crypto_skcipher_encrypt(req) :
- crypto_skcipher_decrypt(req);
+ ret = crypto_wait_req(enc ? crypto_skcipher_encrypt(req) :
+ crypto_skcipher_decrypt(req), &wait);
- switch (ret) {
- case 0:
- break;
- case -EINPROGRESS:
- case -EBUSY:
- wait_for_completion(&result.completion);
- reinit_completion(&result.completion);
- ret = result.err;
- if (!ret)
- break;
- /* fall through */
- default:
+ if (ret) {
pr_err("alg: skcipher%s: %s failed on test %d for %s: ret=%d\n",
- d, e, j, algo, -ret);
+ d, e, j, algo, -ret);
goto out;
}
@@ -1272,21 +1211,10 @@ static int __test_skcipher(struct crypto_skcipher *tfm, int enc,
skcipher_request_set_crypt(req, sg, (diff_dst) ? sgout : sg,
template[i].ilen, iv);
- ret = enc ? crypto_skcipher_encrypt(req) :
- crypto_skcipher_decrypt(req);
+ ret = crypto_wait_req(enc ? crypto_skcipher_encrypt(req) :
+ crypto_skcipher_decrypt(req), &wait);
- switch (ret) {
- case 0:
- break;
- case -EINPROGRESS:
- case -EBUSY:
- wait_for_completion(&result.completion);
- reinit_completion(&result.completion);
- ret = result.err;
- if (!ret)
- break;
- /* fall through */
- default:
+ if (ret) {
pr_err("alg: skcipher%s: %s failed on chunk test %d for %s: ret=%d\n",
d, e, j, algo, -ret);
goto out;
@@ -1462,7 +1390,7 @@ static int test_acomp(struct crypto_acomp *tfm,
int ret;
struct scatterlist src, dst;
struct acomp_req *req;
- struct tcrypt_result result;
+ struct crypto_wait wait;
output = kmalloc(COMP_BUF_SIZE, GFP_KERNEL);
if (!output)
@@ -1486,7 +1414,7 @@ static int test_acomp(struct crypto_acomp *tfm,
}
memset(output, 0, dlen);
- init_completion(&result.completion);
+ crypto_init_wait(&wait);
sg_init_one(&src, input_vec, ilen);
sg_init_one(&dst, output, dlen);
@@ -1501,9 +1429,9 @@ static int test_acomp(struct crypto_acomp *tfm,
acomp_request_set_params(req, &src, &dst, ilen, dlen);
acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- tcrypt_complete, &result);
+ crypto_req_done, &wait);
- ret = wait_async_op(&result, crypto_acomp_compress(req));
+ ret = crypto_wait_req(crypto_acomp_compress(req), &wait);
if (ret) {
pr_err("alg: acomp: compression failed on test %d for %s: ret=%d\n",
i + 1, algo, -ret);
@@ -1516,10 +1444,10 @@ static int test_acomp(struct crypto_acomp *tfm,
dlen = COMP_BUF_SIZE;
sg_init_one(&src, output, ilen);
sg_init_one(&dst, decomp_out, dlen);
- init_completion(&result.completion);
+ crypto_init_wait(&wait);
acomp_request_set_params(req, &src, &dst, ilen, dlen);
- ret = wait_async_op(&result, crypto_acomp_decompress(req));
+ ret = crypto_wait_req(crypto_acomp_decompress(req), &wait);
if (ret) {
pr_err("alg: acomp: compression failed on test %d for %s: ret=%d\n",
i + 1, algo, -ret);
@@ -1563,7 +1491,7 @@ static int test_acomp(struct crypto_acomp *tfm,
}
memset(output, 0, dlen);
- init_completion(&result.completion);
+ crypto_init_wait(&wait);
sg_init_one(&src, input_vec, ilen);
sg_init_one(&dst, output, dlen);
@@ -1578,9 +1506,9 @@ static int test_acomp(struct crypto_acomp *tfm,
acomp_request_set_params(req, &src, &dst, ilen, dlen);
acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- tcrypt_complete, &result);
+ crypto_req_done, &wait);
- ret = wait_async_op(&result, crypto_acomp_decompress(req));
+ ret = crypto_wait_req(crypto_acomp_decompress(req), &wait);
if (ret) {
pr_err("alg: acomp: decompression failed on test %d for %s: ret=%d\n",
i + 1, algo, -ret);
@@ -1997,7 +1925,7 @@ static int do_test_kpp(struct crypto_kpp *tfm, const struct kpp_testvec *vec,
struct kpp_request *req;
void *input_buf = NULL;
void *output_buf = NULL;
- struct tcrypt_result result;
+ struct crypto_wait wait;
unsigned int out_len_max;
int err = -ENOMEM;
struct scatterlist src, dst;
@@ -2006,7 +1934,7 @@ static int do_test_kpp(struct crypto_kpp *tfm, const struct kpp_testvec *vec,
if (!req)
return err;
- init_completion(&result.completion);
+ crypto_init_wait(&wait);
err = crypto_kpp_set_secret(tfm, vec->secret, vec->secret_size);
if (err < 0)
@@ -2024,10 +1952,10 @@ static int do_test_kpp(struct crypto_kpp *tfm, const struct kpp_testvec *vec,
sg_init_one(&dst, output_buf, out_len_max);
kpp_request_set_output(req, &dst, out_len_max);
kpp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- tcrypt_complete, &result);
+ crypto_req_done, &wait);
/* Compute public key */
- err = wait_async_op(&result, crypto_kpp_generate_public_key(req));
+ err = crypto_wait_req(crypto_kpp_generate_public_key(req), &wait);
if (err) {
pr_err("alg: %s: generate public key test failed. err %d\n",
alg, err);
@@ -2055,8 +1983,8 @@ static int do_test_kpp(struct crypto_kpp *tfm, const struct kpp_testvec *vec,
kpp_request_set_input(req, &src, vec->b_public_size);
kpp_request_set_output(req, &dst, out_len_max);
kpp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- tcrypt_complete, &result);
- err = wait_async_op(&result, crypto_kpp_compute_shared_secret(req));
+ crypto_req_done, &wait);
+ err = crypto_wait_req(crypto_kpp_compute_shared_secret(req), &wait);
if (err) {
pr_err("alg: %s: compute shard secret test failed. err %d\n",
alg, err);
@@ -2125,7 +2053,7 @@ static int test_akcipher_one(struct crypto_akcipher *tfm,
struct akcipher_request *req;
void *outbuf_enc = NULL;
void *outbuf_dec = NULL;
- struct tcrypt_result result;
+ struct crypto_wait wait;
unsigned int out_len_max, out_len = 0;
int err = -ENOMEM;
struct scatterlist src, dst, src_tab[2];
@@ -2137,7 +2065,7 @@ static int test_akcipher_one(struct crypto_akcipher *tfm,
if (!req)
goto free_xbuf;
- init_completion(&result.completion);
+ crypto_init_wait(&wait);
if (vecs->public_key_vec)
err = crypto_akcipher_set_pub_key(tfm, vecs->key,
@@ -2166,10 +2094,10 @@ static int test_akcipher_one(struct crypto_akcipher *tfm,
akcipher_request_set_crypt(req, src_tab, &dst, vecs->m_size,
out_len_max);
akcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- tcrypt_complete, &result);
+ crypto_req_done, &wait);
/* Run RSA encrypt - c = m^e mod n;*/
- err = wait_async_op(&result, crypto_akcipher_encrypt(req));
+ err = crypto_wait_req(crypto_akcipher_encrypt(req), &wait);
if (err) {
pr_err("alg: akcipher: encrypt test failed. err %d\n", err);
goto free_all;
@@ -2204,11 +2132,11 @@ static int test_akcipher_one(struct crypto_akcipher *tfm,
sg_init_one(&src, xbuf[0], vecs->c_size);
sg_init_one(&dst, outbuf_dec, out_len_max);
- init_completion(&result.completion);
+ crypto_init_wait(&wait);
akcipher_request_set_crypt(req, &src, &dst, vecs->c_size, out_len_max);
/* Run RSA decrypt - m = c^d mod n;*/
- err = wait_async_op(&result, crypto_akcipher_decrypt(req));
+ err = crypto_wait_req(crypto_akcipher_decrypt(req), &wait);
if (err) {
pr_err("alg: akcipher: decrypt test failed. err %d\n", err);
goto free_all;
--
2.1.4
^ permalink raw reply related
* [PATCH v2 05/11] crypto: move gcm to generic async completion
From: Gilad Ben-Yossef @ 2017-05-29 8:22 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li, Steve French,
Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn
Cc: Eric Biggers, Ofir Drang, Pavel Shilovsky, linux-crypto,
linux-doc, linux-kernel, keyrings, linux-raid, linux-cifs,
samba-technical, linux-fscrypt, linux-ima-devel, linux-ima-user,
linux-security-module
In-Reply-To: <1496046180-21962-1-git-send-email-gilad@benyossef.com>
gcm is starting an async. crypto op and waiting for it complete.
Move it over to generic code doing the same.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
crypto/gcm.c | 32 ++++++--------------------------
1 file changed, 6 insertions(+), 26 deletions(-)
diff --git a/crypto/gcm.c b/crypto/gcm.c
index 3841b5e..fb923a5 100644
--- a/crypto/gcm.c
+++ b/crypto/gcm.c
@@ -16,7 +16,6 @@
#include <crypto/scatterwalk.h>
#include <crypto/hash.h>
#include "internal.h"
-#include <linux/completion.h>
#include <linux/err.h>
#include <linux/init.h>
#include <linux/kernel.h>
@@ -78,11 +77,6 @@ struct crypto_gcm_req_priv_ctx {
} u;
};
-struct crypto_gcm_setkey_result {
- int err;
- struct completion completion;
-};
-
static struct {
u8 buf[16];
struct scatterlist sg;
@@ -98,17 +92,6 @@ static inline struct crypto_gcm_req_priv_ctx *crypto_gcm_reqctx(
return (void *)PTR_ALIGN((u8 *)aead_request_ctx(req), align + 1);
}
-static void crypto_gcm_setkey_done(struct crypto_async_request *req, int err)
-{
- struct crypto_gcm_setkey_result *result = req->data;
-
- if (err == -EINPROGRESS)
- return;
-
- result->err = err;
- complete(&result->completion);
-}
-
static int crypto_gcm_setkey(struct crypto_aead *aead, const u8 *key,
unsigned int keylen)
{
@@ -119,7 +102,7 @@ static int crypto_gcm_setkey(struct crypto_aead *aead, const u8 *key,
be128 hash;
u8 iv[16];
- struct crypto_gcm_setkey_result result;
+ struct crypto_wait wait;
struct scatterlist sg[1];
struct skcipher_request req;
@@ -140,21 +123,18 @@ static int crypto_gcm_setkey(struct crypto_aead *aead, const u8 *key,
if (!data)
return -ENOMEM;
- init_completion(&data->result.completion);
+ crypto_init_wait(&data->wait);
sg_init_one(data->sg, &data->hash, sizeof(data->hash));
skcipher_request_set_tfm(&data->req, ctr);
skcipher_request_set_callback(&data->req, CRYPTO_TFM_REQ_MAY_SLEEP |
CRYPTO_TFM_REQ_MAY_BACKLOG,
- crypto_gcm_setkey_done,
- &data->result);
+ crypto_req_done,
+ &data->wait);
skcipher_request_set_crypt(&data->req, data->sg, data->sg,
sizeof(data->hash), data->iv);
- err = crypto_skcipher_encrypt(&data->req);
- if (err == -EINPROGRESS || err == -EBUSY) {
- wait_for_completion(&data->result.completion);
- err = data->result.err;
- }
+ err = crypto_wait_req(crypto_skcipher_encrypt(&data->req),
+ &data->wait);
if (err)
goto out;
--
2.1.4
^ permalink raw reply related
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