| Message ID | 20190220184847.24640-1-bblock@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scsi: replace GFP_ATOMIC with GFP_KERNEL for sdev allocation | expand |
On Wed, 2019-02-20 at 19:48 +0100, Benjamin Block wrote: > We had a test-report where, under memory pressure, adding LUNs to the > systems would fail (the tests add LUNs strictly in sequence): Hi Benjamin, There are two more instances of GFP_ATOMIC in scsi_scan.c. Have you verified whether or not it is safe to change these into GFP_KERNEL too? Thanks, Bart.
On Wed, Feb 20, 2019 at 11:11:31AM -0800, Bart Van Assche wrote: > On Wed, 2019-02-20 at 19:48 +0100, Benjamin Block wrote: > > We had a test-report where, under memory pressure, adding LUNs to the > > systems would fail (the tests add LUNs strictly in sequence): > > Hi Benjamin, > > There are two more instances of GFP_ATOMIC in scsi_scan.c. Have you verified > whether or not it is safe to change these into GFP_KERNEL too? > No, I was lazy, but I can take a look tomorrow and fix them up as well if they are similar to scsi_alloc_sdev().
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index dd0d516f65e2..e49e6099b852 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -220,7 +220,7 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget, struct Scsi_Host *shost = dev_to_shost(starget->dev.parent); sdev = kzalloc(sizeof(*sdev) + shost->transportt->device_size, - GFP_ATOMIC); + GFP_KERNEL); if (!sdev) goto out;
We had a test-report where, under memory pressure, adding LUNs to the systems would fail (the tests add LUNs strictly in sequence): [ 5525.853432] scsi 0:0:1:1088045124: Direct-Access IBM 2107900 .148 PQ: 0 ANSI: 5 [ 5525.853826] scsi 0:0:1:1088045124: alua: supports implicit TPGS [ 5525.853830] scsi 0:0:1:1088045124: alua: device naa.6005076303ffd32700000000000044da port group 0 rel port 43 [ 5525.853931] sd 0:0:1:1088045124: Attached scsi generic sg10 type 0 [ 5525.854075] sd 0:0:1:1088045124: [sdk] Disabling DIF Type 1 protection [ 5525.855495] sd 0:0:1:1088045124: [sdk] 2097152 512-byte logical blocks: (1.07 GB/1.00 GiB) [ 5525.855606] sd 0:0:1:1088045124: [sdk] Write Protect is off [ 5525.855609] sd 0:0:1:1088045124: [sdk] Mode Sense: ed 00 00 08 [ 5525.855795] sd 0:0:1:1088045124: [sdk] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA [ 5525.857838] sdk: sdk1 [ 5525.859468] sd 0:0:1:1088045124: [sdk] Attached SCSI disk [ 5525.865073] sd 0:0:1:1088045124: alua: transition timeout set to 60 seconds [ 5525.865078] sd 0:0:1:1088045124: alua: port group 00 state A preferred supports tolusnA [ 5526.015070] sd 0:0:1:1088045124: alua: port group 00 state A preferred supports tolusnA [ 5526.015213] sd 0:0:1:1088045124: alua: port group 00 state A preferred supports tolusnA [ 5526.587439] scsi_alloc_sdev: Allocation failure during SCSI scanning, some SCSI devices might not be configured [ 5526.588562] scsi_alloc_sdev: Allocation failure during SCSI scanning, some SCSI devices might not be configured Looking at the code of scsi_alloc_sdev(), and all the calling contexts, there seems to be no reason to use GFP_ATMOIC here. All the different call-contexts use a mutex at some point, and nothing in between that requires no sleeping, as far as I could see. Additionally, the code that allocates the block queue for the device later (scsi_mq_alloc_queue()) already uses GFP_KERNEL. So replace it, and give the allocation a bit of a better chance to succeed, with more ways of reclaim. Signed-off-by: Benjamin Block <bblock@linux.ibm.com> --- drivers/scsi/scsi_scan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)