On Wed, Dec 16 2009 at 3:05am -0500, Mikulas Patocka <mpatocka@xxxxxxxxxx> wrote: > Hi > > I uploaded new shared snapshots here: > http://people.redhat.com/mpatocka/patches/kernel/new-snapshots/devel/ Just a general observation: You have an empty line separating your comment blocks above functions. I think eliminating that empty line would help join the comment block to the function a bit better (as is common in the rest of the kernel). Also, a comment block of the form: /* single line comment above function */ Is generally done as: /* * single line comment above function */ Hopefully I'm not triggering unhappy checkpatch.pl-type thoughts with these recommendations :) > changes: > > - Broken to separate patches, one patch per file. The last patch adds all > the files to Makefile and makes it all compile. It allows you to ack > patches individually. I didn't use stubs to compile intermediate patches, > the problem is that if multiple patches modify the same file, general > modifications to the code become harder. With one-patch-per-file I can > edit it with normal text editor and quilt regenerates the patches. > > - Document on-disk format in dm-multisnap-mikulas-struct.h > > - More functions are commented > > - Removed /*printk ... */ statements. I left some #ifdefed debug routines, > they are non-trivial to write again I'm still seeing one in dm_bufio_client_create() > - Fixed a bug on big-endian systems > > - Exposed interface for snapshots-of-snapshots, tested that they work Where is that interface documented? As an aside, I have some ideas for improving Documentation/device-mapper/dm-multisnapshot.txt I'll just send a patch and we can go from there. > Mikulas BTW, I'm getting the following warning when I build the code: drivers/md/dm-bufio.c: In function ‘write_endio’: drivers/md/dm-bufio.c:725: warning: value computed is not used A cast fixes it: diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c index 1ab5304..be7b89b 100644 --- a/drivers/md/dm-bufio.c +++ b/drivers/md/dm-bufio.c @@ -722,7 +722,7 @@ static void write_endio(struct bio *bio, int error) b->write_error = error; if (unlikely(error)) { struct dm_bufio_client *c = b->c; - cmpxchg(&c->async_write_error, 0, error); + (void)cmpxchg(&c->async_write_error, 0, error); } BUG_ON(!test_bit(B_WRITING, &b->state)); smp_mb__before_clear_bit(); -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel