Re: [PATCH 2/2][RFC] Add dm-crypt target

[Christoph Hellwig <hch@infradead.org>]

[Fruhwirth dropped from Cc list due to his braindead spam "filter"]

On Mon, Dec 22, 2003 at 10:52:36PM +0100, Christophe Saout wrote:
> +#include "dm.h"
> +#include "dm-daemon.h"

Please include driver-private headers after kernel headers.

> +/*
> + * Crypt: maps a linear range of a block device
> + * and encrypts / decrypts at the same time.
> + */
> +struct crypt_c {

crypt_context?  The current name isn't very descriptive..

> +static kmem_cache_t *_io_cache;

Again a rather strange variable name.  What about dm_crypt_pool?

> +
> +/*
> + * Mempool alloc and free functions for the page and io pool
> + */
> +static void *mempool_alloc_page(int gfp_mask, void *data)
> +{
> +	return alloc_page(gfp_mask);
> +}
> +
> +static void mempool_free_page(void *page, void *data)
> +{
> +	__free_page(page);
> +}

Should probably go into generic code one day, along with the slab wrappers.

> +
> +static inline struct page *crypt_alloc_page(struct crypt_c *cc, int gfp_mask)
> +{
> +	return mempool_alloc(cc->page_pool, gfp_mask);
> +}
> +
> +static inline void crypt_free_page(struct crypt_c *cc, struct page *page)
> +{
> +	 mempool_free(page, cc->page_pool);
> +}
> +
> +static inline struct crypt_io *crypt_alloc_io(struct crypt_c *cc)
> +{
> +	return mempool_alloc(cc->io_pool, GFP_NOIO);
> +}
> +
> +static inline void crypt_free_io(struct crypt_c *cc, struct crypt_io *io)
> +{
> +	return mempool_free(io, cc->io_pool);
> +}

Please kill these superflous wrappers.

> +static spinlock_t _kcryptd_lock = SPIN_LOCK_UNLOCKED;
> +static struct bio *_bio_head;
> +static struct bio *_bio_tail;
> +
> +static struct dm_daemon _kcryptd;

Again, rather strange naming..

> +int __init dm_crypt_init(void)

static?

> +void __exit dm_crypt_exit(void)

static?

> +/*
> + * module hooks
> + */

Comment looks slightly superflous :)

Else the code look great and definitly better han cryptoloop from the
conceptual point of view.