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

From: Christophe Saout
Date: Tue Dec 23 2003 - 12:04:31 EST


On Tue, Dec 23, 2003 at 03:43:25PM +0000, Joe Thornber wrote:

> On Tue, Dec 23, 2003 at 10:31:43AM -0500, Jeff Garzik wrote:
> > I agree w/ Christoph... overly defensive programming like this just
> > creates a new class of programmer errors, doesn't really solve anything.
> > It's standard Linux kernel style, and making code look like all other
> > code has benefits in review and debugging. Finally, the programmer
> > should be paying attention to what kernel APIs he/she uses, and add
> > headers accordingly.
>
> ok, I don't feel strongly enough about it to argue, so we'll change it.

Ok, me too. :-)

So how does this look?

--- linux.orig/drivers/md/dm-crypt.c 2003-12-23 16:43:48.409077048 +0100
+++ linux/drivers/md/dm-crypt.c 2003-12-23 16:52:11.946527720 +0100
@@ -4,9 +4,6 @@
* This file is released under the GPL.
*/

-#include "dm.h"
-#include "dm-daemon.h"
-
#include <linux/module.h>
#include <linux/init.h>
#include <linux/bio.h>
@@ -16,6 +13,9 @@
#include <linux/spinlock.h>
#include <asm/scatterlist.h>

+#include "dm.h"
+#include "dm-daemon.h"
+
/*
* per bio private data
*/
@@ -70,10 +70,10 @@
#define MIN_POOL_PAGES 16
#define MIN_BIO_PAGES 8

-static kmem_cache_t *_io_cache;
+static kmem_cache_t *crypt_io_pool;

/*
- * Mempool alloc and free functions for the page and io pool
+ * Mempool alloc and free functions for the page
*/
static void *mempool_alloc_page(int gfp_mask, void *data)
{
@@ -85,26 +85,6 @@
__free_page(page);
}

-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);
-}
-
/*
* Encrypt / decrypt a single sector, source and destination buffers
* are stored in scatterlists. In CBC mode initialise the "previous
@@ -232,7 +212,7 @@
for(i = bio->bi_idx; i < nr_iovecs; i++) {
struct bio_vec *bv = bio_iovec_idx(bio, i);

- bv->bv_page = crypt_alloc_page(cc, gfp_mask);
+ bv->bv_page = mempool_alloc(cc->page_pool, gfp_mask);
if (!bv->bv_page)
break;

@@ -276,7 +256,7 @@

while(bytes) {
struct bio_vec *bv = bio_iovec_idx(bio, i++);
- crypt_free_page(cc, bv->bv_page);
+ mempool_free(bv->bv_page, cc->page_pool);
bytes -= bv->bv_len;
}
}
@@ -301,7 +281,7 @@
if (io->bio)
bio_endio(io->bio, io->bio->bi_size, io->error);

- crypt_free_io(cc, io);
+ mempool_free(io, cc->io_pool);
}

/*
@@ -311,11 +291,11 @@
* interrupt context, so returning bios from read requests get
* queued here.
*/
-static spinlock_t _kcryptd_lock = SPIN_LOCK_UNLOCKED;
-static struct bio *_bio_head;
-static struct bio *_bio_tail;
+static spinlock_t kcryptd_lock = SPIN_LOCK_UNLOCKED;
+static struct bio *kcryptd_bio_head;
+static struct bio *kcryptd_bio_tail;

-static struct dm_daemon _kcryptd;
+static struct dm_daemon kcryptd;

/*
* Fetch a list of the complete bios.
@@ -324,11 +304,11 @@
{
struct bio *bio;

- spin_lock_irq(&_kcryptd_lock);
- bio = _bio_head;
+ spin_lock_irq(&kcryptd_lock);
+ bio = kcryptd_bio_head;
if (bio)
- _bio_head = _bio_tail = NULL;
- spin_unlock_irq(&_kcryptd_lock);
+ kcryptd_bio_head = kcryptd_bio_tail = NULL;
+ spin_unlock_irq(&kcryptd_lock);

return bio;
}
@@ -340,16 +320,16 @@
{
unsigned long flags;

- spin_lock_irqsave(&_kcryptd_lock, flags);
- if (_bio_tail)
- _bio_tail->bi_next = bio;
+ spin_lock_irqsave(&kcryptd_lock, flags);
+ if (kcryptd_bio_tail)
+ kcryptd_bio_tail->bi_next = bio;
else
- _bio_head = bio;
- _bio_tail = bio;
- spin_unlock_irqrestore(&_kcryptd_lock, flags);
+ kcryptd_bio_head = bio;
+ kcryptd_bio_tail = bio;
+ spin_unlock_irqrestore(&kcryptd_lock, flags);
}

-static jiffy_t kcryptd(void)
+static jiffy_t kcryptd_do_work(void)
{
int r;
struct bio *bio;
@@ -493,7 +473,7 @@
}

cc->io_pool = mempool_create(MIN_IOS, mempool_alloc_slab,
- mempool_free_slab, _io_cache);
+ mempool_free_slab, crypt_io_pool);
if (!cc->io_pool) {
ti->error = "dm-crypt: Cannot allocate crypt io mempool";
goto bad1;
@@ -584,7 +564,7 @@
if ((bio_rw(bio) == READ || bio_rw(bio) == READA)
&& bio_flagged(bio, BIO_UPTODATE)) {
kcryptd_queue_bio(bio);
- dm_daemon_wake(&_kcryptd);
+ dm_daemon_wake(&kcryptd);
return 0;
}

@@ -597,7 +577,7 @@
static int crypt_map(struct dm_target *ti, struct bio *bio)
{
struct crypt_c *cc = (struct crypt_c *) ti->private;
- struct crypt_io *io = crypt_alloc_io(cc);
+ struct crypt_io *io = mempool_alloc(cc->io_pool, GFP_NOIO);
struct bio *clone = NULL;
struct convert_context ctx;
unsigned int remaining = bio->bi_size;
@@ -675,7 +655,7 @@
}

/* if no bio has been dispatched yet, we can directly return the error */
- crypt_free_io(cc, io);
+ mempool_free(io, cc->io_pool);
return r;
}

@@ -740,48 +720,45 @@
.status = crypt_status,
};

-int __init dm_crypt_init(void)
+static int __init dm_crypt_init(void)
{
int r;

- _io_cache = kmem_cache_create("dm-crypt_io", sizeof(struct crypt_io),
- 0, 0, NULL, NULL);
- if (!_io_cache)
+ crypt_io_pool = kmem_cache_create("dm-crypt_io", sizeof(struct crypt_io),
+ 0, 0, NULL, NULL);
+ if (!crypt_io_pool)
return -ENOMEM;

- r = dm_daemon_start(&_kcryptd, "kcryptd", kcryptd);
+ r = dm_daemon_start(&kcryptd, "kcryptd", kcryptd_do_work);
if (r) {
DMERR("couldn't create kcryptd: %d", r);
- kmem_cache_destroy(_io_cache);
+ kmem_cache_destroy(crypt_io_pool);
return r;
}

r = dm_register_target(&crypt_target);
if (r < 0) {
DMERR("crypt: register failed %d", r);
- dm_daemon_stop(&_kcryptd);
- kmem_cache_destroy(_io_cache);
+ dm_daemon_stop(&kcryptd);
+ kmem_cache_destroy(crypt_io_pool);
}

return r;
}

-void __exit dm_crypt_exit(void)
+static void __exit dm_crypt_exit(void)
{
int r = dm_unregister_target(&crypt_target);

if (r < 0)
DMERR("crypt: unregister failed %d", r);

- dm_daemon_stop(&_kcryptd);
- kmem_cache_destroy(_io_cache);
+ dm_daemon_stop(&kcryptd);
+ kmem_cache_destroy(crypt_io_pool);
}

-/*
- * module hooks
- */
-module_init(dm_crypt_init)
-module_exit(dm_crypt_exit)
+module_init(dm_crypt_init);
+module_exit(dm_crypt_exit);

MODULE_AUTHOR("Christophe Saout <christophe@xxxxxxxx>");
MODULE_DESCRIPTION(DM_NAME " target for transparent encryption / decryption");

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/