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:
signature.asc
Description: Digital signature
