Re: [RFC 12/12] iscsi-target: Add Makefile/Kconfig and update TCM top level
On Tue, 2011-03-08 at 17:33 +0800, Herbert Xu wrote:
> Nicholas A. Bellinger wrote:
> >
> >> > I should mention this is with the following .config:
> >> >
> >> > CONFIG_CRYPTO_CRC32C=y
> >> > CONFIG_CRYPTO_CRC32C_INTEL=m
>
> This is why you get the unoptimised version. Had you selected
> both as built-in or both as modules, then it would have worked
> as intended.
>
> > What about the following to simply call request_module("crc32c_intel")
> > at module_init() time and top the extra iscsi_login_setup_crypto()
> > code..?
>
> If we're going to do this we should do it in the crypto layer,
> and not litter every single crypto API user with such crap.
>
> Currently we don't invoke request_module unless no implementation
> is reigstered for an algorithm. You can change this so that it
> also invokes request_module if we have not yet done so at least
> once for that algorithm.
>
> Patches are welcome.
>
Ok, fair enough point.. I have addressed this with a new struct
crypto_alg->cra_check_optimized() callback in order for crc32c.ko to
have a method to call request_module("crc32c_intel.ko") after the base
software alg has been loaded.
This is working w/ CONFIG_CRYPTO_CRC32C=y + CONFIG_CRYPTO_CRC32C_INTEL=m
case and should satisfy current (and future) architecture dependent
cases for CRC32C HW offload.
Sending out a patch series for your comments shortly..
Thanks!
--nab
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 12/12] iscsi-target: Add Makefile/Kconfig and update TCM top level
Nicholas A. Bellinger wrote:
>
>> > I should mention this is with the following .config:
>> >
>> > CONFIG_CRYPTO_CRC32C=y
>> > CONFIG_CRYPTO_CRC32C_INTEL=m
This is why you get the unoptimised version. Had you selected
both as built-in or both as modules, then it would have worked
as intended.
> What about the following to simply call request_module("crc32c_intel")
> at module_init() time and top the extra iscsi_login_setup_crypto()
> code..?
If we're going to do this we should do it in the crypto layer,
and not litter every single crypto API user with such crap.
Currently we don't invoke request_module unless no implementation
is reigstered for an algorithm. You can change this so that it
also invokes request_module if we have not yet done so at least
once for that algorithm.
Patches are welcome.
Cheers,
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 12/12] iscsi-target: Add Makefile/Kconfig and update TCM top level
On Fri, 2011-03-04 at 11:00 -0600, James Bottomley wrote:
> On Thu, 2011-03-03 at 12:58 -0800, Nicholas A. Bellinger wrote:
> > On Thu, 2011-03-03 at 09:19 -0500, Christoph Hellwig wrote:
> > > On Wed, Mar 02, 2011 at 01:32:11PM -0800, Nicholas A. Bellinger wrote:
> > > > The kernel code itself that is specific to using the SSE v4.2
> > > > instruction for CRC32C offload are using #ifdef CONFIG_X86 stubs in
> > > > iscsi_target_login.c:iscsi_login_setup_crypto(), and !CONFIG_X86 will
> > > > default to using the unoptimized 1x8 slicing soft CRC32C code. This
> > > > particular piece of logic has been tested on powerpc and arm and is
> > > > funcitoning as expected from the kernel level using the arch independent
> > > > soft code.
> > >
> > > I don't think you need that code at all. The crypto code is structured
> > > to prefer the optimized implementation if it is present. Just stripping
> > > the x86-specific code out and always requesting the plain crc32c
> > > algorithm should give you the optimized one if it is present on your
> > > system.
> > >
> > > Please give it a try.
> > >
> >
> > This is what I originally thought as well, but this ended up not being
> > the case when the logic was originally coded up. I just tried again
> > with .38-rc7 on a 5500 series machine and simply stubbing out the
> > CONFIG_X86 part from iscsi_login_setup_crypto() and calling:
> >
> >crypto_alloc_hash("crc32c", 0, CRYPTO_ALG_ASYNC)
> >
> > does not automatically load and use crc32c_intel.ko when only requesting
> > plain crc32c.
>
> It sounds like there might be a bug in the crypto layer, so the Linux
> way is to make it work as intended.
>
> It's absolutely not acceptable just to pull other layer workarounds into
> drivers.
>
> > The reason for the extra crypto_alloc_hash("crc32c-intel", ...) call in
> > iscsi_login_setup_crypto() is to load crc32c_intel.ko on-demand for
> > cpu_has_xmm4_2 capable machines.
> >
> > I should mention this is with the following .config:
> >
> > CONFIG_CRYPTO_CRC32C=y
> > CONFIG_CRYPTO_CRC32C_INTEL=m
> >
> > This would seem to indicate that CRC32C_INTEL needs to be compiled in
> > (or at least manually loaded) for libcypto to use the optimized
> > instructions when the plain crc32c is called, correct..?
>
> That sounds right. There's probably not an autoload for this on
> recognising sse instructions.
>
I have been thinking about this some more, and modifying libcrypto to be
aware of optimized offload methods for hardware specific modules that it
should load does sound useful, but it seem like overkill to me for only
this particular case.
What about the following to simply call request_module("crc32c_intel")
at module_init() time and top the extra iscsi_login_setup_crypto()
code..?
Thanks,
--nab
[PATCH] iscsi-target: Call request_module("crc32c_intel") during module_init
This patch adds a call during module_init() -> iscsi_target_register_configfs()
to request the loading of crc32c_intel.ko to allow libcrypto to properly use
the optimized offload where available.
It also removes the extra crypto_alloc_hash("crc32c-intel", ...) calls
from iscsi_login_setup_crypto() and removes the unnecessary TPG attribute
crc32c_x86_offload for control this offload from configfs.
Signed-off-by: Nicholas A. Bellinger
---
drivers/target/lio-target/iscsi_target_configfs.c | 18 +++
drivers/target/lio-target/iscsi_target_core.h |4 --
drivers/target/lio-target/iscsi_target_login.c| 34 ++---
drivers/target/lio-target/iscsi_target_tpg.c | 19 ---
drivers/target/lio-target/iscsi_target_tpg.h |1 -
5 files changed, 15 insertions(+), 61 deletions(-)
diff --git a/drivers/target/lio-target/iscsi_target_configfs.c
b/drivers/target/lio-target/iscsi_target_configfs.c
index 76ee4fc..7ba169a 100644
--- a/drivers/target/lio-target/iscsi_target_configfs.c
+++ b/drivers/target/lio-target/iscsi_target_configfs.c
@@ -927,11 +927,6 @@ TPG_ATTR(demo_mode_write_protect, S_IRUGO | S_IWUSR);
*/
DEF_TPG_ATTRIB(prod_mode_write_protect);
TPG_ATTR(prod_mode_write_protect, S_IRUGO | S_IWUSR);
-/*
- * Define iscsi_tpg_attrib_s_crc32c_x86_offload
- */
-DEF_TPG_ATTRIB(crc32c_x86_offload);
-TPG_ATTR(crc32c_x86_offload, S_IRUGO | S_IWUSR);
static struct configfs_attribute *lio_target_tpg_attrib_attrs[] = {
&iscsi_tpg_attrib_authentication.attr,
@@ -942,7 +937,6 @@ static struct configfs_attribute
*lio_target_tpg_attrib_attrs[] = {
&iscsi_tpg_attrib_cache_dynamic_acls.attr,
&iscsi_tpg_attrib_demo_mode_write_protect.attr,
&iscsi_tpg_attrib_prod_mode_write_protect.attr,
- &iscsi_tpg_attrib_crc32c_x86_offload.attr,
NULL,
};
@@ -1525,6 +1519,18 @@ int iscsi_target_register_configfs(void)
lio_target_fabric_configfs = fabric;
printk(KERN_INFO "LIO_TARGET[0] - Set fabric ->"
" lio_target_fabric_configfs\n");
+#ifdef CONFIG_X86
+ /*
+* For cp
Re: [RFC 12/12] iscsi-target: Add Makefile/Kconfig and update TCM top level
On Thu, 2011-03-03 at 12:58 -0800, Nicholas A. Bellinger wrote:
> On Thu, 2011-03-03 at 09:19 -0500, Christoph Hellwig wrote:
> > On Wed, Mar 02, 2011 at 01:32:11PM -0800, Nicholas A. Bellinger wrote:
> > > The kernel code itself that is specific to using the SSE v4.2
> > > instruction for CRC32C offload are using #ifdef CONFIG_X86 stubs in
> > > iscsi_target_login.c:iscsi_login_setup_crypto(), and !CONFIG_X86 will
> > > default to using the unoptimized 1x8 slicing soft CRC32C code. This
> > > particular piece of logic has been tested on powerpc and arm and is
> > > funcitoning as expected from the kernel level using the arch independent
> > > soft code.
> >
> > I don't think you need that code at all. The crypto code is structured
> > to prefer the optimized implementation if it is present. Just stripping
> > the x86-specific code out and always requesting the plain crc32c
> > algorithm should give you the optimized one if it is present on your
> > system.
> >
> > Please give it a try.
> >
>
> This is what I originally thought as well, but this ended up not being
> the case when the logic was originally coded up. I just tried again
> with .38-rc7 on a 5500 series machine and simply stubbing out the
> CONFIG_X86 part from iscsi_login_setup_crypto() and calling:
>
>crypto_alloc_hash("crc32c", 0, CRYPTO_ALG_ASYNC)
>
> does not automatically load and use crc32c_intel.ko when only requesting
> plain crc32c.
It sounds like there might be a bug in the crypto layer, so the Linux
way is to make it work as intended.
It's absolutely not acceptable just to pull other layer workarounds into
drivers.
> The reason for the extra crypto_alloc_hash("crc32c-intel", ...) call in
> iscsi_login_setup_crypto() is to load crc32c_intel.ko on-demand for
> cpu_has_xmm4_2 capable machines.
>
> I should mention this is with the following .config:
>
> CONFIG_CRYPTO_CRC32C=y
> CONFIG_CRYPTO_CRC32C_INTEL=m
>
> This would seem to indicate that CRC32C_INTEL needs to be compiled in
> (or at least manually loaded) for libcypto to use the optimized
> instructions when the plain crc32c is called, correct..?
That sounds right. There's probably not an autoload for this on
recognising sse instructions.
James
James
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 12/12] iscsi-target: Add Makefile/Kconfig and update TCM top level
On Thu, 2011-03-03 at 09:19 -0500, Christoph Hellwig wrote:
> On Wed, Mar 02, 2011 at 01:32:11PM -0800, Nicholas A. Bellinger wrote:
> > The kernel code itself that is specific to using the SSE v4.2
> > instruction for CRC32C offload are using #ifdef CONFIG_X86 stubs in
> > iscsi_target_login.c:iscsi_login_setup_crypto(), and !CONFIG_X86 will
> > default to using the unoptimized 1x8 slicing soft CRC32C code. This
> > particular piece of logic has been tested on powerpc and arm and is
> > funcitoning as expected from the kernel level using the arch independent
> > soft code.
>
> I don't think you need that code at all. The crypto code is structured
> to prefer the optimized implementation if it is present. Just stripping
> the x86-specific code out and always requesting the plain crc32c
> algorithm should give you the optimized one if it is present on your
> system.
>
> Please give it a try.
>
This is what I originally thought as well, but this ended up not being
the case when the logic was originally coded up. I just tried again
with .38-rc7 on a 5500 series machine and simply stubbing out the
CONFIG_X86 part from iscsi_login_setup_crypto() and calling:
crypto_alloc_hash("crc32c", 0, CRYPTO_ALG_ASYNC)
does not automatically load and use crc32c_intel.ko when only requesting
plain crc32c.
The reason for the extra crypto_alloc_hash("crc32c-intel", ...) call in
iscsi_login_setup_crypto() is to load crc32c_intel.ko on-demand for
cpu_has_xmm4_2 capable machines.
I should mention this is with the following .config:
CONFIG_CRYPTO_CRC32C=y
CONFIG_CRYPTO_CRC32C_INTEL=m
This would seem to indicate that CRC32C_INTEL needs to be compiled in
(or at least manually loaded) for libcypto to use the optimized
instructions when the plain crc32c is called, correct..?
--nab
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 12/12] iscsi-target: Add Makefile/Kconfig and update TCM top level
On Wed, Mar 02, 2011 at 01:32:11PM -0800, Nicholas A. Bellinger wrote: > The kernel code itself that is specific to using the SSE v4.2 > instruction for CRC32C offload are using #ifdef CONFIG_X86 stubs in > iscsi_target_login.c:iscsi_login_setup_crypto(), and !CONFIG_X86 will > default to using the unoptimized 1x8 slicing soft CRC32C code. This > particular piece of logic has been tested on powerpc and arm and is > funcitoning as expected from the kernel level using the arch independent > soft code. I don't think you need that code at all. The crypto code is structured to prefer the optimized implementation if it is present. Just stripping the x86-specific code out and always requesting the plain crc32c algorithm should give you the optimized one if it is present on your system. Please give it a try. -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
