On Thu 18-05-23 06:23:11, Christoph Hellwig wrote:
> The long if/else chain obsfucates the actual logic. Tidy it up to be
> more structured. Also drop the whole argument, as it can be trivially
> derived from bdev using bdev_whole, and having the bdev_whole in the
> function makes it easier to follow.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Looks good to me. Just one nit below but regardless of how you decided feel
free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
> +static bool bd_may_claim(struct block_device *bdev, void *holder)
> {
> - if (bdev->bd_holder == holder)
> - return true; /* already a holder */
> - else if (bdev->bd_holder != NULL)
> - return false; /* held by someone else */
> - else if (whole == bdev)
> - return true; /* is a whole device which isn't held */
> -
> - else if (whole->bd_holder == bd_may_claim)
> - return true; /* is a partition of a device that is being partitioned */
> - else if (whole->bd_holder != NULL)
> - return false; /* is a partition of a held device */
> - else
> - return true; /* is a partition of an un-held device */
> + struct block_device *whole = bdev_whole(bdev);
> +
> + if (bdev->bd_holder) {
> + /*
> + * The same holder can always re-claim.
> + */
> + if (bdev->bd_holder == holder)
> + return true;
> + return false;
With this simple condition I'd just do:
/* The same holder can always re-claim. */
return bdev->bd_holder == holder;
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
