Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[Andrew Morton]

On Tue, 20 May 2008 13:58:31 +0900 FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:

> This sets the default dma pad mask to ARCH_KMALLOC_MINALIGN in
> blk_queue_make_request(). It affects only non-coherent platforms.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Cc: Jens Axboe <jens.axboe@oracle.com>
> ---
>  block/blk-settings.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index 8dd8641..781d1bf 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -84,6 +84,11 @@ EXPORT_SYMBOL(blk_queue_softirq_done);
>   **/
>  void blk_queue_make_request(struct request_queue *q, make_request_fn *mfn)
>  {
> +#ifndef ARCH_KMALLOC_MINALIGN
> +#define ARCH_KMALLOC_MINALIGN 1
> +#endif
> +	unsigned int min_align = ARCH_KMALLOC_MINALIGN;
> +
>  	/*
>  	 * set defaults
>  	 */
> @@ -98,6 +103,7 @@ void blk_queue_make_request(struct request_queue *q, make_request_fn *mfn)
>  	blk_queue_max_sectors(q, SAFE_MAX_SECTORS);
>  	blk_queue_hardsect_size(q, 512);
>  	blk_queue_dma_alignment(q, 511);
> +	blk_queue_dma_pad(q, min_align - 1);
>  	blk_queue_congestion_threshold(q);
>  	q->nr_batching = BLK_BATCH_REQ;

urgh.  This ARCH_KMALLOC_MINALIGN thing has the smell of an expedient
hack which is now growing.

Look at what crypto did (which seems to be a lot worse):

/*
 * The macro CRYPTO_MINALIGN_ATTR (along with the void * type in the actual
 * declaration) is used to ensure that the crypto_tfm context structure is
 * aligned correctly for the given architecture so that there are no alignment
 * faults for C data types.  In particular, this is required on platforms such
 * as arm where pointers are 32-bit aligned but there are data types such as
 * u64 which require 64-bit alignment.
 */
#if defined(ARCH_KMALLOC_MINALIGN)
#define CRYPTO_MINALIGN ARCH_KMALLOC_MINALIGN
#elif defined(ARCH_SLAB_MINALIGN)
#define CRYPTO_MINALIGN ARCH_SLAB_MINALIGN
#else
#define CRYPTO_MINALIGN __alignof__(unsigned long long)
#endif

So here you're using it for "dma aligment" whereas crypto is using it
(or ARCH_SLAB_MINALIGN!) for "cpu 64-bit alignment".


Why does ARCH_KMALLOC_MINALIGN even exist?  What is its mandate?  Sigh.


It's not really related to your patch (although your patch compounds
the problem a little).  But we should sit down and work out what we
actually want to do here.  Something like:

In each architecture's arch/foo/Kconfig, define

	CONFIG_ARCH_DMA_ALIGN

and

	CONFIG_ARCH_64BIT_POINTER_ALIGN

and then use them.  Note that these have nothing to do with each other,
as far as I can tell.

Which leaves the question: "what should slab use"?  Maybe
CONFIG_ARCH_DMA_ALIGN?  But that depends what ARCH_KMALLOC_MINALIGN is
supposed to exist for.

ick.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[Herbert Xu]

On Tue, May 20, 2008 at 02:31:29AM -0700, Andrew Morton wrote:
>
> So here you're using it for "dma aligment" whereas crypto is using it
> (or ARCH_SLAB_MINALIGN!) for "cpu 64-bit alignment".

No the 64-bit alignment is just an example.  The purpose of
CRYPTO_MINALIGN is pretty much the same as ARCH_KMALLOC_MINALIGN,
i.e., the minimum alignment guaranteed by kmalloc.  The only
reason it exists is because ARCH_KMALLOC_MINALIGN isn't defined
on all platforms.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[Andrew Morton]

On Tue, 20 May 2008 17:38:20 +0800 Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Tue, May 20, 2008 at 02:31:29AM -0700, Andrew Morton wrote:
> >
> > So here you're using it for "dma aligment" whereas crypto is using it
> > (or ARCH_SLAB_MINALIGN!) for "cpu 64-bit alignment".
> 
> No the 64-bit alignment is just an example.  The purpose of
> CRYPTO_MINALIGN is pretty much the same as ARCH_KMALLOC_MINALIGN,
> i.e., the minimum alignment guaranteed by kmalloc.  The only
> reason it exists is because ARCH_KMALLOC_MINALIGN isn't defined
> on all platforms.

I'm struggling to understand what you're saying here.

The comment you have there over the CRYPTO_MINALIGN definition is quite
specific.  Is it wrong?

Whether the mapping between CRYPTO_MINALIGN and ARCH_KMALLOC_MINALIGN
is abusive is (I find) hard to say, because first one would need to be
able to say what ARCH_KMALLOC_MINALIGN is for.  I expect it was for DMA
purposes.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[FUJITA Tomonori]

On Tue, 20 May 2008 02:52:31 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Tue, 20 May 2008 17:38:20 +0800 Herbert Xu <herbert@gondor.apana.org.au> wrote:
> 
> > On Tue, May 20, 2008 at 02:31:29AM -0700, Andrew Morton wrote:
> > >
> > > So here you're using it for "dma aligment" whereas crypto is using it
> > > (or ARCH_SLAB_MINALIGN!) for "cpu 64-bit alignment".
> > 
> > No the 64-bit alignment is just an example.  The purpose of
> > CRYPTO_MINALIGN is pretty much the same as ARCH_KMALLOC_MINALIGN,
> > i.e., the minimum alignment guaranteed by kmalloc.  The only
> > reason it exists is because ARCH_KMALLOC_MINALIGN isn't defined
> > on all platforms.
> 
> I'm struggling to understand what you're saying here.
> 
> The comment you have there over the CRYPTO_MINALIGN definition is quite
> specific.  Is it wrong?
> 
> Whether the mapping between CRYPTO_MINALIGN and ARCH_KMALLOC_MINALIGN
> is abusive is (I find) hard to say, because first one would need to be
> able to say what ARCH_KMALLOC_MINALIGN is for.  I expect it was for DMA
> purposes.

Yeah, ARCH_KMALLOC_MINALIGN is for DMA. Non coherent platforms define
it.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[Herbert Xu]

On Tue, May 20, 2008 at 02:52:31AM -0700, Andrew Morton wrote:
> 
> The comment you have there over the CRYPTO_MINALIGN definition is quite
> specific.  Is it wrong?

No it's not wrong, but the comment isn't about CRYPTO_MINALIGN.
The comment is talking about CRYPTO_MINALIGN_ATTR, which uses
CRYPTO_MINALIGN.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[FUJITA Tomonori]

On Tue, 20 May 2008 17:38:20 +0800
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Tue, May 20, 2008 at 02:31:29AM -0700, Andrew Morton wrote:
> >
> > So here you're using it for "dma aligment" whereas crypto is using it
> > (or ARCH_SLAB_MINALIGN!) for "cpu 64-bit alignment".
> 
> No the 64-bit alignment is just an example.  The purpose of
> CRYPTO_MINALIGN is pretty much the same as ARCH_KMALLOC_MINALIGN,
> i.e., the minimum alignment guaranteed by kmalloc.  The only
> reason it exists is because ARCH_KMALLOC_MINALIGN isn't defined
> on all platforms.

struct ablkcipher_request {
	struct crypto_async_request base;

	unsigned int nbytes;

	void *info;

	struct scatterlist *src;
	struct scatterlist *dst;

	void *__ctx[] CRYPTO_MINALIGN_ATTR;
};

Does someone do DMA from/to __ctx?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[Herbert Xu]

On Tue, May 20, 2008 at 10:25:25PM +0900, FUJITA Tomonori wrote:
>
> Does someone do DMA from/to __ctx?

Nobody.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[FUJITA Tomonori]

On Tue, 20 May 2008 23:34:24 +0800
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Tue, May 20, 2008 at 10:25:25PM +0900, FUJITA Tomonori wrote:
> >
> > Does someone do DMA from/to __ctx?
> 
> Nobody.

Then, you don't need to use ARCH_KMALLOC_MINALIGN. 8 bytes align works
for you on all the architectures.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[Herbert Xu]

On Wed, May 21, 2008 at 01:09:41AM +0900, FUJITA Tomonori wrote:
>
> Then, you don't need to use ARCH_KMALLOC_MINALIGN. 8 bytes align works
> for you on all the architectures.

DMA isn't the only thing that requires alignment.  The CPU needs
it too.  Also using a constant like 8 is broken because if we used
a value larger than the alignment guaranteed by kmalloc then the
context may end up unaligned.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[FUJITA Tomonori]

On Wed, 21 May 2008 09:26:22 +0800
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Wed, May 21, 2008 at 01:09:41AM +0900, FUJITA Tomonori wrote:
> >
> > Then, you don't need to use ARCH_KMALLOC_MINALIGN. 8 bytes align works
> > for you on all the architectures.
> 
> DMA isn't the only thing that requires alignment.  The CPU needs
> it too.  Also using a constant like 8 is broken because if we used
> a value larger than the alignment guaranteed by kmalloc then the
> context may end up unaligned.

ARCH_KMALLOC_MINALIGN represents DMA alignment since we guarantee
kmalloced buffers can be used for DMA.

Only non coherent architecutures defines ARCH_KMALLOC_MINALIGN to the
cache line size since an DMA'able object within a structure isn't
sharing a cache line with some other object (note that it not about
only alignment).

For your case, the alignment requirement for a pointer is appropriate
(it's about the CPU alignment requirement that you talk about. 8 bytes
alignment always works).

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[Herbert Xu]

On Wed, May 21, 2008 at 10:36:51AM +0900, FUJITA Tomonori wrote:
>
> ARCH_KMALLOC_MINALIGN represents DMA alignment since we guarantee
> kmalloced buffers can be used for DMA.

That may be why it was created, but that is not its only application.
In particular, it forms part of the calculation of the minimum
alignment guaranteed by kmalloc which is why it's used in crpyto.

Of course, if some kind soul would move this calculation into a
header file then we wouldn't be having this discussion.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[FUJITA Tomonori]

On Wed, 21 May 2008 11:16:46 +0800
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Wed, May 21, 2008 at 10:36:51AM +0900, FUJITA Tomonori wrote:
> >
> > ARCH_KMALLOC_MINALIGN represents DMA alignment since we guarantee
> > kmalloced buffers can be used for DMA.
> 
> That may be why it was created, but that is not its only application.

Currently, it's only applicaiton.


> In particular, it forms part of the calculation of the minimum
> alignment guaranteed by kmalloc which is why it's used in crpyto.
> 
> Of course, if some kind soul would move this calculation into a
> header file then we wouldn't be having this discussion.

As explained, with the current way we define ARCH_KMALLOC_MINALIGN,
crypto doesn't need to use it. But to make it clear, we had better
clean up these defines, such as renaming it an appropriate name like
ARCH_DMA_ALIGN.

I'll send patches shortly.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[Herbert Xu]

On Wed, May 21, 2008 at 03:54:14PM +0900, FUJITA Tomonori wrote:
>
> As explained, with the current way we define ARCH_KMALLOC_MINALIGN,
> crypto doesn't need to use it. But to make it clear, we had better
> clean up these defines, such as renaming it an appropriate name like
> ARCH_DMA_ALIGN.

No you don't understand the way crypto is using it.  We need to
know exactly the minimum alignment guaranteed by kmalloc.  Too much
or too little are both buggy.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[FUJITA Tomonori]

On Wed, 21 May 2008 16:47:00 +0800
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Wed, May 21, 2008 at 03:54:14PM +0900, FUJITA Tomonori wrote:
> >
> > As explained, with the current way we define ARCH_KMALLOC_MINALIGN,
> > crypto doesn't need to use it. But to make it clear, we had better
> > clean up these defines, such as renaming it an appropriate name like
> > ARCH_DMA_ALIGN.
> 
> No you don't understand the way crypto is using it.  We need to
> know exactly the minimum alignment guaranteed by kmalloc.  Too much
> or too little are both buggy.

Why do crypto need to know exactly the minimum alignment guaranteed by
kmalloc? Can you tell me an example how the alignment breaks crypto?

For me, the way crypto use it is idential to what the hostdata in
struct Scsi_Host does.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[Herbert Xu]

On Wed, May 21, 2008 at 06:34:45PM +0900, FUJITA Tomonori wrote:
>
> Why do crypto need to know exactly the minimum alignment guaranteed by
> kmalloc? Can you tell me an example how the alignment breaks crypto?

It's used to make the context aligned so that for most algorithms
we can get to the context without going through ALIGN_PTR.  Only
algorithms requiring alignments bigger than that offered by kmalloc
would have to use ALIGN_PTR.  This is important because the context
is used on the fast path, i.e., for AES every block has to access
the context.

If we used an alignment value is bigger than that guaranteed by
kmalloc then this would break because the context may end up
unaligned.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[FUJITA Tomonori]

On Wed, 21 May 2008 18:05:29 +0800
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Wed, May 21, 2008 at 06:34:45PM +0900, FUJITA Tomonori wrote:
> >
> > Why do crypto need to know exactly the minimum alignment guaranteed by
> > kmalloc? Can you tell me an example how the alignment breaks crypto?
> 
> It's used to make the context aligned so that for most algorithms
> we can get to the context without going through ALIGN_PTR.  Only

How many bytes does the context need to be aligned?

I'm still not sure what you mean. You referred to aes, so let me use
aes as an example.

I think 'we can get to the context' means that accessing to
crypto_aes_ctx from struct crypto_tfm like this:

struct crypto_aes_ctx *ctx = crypto_tfm_ctx(tfm);


struct crypto_tfm and crypto_tfm_ctx are defined as:

struct crypto_tfm {

	u32 crt_flags;

	union {
		struct ablkcipher_tfm ablkcipher;
		struct aead_tfm aead;
		struct blkcipher_tfm blkcipher;
		struct cipher_tfm cipher;
		struct hash_tfm hash;
		struct compress_tfm compress;
	} crt_u;

	struct crypto_alg *__crt_alg;

	void *__crt_ctx[] CRYPTO_MINALIGN_ATTR;
};

static inline void *crypto_tfm_ctx(struct crypto_tfm *tfm)
{
	return tfm->__crt_ctx;
}

struct crypto_aes_ctx is placed right after struct crypto_tfm.

My question is why __crt_ctx needs ARCH_KMALLOC_MINALIGN alignment,
e.g., could be 128 bytes.


> algorithms requiring alignments bigger than that offered by kmalloc
> would have to use ALIGN_PTR.  This is important because the context
> is used on the fast path, i.e., for AES every block has to access
> the context.

Why do algorithms require alignments bigger than ARCH_KMALLOC_MINALIGN?


> If we used an alignment value is bigger than that guaranteed by
> kmalloc then this would break because the context may end up
> unaligned.
> 
> Cheers,
> -- 
> Visit Openswan at http://www.openswan.org/
> Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
> 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-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[Herbert Xu]

On Wed, May 21, 2008 at 08:01:12PM +0900, FUJITA Tomonori wrote:
>
> Why do algorithms require alignments bigger than ARCH_KMALLOC_MINALIGN?

Because the hardware may require it.  For example, the VIA Padlock
will fault unless the buffers are 16-byte aligned (it being an
x86-32 platform).

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[FUJITA Tomonori]

On Wed, 21 May 2008 19:25:54 +0800
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Wed, May 21, 2008 at 08:01:12PM +0900, FUJITA Tomonori wrote:
> >
> > Why do algorithms require alignments bigger than ARCH_KMALLOC_MINALIGN?
> 
> Because the hardware may require it.  For example, the VIA Padlock
> will fault unless the buffers are 16-byte aligned (it being an
> x86-32 platform).

OK, thanks. So it's about hardware requrement. Let me make sure if I
understand crypto alignment issue.

__crt_ctx needs ARCH_KMALLOC_MINALIGN alignment only because of crypto
hardware. If I misunderstand it, can you answer my question in the
previous mail (it's the part that you cut)? That is, why does
__crt_ctx need ARCH_KMALLOC_MINALIGN alignment with software
algorithms.

The VIA Padlock likes 16-byte aligned __crt_ctx. On x86-32 platform,
ARCH_KMALLOC_MINALIGN is not defined, so __crt_ctx is 8-byte
aligned. struct aes_ctx of The VIA Padlock may not be aligned so you
may need ALIGN hack every time.

But ARCH_KMALLOC_MINALIGN is 128 bytes on some architectures. In this
case, __crt_ctx is 128-byte aligned and struct aes_ctx of The VIA
Padlock is guaranteed to be aligned nicely.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[Herbert Xu]

On Wed, May 21, 2008 at 09:09:58PM +0900, FUJITA Tomonori wrote:
>
> OK, thanks. So it's about hardware requrement. Let me make sure if I
> understand crypto alignment issue.
> 
> __crt_ctx needs ARCH_KMALLOC_MINALIGN alignment only because of crypto
> hardware. If I misunderstand it, can you answer my question in the
> previous mail (it's the part that you cut)? That is, why does
> __crt_ctx need ARCH_KMALLOC_MINALIGN alignment with software
> algorithms.

Because the same structure is used for all algorithms!

Why is this so hard to understand?
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[FUJITA Tomonori]

On Wed, 21 May 2008 20:22:18 +0800
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Wed, May 21, 2008 at 09:09:58PM +0900, FUJITA Tomonori wrote:
> >
> > OK, thanks. So it's about hardware requrement. Let me make sure if I
> > understand crypto alignment issue.
> > 
> > __crt_ctx needs ARCH_KMALLOC_MINALIGN alignment only because of crypto
> > hardware. If I misunderstand it, can you answer my question in the
> > previous mail (it's the part that you cut)? That is, why does
> > __crt_ctx need ARCH_KMALLOC_MINALIGN alignment with software
> > algorithms.
> 
> Because the same structure is used for all algorithms!

No, you misunderstand my question. I meant, software algorithms don't
need ARCH_KMALLOC_MINALIGN alignment for __crt_ctx and if we are fine
with using the ALIGN hack for crypto hardware every time (like
aes_ctx_common), crypto doesn't need ARCH_KMALLOC_MINALIGN alignment
for __crt_ctx. Is this right?


> 
> Why is this so hard to understand?

Because there are few architecture that defines
ARCH_KMALLOC_MINALIGN. So if crypto hardware needs alignement, it's
likely the hardware alignement is larger than __crt_ctx alignment. As
a result, you have to use ALIGN_PTR. So It's hard to understand using
ARCH_KMALLOC_MINALIGN here. I don't know about crypto hardware, but I
wonder if we can use a static alignment like 64 bytes here, which may
work for most of crypto hardware. Or if there are not many users of
crypto hardware, it may be fine to use ALIGN_PTR for the hardware.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[FUJITA Tomonori]

On Wed, 21 May 2008 21:46:24 +0900
FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:

> On Wed, 21 May 2008 20:22:18 +0800
> Herbert Xu <herbert@gondor.apana.org.au> wrote:
> 
> > On Wed, May 21, 2008 at 09:09:58PM +0900, FUJITA Tomonori wrote:
> > >
> > > OK, thanks. So it's about hardware requrement. Let me make sure if I
> > > understand crypto alignment issue.
> > > 
> > > __crt_ctx needs ARCH_KMALLOC_MINALIGN alignment only because of crypto
> > > hardware. If I misunderstand it, can you answer my question in the
> > > previous mail (it's the part that you cut)? That is, why does
> > > __crt_ctx need ARCH_KMALLOC_MINALIGN alignment with software
> > > algorithms.
> > 
> > Because the same structure is used for all algorithms!
> 
> No, you misunderstand my question. I meant, software algorithms don't
> need ARCH_KMALLOC_MINALIGN alignment for __crt_ctx and if we are fine
> with using the ALIGN hack for crypto hardware every time (like
> aes_ctx_common), crypto doesn't need ARCH_KMALLOC_MINALIGN alignment
> for __crt_ctx. Is this right?
> 
> 
> > 
> > Why is this so hard to understand?
> 
> Because there are few architecture that defines
> ARCH_KMALLOC_MINALIGN. So if crypto hardware needs alignement, it's
> likely the hardware alignement is larger than __crt_ctx alignment. As
> a result, you have to use ALIGN_PTR. So It's hard to understand using
> ARCH_KMALLOC_MINALIGN here. I don't know about crypto hardware, but I
> wonder if we can use a static alignment like 64 bytes here, which may
> work for most of crypto hardware. Or if there are not many users of

Oops, scratch the static alignment. It's impossible.


> crypto hardware, it may be fine to use ALIGN_PTR for the hardware.

I still wonder it's acceptable or not.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[Herbert Xu]

On Wed, May 21, 2008 at 09:55:15PM +0900, FUJITA Tomonori wrote:
>
> > crypto hardware, it may be fine to use ALIGN_PTR for the hardware.
> 
> I still wonder it's acceptable or not.

Normally I would say yes.  But because the context poitner is
used on the most performance-critical path of the crypto API,
I'd rather not use it unless necessary.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[Herbert Xu]

On Wed, May 21, 2008 at 09:46:24PM +0900, FUJITA Tomonori wrote:
>
> No, you misunderstand my question. I meant, software algorithms don't
> need ARCH_KMALLOC_MINALIGN alignment for __crt_ctx and if we are fine
> with using the ALIGN hack for crypto hardware every time (like
> aes_ctx_common), crypto doesn't need ARCH_KMALLOC_MINALIGN alignment
> for __crt_ctx. Is this right?

The padlock isn't the only hardware device that will require
such alignment.  Now that we have the async interface there will
be more.

> Because there are few architecture that defines
> ARCH_KMALLOC_MINALIGN. So if crypto hardware needs alignement, it's

You keep going back to ARCH_KMALLOC_MINALIGN.  But this has *nothing*
to do with ARCH_KMALLOC_MINALIGN.  The only reason it appears at
all in the crypto code is because it's one of the parameters used
to calculate the minimum alignment guaranteed by kmalloc.

If there were a macro KMALLOC_MINALIGN which did what its name says
then I'd gladly use it.

Cheeres,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[FUJITA Tomonori]

On Wed, 21 May 2008 21:18:11 +0800
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Wed, May 21, 2008 at 09:46:24PM +0900, FUJITA Tomonori wrote:
> >
> > No, you misunderstand my question. I meant, software algorithms don't
> > need ARCH_KMALLOC_MINALIGN alignment for __crt_ctx and if we are fine
> > with using the ALIGN hack for crypto hardware every time (like
> > aes_ctx_common), crypto doesn't need ARCH_KMALLOC_MINALIGN alignment
> > for __crt_ctx. Is this right?
> 
> The padlock isn't the only hardware device that will require
> such alignment.  Now that we have the async interface there will
> be more.

Ok, so it's all about crypto hardware requirement. In other words, if
we accept for potential performance drop of crypto hardware, crypto
can drop this alignment.


> > Because there are few architecture that defines
> > ARCH_KMALLOC_MINALIGN. So if crypto hardware needs alignement, it's
> 
> You keep going back to ARCH_KMALLOC_MINALIGN.  But this has *nothing*
> to do with ARCH_KMALLOC_MINALIGN.  The only reason it appears at
> all in the crypto code is because it's one of the parameters used
> to calculate the minimum alignment guaranteed by kmalloc.

No, you misunderstand what I meant. I'm talking about the minimum
alignment guaranteed by kmalloc too.

What I'm trying to asking you, on the majority of architectures, the
minimum alignment guaranteed by kmalloc (8 bytes) is too small for
algorithms requiring alignments (that is, crypto hardware requiring
alignments). As a result, the former in the following your logic
doesn't happens for most of us. Your logic is:

=
It's used to make the context aligned so that for most algorithms we
can get to the context without going through ALIGN_PTR. Only
algorithms requiring alignments bigger than that offered by kmalloc
would have to use ALIGN_PTR.
=

The former preferable path (algorithms requiring alignments are
smaller than the minimum alignment guaranteed by kmalloc) happens only
on some powerpc, arm, and mips architectures. Do I misunderstand
something?

If you put a hack in __crypto_alloc_tfm and crypto_free_tfm to return
aligned tmf to algorithms (and use aligned attribute for __crt_ctx),
then the your logic would makes sense.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[David Miller, FUJITA Tomonori]

> On Wed, 21 May 2008 21:18:11 +0800
> Herbert Xu <herbert@gondor.apana.org.au> wrote:
> 
> > On Wed, May 21, 2008 at 09:46:24PM +0900, FUJITA Tomonori wrote:
> > >
> > > No, you misunderstand my question. I meant, software algorithms don't
> > > need ARCH_KMALLOC_MINALIGN alignment for __crt_ctx and if we are fine
> > > with using the ALIGN hack for crypto hardware every time (like
> > > aes_ctx_common), crypto doesn't need ARCH_KMALLOC_MINALIGN alignment
> > > for __crt_ctx. Is this right?
> > 
> > The padlock isn't the only hardware device that will require
> > such alignment.  Now that we have the async interface there will
> > be more.
> 
> Ok, so it's all about crypto hardware requirement. In other words, if
> we accept for potential performance drop of crypto hardware, crypto
> can drop this alignment.

It sounds to me that Herbert is saying that the VIA crypto hardware
will malfunction if not given an aligned address, rather than simply
go more slowly.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[Herbert Xu]

On Wed, May 21, 2008 at 06:19:45PM -0700, David Miller wrote:
>
> > Ok, so it's all about crypto hardware requirement. In other words, if
> > we accept for potential performance drop of crypto hardware, crypto
> > can drop this alignment.
> 
> It sounds to me that Herbert is saying that the VIA crypto hardware
> will malfunction if not given an aligned address, rather than simply
> go more slowly.

Yes, in general hardware crypto that needs alignment requires it.
In VIA's case it will generate a GPF.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[FUJITA Tomonori]

On Wed, 21 May 2008 18:19:45 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Date: Thu, 22 May 2008 10:14:12 +0900
> 
> > On Wed, 21 May 2008 21:18:11 +0800
> > Herbert Xu <herbert@gondor.apana.org.au> wrote:
> > 
> > > On Wed, May 21, 2008 at 09:46:24PM +0900, FUJITA Tomonori wrote:
> > > >
> > > > No, you misunderstand my question. I meant, software algorithms don't
> > > > need ARCH_KMALLOC_MINALIGN alignment for __crt_ctx and if we are fine
> > > > with using the ALIGN hack for crypto hardware every time (like
> > > > aes_ctx_common), crypto doesn't need ARCH_KMALLOC_MINALIGN alignment
> > > > for __crt_ctx. Is this right?
> > > 
> > > The padlock isn't the only hardware device that will require
> > > such alignment.  Now that we have the async interface there will
> > > be more.
> > 
> > Ok, so it's all about crypto hardware requirement. In other words, if
> > we accept for potential performance drop of crypto hardware, crypto
> > can drop this alignment.
> 
> It sounds to me that Herbert is saying that the VIA crypto hardware
> will malfunction if not given an aligned address, rather than simply
> go more slowly.

I understand that.

VIA crypto driver has the following code to get proper alignment:

static inline struct aes_ctx *aes_ctx_common(void *ctx)
{
	unsigned long addr = (unsigned long)ctx;
	unsigned long align = PADLOCK_ALIGNMENT;

	if (align <= crypto_tfm_ctx_alignment())
		align = 1;
	return (struct aes_ctx *)ALIGN(addr, align);
}

What he insists is:

When crypto hardware alignment is smaller than the minimum alignment
guaranteed by kmalloc, the above function is faster since ALIGN is
nullified. That's why crypto uses the minimum alignment guaranteed by
kmalloc.


What I asking is:

On most architectures, the minimum alignment guaranteed by kmalloc is
too small (8 bytes). This ideal story doesn't happen to most of us.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[Herbert Xu]

On Thu, May 22, 2008 at 10:32:21AM +0900, FUJITA Tomonori wrote:
>
> What I asking is:
> 
> On most architectures, the minimum alignment guaranteed by kmalloc is
> too small (8 bytes). This ideal story doesn't happen to most of us.

Right, so the real issue is that what we have here is a lower
bound of the kmalloc alignment.  In reality, the kmalloc return
values are cache-line aligned when debugging is off.  So if you
can think of a way of getting a better bound at compile time,
I'm all ears.

Otherwise this discussion seems to be completely pointless.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] block: use ARCH_KMALLOC_MINALIGN as the default dma pad mask

[Paul Mundt]

On Tue, May 20, 2008 at 02:31:29AM -0700, Andrew Morton wrote:
> Why does ARCH_KMALLOC_MINALIGN even exist?  What is its mandate?  Sigh.
> 
> It's not really related to your patch (although your patch compounds
> the problem a little).  But we should sit down and work out what we
> actually want to do here.  Something like:
> 
> In each architecture's arch/foo/Kconfig, define
> 
> 	CONFIG_ARCH_DMA_ALIGN
> 
> and
> 
> 	CONFIG_ARCH_64BIT_POINTER_ALIGN
> 
> and then use them.  Note that these have nothing to do with each other,
> as far as I can tell.
> 
> Which leaves the question: "what should slab use"?  Maybe
> CONFIG_ARCH_DMA_ALIGN?  But that depends what ARCH_KMALLOC_MINALIGN is
> supposed to exist for.
> 
> ick.
> 
The only platforms that set ARCH_KMALLOC_MINALIGN appear to do so for DMA
alignment reasons, so your Kconfig option there seems reasonable.

The ARCH_SLAB_MINALIGN you can blame me for:

	http://marc.info/?l=linux-kernel&m=110227138116749&w=2

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>