Re: [PATCH/Resend] md: Push down data integrity code to personalities.

[Andre Noll <maan@systemlinux.org>]

[-- Attachment #1: Type: text/plain, Size: 1838 bytes --]

On 15:06, Neil Brown wrote:
> > Here is a revised patch that puts calls to the new functions into
> > the ->hot_*_disk methods as you propose.
> 
> Thanks.  Much better.
> Calling md_integrity_check from the ->error routines isn't a good idea
> though.  They can be called in interrupt context, and
> md_integrity_check can call kmalloc(..., GFP_KERNEL) which might try
> to sleep.  That would be bad.
> We don't need the call in ->error, having it in ->hot_remove_disk is
> adequate, as that is called soon after any failure (it doesn't wait
> for the sysadmin to "mdadm --remove ...".).
> 
> So I have made that change and updated the comment accordingly.

Thanks. Sorry if this is a FAQ, but how can one tell whether a given
function may be called in interrupt context? Is there a better way
than recursively checking all its callers?

> > Side note: The patch causes the new gcc warning
> > 
> > 	drivers/md/md.c: In function 'md_integrity_add_rdev':
> > 	drivers/md/md.c:1546: warning: unused variable 'gd'
> > 
> > if data integrity is not compiled in. Any ideas on how to avoid it
> > without introducing an #ifdef-mess are welcome.
> 
> I just replaced ever occurrence of "gd" with "mddev->gendisk".  It
> isn't too ugly.
> Alternately, blkdev.h could have
> 
> static inline struct blk_integrity *blk_get_integrity(struct gendisk *disk)
> {
> 	return NULL;
> }
> 
> in place of
> 
> #define blk_get_integrity(a)			(0)
> 
> and then there would be no complaints about unused variables, and
> slightly improved type checking.

IMO this second alternative is be a bit cleaner and it might help
other users of blk_get_integrity() as well. Martin, are you OK with
this change to blkdev.h?

Regards
Andre
-- 
The only person who always got his work done by Friday was Robinson Crusoe

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]