Don't use unnecessary GFP_ATOMIC in sg.c v2

Don't use unnecessary GFP_ATOMIC in sg.c v2

[Update for the previous version of the patch]

sg.c uses GFP_ATOMIC for a lot of allocations where it isn't necessary. There are no locks hold and I don't see any other reasons why GFP_ATOMIC should be needed here. So remove them.

Depends on the earlier GFP_DMA patchkit, but only because it happens to patch the same places (no real functional dependency)

Tested by burning a CD on a kernel with all lock/sleep/etc. debugging enabled.

v2: Actually always use GFP_KERNEL instead of 0 (which is equivalent to
GFP_ATOMIC). Retested.

Signed-off-by Andi Kleen <[email protected]>

---
 drivers/scsi/sg.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Index: linux/drivers/scsi/sg.c
===================================================================

drivers/scsi/sg.c +5 -6

@@ -745,7 +745,7 @@ sg_common_write(Sg_fd * sfp, Sg_request 
 	if (scsi_execute_async(sdp->device, cmnd, hp->cmd_len, data_dir, srp->data.buffer,
 				hp->dxfer_len, srp->data.k_use_sg, timeout,
 				SG_DEFAULT_RETRIES, srp, sg_cmd_done,
-				GFP_ATOMIC)) {
+				GFP_KERNEL)) {
 		SCSI_LOG_TIMEOUT(1, printk("sg_common_write: scsi_execute_async failed\n"));
 		/*
 		 * most likely out of mem, but could also be a bad map

@@ -1654,7 +1654,7 @@ static int
 sg_build_sgat(Sg_scatter_hold * schp, const Sg_fd * sfp, int tablesize)
 {
 	int sg_bufflen = tablesize * sizeof(struct scatterlist);
-	gfp_t gfp_flags = GFP_ATOMIC | __GFP_NOWARN;
+	gfp_t gfp_flags = GFP_KERNEL | __GFP_NOWARN;
 
 	schp->buffer = kzalloc(sg_bufflen, gfp_flags);
 	if (!schp->buffer)

@@ -1694,7 +1694,7 @@ st_map_user_pages(struct scatterlist *sg
 	if (count == 0)
 		return 0;
 
-	if ((pages = kmalloc(max_pages * sizeof(*pages), GFP_ATOMIC)) == NULL)
+	if ((pages = kmalloc(max_pages * sizeof(*pages), GFP_KERNEL)) == NULL)
 		return -ENOMEM;
 
         /* Try to fault in all of the necessary pages */

@@ -2323,7 +2323,7 @@ sg_add_sfp(Sg_device * sdp, int dev)
 	unsigned long iflags;
 	int bufflen;
 
-	sfp = kzalloc(sizeof(*sfp), GFP_ATOMIC | __GFP_NOWARN);
+	sfp = kzalloc(sizeof(*sfp), GFP_KERNEL|__GFP_NOWARN);
 	if (!sfp)
 		return NULL;
 

@@ -2452,7 +2452,7 @@ sg_page_malloc(int rqSz, int *retSzp)
 	if ((rqSz <= 0) || (NULL == retSzp))
 		return resp;
 
-	page_mask = GFP_ATOMIC | __GFP_COMP | __GFP_NOWARN;
+	page_mask = GFP_KERNEL | __GFP_COMP | __GFP_NOWARN;
 
 	for (order = 0, a_size = PAGE_SIZE; a_size < rqSz;
 	     order++, a_size <<= 1) ;
-

To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Oh no, not again. This isn't the first time kernel folks have tried to demote the sg driver's GFP_ATOMIC to GFP_KERNEL.

In the past it has ended in grief. The driver was written to attempt _fast_ allocation and if that failed then:

- lessen the requested allocation (e.g. smaller elements but more of them in a scatter gather list), or

- report the error back to the user (i.e. ENOMEM) assuming that they are knowledgeable enough to do something about it (e.g. do two smaller READs rather than one larger one).