Re: [PATCH V4 10/16] block: mq-deadline: Add zoned block device data

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/24/17 17:14, Christoph Hellwig wrote:
>> +
>> +	struct request_queue *q;
> 
> Do you really need the queue backpointer?  At least as far as this
> patch is concerned we could just pass the queue on to
> deadline_enable_zones_wlock and be fine.  And in general we should
> always passing the q, as we can trivial go from queue to deadline_data
> using queue->elevator->elevator_data.

This is for the sysfs zones_wlock store function which does not give the
queue. Instead of this backpointer, I can copy the queue node, number of
zones and zone model so that cdeadline_enable_zones_wlock() can be
called equally from init_queue context and from the sysfs zones_wlock
store context.

>> +static int deadline_zoned_init_queue(struct request_queue *q,
>> +				     struct deadline_data *dd)
>> +{
>> +	if (!blk_queue_is_zoned(q) ||
>> +	    !blk_queue_nr_zones(q)) {
> 
> Shouldn't !blk_queue_nr_zones(q) be enough?  If not both conditionals
> could easily fit into the same line, and I'd be tempted to move them
> to the caller and call deadline_enable_zones_wlock straight from there.

OK. Will update.

>> @@ -341,6 +387,15 @@ static int dd_init_queue(struct request_queue *q, struct elevator_type *e)
>>  	spin_lock_init(&dd->lock);
>>  	INIT_LIST_HEAD(&dd->dispatch);
>>  
>> +	dd->q = q;
>> +	spin_lock_init(&dd->zone_lock);
>> +	ret = deadline_zoned_init_queue(q, dd);
>> +	if (ret) {
>> +		kfree(dd);
>> +		kobject_put(&eq->kobj);
>> +		return ret;
>> +	}
>> +
>>  	q->elevator = eq;
>>  	return 0;
> 
> This should probably grow goto based unwinding, e.g.

OK. Will update.

Thanks !

-- 
Damien Le Moal
Western Digital Research



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux