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@xxxxxxxx> Please avoid sending me Word or PowerPoint attachments. See http://www.fsf.org/philosophy/no-word-attachments.html