On 07/22/2011 11:31 AM, FUJITA Tomonori wrote:
>> +/**
>> + * scsi_destroy_bsg_job - routine to teardown/delete a bsg job
>> + * @job: scsi_bsg_job that is to be torn down
>> + */
>> +static void scsi_destroy_bsg_job(struct scsi_bsg_job *job)
>> +{
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&job->job_lock, flags);
>> + if (job->ref_cnt) {
>> + spin_unlock_irqrestore(&job->job_lock, flags);
>> + return;
>> + }
>> + spin_unlock_irqrestore(&job->job_lock, flags);
>
> Is this actually a reference counter? Doesn't look like how we use a
> reference counter? If scsi_bsg_job needs it, it would be better to
> just put kref in it?
Yeah, I do not think it is needed. That and the state_flags should not
be needed (state flag seems to duplicate the request state handling). I
have patches to remove that code. I was going to send them separately
though, so just in case I/we are wrong it can be git reverted. I guess
it can just be merged in this patch though.
>> +void scsi_bsg_remove(struct request_queue *q)
>> +{
>> + struct request *req; /* block request */
>> + int counts; /* totals for request_list count and starved */
>
> This function looks hacky a bit (e.g. peeking at the queue internal
> values such as count and starved). Can we do this in a different way
> (less hacky way)?
>
>> + }
>> +
>> + msleep(200); /* allow bsg to possibly finish */
>
> Hmm, what's it? Do we need to fix something about bsg?
James Smart had added this in 78d16341facf829a71b6f7c68ec5511b9c168060.
I do not think it can be easily fixed. I think he is working on a better
long term fix.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
