[dm-devel] Re: Shared snapshots

[Mike Snitzer]

On Wed, Dec 16 2009 at  3:05am -0500,
Mikulas Patocka <mpatocka at redhat.com> 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();