[dm-devel] Another experimental dm target... an encryption target

[Christophe Saout]

Am Di, 2003-07-29 um 16.33 schrieb Joe Thornber:

> Very impressive, I can see we're going to have to think up something
> harder for you to do ;)

Thanks. :)

> > 1. I could save a mempool if I could access the target_io structure (I
> > could, but that's really ugly). That's because I need to access my
> > crypt_c structure in my endio function. Because I also need to access
> > the original bio and I can't put two pointers in cloned_bio->bi_private
> > I have to allocate a crypt_io structure that contains both pointers.
> 
> I can't see a way around this mempool ATM,

Before I introduced the use of a mempool I copied the definition of
struct target_io and used the fact that the bio that's passed as
parameter of the ->map function has this structure in the bi_private
field. I then in turn put the address of this bio in the bi_private
field of my clone. But, as I said, that's very ugly because one should
not rely on that and that's why target_io is private to dm.c anyway.
That's why i fixed it by introducing this additional structure that's
allocated using a mempool. The structure is very small though and I
think mempools are efficient (at least more efficient than the whole
encryption thing anyway) so it really shouldn't be a performance
bottleneck.

> though you don't need the io->clone field ?

Yes, that's right. I put it there for completeness reasons in the
beginning but it turned out that I never need it because I already have
that bio anyway.

I think I added it because I wanted to modify the worker thread queue to
use the crypt_io structure instead of the cloned bios but I then
realized that I would need additional next pointers and that keeping the
bios in the queue was just fine.

So yes, you can safely drop this field (in the structure definition and
the assignment in crypt_map).

> > The bios device-mapper gives to the target are never sent down. I've got
> > the same problem in dm-file (at least I don't need to clone them again
> > so it's not a too big waste).
> 
> That's fine, md and dm-raid1 work in the same way.

Ok.

> > So you see, at lot of room for improvements. ;)
> 
> The main problem I have with the code is with crypt_alloc_buffer(), I
> realise this is not your code but copied this from loop.c.

Yes, you got me... I saw that you are using page pools in the current dm
2.4 implementation. That's why I wanted to wait and possibly use that
implementation instead. Or should I implement my own little pool?

> I would
> far rather you managed a mempool of struct pages rather than doing
> this hacky repeat until alloc works stuff.  Of course this may then
> mean you have to split v. large bios.

Yes, I could do this. Does this mean that I have to wait for a split
write to return first before I can continue to write the next part of
the bio? When the pool runs empty?

I could do something like this:

1. try to allocate pages with the nowait option (so it fails if the
system needs to swap out or something)
2. if that fails try to use some pages from the pool, so it's only used
in emergency mode
3. if that fails and you've already got some pages, submit a partial bio
and
4. try to wait for the pool to refill (if it's empty that means that
other write request are currently using them and they will definitely
return some day so you won't deadlock here).

When writes terminate you can put every page back on the pool even those
that originally didn't come from the pool until the pool reaches its
maximum number of pages.

Would that be a strategy? Or is it too complicated?

> How much testing has this had ?  Patrick Caulfield tends to break my
> code by running a couple of:
> 
> find /usr|cpio -pmd /mnt/ &
> 
> on a low memory config with the dm device mounted on /mnt.  I suggest
> you try the same, I really don't trust any code that has come from
> loop.c.

Yes, I don't think that code's too clever either. If you are out of
memory, your swap is full and the kernel wants to write out dirty pages
to reclaim memory but the writes are stuck waiting for buffers to do the
encryption, you run into a deadlock.

--
Christophe Saout <christophe at saout.de>
Please avoid sending me Word or PowerPoint attachments.
See http://www.fsf.org/philosophy/no-word-attachments.html