Re: [PATCH 3/8] scsi: megaraid_sas - add module param max_sectors, cmd_per_lun

[Brian King <brking@linux.vnet.ibm.com>]

bo yang wrote:

> +static ssize_t
> +sysfs_max_sectors_read(struct kobject *kobj,
> +			struct bin_attribute *bin_attr,
> +			char *buf, loff_t off, size_t count)
> +{
> +	struct Scsi_Host *host = class_to_shost(container_of(kobj,
> +					struct class_device, kobj));
> +	struct megasas_instance *instance =
> +				(struct megasas_instance *)host->hostdata;
> +
> +	count = sprintf(buf, "%u\n", instance->max_sectors_per_req);
> +
> +	return count+1;
> +}
> +
> +static struct bin_attribute sysfs_max_sectors_attr = {
> +	.attr = {
> +		.name = "max_sectors",
> +		.mode = S_IRUSR|S_IRGRP|S_IROTH,
> +		.owner = THIS_MODULE,
> +	},
> +	.size = 7,
> +	.read = sysfs_max_sectors_read,
> +};

Why is this implemented as a binary sysfs attribute? Also, can you use
the existing shost_attrs infrastructure that's in the scsi_host_template
like megaraid_mbox uses?


> +
> +	/*
> +	 * Check if the module parameter value for max_sectors can be used
> +	 */
> +	if (max_sectors && max_sectors <= instance->max_sectors_per_req)
> +		instance->max_sectors_per_req = max_sectors;
> +	else {
> +		if (max_sectors)
> +			printk(KERN_INFO
> +				"megasas: max_sectors should be > 0 and"
> +				"<= %d\n",
> +				instance->max_sectors_per_req);
> +	}

Could be simplified to an else if, which would remove one indent level...

-Brian

-- 
Brian King
Linux on Power Virtualization
IBM Linux Technology Center