Re: [dm-devel] [PATCH 25/27] block: remove the discard_zeroes_data flag
On Wed, May 10, 2017 at 09:50:35PM -0700, Nicholas A. Bellinger wrote: > 1) Expose a block_device or request_queue bit to signal 'real LBPRZ' > support up to IBLOCK, in order to maintain SCSI target feature > compatibility. No way. If you want to zero use REQ_OP_WRITE_ZEROES.. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] multipath-tools:Prioritizer based on a time-delay algorithm
Hi Xose and Christophe, On 2017/5/11 6:36, Xose Vazquez Perez wrote: > On 05/08/2017 05:58 AM, Yang Feng wrote: > >> Prioritizer for device mapper multipath, where the corresponding priority >> values of specific paths are provided by a time-delay algorithm. And the >> time-delay algorithm is dependent on the following arguments(delay_interval, >> cons_num). > This new feature should be documented in multipath/multipath.conf.5 > multipath/multipath.conf.5 has be documented in the following patch. >> diff --git a/libmultipath/checkers/Makefile b/libmultipath/checkers/Makefile >> index 4970fc0..7e433ca 100644 >> --- a/libmultipath/checkers/Makefile >> +++ b/libmultipath/checkers/Makefile >> @@ -14,19 +14,16 @@ LIBS= \ >> libcheckemc_clariion.so \ >> libcheckhp_sw.so \ >> libcheckrdac.so >> -ifneq ($(ENABLE_RADOS),0) >> -LIBS += libcheckrbd.so >> -endif > > Is it right? > > Thanks, fixed as the flollowing patch. --- Prioritizer for device mapper multipath, where the corresponding priority values of specific paths are provided by a time-delay algorithm. And the time-delay algorithm is dependent on the following arguments(delay_interval, cons_num). The principle of the algorithm is illustrated as follows: 1. By sending a certain number "cons_num" of read IOs to the current path continuously, the IOs' average delay can be calculated. 2. According to the average delay of each path and the weight value "delay_interval", the priority "rc" of each path can be provided. delay_interval delay_interval delay_interval delay_interval |---|---|---||---| |priority rank 1|priority rank 2|priority rank 3|... |priority rank x| |---|---|---||---| Priority Rank Partitioning --- libmultipath/Makefile | 2 +- libmultipath/checkers/Makefile | 4 +- libmultipath/checkers/emc_clariion.c| 2 +- libmultipath/checkers/libsg.c | 94 libmultipath/checkers/libsg.h | 9 -- libmultipath/checkers/readsector0.c | 2 +- libmultipath/libsg.c| 94 libmultipath/libsg.h| 9 ++ libmultipath/prioritizers/Makefile | 6 +- libmultipath/prioritizers/delayedpath.c | 246 libmultipath/prioritizers/delayedpath.h | 14 ++ multipath/multipath.conf.5 | 19 +++ 12 files changed, 392 insertions(+), 109 deletions(-) delete mode 100644 libmultipath/checkers/libsg.c delete mode 100644 libmultipath/checkers/libsg.h create mode 100644 libmultipath/libsg.c create mode 100644 libmultipath/libsg.h create mode 100644 libmultipath/prioritizers/delayedpath.c create mode 100644 libmultipath/prioritizers/delayedpath.h diff --git a/libmultipath/Makefile b/libmultipath/Makefile index 1f5ec25..a4d725a 100644 --- a/libmultipath/Makefile +++ b/libmultipath/Makefile @@ -41,7 +41,7 @@ OBJS = memory.o parser.o vector.o devmapper.o callout.o \ structs.o discovery.o propsel.o dict.o \ pgpolicies.o debug.o defaults.o uevent.o time-util.o \ switchgroup.o uxsock.o print.o alias.o log_pthread.o \ - log.o configure.o structs_vec.o sysfs.o prio.o checkers.o \ + log.o configure.o structs_vec.o sysfs.o libsg.o prio.o checkers.o \ lock.o waiter.o file.o wwids.o prioritizers/alua_rtpg.o all: $(LIBS) diff --git a/libmultipath/checkers/Makefile b/libmultipath/checkers/Makefile index 4970fc0..7e433ca 100644 --- a/libmultipath/checkers/Makefile +++ b/libmultipath/checkers/Makefile @@ -14,19 +14,16 @@ LIBS= \ libcheckemc_clariion.so \ libcheckhp_sw.so \ libcheckrdac.so ifneq ($(ENABLE_RADOS),0) LIBS += libcheckrbd.so endif all: $(LIBS) libcheckrbd.so: rbd.o $(CC) $(LDFLAGS) $(SHARED_FLAGS) -o $@ $^ -lrados -ludev -libcheckdirectio.so: libsg.o directio.o +libcheckdirectio.so: ../libsg.o directio.o $(CC) $(LDFLAGS) $(SHARED_FLAGS) -o $@ $^ -laio -libcheck%.so: libsg.o %.o +libcheck%.so: ../libsg.o %.o $(CC) $(LDFLAGS) $(SHARED_FLAGS) -o $@ $^ install: diff --git a/libmultipath/checkers/emc_clariion.c b/libmultipath/checkers/emc_clariion.c index 9c1ffed..e4ba757 100644 --- a/libmultipath/checkers/emc_clariion.c +++ b/libmultipath/checkers/emc_clariion.c @@ -12,7 +12,7 @@ #include #include "../libmultipath/sg_include.h" -#include "libsg.h" +#include "../libmultipath/libsg.h" #include "checkers.h" #include "debug.h" #include "memory.h" diff --git a/libmultipath/checkers/libsg.c b/libmultipath/checkers/libsg.c deleted file mode 100644 index 958ea92..000 --- a/libmultipath/checkers/libsg.c +++ /dev/null @@ -1,94 +0,0 @@ -/* - * Copyright (c) 2004, 2005 Christophe Varoqui - */ -#include -#include -#include -#include - -#include "checkers.h" -#include "libsg.h" -#include "../libmultipath/sg_incl
Re: [dm-devel] [PATCH 4/5] kpartx: default to running in sync mode
On 04/25/2017 12:39 AM, Benjamin Marzinski wrote:
> When users run kpartx, they would naturally assume that when it
> completes, the devices have been created. However, kpartx runs in async
> mode by default. This seems like it is likely to trip up users. So,
> switch the default to sync mode, add a -n option to enable async mode,
> and set async mode when kpartx is called by the udev rules.
>[...]
> diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
> index 58e60ff..d1edd5e 100644
> --- a/kpartx/kpartx.c
> +++ b/kpartx/kpartx.c
>[...]
> -static char short_opts[] = "rladfgvp:t:su";
> +static char short_opts[] = "rladfgvp:t:snu";
>[...]
> - printf("\t-s sync mode. Don't return until the partitions are
> created\n");
> + printf("\t-n nosync mode. Return before the partitions are created\n");
> + printf("\t-s sync mode. Don't return until the partitions are created.
> Default.\n");
> return 1;
>[...]> + case 'n':
> + udev_sync = 0;
> + break;
New flags should be documented in its man page.
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] multipath-tools:Prioritizer based on a time-delay algorithm
On 05/08/2017 05:58 AM, Yang Feng wrote: > Prioritizer for device mapper multipath, where the corresponding priority > values of specific paths are provided by a time-delay algorithm. And the > time-delay algorithm is dependent on the following arguments(delay_interval, > cons_num). This new feature should be documented in multipath/multipath.conf.5 > diff --git a/libmultipath/checkers/Makefile b/libmultipath/checkers/Makefile > index 4970fc0..7e433ca 100644 > --- a/libmultipath/checkers/Makefile > +++ b/libmultipath/checkers/Makefile > @@ -14,19 +14,16 @@ LIBS= \ > libcheckemc_clariion.so \ > libcheckhp_sw.so \ > libcheckrdac.so > -ifneq ($(ENABLE_RADOS),0) > -LIBS += libcheckrbd.so > -endif Is it right? -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] need to pick a solution for dm-crypt IV generation and do it! [was: Re: dm: submit stacked requests in irq enabled context]
On Wed, May 10, 2017 at 5:45 PM, Mike Snitzer wrote: > On Wed, May 10 2017 at 9:37am -0400, > Gilad Ben-Yossef wrote: > >> On Wed, May 10, 2017 at 11:49 AM, Neeraj Soni >> wrote: >> > Hi Keith, >> > >> > Request based dm (dm-req-crypt) is being used for Disk Encryption solution >> > in Android used by Google. Also as i mentioned reverting this fix improves >> > the RR/RW numbers so this proves the request based dm is coming into path >> > and is being used. >> >> Sadly, that is an out of tree module. >> >> Does it still use Qcom specific APIs in its implementation (qcrypto_* funcs)? >> It did the last time I've checked - and the driver that implements >> those is not upstream either... >> >> It makes it difficult to help - which is a shame since I am interested >> in enabling higher performance >> of dm-crypt when using HW based crypto transformation myself. > > I have absolutely no interest in request-based dm-crypt. It is a hack > to work-around limitations in crypto IV generation. I agree. This is why I've said I'm interested in a high performance dm-crypt, not "request based dm-crypt". They are trying to solve the same problem but with the wrong solution and doing so out of upstream. As the parlance of our time seems to go... sad. :-) Cheers, Gilad -- Gilad Ben-Yossef Chief Coffee Drinker "If you take a class in large-scale robotics, can you end up in a situation where the homework eats your dog?" -- Jean-Baptiste Queru -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] need to pick a solution for dm-crypt IV generation and do it! [was: Re: dm: submit stacked requests in irq enabled context]
On Wed, May 10 2017 at 9:37am -0400, Gilad Ben-Yossef wrote: > On Wed, May 10, 2017 at 11:49 AM, Neeraj Soni wrote: > > Hi Keith, > > > > Request based dm (dm-req-crypt) is being used for Disk Encryption solution > > in Android used by Google. Also as i mentioned reverting this fix improves > > the RR/RW numbers so this proves the request based dm is coming into path > > and is being used. > > Sadly, that is an out of tree module. > > Does it still use Qcom specific APIs in its implementation (qcrypto_* funcs)? > It did the last time I've checked - and the driver that implements > those is not upstream either... > > It makes it difficult to help - which is a shame since I am interested > in enabling higher performance > of dm-crypt when using HW based crypto transformation myself. I have absolutely no interest in request-based dm-crypt. It is a hack to work-around limitations in crypto IV generation. If "google" is foolish enough to deploy out-of-tree request-based dm-crypt in their android kernel then they are easily capable of reverting the commit in question to prop up their short-cited decision. The correct way forward is to follow through with the crypto work discussed here (pick a solution to implement and make it happen): https://www.redhat.com/archives/dm-devel/2017-March/msg00044.html and here: https://www.redhat.com/archives/dm-devel/2017-March/msg00053.html and here: https://www.redhat.com/archives/dm-devel/2017-April/msg00132.html Mike -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH 25/27] block: remove the discard_zeroes_data flag
On Mon, May 08, 2017 at 11:46:14PM -0700, Nicholas A. Bellinger wrote: > That said, simply propagating up q->limits.max_write_zeroes_sectors as > dev_attrib->unmap_zeroes_data following existing code still looks like > the right thing to do. It is not. Martin has decoupled write same/zeroes support from discard support. Any device will claim to support it initially, and we'll only clear the flag if a Write Same command fails. So even if LBPRZ is not set you can trivially get into a situation where discard is supported through UNMAP, and you'll incorrectly set LBPRZ and will cause data corruption. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] dm: submit stacked requests in irq enabled context
On Wed, May 10, 2017 at 11:49 AM, Neeraj Soni wrote: > Hi Keith, > > Request based dm (dm-req-crypt) is being used for Disk Encryption solution > in Android used by Google. Also as i mentioned reverting this fix improves > the RR/RW numbers so this proves the request based dm is coming into path > and is being used. Sadly, that is an out of tree module. Does it still use Qcom specific APIs in its implementation (qcrypto_* funcs)? It did the last time I've checked - and the driver that implements those is not upstream either... It makes it difficult to help - which is a shame since I am interested in enabling higher performance of dm-crypt when using HW based crypto transformation myself. Gilad -- Gilad Ben-Yossef Chief Coffee Drinker "If you take a class in large-scale robotics, can you end up in a situation where the homework eats your dog?" -- Jean-Baptiste Queru -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] dm ioctl: make dm_open and dm_release release
On 10/05/17 10:18, Colin King wrote:
> From: Colin Ian King
>
> Making dm_open and dm_release static fixes the sparse warnings:
>
> warning: symbol 'dm_open' was not declared. Should it be static?
> warning: symbol 'dm_release' was not declared. Should it be static?
>
> Fixes: ab2c6224f4c0c ("dm: a basic support for using the select or poll
> function")
> Signed-off-by: Colin Ian King
> ---
> drivers/md/dm-ioctl.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
> index 70dc5db90ef2..d45c68115d4c 100644
> --- a/drivers/md/dm-ioctl.c
> +++ b/drivers/md/dm-ioctl.c
> @@ -1893,7 +1893,7 @@ static long dm_compat_ctl_ioctl(struct file *file, uint
> command, ulong u)
> #define dm_compat_ctl_ioctl NULL
> #endif
>
> -int dm_open(struct inode *inode, struct file *filp)
> +static int dm_open(struct inode *inode, struct file *filp)
> {
> int r;
> struct dm_file *priv;
> @@ -1911,7 +1911,7 @@ int dm_open(struct inode *inode, struct file *filp)
> return 0;
> }
>
> -int dm_release(struct inode *inode, struct file *filp)
> +static int dm_release(struct inode *inode, struct file *filp)
> {
> kfree(filp->private_data);
> return 0;
>
Sorry, ignore this, it had a mistake in the subject line
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH][V2] dm ioctl: make dm_open and dm_release release static
From: Colin Ian King
Making dm_open and dm_release static fixes the sparse warnings:
warning: symbol 'dm_open' was not declared. Should it be static?
warning: symbol 'dm_release' was not declared. Should it be static?
Fixes: ab2c6224f4c0c ("dm: a basic support for using the select or poll
function")
Signed-off-by: Colin Ian King
---
drivers/md/dm-ioctl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 70dc5db90ef2..d45c68115d4c 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1893,7 +1893,7 @@ static long dm_compat_ctl_ioctl(struct file *file, uint
command, ulong u)
#define dm_compat_ctl_ioctl NULL
#endif
-int dm_open(struct inode *inode, struct file *filp)
+static int dm_open(struct inode *inode, struct file *filp)
{
int r;
struct dm_file *priv;
@@ -1911,7 +1911,7 @@ int dm_open(struct inode *inode, struct file *filp)
return 0;
}
-int dm_release(struct inode *inode, struct file *filp)
+static int dm_release(struct inode *inode, struct file *filp)
{
kfree(filp->private_data);
return 0;
--
2.11.0
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] dm: submit stacked requests in irq enabled context
Hi Keith, Request based dm (dm-req-crypt) is being used for Disk Encryption solution in Android used by Google. Also as i mentioned reverting this fix improves the RR/RW numbers so this proves the request based dm is coming into path and is being used. Neeraj -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project On 5/10/2017 12:47 PM, Christoph Hellwig wrote: On Tue, May 09, 2017 at 11:55:55AM -0400, Keith Busch wrote: I have recently started using kernel 4.4 on a Android device and ran Androbench to check storage read/write performance. I found that the Random Read (RR) and Random write(RW) performance with Full Disk Encryption is degraded compared to no Disk Encryption. Initially i dm-crypt doesn't even use request based device mapper. Something odd must be going on with your benchmarks. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH] dm ioctl: make dm_open and dm_release release
From: Colin Ian King
Making dm_open and dm_release static fixes the sparse warnings:
warning: symbol 'dm_open' was not declared. Should it be static?
warning: symbol 'dm_release' was not declared. Should it be static?
Fixes: ab2c6224f4c0c ("dm: a basic support for using the select or poll
function")
Signed-off-by: Colin Ian King
---
drivers/md/dm-ioctl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 70dc5db90ef2..d45c68115d4c 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1893,7 +1893,7 @@ static long dm_compat_ctl_ioctl(struct file *file, uint
command, ulong u)
#define dm_compat_ctl_ioctl NULL
#endif
-int dm_open(struct inode *inode, struct file *filp)
+static int dm_open(struct inode *inode, struct file *filp)
{
int r;
struct dm_file *priv;
@@ -1911,7 +1911,7 @@ int dm_open(struct inode *inode, struct file *filp)
return 0;
}
-int dm_release(struct inode *inode, struct file *filp)
+static int dm_release(struct inode *inode, struct file *filp)
{
kfree(filp->private_data);
return 0;
--
2.11.0
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] dm: submit stacked requests in irq enabled context
On Tue, May 09, 2017 at 11:55:55AM -0400, Keith Busch wrote: > > I have recently started using kernel 4.4 on a Android device and ran > > Androbench to check storage read/write performance. I found that the > > Random Read (RR) and Random write(RW) performance with Full Disk > > Encryption is degraded compared to no Disk Encryption. Initially i dm-crypt doesn't even use request based device mapper. Something odd must be going on with your benchmarks. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] dm: submit stacked requests in irq enabled context
As of now i can not move to the latest stable version (4.11 is what i see in https://www.kernel.org/) since we do not have support available for that. I assume that this patch will be present even in 4.11 so effectively if no remedy is brought in in 4.11 the problem will still exist since we are dealing with an additional thread and scheduling delays. Neeraj -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project On 5/9/2017 9:25 PM, Keith Busch wrote: On Tue, May 09, 2017 at 03:19:31PM +0530, Neeraj Soni wrote: + Alasdair and dm-devel for awareness and inputs. On 5/9/2017 12:26 PM, Neeraj Soni wrote: Hi Keith/Snitzer, I have recently started using kernel 4.4 on a Android device and ran Androbench to check storage read/write performance. I found that the Random Read (RR) and Random write(RW) performance with Full Disk Encryption is degraded compared to no Disk Encryption. Initially i thought this must the issue with the storage part used and i compared the performance of similar storage part on a device that was using Android with kernel 3.18. I found that with no Disk Encryption the performance was equivalent to the device which was using 4.4 but with Disk Encryption there was degradation in RR (~20%) and RW(10%). I then tried to compare the changes that was brought in kernel 4.4 in Full Disk Encryption path. I came across the patch mentioned in subject and found that now a worker thread is scheduled in dm_request_fn() to process the requests instead of directly invoking map_request() as was in kernel 3.18. I reverted this patch and found that the RR and RW performance was now closer to what we have without Disk Encryption. From the commit message i understand that this change is significant and will be required for blk-mq support but have you came across such degradation issue with your patch and do we have any fix for this degradation available? Just for comparison, could you check performance on a more recent stable kernel? -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
