Skip to content

Commit

Permalink
[SCSI] don't change sdev starvation list order without request dispat…
Browse files Browse the repository at this point in the history
…ched

The sdev is deleted from starved list and then try to dispatch from this
device. It's quite possible the sdev can't eventually dispatch a request,
then the sdev will be in starved list tail. This isn't fair.
There are two cases here:
1. unplug path. scsi_request_fn() calls to scsi_target_queue_ready(), then
the dev is removed from starved list, but quite possible host queue isn't
ready, the dev is moved to starved list without dispatching any request.
2. scsi_run_queue path. It deletes the dev from starved list first (both
global and local starved lists), then handles the dev. Then we could have
the same process like case 1.

This patch fixes the first case. Case 2 isn't fixed, because there is a
rare case scsi_run_queue finds host isn't busy but scsi_request_fn finds
host is busy (other CPU is faster to get host queue depth). Not deleting
the dev from starved list in scsi_run_queue will keep scsi_run_queue
looping (though this is very rare case, because host will become busy).
Fortunately fixing case 1 already gives big improvement for starvation in
my test. In a 12 disk JBOD setup, running file creation under EXT4, this
gives 12% more throughput.

Signed-off-by: Shaohua Li <shaohua.li@intel.com>
Signed-off-by: James Bottomley <JBottomley@Parallels.com>
  • Loading branch information
Shaohua Li authored and James Bottomley committed Jan 16, 2012
1 parent 05b080f commit 466c08c
Showing 1 changed file with 1 addition and 6 deletions.
7 changes: 1 addition & 6 deletions drivers/scsi/scsi_lib.c
Original file line number Diff line number Diff line change
Expand Up @@ -1316,15 +1316,10 @@ static inline int scsi_target_queue_ready(struct Scsi_Host *shost,
}

if (scsi_target_is_busy(starget)) {
if (list_empty(&sdev->starved_entry))
list_add_tail(&sdev->starved_entry,
&shost->starved_list);
list_move_tail(&sdev->starved_entry, &shost->starved_list);
return 0;
}

/* We're OK to process the command, so we can't be starved */
if (!list_empty(&sdev->starved_entry))
list_del_init(&sdev->starved_entry);
return 1;
}

Expand Down

0 comments on commit 466c08c

Please sign in to comment.